This project is archived and is in readonly mode.
Fix the gem dependency system in Rails
Reported by David Dollar | March 10th, 2009 @ 02:51 PM
This patch addresses a lot of usability issues in the rake gems:* tasks. They are currently unusable for most situations, and are quite fragile.
-
Fixes the chicken/egg problem present in the current gem system when gems are defined in the config that are not yet installed.
-
Makes the gem system understand development vs. runtime dependencies, removing the need to have hoe as a dependency of your production app.
-
Makes the gem 'unpacking' system a lot less fragile.
-
Significantly cleans up the code associated with the gem system, making it much less opaque.
-
Deprecates the rake tasks gems:install:dependencies and gems:unpack:dependencies. The recursiveness is default in gems:install and gems:unpack now.
Comments and changes to this ticket
-
David Dollar March 10th, 2009 @ 02:52 PM
This change can also be found at:
-
CancelProfileIsBroken March 10th, 2009 @ 02:56 PM
- Assigned user set to Matt Jones
- Tag changed from gems to gems, patch
- Title changed from [PATCH] Fix the gem dependency system in Rails to Fix the gem dependency system in Rails
-
knewter (at gmail) March 10th, 2009 @ 04:34 PM
+1
Saying this patch can't make it into Rails 2.3 is ~ tantamount to saying 'rails 2.3 plans on having a broken gem config system.'
the gem tasks are great, but they'd be 10x better if they worked right today.
-
Pratik March 10th, 2009 @ 04:44 PM
Hey I'll happily push this if Matt gives it a +1. I am just not very keen on making big changes 3 days before the release.
-
José Valim March 10th, 2009 @ 06:33 PM
This looks really good! :)
I just don't agree with this feature:
"Deprecates the rake tasks gems:install:dependencies and gems:unpack:dependencies. The recursiveness is default in gems:install and gems:unpack now."
It would generate a lot of surprises. And I would prefer that it unpack the dependencies only when I ask to. For example, any gem using newgem would unpack a lot of unneeded dependencies on my app.
-
David Dollar March 10th, 2009 @ 06:51 PM
Jose: A lot of your complaint is solved by the fact that the gem system in this patch understands development dependencies so you wouldn't have that gem clutter.
-
David Dollar March 10th, 2009 @ 06:53 PM
Also, gems:install already installed dependencies, since it just farmed its behaviour out to the 'gem' binary which by default installs dependencies.
-
David Dollar March 10th, 2009 @ 07:01 PM
As further argument for making recursiveness the default, gems:unpack is there to make your app resistant to the state of the server it's deployed on. If you don't get the dependencies, unpacking isn't very useful.
-
Matt Jones March 10th, 2009 @ 07:42 PM
I've got a number of issues with this patch, but while I'm writing them up I'll post the show-stopper.
While there are some concerns about building gems that aren't required by the current environment (#1793), this patch completely breaks the most common cases by loading the environment before building gems.
Quick example: - start with a blank Rails 2.3.1 app with git clone in vendor - add "config.gem 'RedCloth'" to environment.rb - rake gems:unpack
After that step, NOTHING works. Executing rake gems:build gets:
(in /Users/mattjones/scratch/rc2app) /Users/mattjones/scratch/rc2app/vendor/gems/RedCloth-4.0.1/lib/redcloth_scan.bundle: [BUG] Bus Error ruby 1.8.6 (2007-03-13) [i686-darwin8.9.1] Abort trap
Note that this is an extreme example - some gems just spit out lots of backtraces if native parts aren't present. (hpricot, for example)
I'll also note that there is a workaround for this: use "rake gems:build" to do the initial unpack. While this does work, it requires that users know that there are native extensions with this problem beforehand. It also breaks the deployment scenario where gems are unpacked but not built in the repository.
I'll post a few more comments shortly, but the issue noted above is the biggest concern.
-
Stephen Touset March 10th, 2009 @ 08:35 PM
What it comes down to is that config.gem has been broken out of the box in new and innovative ways every single release since it's been available.
David's patch makes it work out of the box. Period.
-
Matt Jones March 10th, 2009 @ 08:52 PM
Here's the rundown of the other issues:
-
gems:unpack no longer skips framework gems. Several problems:
- gems that depend on rake will unpack it to vendor/gems. This has potential for creating severe Heisenbugs, where the system-local version of Rake is used for command-line rake, while other uses (Rake::Task[].invoke, for example) from within Rails will use the frozen version.
- gems that depend on Rails will try to unpack Rails gems to vendor/gems. This is BAD. Not only will it NOT WORK, but it will confuse users.
-
gems:unpack is also useful for things like fixing bugs in gems. Removing the GEM= functionality means that's not as practical as before.
-
Failed builds will leave Makefiles behind (for debugging), which will break the test in GemDependency#built? and prevent a second attempt. The Makefiles should be trusted to control what needs to be recompiled.
Other minor issues:
-
There never was an install:dependencies, so deprecating it doesn't make sense.
-
Install shouldn't need to recurse, as the gem command already does (and skips development dependencies)
-
Renaming refresh_specs (to just refresh) invalidates all the old documentation, and obscures the purpose of the task. Naive users might assume that it downloads newer versions of their frozen gems...
-
gem_builder is not needed most of the time; the require was explicitly moved to where it was used in http://github.com/rails/rails/co...
Finally, I'm interested to know what drove this patch - which gems weren't working correctly? Some oddly-packed gems (hoe, for instance) create edge cases that need to be addressed (see #2097). "The system doesn't work" doesn't make it easy to debug...
-
-
David Dollar March 10th, 2009 @ 10:09 PM
I have attached a new patch that addresses your concerns.
As far as what is broken currently (on a checkout of tag 2.3.1):
config.gem 'RedCloth' config.gem 'aws-s3', :lib => 'aws/s3'
results in
[~/Code/RailsApp] rake gems (in /Users/david/Code/RailsApp) - [I] RedCloth - [I] aws-s3 - [I] xml-simple = 1.0.12 - [I] builder = 2.1.2 - [I] mime-types = 1.16 - [ ] archive-tar-minitar ~> 0.5 - [ ] nokogiri ~> 1.2 - [ ] rcov ~> 0.8 - [ ] hoe >= 1.8.3 I = Installed F = Frozen R = Framework (loaded before rails starts) [david@neon.local] [00:02] [~/Code/RailsApp] rake gems:unpack:dependencies (in /Users/david/Code/RailsApp) WARNING: Installing to ~/.gem since /Library/Ruby/Gems/1.8 and /usr/bin aren't both writable. Unpacked gem: '/Users/david/Code/RailsApp/vendor/gems/RedCloth-3.0.4' Unpacked gem: '/Users/david/Code/RailsApp/vendor/gems/aws-s3-0.5.1' Unpacked gem: '/Users/david/Code/RailsApp/vendor/gems/xml-simple-1.0.12' Unpacked gem: '/Users/david/Code/RailsApp/vendor/gems/mime-types-1.16' Unpacked gem: '/Users/david/Code/RailsApp/vendor/gems/rcov-0.8.1.2.0' Unpacked gem: '/Users/david/Code/RailsApp/vendor/gems/hoe-1.8.3' Unpacked gem: '/Users/david/Code/RailsApp/vendor/gems/rubyforge-1.0.2' [david@neon.local] [00:30] [~/Code/RailsApp] rake gems (in /Users/david/Code/RailsApp) - [F] RedCloth - [F] aws-s3 - [F] xml-simple = 1.0.12 - [I] builder = 2.1.2 - [F] mime-types = 1.16 - [ ] archive-tar-minitar ~> 0.5 - [ ] nokogiri ~> 1.2 - [ ] rcov ~> 0.8 - [ ] hoe >= 1.8.3
Notice that things are unpacked and then not recognized as being so. When digging down into the reasons, the gem code is simply not well-architected, so this patch refactors large parts of it.
-
David Dollar March 10th, 2009 @ 10:10 PM
Also the current system is inconsistent in its approach to handling development dependencies.
-
Matt Jones March 11th, 2009 @ 07:41 AM
@DDollar - thanks for the feedback.
Some remaining issues with the patch: - automatic recursion on dependencies is still not always helpful. Example: acts_as_reportable depends on ruport, which in turn depends on fastercsv and pdf-writer. It's not useful to pull in all 3 dependent gems just to patch a bug in acts_as_reportable. If I tell Rails, "unpack gem X", I want it to unpack gem X. unpack:dependencies is a clear way of saying, "Yes, unpack the dependencies".
-
a minor quibble, but the sense of the tests for GEM= for blank inputs has been reversed. Before, it would act on all gems, now it acts on none of the gems. I don't know if anybody uses it that way (maybe in a script?), but it seems like a bad idea to break it.
-
the refactoring changes are nice, but they run into the "public/private API" problem. Changing the interface of Rails::GemDependency this late in the release cycle for aesthetic reasons doesn't seem like a good idea. Again, I can't think of any project using those functions, but why break things?
-
using frozen? instead of directly checking for the existence of the target unpack directory will re-introduce #2097, which can cause unpacking to fail.
-
The GEM= setting needs to be consistently used, for install/unpack/build/refresh_specs.
-
change to frozen? test will fail to detect gems frozen in un-versioned directories (see #533). This isn't technically supported, but again, why break it?
-
the line in GemDependency#specification that alters the version requirement to match an already loaded gem:
version_requirements = Gem::Requirement.create("=#{existing_spec.version}")
..doesn't work anymore. This is a nasty gotcha with attr_accessor - the line above assigns to a local variable rather than calling the version_requirements= method.
While I agree that :development dependencies are not well handled, note that they are very new in Rubygems as well. Gemspec files written by pre-1.3 versions of Rubygems don't correctly indicate development dependencies, as they don't check for the rubygems version correctly. Example from hoe-0.8.2.gemspec:
if s.respond_to? :specification_version then current_version = Gem::Specification::CURRENT_SPECIFICATION_VERSION s.specification_version = 2 if current_version >= 3 then s.add_runtime_dependency(%q<rubyforge>, [">= 1.0.1"]) s.add_runtime_dependency(%q<rake>, [">= 0.8.3"]) else s.add_dependency(%q<rubyforge>, [">= 1.0.1"]) s.add_dependency(%q<rake>, [">= 0.8.3"]) end else s.add_dependency(%q<rubyforge>, [">= 1.0.1"]) s.add_dependency(%q<rake>, [">= 0.8.3"]) end
That current_version >= 3 test won't pass on any released version of Rubygems (as of 1.3.1). The 1.3 series generates a different test based on the Rubygems version explicitly.
HOWEVER, even the new 1.3 test doesn't help Rails, as .specification files are not Ruby code. They are YAMLized from the specs in memory, meaning that they didn't have the :runtime/:development flags when dumped from Rubygems < 1.3 (ie any version of Rails before 2.2.2).
In the light of all that, it is probably better to preserve the existing behavior for 2.3.
The "rake gems" printing code could use some improvements. There are really a few more cases than the current display shows:
-
installed, loaded gem (currently "I")
-
frozen, loaded gem (currently "F")
-
framework gem (currently "R")
-
gem not found (currently " ")
-
gem specified with :lib => false (currently " " as well)
-
runtime dependency not loaded (currently " " as well)
-
development dependency not loaded (" " as well)
As noted above, the last two are still not well supported. What do you think about displaying unloaded gems' status with, say, a lowercase letter? So your output above would now be:
[~/Code/RailsApp] rake gems (in /Users/david/Code/RailsApp) - [F] RedCloth - [F] aws-s3 - [F] xml-simple = 1.0.12 - [I] builder = 2.1.2 - [F] mime-types = 1.16 - [f] archive-tar-minitar ~> 0.5 - [f] nokogiri ~> 1.2 - [f] rcov ~> 0.8 - [f] hoe >= 1.8.3
-
-
David Dollar March 11th, 2009 @ 12:40 PM
While I agree with some of your issues and disagree with others, I'm a bit incredulous at "In the light of all that, it is probably better to preserve the existing behavior for 2.3."
The existing system does not work. Look in my example posted before. Nokogiri never got unpacked. Why? How can I unpack it? The system falls down in so many places that I always threw my hands up in disgust at it until I rewrote it.
Let me say that again. The current system does not work
-
David Dollar March 11th, 2009 @ 01:24 PM
A new patch is attached.
-
automatic recursion on dependencies is still not always helpful.
- Agreed. unpack and unpack:dependencies are now back.
-
a minor quibble, but the sense of the tests for GEM= for blank inputs has been reversed. Before, it would act on all gems, now it acts on none of the gems. I don't know if anybody uses it that way (maybe in a script?), but it seems like a bad idea to break it.
- Are you really suggesting that rake GEM= gems:unpack should unpack all gems? It seems like bikeshedding at this point.
-
The refactoring changes are nice, but they run into the "public/private API" problem. Changing the interface of Rails::GemDependency this late in the release cycle for aesthetic reasons doesn't seem like a good idea. Again, I can't think of any project using those functions, but why break things?
- Because they don't work. Keeping an existing broken system for the sake of keeping an existing system is silly.
-
using frozen? instead of directly checking for the existence of the target unpack directory will re-introduce #2097, which can cause unpacking to fail.
- I think you are referring to the old gem system which checked for loaded? The new code checks frozen? which in turn checks for the existence of the target unpack directory
-
The GEM= setting needs to be consistently used, for install/unpack/build/refresh_specs.
- I agree, and the patch is updated.
-
Change to frozen? test will fail to detect gems frozen in un-versioned directories (see #533). This isn't technically supported, but again, why break it?
- The bug you linked to doesn't have anything to do with the gem system as far as I can tell.
-
-
David Dollar March 11th, 2009 @ 01:27 PM
I've gone ahead and made the change to support GEM= working on all gems. New patch attached.
-
Stephen Bannasch March 11th, 2009 @ 09:15 PM
Here's what my config.gems statements look like in my test rails app:
config.gem 'RedCloth' config.gem 'aws-s3', :lib => 'aws/s3' config.gem "haml", :version => '2.0.9' config.gem "nokogiri"
In rails 2.3.1 edge:
last commit:
commit f2c7508befb085ffe19ec7fb9ca2e6919cc919c9 Author: Russ Smith <russ@bashme.org> Date: Wed Mar 11 12:50:24 2009 -0500 Update bundled Rack to fix Litespeed compatibility [[#2198](/projects/8994/tickets/2198 "Ticket #2198") state:resolved] Signed-off-by: Joshua Peek <josh@joshpeek.com>
I get this behavior freezing and building gems (the development gems are listed but not installed):
[test23 (master)]$ rake gems (in /Users/stephen/dev/ruby/src/gems/test23) - [I] RedCloth - [I] aws-s3 - [I] xml-simple = 1.0.12 - [I] builder = 2.1.2 - [I] mime-types = 1.16 - [ ] archive-tar-minitar ~> 0.5 - [I] nokogiri = 1.2.1 - [ ] rcov ~> 0.8 - [ ] hoe >= 1.8.3 - [I] haml = 2.0.9 - [I] nokogiri I = Installed F = Frozen R = Framework (loaded before rails starts) [test23 (master)]$ rake gems:unpack:dependencies (in /Users/stephen/dev/ruby/src/gems/test23) WARNING: Installing to ~/.gem since /Library/Ruby/Gems/1.8 and /usr/bin aren't both writable. Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/RedCloth-4.1.9' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/aws-s3-0.5.1' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/haml-2.0.9' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/nokogiri-1.2.1' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/xml-simple-1.0.12' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/mime-types-1.16' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/archive-tar-minitar-0.5.2' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/hoe-1.9.0' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/rubyforge-1.0.3' [test23 (master)]$ rake gems:build (in /Users/stephen/dev/ruby/src/gems/test23) Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/archive-tar-minitar-0.5.2' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/aws-s3-0.5.1' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/haml-2.0.9' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/hoe-1.9.0' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/mime-types-1.16' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/nokogiri-1.2.1' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/RedCloth-4.1.9' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/rubyforge-1.0.3' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/xml-simple-1.0.12' [test23 (master)]$ rake gems (in /Users/stephen/dev/ruby/src/gems/test23) - [F] RedCloth - [F] aws-s3 - [F] xml-simple = 1.0.12 - [I] builder = 2.1.2 - [F] mime-types = 1.16 - [ ] archive-tar-minitar ~> 0.5 - [F] nokogiri = 1.2.1 - [ ] rcov ~> 0.8 - [ ] hoe >= 1.8.3 - [F] haml = 2.0.9 - [F] nokogiri I = Installed F = Frozen R = Framework (loaded before rails starts)
With David Dollar's latest patch:
[rails (master)]$ git co gem_dependency Switched to branch "gem_dependency" [rails (gem_dependency)]$ git log commit 15e782dbe0648f10b49073c7fc0562e4a99a5482 Author: David Dollar <ddollar@gmail.com> Date: Tue Mar 10 10:44:24 2009 -0400 Fixes a lot of brokenness in the gems rake tasks. * Fixes the chicken/egg problem present in the current gem system when gems are defined in the config that are not yet installed. * Makes the gem system understand development vs. runtime dependencies, removing the need to have hoe as a dependency of your production app. * Makes the gem 'unpacking' system a lot less fragile. * Significantly cleans up the code associated with the gem system, making it much less opaque. commit f2c7508befb085ffe19ec7fb9ca2e6919cc919c9 Author: Russ Smith <russ@bashme.org> Date: Wed Mar 11 12:50:24 2009 -0500 Update bundled Rack to fix Litespeed compatibility [[#2198](/projects/8994/tickets/2198 "Ticket #2198") state:resolved] Signed-off-by: Joshua Peek <josh@joshpeek.com> [rails (gem_dependency)]$ cd - /Users/stephen/dev/ruby/src/gems/test23 [test23 (master)]$ rm -rf vendor/gems/* [test23 (master)]$ rake gems (in /Users/stephen/dev/ruby/src/gems/test23) - [I] RedCloth - [I] aws-s3 - [I] xml-simple - [R] builder - [I] mime-types - [I] haml = 2.0.9 - [I] nokogiri I = Installed F = Frozen R = Framework (loaded before rails starts) [test23 (master)]$ rake gems:unpack:dependencies (in /Users/stephen/dev/ruby/src/gems/test23) WARNING: Installing to ~/.gem since /Library/Ruby/Gems/1.8 and /usr/bin aren't both writable. Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/RedCloth-4.1.9' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/aws-s3-0.5.1' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/xml-simple-1.0.12' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/mime-types-1.16' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/haml-2.0.9' Unpacked gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/nokogiri-1.2.1' [test23 (master)]$ rake gems:build (in /Users/stephen/dev/ruby/src/gems/test23) Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/RedCloth-4.1.9' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/aws-s3-0.5.1' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/xml-simple-1.0.12' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/mime-types-1.16' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/haml-2.0.9' Built gem: '/Users/stephen/dev/ruby/src/gems/test23/vendor/gems/nokogiri-1.2.1' [test23 (master)]$ rake gems (in /Users/stephen/dev/ruby/src/gems/test23) - [F] RedCloth - [F] aws-s3 - [F] xml-simple - [R] builder - [F] mime-types - [F] haml = 2.0.9 - [F] nokogiri I = Installed F = Frozen R = Framework (loaded before rails starts) [test23 (master)]$ ls vendor/gems/ RedCloth-4.1.9 haml-2.0.9 nokogiri-1.2.1 aws-s3-0.5.1 mime-types-1.16 xml-simple-1.0.12
-
Matt Jones March 12th, 2009 @ 09:42 AM
- Milestone cleared.
A couple things:
-
first off, apologies if I was a little pissy in the previous posts. The 2.3 final is colliding badly with my work schedule; too much coding, not enough sleep.
-
I got the ticket reference wrong above; it should have been #553.
Anyways, I've attached a patch that incorporates a majority of the suggestions presented here. From the patch:
- gems:unpack:dependencies now only unpacks runtime dependencies. Use gems:unpack:all_dependencies to get the old behavior. - gems task now shows status of gems not currently loaded (using lowercase letters). Useful for inspecting dependencies of gems (ruport, for example) that don't load dependencies at startup. - hardened methods in Rails::GemDependency against Gem::Exception from mismatched versions. Install/unpack/etc. should complain and continue. - warn users when trying to unpack gems not installed on the system, rather than silently ignoring them.
Things not included from the previous patches:
-
the unpack task doesn't invoke the install task. This mirrors the operation of rubygems - 'gem unpack' doesn't install gems either.
-
the internal refactorings. As stated before, changing the entire API of GemDependency objects between RC2 and final just isn't practical.
Let me know if you find anything else. Thanks!
-
David Dollar March 12th, 2009 @ 01:38 PM
I actually disagree that unpack should not install first. The analogy to the gem system doesn't really apply. With rake gems:unpack you are trying to unpack the dependencies defined by your application. With gem unpack you are trying to unpack one-and-only-one gem.
I'd argue that 99% of the time, someone that is trying to rake gems:unpack wants all of their config.gem dependencies unpacked, unless they are specifying one with GEM=
It seems to make sense to install the necessary dependencies to achieve what the user is going for.
-
Jordi Bunster March 12th, 2009 @ 02:03 PM
If I may, I think you folks are looking for something like gems:vendor.
You can have gems:install, gems:unpack, gems:unpack:dependencies, etc, all stay the same, and then have gems:vendor do what you want: vendor all you need, recursively.
This is just a suggestion, btw. Not really all that interested in what color we paint it, I just want it to work.
-
Repository March 13th, 2009 @ 10:25 AM
- State changed from new to resolved
(from [99d75a7b02bf430a124b9c3e2515850959d78acf]) Makes the gem system understand development vs. runtime dependencies [#2195 state:resolved]
The patch also fixes:
-
Fixes the chicken/egg problem present in the current gem system when gems are defined in the config that are not yet installed.
-
Remove the need to have hoe as a dependency of your production app.
-
Makes the gem 'unpacking' system a lot less fragile.
Signed-off-by: Matt Jones al2o3cr@gmail.com Signed-off-by: Pratik Naik pratiknaik@gmail.com http://github.com/rails/rails/co...
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 2195 Fix the gem dependency system in Rails (from [99d75a7b02bf430a124b9c3e2515850959d78acf]) Makes t...