unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / Atom feed
* [PATCH] limit rack version for ruby compatibility
@ 2016-01-08 18:34 Adam Duke
  2016-01-08 19:18 ` Eric Wong
  0 siblings, 1 reply; 12+ messages in thread
From: Adam Duke @ 2016-01-08 18:34 UTC (permalink / raw)
  To: unicorn-public

From 6f1cb0ae4b63bd1906fd83d154dae1d1f2b35407 Mon Sep 17 00:00:00 2001
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

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
   s.add_dependency(%q<kgio>, '~> 2.6')
   s.add_dependency(%q<raindrops>, '~> 0.7')

-- 
2.6.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-08 18:34 [PATCH] limit rack version for ruby compatibility Adam Duke
@ 2016-01-08 19:18 ` Eric Wong
  2016-01-08 21:50   ` Aaron Patterson
  0 siblings, 1 reply; 12+ messages in thread
From: Eric Wong @ 2016-01-08 19:18 UTC (permalink / raw)
  To: Adam Duke; +Cc: unicorn-public, Aaron Patterson, rack-devel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-08 19:18 ` Eric Wong
@ 2016-01-08 21:50   ` Aaron Patterson
  2016-01-08 21:56     ` Aaron Patterson
  2016-01-08 22:37     ` Eric Wong
  0 siblings, 2 replies; 12+ messages in thread
From: Aaron Patterson @ 2016-01-08 21:50 UTC (permalink / raw)
  To: rack-devel; +Cc: Adam Duke, unicorn-public

[-- Attachment #1: Type: text/plain, Size: 2839 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/


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-08 21:50   ` Aaron Patterson
@ 2016-01-08 21:56     ` Aaron Patterson
  2016-01-08 22:13       ` Adam Duke
  2016-01-08 22:37     ` Eric Wong
  1 sibling, 1 reply; 12+ messages in thread
From: Aaron Patterson @ 2016-01-08 21:56 UTC (permalink / raw)
  To: Aaron Patterson; +Cc: rack-devel, Adam Duke, unicorn-public

[-- Attachment #1: Type: text/plain, Size: 1767 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/


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-08 21:56     ` Aaron Patterson
@ 2016-01-08 22:13       ` Adam Duke
  2016-01-08 22:17         ` Aaron Patterson
  0 siblings, 1 reply; 12+ messages in thread
From: Adam Duke @ 2016-01-08 22:13 UTC (permalink / raw)
  To: Aaron Patterson; +Cc: rack-devel, unicorn-public

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.

On Fri, Jan 8, 2016 at 4:56 PM, Aaron Patterson
<tenderlove@ruby-lang.org> wrote:
> 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/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-08 22:13       ` Adam Duke
@ 2016-01-08 22:17         ` Aaron Patterson
  0 siblings, 0 replies; 12+ messages in thread
From: Aaron Patterson @ 2016-01-08 22:17 UTC (permalink / raw)
  To: Adam Duke; +Cc: Aaron Patterson, rack-devel, unicorn-public

[-- Attachment #1: Type: text/plain, Size: 642 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/


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-08 21:50   ` Aaron Patterson
  2016-01-08 21:56     ` Aaron Patterson
@ 2016-01-08 22:37     ` Eric Wong
  2016-01-08 23:19       ` Aaron Patterson
  1 sibling, 1 reply; 12+ messages in thread
From: Eric Wong @ 2016-01-08 22:37 UTC (permalink / raw)
  To: Aaron Patterson; +Cc: rack-devel, Adam Duke, unicorn-public

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-08 22:37     ` Eric Wong
@ 2016-01-08 23:19       ` Aaron Patterson
  2016-01-21 17:12         ` Adam Duke
  0 siblings, 1 reply; 12+ messages in thread
From: Aaron Patterson @ 2016-01-08 23:19 UTC (permalink / raw)
  To: rack-devel; +Cc: Aaron Patterson, Adam Duke, unicorn-public

[-- Attachment #1: Type: text/plain, Size: 1859 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/


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-08 23:19       ` Aaron Patterson
@ 2016-01-21 17:12         ` Adam Duke
  2016-01-21 20:12           ` Eric Wong
  0 siblings, 1 reply; 12+ messages in thread
From: Adam Duke @ 2016-01-21 17:12 UTC (permalink / raw)
  To: Aaron Patterson; +Cc: rack-devel, unicorn-public

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.

From 2f3b39edb5d477e0efcbe5ce55af37ddea289e9e Mon Sep 17 00:00:00 2001
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

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/unicorn.gemspec b/unicorn.gemspec
index 1099361..16607ac 100644
--- a/unicorn.gemspec
+++ b/unicorn.gemspec
@@ -35,7 +35,7 @@
   # 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>)
+  s.add_dependency(%q<rack>, '< 2')
   s.add_dependency(%q<kgio>, '~> 2.6')
   s.add_dependency(%q<raindrops>, '~> 0.7')

-- 
2.6.4

On Fri, Jan 8, 2016 at 6:19 PM, Aaron Patterson
<tenderlove@ruby-lang.org> wrote:
> 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/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-21 17:12         ` Adam Duke
@ 2016-01-21 20:12           ` Eric Wong
  2016-01-21 22:09             ` Aaron Patterson
  2016-01-27  0:47             ` Eric Wong
  0 siblings, 2 replies; 12+ messages in thread
From: Eric Wong @ 2016-01-21 20:12 UTC (permalink / raw)
  To: Adam Duke; +Cc: Aaron Patterson, rack-devel, unicorn-public

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-21 20:12           ` Eric Wong
@ 2016-01-21 22:09             ` Aaron Patterson
  2016-01-27  0:47             ` Eric Wong
  1 sibling, 0 replies; 12+ messages in thread
From: Aaron Patterson @ 2016-01-21 22:09 UTC (permalink / raw)
  To: rack-devel; +Cc: Adam Duke, Aaron Patterson, unicorn-public

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/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] limit rack version for ruby compatibility
  2016-01-21 20:12           ` Eric Wong
  2016-01-21 22:09             ` Aaron Patterson
@ 2016-01-27  0:47             ` Eric Wong
  1 sibling, 0 replies; 12+ messages in thread
From: Eric Wong @ 2016-01-27  0:47 UTC (permalink / raw)
  To: unicorn-public; +Cc: Adam Duke, Aaron Patterson

Eric Wong <e@80x24.org> wrote:
> I prefer we drop the rack dependency entirely.  We don't use rack
> for much.  This seems doable, (totally untested) patch below:

(removed rack-devel from Cc:)

Tested and pushed to unicorn.git master with minor changes
as commit 3d69a6f064078eeb28c1819725d3715ce6905374

* use nil instead of '[]' for empty splat to reduce garbage
* rack is a development dependency, not removed as a dep entirely

    http://bogomips.org/unicorn.git/patch?id=3d69a6f064078

Subject: rack is optional at runtime, required for dev

We do not want to pull in a newer or older version of rack depending
on an the application running under it requires.  Furthermore, it
has always been possible to use unicorn without any middleware at
all.

Without rack, we'll be missing descriptive status text in the first
response line, but any valid HTTP/1.x parser should be able to
handle it properly.

ref:
 http://bogomips.org/unicorn-public/20160121201255.GA6186@dcvr.yhbt.net/t/#u

Thanks-to: Adam Duke <adam.v.duke@gmail.com>
Thanks-to: Aaron Patterson <tenderlove@ruby-lang.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-08 18:34 [PATCH] limit rack version for ruby compatibility Adam Duke
2016-01-08 19:18 ` Eric Wong
2016-01-08 21:50   ` Aaron Patterson
2016-01-08 21:56     ` Aaron Patterson
2016-01-08 22:13       ` Adam Duke
2016-01-08 22:17         ` Aaron Patterson
2016-01-08 22:37     ` Eric Wong
2016-01-08 23:19       ` Aaron Patterson
2016-01-21 17:12         ` Adam Duke
2016-01-21 20:12           ` Eric Wong
2016-01-21 22:09             ` Aaron Patterson
2016-01-27  0:47             ` Eric Wong

unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://bogomips.org/unicorn-public
	git clone --mirror http://ou63pmih66umazou.onion/unicorn-public

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.unicorn

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox