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

Fix build on Windows (restore tzinfo dependency) #1767

Closed
wants to merge 1 commit into from

Conversation

kungfux
Copy link
Collaborator

@kungfux kungfux commented May 20, 2024

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Description

Build is failing on Windows.

$ ruby -v
ruby 3.3.1 (2024-04-23 revision c56cd86388) [x64-mingw-ucrt]

$ ./tools/run

> bundle exec jekyll s -l -H 127.0.0.1

Configuration file: C:/Projects/jekyll-theme-chirpy/_config.yml
  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. If you've run Jekyll with `bundle exec`, ensure that you have included the tzinfo gem in your Gemfile as well. The full error message from Ruby is: 'cannot load such file -- tzinfo' If you run into trouble, you can find helpful resources at https://jekyllrb.com/help/!
jekyll 4.3.3 | Error:  tzinfo
C:/Ruby33-x64/lib/ruby/gems/3.3.0/gems/jekyll-4.3.3/lib/jekyll/external.rb:70:in `rescue in block in require_with_graceful_fail': tzinfo (Jekyll::Errors::MissingDependencyException)

Additional context

Root cause: f87fdd0

Can we restore the dependency or is there a better solution?

@kungfux kungfux requested a review from cotes2020 May 20, 2024 21:51
@seaque
Copy link

seaque commented May 22, 2024

For now you can add this to Gemfile

# Windows and JRuby does not include zoneinfo files, so bundle the tzinfo-data gem
# and associated library.
platforms :mingw, :x64_mingw, :mswin, :jruby do
  gem "tzinfo", ">= 1", "< 3"
  gem "tzinfo-data"
end

# Performance-booster for watching directories on Windows
gem "wdm", "~> 0.1.1", :platforms => [:mingw, :x64_mingw, :mswin]

# Lock `http_parser.rb` gem to `v0.6.x` on JRuby builds since newer versions of the gem
# do not have a Java counterpart.
gem "http_parser.rb", "~> 0.6.0", :platforms => [:jruby]

@kungfux kungfux changed the title Restore tzinfo dependency to fix Windows build Restore tzinfo dependency May 22, 2024
@cotes2020
Copy link
Owner

This project is essentially a gem, so it is not appropriate for its Gemfile to contain other gem dependencies that are required for a particular platform (Windows), which is exactly what Jekyll does.

To verify this, you can run the command jekyll new-theme my-theme to create a theme my-theme and check the Gemfile inside, which reads:

# frozen_string_literal: true

source "https://rubygems.org"
gemspec

@kungfux
Copy link
Collaborator Author

kungfux commented May 25, 2024

It is not appropriate for your Gemfile to contain other Gem dependencies required for a particular platform.

It depends. I haven't found much information on possible recommendations, other than 9146. However, it's up to us to decide whether we want to allow users running theme locally on Windows to face the same problem or not.

An alternative approach would be to add this information to the wiki. However, I don't think it is a good option to force people to spend time trying to find a solution.

I urge you to think about which way to go: have a built-in fix to support Windows users, or add information to the wiki. Is there a downside for us to the first option?

Thanks.

@huanyushi
Copy link
Contributor

I agree with @kungfux .

I am a Windows user. After updating to v7.0.0, I will encounter the same error. Therefore I need to manually add tzinfo and tzinfo-data. I used to think that there was something wrong with my own configuration process, but now it seems that is not the case.

@cotes2020
Copy link
Owner

Let me elaborate a bit more: the Gemfile of the main repository (jekyll-theme-chirpy) serves to build the gem on Unix-like platforms, and it would be pointless to add dependencies needed for Windows. As you can see, all development/testing/releasing of the main repository is done on Unix-like machines.

So how to make it easier for users to run Jekyll out of the box on Windows, I think we should restore tzinfo / tzinfo-data in the chirpy-starter's Gemfile instead of the main repository.

As for the necessity of adding Windows timezone deps instructions to the Wiki, I think it's optional since they're already in the Jekyll docs and should be visible to new users when installing Jekyll, but I'm open to it if you insist.


I haven't found much information on possible recommendations, other than 9146

I think this comment will help you understand the situation: jekyll/jekyll#5935 (comment)

@kungfux
Copy link
Collaborator Author

kungfux commented May 26, 2024

the Gemfile of the main repository (jekyll-theme-chirpy) serves to build the gem on Unix-like platforms

That's true for actions and maybe local development for some number of users, but not for everyone.

and it would be pointless to add dependencies needed for Windows

I'm the one who needs this added to both the starter and main repositories. I'm not going to estimate the number of Windows/Unix-like users and mention the availability of the GitHub workspace as a workaround. There is at least one user who has the problem of doing development on a Windows machine, and there is a chance that others will come along and file a ticket. I find it useful to build and see the result before committing changes to production. While I can modify and commit the Gemfile in my own blog instance, I have to keep the modified Gemfile locally while working with the main repository.

IMO, if the extra dependency does not cause problems in the build process on Unix-like systems, and the only reason to remove it was to have a cleaner gem spec, then it is worse to have it by default. I trust your experience and believe you will make the right decision. I just wanted to share my experience regarding this change.

Thanks for your time and for sharing useful links that I missed.


P.S. Interesting point in that Jekyll blog post:

While ‘new’ blogs created with Jekyll v3.4 and greater, will have the following added to their Gemfile by default, existing sites will have to update their Gemfile (and installed gems) to enable development on Windows.

I have tried and created a new theme with Jekyll 4.3.3 and no tzinfo dependency has been added. However, I'm able to run jekyll build without any problems. Not sure why.

@cotes2020
Copy link
Owner

There is at least one user who has the problem of doing development on a Windows machine, and there is a chance that others will come along and file a ticket ...

Thanks for sharing this valuable experience, a better approach might be to customize a Docker development environment so that there are no cross-platform dependencies to worry about.

In the past, the Gemfile in this repository has always had platform-specific dependencies because it was originally a regular Jekyll project (my personal website), and its dependencies were created with the jekyll new command. But since v3.0.0, it was converted to a gem-based theme, but I didn't clean up these non-generic deps in time until v7.0.0 development, when I felt it was time to deal with this legacy issue.

I'm able to run jekyll build without any problems. Not sure why ...

Maybe your machine has installed tzinfo/tzinfo-data with the command gem install <pkg>?

@kungfux kungfux added the wontfix This will not be worked on label May 27, 2024
@kungfux
Copy link
Collaborator Author

kungfux commented May 27, 2024

I see your point. I'll elaborate on that option and summarize results somewhere afterwards. Thanks.

@kungfux kungfux closed this May 27, 2024
@cotes2020
Copy link
Owner

I'm considering using VS Code devcontainer to improve the development environment setup.

@kungfux
Copy link
Collaborator Author

kungfux commented May 27, 2024

That's what I've started to try following this guide as a starting point. However, it should be adapted to be suitable and fully automated. Preliminary, it looks interesting but I'm a bit concerned about the required additional resources. I will see how it goes :)

@cotes2020
Copy link
Owner

cotes2020 commented May 27, 2024

I've been experimenting for a few hours with reference to the VS Codoe docs and will submit a PR later to improve it with you.

@kungfux kungfux changed the title Restore tzinfo dependency Fix build on Windows (restore tzinfo dependency) May 27, 2024
@cotes2020 cotes2020 mentioned this pull request May 27, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants