Re: [PATCH] limit rack version for ruby compatibility

Re: [PATCH] limit rack version for ruby compatibility

[Eric Wong]

Adam Duke <adamduke@twitter.com> wrote:
> From: Adam Duke <adam.v.duke@gmail.com>
> Date: Fri, 8 Jan 2016 13:06:31 -0500
> Subject: [PATCH] limit rack version for ruby compatibility
> 
> rack introduced a dependency on ruby 2.2.2 or greater in
> https://github.com/rack/rack/commit/771d94e5dbe53058160a1f8a4cc56384c1d2a048

Cc-ing rack-devel + Aaron

Yikes!  ruby-core still supports Ruby 2.1 and possibly even 2.0.0

And there doesn't seem to be any documentation on why Ruby 2.2.x
is needed in the first place for rack.git
commit a2fe30a5e70371c89c1b29fdc2dc5f8027bc5fe6

	http://bogomips.org/mirrors/rack.git/patch?id=a2fe30a5e70371c8

Aaron?

> In order to maintain support for ruby versions less than 2.2.2, limit
> the rack dependency to supported versions for the current ruby.
> ---
>  unicorn.gemspec | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/unicorn.gemspec b/unicorn.gemspec
> index 1099361..ce7080a 100644
> --- a/unicorn.gemspec
> +++ b/unicorn.gemspec
> @@ -35,7 +35,11 @@
>    # up/downgrade to any other version, the Rack dependency may be
>    # commented out.  Nevertheless, upgrading to Rails 2.3.4 or later is
>    # *strongly* recommended for security reasons.
> -  s.add_dependency(%q<rack>)
> +  if RUBY_VERSION < '2.2.2'
> +    s.add_dependency(%q<rack>, '~> 1.6.4')
> +  else
> +    s.add_dependency(%q<rack>)
> +  end

Interesting, I built a gem with RubyGems 2.5.1 and this conditional
was preserved in the gemspec.  I tried this in the past (2009/2010?)
and any conditionals written like this got clobbered in the final
gemspec.

In other words, conditionals used to be evaluated at "gem build" time,
not "gem install" time.  We should check when this improvement was
introduced into RubyGems should we go this route.

Also, maybe '~> 1.6.4' is too strict, '~> 1.6' could be better in case
a rack 1.7 comes out in parallel to rack 2.0

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] limit rack version for ruby compatibility

[Aaron Patterson]

[-- Attachment #1: Type: text/plain, Size: 3059 bytes --]

On Fri, Jan 08, 2016 at 07:18:07PM +0000, Eric Wong wrote:
> Adam Duke <adamduke@twitter.com> wrote:
> > From: Adam Duke <adam.v.duke@gmail.com>
> > Date: Fri, 8 Jan 2016 13:06:31 -0500
> > Subject: [PATCH] limit rack version for ruby compatibility
> > 
> > rack introduced a dependency on ruby 2.2.2 or greater in
> > https://github.com/rack/rack/commit/771d94e5dbe53058160a1f8a4cc56384c1d2a048
> 
> Cc-ing rack-devel + Aaron
> 
> Yikes!  ruby-core still supports Ruby 2.1 and possibly even 2.0.0
> 
> And there doesn't seem to be any documentation on why Ruby 2.2.x
> is needed in the first place for rack.git
> commit a2fe30a5e70371c89c1b29fdc2dc5f8027bc5fe6
> 
> 	http://bogomips.org/mirrors/rack.git/patch?id=a2fe30a5e70371c8
> 
> Aaron?

The main reason I bumped it up to Ruby 2.2.x is because that will be the
minimum version of Ruby I'll be stuck with throughout Rack 2.x's
lifetime.  IOW, I can't drop Ruby versions in anything but a major
release so I'm being conservative and only going with the latest (at the
time that was 2.2).

I could be convinced to bring down the version number, but I'd like to
know why first. :)

> > In order to maintain support for ruby versions less than 2.2.2, limit
> > the rack dependency to supported versions for the current ruby.
> > ---
> >  unicorn.gemspec | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/unicorn.gemspec b/unicorn.gemspec
> > index 1099361..ce7080a 100644
> > --- a/unicorn.gemspec
> > +++ b/unicorn.gemspec
> > @@ -35,7 +35,11 @@
> >    # up/downgrade to any other version, the Rack dependency may be
> >    # commented out.  Nevertheless, upgrading to Rails 2.3.4 or later is
> >    # *strongly* recommended for security reasons.
> > -  s.add_dependency(%q<rack>)
> > +  if RUBY_VERSION < '2.2.2'
> > +    s.add_dependency(%q<rack>, '~> 1.6.4')
> > +  else
> > +    s.add_dependency(%q<rack>)
> > +  end
> 
> Interesting, I built a gem with RubyGems 2.5.1 and this conditional
> was preserved in the gemspec.  I tried this in the past (2009/2010?)
> and any conditionals written like this got clobbered in the final
> gemspec.

I wonder if that's true even after you upload to rubygems.org.  I'd
guess it's not true as they don't want to support arbitrary ruby code
for specs.

> In other words, conditionals used to be evaluated at "gem build" time,
> not "gem install" time.  We should check when this improvement was
> introduced into RubyGems should we go this route.
> 
> Also, maybe '~> 1.6.4' is too strict, '~> 1.6' could be better in case
> a rack 1.7 comes out in parallel to rack 2.0

Agree here.  1.7 may be possible, and I want to make the guarantee that
its API is backwards compatible with 1.6.

-- 
Aaron Patterson
http://tenderlovemaking.com/

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH] limit rack version for ruby compatibility

[Aaron Patterson]

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

On Fri, Jan 08, 2016 at 01:50:46PM -0800, Aaron Patterson wrote:
> On Fri, Jan 08, 2016 at 07:18:07PM +0000, Eric Wong wrote:
> > Adam Duke <adamduke@twitter.com> wrote:
> > > From: Adam Duke <adam.v.duke@gmail.com>
> > > Date: Fri, 8 Jan 2016 13:06:31 -0500
> > > Subject: [PATCH] limit rack version for ruby compatibility
> > > 
> > > rack introduced a dependency on ruby 2.2.2 or greater in
> > > https://github.com/rack/rack/commit/771d94e5dbe53058160a1f8a4cc56384c1d2a048
> > 
> > Cc-ing rack-devel + Aaron
> > 
> > Yikes!  ruby-core still supports Ruby 2.1 and possibly even 2.0.0
> > 
> > And there doesn't seem to be any documentation on why Ruby 2.2.x
> > is needed in the first place for rack.git
> > commit a2fe30a5e70371c89c1b29fdc2dc5f8027bc5fe6
> > 
> > 	http://bogomips.org/mirrors/rack.git/patch?id=a2fe30a5e70371c8
> > 
> > Aaron?
> 
> The main reason I bumped it up to Ruby 2.2.x is because that will be the
> minimum version of Ruby I'll be stuck with throughout Rack 2.x's
> lifetime.  IOW, I can't drop Ruby versions in anything but a major
> release so I'm being conservative and only going with the latest (at the
> time that was 2.2).
> 
> I could be convinced to bring down the version number, but I'd like to
> know why first. :)

Oh, I forgot to mention that I don't mind eliminating the Ruby version
requirement as long as we put something in the README that says we only
guarantee it works on 2.2.x and up.  Older versions could be "best
effort".  I'm just afraid to do something like that because I really
don't want to maintain 1.8 and 1.9 baggage (for example).  I used the
gemspec to clearly announce the Ruby versions I actually test with.

-- 
Aaron Patterson
http://tenderlovemaking.com/

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH] limit rack version for ruby compatibility

[Aaron Patterson]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

On Fri, Jan 08, 2016 at 05:13:19PM -0500, Adam Duke wrote:
> Is it reasonable to assume that any rack release that includes bumping
> the ruby requirement to 2.2.2 would require a major version bump of
> rack?
> 
> The dependency in the unicorn gemspec could be as simple as '< 2' if
> that is the case.

Not totally sure if I follow what you're saying, but yes, bumping the
Ruby version will always necessitate bumping the major version of Rack.
IOW, Rack 2.X will support Ruby 2.2.2 and up indefinitely, and dropping
any version of Ruby would require bumping Rack to 3.X.

-- 
Aaron Patterson
http://tenderlovemaking.com/

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH] limit rack version for ruby compatibility

[Eric Wong]

Aaron Patterson <tenderlove@ruby-lang.org> wrote:
> The main reason I bumped it up to Ruby 2.2.x is because that will be the
> minimum version of Ruby I'll be stuck with throughout Rack 2.x's
> lifetime.  IOW, I can't drop Ruby versions in anything but a major
> release so I'm being conservative and only going with the latest (at the
> time that was 2.2).
> 
> I could be convinced to bring down the version number, but I'd like to
> know why first. :)

Because other people are _always_ slow to upgrade :)

However, I suppose it's fine to bring the requirement up with a
major version bump of Rack.  I don't want to burden you with
old cruft, either.

unicorn may also be able to drop the dependency on rack by
lazy loading:

* Rack::Utils::HTTP_STATUS_CODES is the main thing we use from
  Rack at runtime; and unicorn would actually function fine if
  the hash were empty; HTTP status lines would just be short
  and non-descriptive.

* The Rack::Builder dependency can be optional, even.

Fwiw, I plan to support Rack 1.x and Ruby 1.9.3 under unicorn for a few
more years because of LTS distros.  New versions take priority, of
course.

> On Fri, Jan 08, 2016 at 07:18:07PM +0000, Eric Wong wrote:
> > Adam Duke <adamduke@twitter.com> wrote:
> > > +++ b/unicorn.gemspec
> > > @@ -35,7 +35,11 @@
> > >    # up/downgrade to any other version, the Rack dependency may be
> > >    # commented out.  Nevertheless, upgrading to Rails 2.3.4 or later is
> > >    # *strongly* recommended for security reasons.
> > > -  s.add_dependency(%q<rack>)
> > > +  if RUBY_VERSION < '2.2.2'
> > > +    s.add_dependency(%q<rack>, '~> 1.6.4')
> > > +  else
> > > +    s.add_dependency(%q<rack>)
> > > +  end
> > 
> > Interesting, I built a gem with RubyGems 2.5.1 and this conditional
> > was preserved in the gemspec.  I tried this in the past (2009/2010?)
> > and any conditionals written like this got clobbered in the final
> > gemspec.
> 
> I wonder if that's true even after you upload to rubygems.org.  I'd
> guess it's not true as they don't want to support arbitrary ruby code
> for specs.

Ah, you're right.  I was looking at the gemspec which is distributed
with the gem source and not the regenerated gemspec which RubyGems
actually uses.

So yeah, it looks like Adam's patch only affects the gem build process.

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] limit rack version for ruby compatibility

[Aaron Patterson]

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

On Fri, Jan 08, 2016 at 10:37:32PM +0000, Eric Wong wrote:
> Aaron Patterson <tenderlove@ruby-lang.org> wrote:
> > The main reason I bumped it up to Ruby 2.2.x is because that will be the
> > minimum version of Ruby I'll be stuck with throughout Rack 2.x's
> > lifetime.  IOW, I can't drop Ruby versions in anything but a major
> > release so I'm being conservative and only going with the latest (at the
> > time that was 2.2).
> > 
> > I could be convinced to bring down the version number, but I'd like to
> > know why first. :)
> 
> Because other people are _always_ slow to upgrade :)

Yes, exactly. I am betting that by the time people upgrade to Rack 2.0,
Ruby 2.2.2 will be old hat (Ruby 2.3 has been released already!)  ;)

> However, I suppose it's fine to bring the requirement up with a
> major version bump of Rack.  I don't want to burden you with
> old cruft, either.
> 
> unicorn may also be able to drop the dependency on rack by
> lazy loading:
> 
> * Rack::Utils::HTTP_STATUS_CODES is the main thing we use from
>   Rack at runtime; and unicorn would actually function fine if
>   the hash were empty; HTTP status lines would just be short
>   and non-descriptive.
> 
> * The Rack::Builder dependency can be optional, even.
> 
> Fwiw, I plan to support Rack 1.x and Ruby 1.9.3 under unicorn for a few
> more years because of LTS distros.  New versions take priority, of
> course.

Ok.  Let me know if there's anything I can do to help.  Removing the
strict requirement from the gemspec *is* on the table, as long as we
document the supported versions in the README.  I don't plan on using
anything that would be specific to Ruby 2.2.2 and up, but I don't want
to be burdened by older ones either.  A simple comment in the README
would suffice.

-- 
Aaron Patterson
http://tenderlovemaking.com/

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH] limit rack version for ruby compatibility

[Eric Wong]

Adam Duke <adamduke@twitter.com> wrote:
> Following up on this, it seems to me like keeping the Ruby 2.2.2
> requirement is the right way to go for rack. If the unicorn project
> wants to continue support for older rubies, the unicorn gemspec should
> be changed to limit the rack dependency to '< 2'. If rack 2.0.0 is
> released and there is no limit on the dependency in unicorn's gemspec,
> it seems to me like any deployments that are not running Ruby 2.2.2
> will fail.

I prefer we drop the rack dependency entirely.  We don't use rack
for much.  This seems doable, (totally untested) patch below:

diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index b0e6bd1..a3eae5e 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -1,9 +1,14 @@
 # -*- encoding: binary -*-
 require 'etc'
 require 'stringio'
-require 'rack'
 require 'kgio'
 
+begin
+  require 'rack'
+rescue LoadError
+  warn 'rack not available, functionality reduced'
+end
+
 # :stopdoc:
 # Unicorn module containing all of the classes (include C extensions) for
 # running a Unicorn web server.  It contains a minimalist HTTP server with just
@@ -32,6 +37,9 @@ module Unicorn
   def self.builder(ru, op)
     # allow Configurator to parse cli switches embedded in the ru file
     op = Unicorn::Configurator::RACKUP.merge!(:file => ru, :optparse => op)
+    if ru =~ /\.ru$/ && !defined?(Rack::Builder)
+      abort "rack and Rack::Builder must be available for processing #{ru}"
+    end
 
     # Op is going to get cleared before the returned lambda is called, so
     # save this value so that it's still there when we need it:
@@ -53,32 +61,33 @@ def self.builder(ru, op)
 
       return inner_app if no_default_middleware
 
+      middleware = { # order matters
+        ContentLength: [],
+        Chunked: [],
+        CommonLogger: [ $stderr ],
+        ShowExceptions: [],
+        Lint: [],
+        TempfileReaper: []
+      }
+
       # return value, matches rackup defaults based on env
       # Unicorn does not support persistent connections, but Rainbows!
       # and Zbatery both do.  Users accustomed to the Rack::Server default
       # middlewares will need ContentLength/Chunked middlewares.
       case ENV["RACK_ENV"]
       when "development"
-        Rack::Builder.new do
-          use Rack::ContentLength
-          use Rack::Chunked
-          use Rack::CommonLogger, $stderr
-          use Rack::ShowExceptions
-          use Rack::Lint
-          use Rack::TempfileReaper if Rack.const_defined?(:TempfileReaper)
-          run inner_app
-        end.to_app
       when "deployment"
-        Rack::Builder.new do
-          use Rack::ContentLength
-          use Rack::Chunked
-          use Rack::CommonLogger, $stderr
-          use Rack::TempfileReaper if Rack.const_defined?(:TempfileReaper)
-          run inner_app
-        end.to_app
+        middleware.delete(:ShowExceptions)
+        middleware.delete(:Lint)
       else
-        inner_app
+        return inner_app
       end
+      Rack::Builder.new do
+        middleware.each do |m, args|
+          use(Rack.const_get(m), *args) if Rack.const_defined?(m)
+        end
+        run inner_app
+      end.to_app
     end
   end
 
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index 7b446c2..ec128e4 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -10,10 +10,13 @@
 # is the job of Rack, with the exception of the "Date" and "Status" header.
 module Unicorn::HttpResponse
 
+  STATUS_CODES = defined?(Rack::Utils::HTTP_STATUS_CODES) ?
+                 Rack::Utils::HTTP_STATUS_CODES : {}
+
   # internal API, code will always be common-enough-for-even-old-Rack
   def err_response(code, response_start_sent)
     "#{response_start_sent ? '' : 'HTTP/1.1 '}" \
-      "#{code} #{Rack::Utils::HTTP_STATUS_CODES[code]}\r\n\r\n"
+      "#{code} #{STATUS_CODES[code]}\r\n\r\n"
   end
 
   # writes the rack_response to socket as an HTTP response
@@ -23,7 +26,7 @@ def http_response_write(socket, status, headers, body,
 
     if headers
       code = status.to_i
-      msg = Rack::Utils::HTTP_STATUS_CODES[code]
+      msg = STATUS_CODES[code]
       start = response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
       buf = "#{start}#{msg ? %Q(#{code} #{msg}) : status}\r\n" \
             "Date: #{httpdate}\r\n" \
diff --git a/unicorn.gemspec b/unicorn.gemspec
index 2728373..c3f8267 100644
--- a/unicorn.gemspec
+++ b/unicorn.gemspec
@@ -31,11 +31,11 @@
   # version requirements here.
   s.required_ruby_version = '< 3.0'
 
-  # for people that are absolutely stuck on Rails 2.3.2 and can't
-  # up/downgrade to any other version, the Rack dependency may be
-  # commented out.  Nevertheless, upgrading to Rails 2.3.4 or later is
-  # *strongly* recommended for security reasons.
-  s.add_dependency(%q<rack>)
+  # We do not have a hard dependency on rack, it's possible to load
+  # things which respond to #call.  HTTP status lines in responses
+  # won't have descriptive text, only the numeric status.
+  # s.add_dependency(%q<rack>)
+
   s.add_dependency(%q<kgio>, '~> 2.6')
   s.add_dependency(%q<raindrops>, '~> 0.7')

...  Not touching the untested, ancient old_rails stuff, yet

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] limit rack version for ruby compatibility

[Aaron Patterson]

On Thu, Jan 21, 2016 at 08:12:55PM +0000, Eric Wong wrote:
> Adam Duke <adamduke@twitter.com> wrote:
> > Following up on this, it seems to me like keeping the Ruby 2.2.2
> > requirement is the right way to go for rack. If the unicorn project
> > wants to continue support for older rubies, the unicorn gemspec should
> > be changed to limit the rack dependency to '< 2'. If rack 2.0.0 is
> > released and there is no limit on the dependency in unicorn's gemspec,
> > it seems to me like any deployments that are not running Ruby 2.2.2
> > will fail.
> 
> I prefer we drop the rack dependency entirely.  We don't use rack
> for much.  This seems doable, (totally untested) patch below:

[snip]

Ah, that was a much smaller patch than I had anticipated! :D

-- 
Aaron Patterson
http://tenderlovemaking.com/

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.