Skip to content

Restore Ruby 1.9.1 compatibility #173

Merged
merged 1 commit into from May 25, 2011

4 participants

@sferik
sferik commented May 25, 2011

The latest Rack release (1.3.0) breaks on Ruby 1.9.1 with the error:

NoMethodError: undefined method `decode_www_form_component' for URI:Module

Apparently, URI.encode_www_form_component and URI.encode_www_form_component weren't added until Ruby 1.9.2.

This patch restores Ruby 1.9.1 compatibility.

@sferik sferik Restore Ruby 1.9.1 compatibility
URI.decode_www_form_component and URI.encode_www_form_component were not
added until Ruby 1.9.2.
f043f32
@tenderlove
Official Rack repositories member

Are we sure we want to do this? Even ruby-core doesn't support 1.9.1. I'd hate to advertise support for such a buggy ruby.

@sferik
sferik commented May 25, 2011

Despite the fact that Ruby 1.9.1 is not supported by Ruby Core, people are still using it to run production applications. I'm not convinced that the release of Rack 1.3.0 should be the forcing function for them to upgrade to 1.9.2.

Updating a system's version of Ruby can be difficult with multiple applications and scripts running on the same box. On some hosts, users don't even have permission to do so.

@tenderlove
Official Rack repositories member

@sferik The same argument could be made about any version of Ruby. People still run 1.8.2 in production, but is it worth our time to jump through hoops to support it?

@sferik
sferik commented May 25, 2011

@tenderlove If supporting Ruby 1.9.1 in Rack required jumping through hoops or sacrificing performance, I think you could make a compelling argument against it, but this seems more like an oversight.

Please keep in mind that this decision has implications beyond Rack itself. By dropping support for Ruby 1.9.1 in Rack, you're also dropping support for Ruby 1.9.1 in over 500 gems that list Rack as a dependency (or, more precisely, forcing them to lock Rack to version 1.2.x).

If you want to drop support for Ruby 1.9.1 (or Ruby 1.8.6) in Rack, by all means start a discussion about it that involves the Rack community. In this case, dropping support seemed more like an oversight than a conscious decision, which is why I thought it would be helpful to offer this patch.

It would make things a lot clearer if this gem had a Requirements section in the README (see Sinatra's README for a good example) that explicitly states which versions of Ruby are supported. Without that, how am I supposed to know whether a patch like this is appropriate or not?

Are there some additional hoops I'm not aware of that are required to support Ruby 1.9.1?

@sferik
sferik commented May 25, 2011

One additional question: Does Rack aspire to adhere to Semantic Versioning?

@raggi
Official Rack repositories member
raggi commented May 25, 2011

Dude, don't ever say "by all means start a discussion about it that involves the Rack community" when I put out the beta weeks before release and no one had the presence of mind to even try it.

@raggi raggi merged commit f42d003 into rack:master May 25, 2011
@tenderlove
Official Rack repositories member

@sferik I agree, I think we need to be more clear in the README for sure. I don't really have time to support rubies besides 1.8.7, 1.9.2, and 1.9.3 (since those are what we use at work). I think it would be awesome if we could get someone on the rack team (like you) that would support other versions.

As for community appeal, I think we could go by rubygems.org download stats. Here are some numbers I could track down:

[aaron@higgins Downloads]$ gzcat gemcutter_org_access_log.1.gz | grep -o 'Ruby/1\.[8,9]\.[0-9]*' | sort | uniq -c
 884 Ruby/1.8.4
75293 Ruby/1.8.5
640145 Ruby/1.8.6
10780321 Ruby/1.8.7
2229 Ruby/1.8.8
5220 Ruby/1.9.0
106098 Ruby/1.9.1
3982276 Ruby/1.9.2
13364 Ruby/1.9.3
[aaron@higgins Downloads]$ gzcat gemcutter_org_access_log.1.gz | head -1
184.57.2.113 - - [08/May/2011:02:23:15 -0400] "HEAD /yaml HTTP/1.1" 302 - "-" "-"
[aaron@higgins Downloads]$ gzcat gemcutter_org_access_log.1.gz | tail -1
174.129.187.249 - - [15/May/2011:02:22:58 -0400] "GET /specs.4.8.gz HTTP/1.1" 302 - "-" "RubyGems/1.3.7 x86_64-linux Ruby/1.8.7 (2010-04-19 patchlevel 253)"
[aaron@higgins Downloads]$ ls -alh gemcutter_org_access_log.1.gz 
-rw-r--r--@ 1 aaron  staff   284M May 25 11:16 gemcutter_org_access_log.1.gz
[aaron@higgins Downloads]$ gzcat gemcutter_org_access_log.1.gz | wc -l
 20462297
[aaron@higgins Downloads]$

zomg graph

I pulled 1.9.3 and 1.8.8, since they are not released yet.

@sferik
sferik commented May 25, 2011

@raggi I appreciate you releasing a beta gem but that's very different from publicly declaring your intent to break backwards compatibility. From what I have gathered, this release was not an intentional attempt to break backwards compatibility, it was an accident, and the code on line 9 of lib/rack/utils.rb is a bug.

This is the sequence of relevant events, as I see them:

  1. A gem was released that accidentally removed support for Ruby 1.9.1
  2. A patch was offered to restore support for Ruby 1.9.1
  3. That patch was not applied because "Ruby 1.9.1 isn't supported"

I'm not saying you have to support Ruby 1.9.1, but it would be nice if:

  1. That was a conscious decision, instead of an accident
  2. That fact was stated in the README

Since I maintain gems that depend on Rack, I will stop testing them against Ruby 1.9.1 and will stop telling users of my gems that they support Ruby 1.9.1. Until recently, I was under the impression that Ruby 1.9.1 was supported because all of my tests passed and it wasn't stated otherwise.

Publicly stating that you don't support Ruby 1.9.1 would also stop me from wasting my time submitting patches such as this one and would save you from having to engage in discussions such as this one.

@sferik
sferik commented May 25, 2011

@raggi Thanks for merging this pull request. I feel bad that we keep butting heads on these issues. I really appreciate all the work you put into Rack. I'm raising these issues because I, too, care about Rack and want it to be awesome!

@tenderlove Thanks for posting those stats. I would be honored to accept responsibility for maintaining Rack compatibility with additional Ruby implementations including MRI 1.8.6, MRI 1.9.1, REE, Rubinius, and JRuby. Let's figure out which versions Rack actually wants to support and make an announcement on the README. We could also modify update the Gem::Specification#required_ruby_version accordingly. The official Continuous Integration server should also be updated to include all officially supported versions (and we should link to the CI server from the README).

@sferik
sferik commented Jun 24, 2011

Any idea when version 1.3.1 might be released? It's been about a month since this patch was merged and there seem to be a number of additional changes in master since 1.3.0 was released: 1.3.0...master /cc @rkh

@rkh
Official Rack repositories member
rkh commented Jun 24, 2011

I already backported this and other commits to the 1.3 branch. I can prep everything else (changes, announcement), but I don't have the permission to push a new gem.

@sideci-sample sideci-sample pushed a commit to sideci-sample/sideci-sample-omniauth that referenced this pull request Apr 23, 2014
@sferik sferik Don't test against Ruby 1.9.1 until Rack restores compatibility 9ddad05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.