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

[7-0-stable] Add lower bound to Listen gem requirement #48762

Merged
merged 2 commits into from Jul 19, 2023

Conversation

skipkayhil
Copy link
Member

Backports #48622 and #47002 to 7-0-stable

This is useful because #47774 was backported, and wait_for_state isn't available until listen 3.3

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

A user reported seeing the following error when trying to use the
EventedFileUpdateChecker in their app:

```shell
$ rails server
=> Booting Puma
=> Rails 7.0.4 application starting in development
=> Run `bin/rails server --help` for more startup options
Exiting
/usr/local/lib/ruby/gems/3.0.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require': cannot load such file -- listen (LoadError)
	from /usr/local/lib/ruby/gems/3.0.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
	from /usr/local/lib/ruby/gems/3.0.0/gems/activesupport-7.0.4/lib/active_support/evented_file_update_checker.rb:6:in `<top (required)>'
	from /usr/local/lib/ruby/gems/3.0.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
	from /usr/local/lib/ruby/gems/3.0.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
	from /home/ross/Data/EmySystem/newentry/Entry2/config/environments/development.rb:84:in `block in <top (required)>'
	from /usr/local/lib/ruby/gems/3.0.0/gems/railties-7.0.4/lib/rails/railtie.rb:257:in `instance_eval'
	from /usr/local/lib/ruby/gems/3.0.0/gems/railties-7.0.4/lib/rails/railtie.rb:257:in `configure'
	from /home/ross/Data/EmySystem/newentry/Entry2/config/environments/development.rb:3:in `<top (required)>'
        ...
```

While we were able to direct them to the proper fix (add listen to
their app's Gemfile), the error message does not really point them in
that direction on its own.

This commit improves the error message by using Kernel#gem to indicate
that the gem should be in the user's bundle. This is the same approach
used for other not-specified dependencies in Rails, such as the various
database drivers in their Active Record adapters.

New error message:
```shell
$ bin/rails r "ActiveSupport::EventedFileUpdateChecker"
/home/hartley/.cache/asdf/installs/ruby/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/rubygems_integration.rb:276:in `block (2 levels) in replace_gem': listen is not part of the bundle. Add it to your Gemfile. (Gem::LoadError)
        from /home/hartley/src/github.com/skipkayhil/rails/activesupport/lib/active_support/evented_file_update_checker.rb:3:in `<top (required)>'
        from /home/hartley/.cache/asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
        from /home/hartley/.cache/asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
	...
```
An issue was recently opened with the following error message:

```
.../activesupport-7.0.6/lib/active_support/evented_file_update_checker.rb:120:in `start': undefined method `wait_for_state' for #<Listen::Listener ...>
```

The issue is that the user was using Listen 3.0.8, however the
`wait_for_state` method was [added][1] in Listen 3.3.0

We can make the error message a little better by defining a lower bound
on Listen 3.3.0 (like we do for other optional dependencies):

```
.../bundler/rubygems_integration.rb:280:in `block (2 levels) in replace_gem': can't activate listen (~> 3.3), already activated listen-3.0.8. Make sure all dependencies are added to Gemfile. (Gem::LoadError)
```

There is definitely still room for improvement here, but this should
be much more helpful in figuring out that the issue is a bad Listen
version and not a bug in Rails.

[1]: guard/listen@12b4fc5
skipkayhil referenced this pull request Jul 19, 2023
…e-update-checker

Fix race condition in evented file update checker
@eileencodes eileencodes merged commit c02b40c into rails:7-0-stable Jul 19, 2023
3 checks passed
@skipkayhil skipkayhil deleted the hm-backport-48622 branch July 19, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants