New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a module to re-define `ENV["TZ"]` in Windows #5612

Merged
merged 7 commits into from Dec 9, 2016

Conversation

Projects
None yet
5 participants
@ashmaroli
Member

ashmaroli commented Nov 30, 2016

Finally managed to crack this issue.
With this in place, Jekyll on Windows will be able to reset ENV["TZ"] to the IANA TimeZone specified in the config.yml

Points To Consider:

  • Windows users need to have the following added to their Gemfile (and installed):
    • gem "tzinfo-data"
  • A side-effect to this is that now only valid IANA Timezones will be accepted. Earlier Windows users were able to pass UTC-04:00 etc as a workaround. Those sites will essentially require a re-configuring.

--
Addresses: #5414
/cc @jekyll/windows

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Nov 30, 2016

Member

DST & Failing Test..

  • Travis which doesn't use this module passes all test leading to the inference that America/New_York on Travis is still at UTC-04:00 even though DST is not active currently.
  • AppVeyor which sets TZ using this module fails as it uses the correct timezone-offset at UTC-05:00
Member

ashmaroli commented Nov 30, 2016

DST & Failing Test..

  • Travis which doesn't use this module passes all test leading to the inference that America/New_York on Travis is still at UTC-04:00 even though DST is not active currently.
  • AppVeyor which sets TZ using this module fails as it uses the correct timezone-offset at UTC-05:00
@parkr

parkr approved these changes Nov 30, 2016

Wow, this is really great work. Just one question about how we handle a missing dependency!

@@ -84,6 +84,10 @@ def gemfile_contents
group :jekyll_plugins do
gem "jekyll-feed", "~> 0.6"
end
# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]

This comment has been minimized.

@parkr

parkr Nov 30, 2016

Member

How does this work when you don't have the gen installed?

@parkr

parkr Nov 30, 2016

Member

How does this work when you don't have the gen installed?

This comment has been minimized.

@ashmaroli

ashmaroli Nov 30, 2016

Member

When the gem tzinfo-data is missing, Jekyll will raise MissingDependencyException w.r.t the parent gem tzinfo

Dependency Error: Yikes! It looks like you don't have tzinfo or one of its dependencies
installed. In order to use Jekyll as currently configured, you'll need to install this gem.
The full error message from Ruby is: 'cannot load such file -- tzinfo' 
If you run into trouble, you can find helpful resources at http://jekyllrb.com/help/!

jekyll 3.3.1 | Error:  tzinfo

If they then erroneously add gem tzinfo alone to their Gemfile and run Jekyll, then a TZInfo::DataSourceNotFound exception will be raised by tzinfo gem as Windows doesnt have a native zoneinfo source.:

jekyll 3.3.1 | Error:  No source of timezone data could be found.
Please refer to http://tzinfo.github.io/datasourcenotfound for help resolving this error.
@ashmaroli

ashmaroli Nov 30, 2016

Member

When the gem tzinfo-data is missing, Jekyll will raise MissingDependencyException w.r.t the parent gem tzinfo

Dependency Error: Yikes! It looks like you don't have tzinfo or one of its dependencies
installed. In order to use Jekyll as currently configured, you'll need to install this gem.
The full error message from Ruby is: 'cannot load such file -- tzinfo' 
If you run into trouble, you can find helpful resources at http://jekyllrb.com/help/!

jekyll 3.3.1 | Error:  tzinfo

If they then erroneously add gem tzinfo alone to their Gemfile and run Jekyll, then a TZInfo::DataSourceNotFound exception will be raised by tzinfo gem as Windows doesnt have a native zoneinfo source.:

jekyll 3.3.1 | Error:  No source of timezone data could be found.
Please refer to http://tzinfo.github.io/datasourcenotfound for help resolving this error.
Show outdated Hide outdated lib/jekyll.rb
@@ -119,7 +119,7 @@ def configuration(override = {})
# Returns nothing
# rubocop:disable Style/AccessorMethodName
def set_timezone(timezone)
ENV["TZ"] = timezone
ENV["TZ"] = Utils::Platforms.windows? ? Utils::WinTZ.calculate(timezone) : timezone

This comment has been minimized.

@envygeeks

envygeeks Nov 30, 2016

Contributor

This needs to be really_windows?. BashOnWindows uses GNU/Utils.

@envygeeks

envygeeks Nov 30, 2016

Contributor

This needs to be really_windows?. BashOnWindows uses GNU/Utils.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Dec 1, 2016

Member

@DirtyF I've added a small piece of documentation to go along with this pull.

Member

ashmaroli commented Dec 1, 2016

@DirtyF I've added a small piece of documentation to go along with this pull.

@oe

oe approved these changes Dec 6, 2016

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Dec 9, 2016

Member

@jekyllbot: merge +bug

Member

parkr commented Dec 9, 2016

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit 1049802 into jekyll:master Dec 9, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot added bug fix labels Dec 9, 2016

jekyllbot added a commit that referenced this pull request Dec 9, 2016

parkr added a commit that referenced this pull request Dec 16, 2016

parkr added a commit that referenced this pull request Dec 16, 2016

Merge branch 'master' into liquid-4
* master: (40 commits)
  Update history to reflect merge of #5658 [ci skip]
  Fix a couple of typos in the docs
  Update history to reflect merge of #5657 [ci skip]
  Update variables.md
  Update history to reflect merge of #5653 [ci skip]
  Improve Permalinks documentation.
  Update history to reflect merge of #5652 [ci skip]
  Use `assert_nil` instead of `assert_equal nil`
  Update history to reflect merge of #5513 [ci skip]
  Ignore symlinked file in windows
  Update history to reflect merge of #5612 [ci skip]
  Update history to reflect merge of #5643 [ci skip]
  Update Core team list in README
  Update history to reflect merge of #5641 [ci skip]
  use backticks for Gemfile for consistency since in the next sentence _config.yml file has backtick
  add a set of steps in site_configuration.feature
  update documentation for Windows
  narrow it down to only Windows
  revert and adjust site_configuration.feature
  add 'tzinfo-data' gem to generated Gemfile
  ...

@ashmaroli ashmaroli referenced this pull request Mar 5, 2017

Closed

Install of Jekyll 3.4.1 gem does not install tzinfo #5935

5 of 5 tasks complete

@ashmaroli ashmaroli deleted the ashmaroli:win-tz branch Oct 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment