System spec failures after upgrading to 1.0.1

[tagliala]

Hello, after this upgrade CI started failing with:

     1.1) Failure/Error: register :puma, Puma
          
          NoMethodError:
            undefined method `register' for Rackup::Handler:Module

I've tried to clone and bisect, but I'm not familiar with this repo, I don't understand how it is supposed to work in local

This is the only change in the code:

diff --git a/Gemfile.lock b/Gemfile.lock
index 7dafccb..79183b0 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -400,7 +400,7 @@ GEM
       rack (< 3)
     rack-test (2.1.0)
       rack (>= 1.3)
-    rackup (1.0.0)
+    rackup (1.0.1)
       rack (< 3)
       webrick
     rails (7.2.1.1)

    puma (6.4.3)
      nio4r (~> 2.0)

    rack (2.2.10)

[ioquatix]

What version of Puma and Rack are you using?

[tagliala]

    puma (6.4.3)
      nio4r (~> 2.0)

    rack (2.2.10)

(Can't upgrade to Rack 3, sorry)

[ioquatix]

It looks like a problem with Puma:

https://github.com/puma/puma/blob/v6.4.3/lib/rack/handler/puma.rb#L117-L141

if Object.const_defined? :Rackup

is probably incorrect.

There should be two separate files, lib/rack/handler/puma.rb and lib/rackup/handler/puma.rb. The former is loaded by Rack v2 and the latter by Rackup v2. Having only one file with the detection logic is, basically, looking incorrect to me.

cc @MSP-Greg

[tagliala]

Thanks, should I report this to Puma?

In the meanwhile, here it is a reproducible test case: diowa/ruby3-rails7-bootstrap-heroku#995

[ioquatix]

Yes, I think we should report this to Puma.

Here is how I do it in Falcon:

https://github.com/socketry/falcon/blob/main/lib/rackup/handler/falcon.rb

It should be easy to adapt.

[tagliala]

Thanks, I'll do that

Apologies for this Small OT:

I've noticed that neither rack nor rackup explicitly set the spec.metadata['rubygems_mfa_required'] = 'true' metadata for multi-factor authentication. While not a requirement for rack itself, given its implicitly required by its high download count, this change display an MFA recommendation in the RubyGems sidebar. This increased visibility could encourage more users to enable MFA and helps with some gem parsers.

Would you accept PRs including this metadata?

Ref:

• https://blog.rubygems.org/2022/08/15/requiring-mfa-on-popular-gems.html
• https://guides.rubygems.org/mfa-requirement-opt-in/

[ioquatix]

Yes, I would accept that PR, thanks for being so gracious about asking.

(I see no reason why we couldn't also add that to rack-session and rack head).

[tagliala]

(I see no reason why we couldn't also add that to rack-session and rack head).

There should be one PRs per repo with (optional?) backports to different branches for each supported version, thanks.

I'll do that

[MSP-Greg]

Seems like a breaking change in 1.0.1.

I believe we can fix it in Puma by changing

if Object.const_defined? :Rackup

to

if Object.const_defined?(:Rackup) && ::Rackup.const_defined?(:Handler)

[ioquatix]

As there is nothing to be done here to fix the issue, I will close it. Thanks for your report!