lib/rack/handler/puma.rb - fix for rackup v1.0.1, adjust Gemfile #3532
Conversation
|
This PR looks like a reasonable MVP. I believe you should consider the following (maybe later):
Example of how to implement the shims effectively:
That's not accurate. Thanks for your efforts. |
|
Hmm, why can't I get the tests to fail without this change? Here's what I tried $ git l -1
3f251ae4 (HEAD -> master, dentarg/master) 2024-10-22 11:37:08 -0500 [CI] fix #3325, needs method rename (#3530) (MSP-Greg)
~/src/puma master*
$ git diff
diff --git a/Gemfile b/Gemfile
index 97038b9e..6dfddf50 100644
--- a/Gemfile
+++ b/Gemfile
@@ -11,20 +11,16 @@ gem "minitest-retry"
gem "minitest-proveit"
gem "minitest-stub-const"
-use_rackup = false
-rack_vers =
- case ENV['PUMA_CI_RACK']&.strip
- when 'rack2'
- '~> 2.2'
- when 'rack1'
- '~> 1.6'
- else
- use_rackup = true
- '>= 2.2'
- end
-
-gem "rack", rack_vers
-gem "rackup" if use_rackup
+case ENV['PUMA_CI_RACK']&.strip
+when 'rack2'
+ gem "rackup", '~> 1.0'
+ gem "rack" , '~> 2.2'
+when 'rack1'
+ gem "rack" , '~> 1.6'
+else
+ gem "rackup", '>= 2.0'
+ gem "rack" , '>= 2.2'
+end
gem "jruby-openssl", :platform => "jruby"
~/src/puma master*
$ PUMA_CI_RACK=rack2 bundle
Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
Fetching rubocop 1.67.0
Installing rubocop 1.67.0
Bundle complete! 15 Gemfile dependencies, 28 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.
1 installed gem you directly depend on is looking for funding.
Run `bundle fund` for details
~/src/puma master*
$ PUMA_CI_RACK=rack2 bundle show | grep rack
* rack (2.2.10)
* rackup (1.0.1)
~/src/puma master*
$ PUMA_CI_RACK=rack2 bundle exec ./test/runner -v test_rack_handler
Run options: -v --seed 4592
# Running:
TestRackUp::TestUserSuppliedOptionsIsNotPresent#test_user_port_wins_over_config = 0.02 s = .
TestRackUp::TestUserSuppliedOptionsIsNotPresent#test_config_wins_over_default = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsIsNotPresent#test_user_port_wins_over_default = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsIsNotPresent#test_user_log_requests_wins_over_file_config = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsIsNotPresent#test_default_log_request_when_no_config_file = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsIsNotPresent#test_file_log_requests_wins_over_default_config = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsIsNotPresent#test_user_port_wins_over_default_when_user_supplied_is_blank = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsIsNotPresent#test_default_port_when_no_config_file = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsHostIsSet#test_absolute_unix_host = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsHostIsSet#test_ipv6_host_supplied_port_default = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsHostIsSet#test_abstract_unix_host = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsHostIsSet#test_relative_unix_host = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsHostIsSet#test_ssl_host_supplied_port_default = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsHostIsSet#test_host_uses_supplied_port_default = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsPortIsSet#test_port_wins_over_config = 0.00 s = .
TestRackUp::RackUp#test_bin = /Users/dentarg/.arm64_rubies/3.3.5/bin/rackup:25: warning: ostruct was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
You can add ostruct to your Gemfile or gemspec to silence this warning.
0.86 s = .
TestRackUp::TestPathHandler#test_handler_boots = 0.50 s = .
TestRackUp::TestUserSuppliedOptionsIsEmpty#test_default_host_when_using_config_file = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsIsEmpty#test_config_file_wins_over_port = 0.00 s = .
TestRackUp::TestUserSuppliedOptionsIsEmpty#test_default_host_when_using_config_file_with_explicit_host = 0.00 s = .
TestRackUp::TestOnBootedHandler#test_on_booted = 0.51 s = .
Fabulous run in 1.894555s, 11.0844 runs/s, 13.1957 assertions/s.
21 runs, 25 assertions, 0 failures, 0 errors, 0 skips |
|
Also when I played around "by hand", I could not reproduce either of rack/rackup#13 rack/rackup#19, what am I missing? What's triggering the problem for folks? $ cat Gemfile
# frozen_string_literal: true
source "https://rubygems.org"
gem "rackup", "1.0.0"
# gem "rackup", "1.0.1"
gem "puma"
$ b e rackup --server puma
ostruct was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
You can add ostruct to your Gemfile or gemspec to silence this warning.
Puma starting in single mode...
* Puma version: 6.4.3 (ruby 3.3.5-p100) ("The Eagle of Durango")
* Min threads: 0
* Max threads: 5
* Environment: development
* PID: 27190
* Listening on http://127.0.0.1:9292
* Listening on http://[::1]:9292
Use Ctrl-C to stop
^C- Gracefully stopping, waiting for requests to finish
=== puma shutdown: 2024-10-23 22:18:55 +0200 ===
- Goodbye! |
|
I think the issue is running Capybara, which starts Puma with puma_rack_handler = defined?(Rackup::Handler::Puma) ? Rackup::Handler::Puma : Rack::Handler::PumaI haven't yet thought of a way to add a test. Also, Capybara no longer tests against Rack 2. I.think. |
|
I still can't reproduce... I was about to say $ bundle
Fetching gem metadata from https://rubygems.org/..
Resolving dependencies...
Using rackup 1.0.1 (was 1.0.0)
Bundle complete! 2 Gemfile dependencies, 6 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.
~/code-snippets/rackup-test
$ bundle exec ruby -rrack/handler/puma -e 'puma_rack_handler = defined?(Rackup::Handler::Puma) ? Rackup::Handler::Puma : Rack::Handler::Puma; pp puma_rack_handler'
Rack::Handler::PumaSo the it is the
$ bundle exec ruby -rrackup -rrack/handler/puma -e ''
/Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/gems/3.3.0/gems/puma-6.4.3/lib/rack/handler/puma.rb:126:in `<module:Handler>': undefined method `register' for module Rackup::Handler (NoMethodError)
register :puma, Puma
^^^^^^^^
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/gems/3.3.0/gems/puma-6.4.3/lib/rack/handler/puma.rb:120:in `<module:Rackup>'
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/gems/3.3.0/gems/puma-6.4.3/lib/rack/handler/puma.rb:119:in `<top (required)>'
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `require'
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `block (2 levels) in replace_require'
$ bundle exec ruby -rrackup -rrack/handler/puma -e ''
/Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/gems/3.3.0/gems/rackup-1.0.0/lib/rackup.rb:6:in `require_relative': cannot load such file -- /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/gems/3.3.0/gems/rackup-1.0.0/lib/rackup/handler (LoadError)
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/gems/3.3.0/gems/rackup-1.0.0/lib/rackup.rb:6:in `<top (required)>'
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `require'
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `block (2 levels) in replace_require' |
|
Actually, the code we try to run doesn't matter, it is the |
|
Hehe, so rackup 1.0.0 has been broken for quite some time but anyone using it only with Capybara hasn't noticed, because Capybara wraps $ b e ruby -r rackup -e ''
/Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/gems/3.3.0/gems/rackup-1.0.0/lib/rackup.rb:6:in `require_relative': cannot load such file -- /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/gems/3.3.0/gems/rackup-1.0.0/lib/rackup/handler (LoadError)
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/gems/3.3.0/gems/rackup-1.0.0/lib/rackup.rb:6:in `<top (required)>'
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `require'
from /Users/dentarg/.arm64_rubies/3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75:in `block (2 levels) in replace_require'When that was fixed today, the bug in Puma was uncovered. |
|
Hello, I've tested diowa/ruby3-rails7-bootstrap-heroku#996 against this branch and it's working again Please let me know if I can do something, I will try |
| ensure | ||
| if pid | ||
| if Puma::IS_WINDOWS | ||
| `taskkill /F /PID #{pid}` |
There was a problem hiding this comment.
Can you not use Process.kill?
There was a problem hiding this comment.
I haven't checked for a while, but Windows doesn't handle signals the same as *nix or macos.
I just copied the basic code from test_bin, which I think I wrote. I'll have a look. Might be tomorrow AM (US).
There was a problem hiding this comment.
Thanks. I looked at this, and with possible errors being raised, or extra messages to the CI log, this is probably the easiest way to shut a process down, especially with multiple OS's.
But while looking at it, I noticed that I had a bash command line for IO.popen, so I updated it to work with Windows by using the env parameter for passing RUBYOPT.
We don't test Rack2/Rackup on Windows in CI.
|
I added a test. The test fails on master (rack2 jobs) when the test and Gemfile changes are added to master. Also, all the |
test/test_rack_handler.rb
Outdated
| #io = IO.popen "RUBY_OPT='-rrack/handler' rackup -p 0" | ||
| io = IO.popen "RUBYOPT='-rbundler/setup -rrack/version -rrack/handler -rrackup -rrack/handler/puma' ruby -e 'puts Rackup::VERSION'" |
There was a problem hiding this comment.
Why the commented line? Is it good to have around if debugging this?
There was a problem hiding this comment.
Thanks. No, something I should have removed. Yesterday had a lot of bad disruptions...
As you mentioned above, it's about the requires. Writing tests that fail in master and pass in a PR may require some iterations, which requires switching branches and/or OS's (and moving code between them), etc. Or, more room for missing debugging statements or unneeded comments. Sorry, I've done it before...
Fixed.
1d2bfef to
45d8c1e
Compare
45d8c1e to
37edc2f
Compare
|
Thanks 🙏 |
…a#3532) * lib/rack/handler/puma.rb - fix for rackup v1.0.1 * [CI] test_rack_handler.rb - add `test_rackup1`
|
We decided not to wait for a release and instead changed to use a forked Puma: gem 'puma', github: 'nimbleind/puma', branch: 'fix-rack'Anyone can use this branch if you like. It takes the last release of 6.4.3 and adds only the commit that fixes the rackup issue. This unblocked 12 gem updates for us. Edit: This has been in production statusgator.com for a week without issue so far. A release would be great but we are running just fine with this workaround. |
|
Just one more person asking humbly for a release 🙏🏾 |
|
Is there anything we can do to help get this released? |
There is currently a [bug in Puma](puma/puma#3532) that causes issues with v1.0.1 of rackup.
There is currently a [bug in Puma](puma/puma#3532) that causes issues with v1.0.1 of rackup.
There is currently a [bug in Puma](puma/puma#3532) that causes issues with v1.0.1 of rackup.
There is currently a [bug in Puma](puma/puma#3532) that causes issues with v1.0.1 of rackup.
There is currently a [bug in Puma](puma/puma#3532) that causes issues with v1.0.1 of rackup.
There is currently a [bug in Puma](puma/puma#3532) that causes issues with v1.0.1 of rackup.
Quick heads up that v6.5.0 has been released. 🚀 |
|
Thanks everyone, and apologies for this glitch in the matrix, may they be few and far between! |
|
Thank you! 🙏 ❤️ |
Puma 6.5.0 contains a fix that allows rackup versions higher than 1.0.1 to work. See puma/puma#3532
Puma 6.5.0 contains a fix that allows rackup versions higher than 1.0.1 to work. See puma/puma#3532
Description
This almost falls in the category of "Don't get me started...".
It will break things.
Previous versions of
Rackupalso definedRackup::Handler, and Puma assumed that ifRackupwas defined, so wasRackup::Handler. That is not the case with v1.0.1. All it defines isRackup::VERSION. It's simply a Rack 2 compatible version that makes Gemfile logic simpler. Its gemspec also contains:This PR now checks for the existence of
Rackup::HandlerCloses #3531
Your checklist for this pull request
[ci skip]to the title of the PR.#issue" to the PR description or my commit messages.