Skip to content
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

ashmaroli
Copy link
Member

@ashmaroli 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
Copy link
Member Author

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

Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ashmaroli
Copy link
Member Author

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

this set of steps allow the test to pass when DST in not currently active.
They may fail when DST becomes active.
@parkr
Copy link
Member

parkr commented Dec 9, 2016

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit 1049802 into jekyll:master Dec 9, 2016
jekyllbot added a commit that referenced this pull request Dec 9, 2016
parkr pushed a commit that referenced this pull request Dec 16, 2016
parkr added a commit that referenced this pull request Dec 16, 2016
* 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 deleted the win-tz branch October 29, 2017 17:44
@jekyll jekyll locked and limited conversation to collaborators Jul 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants