Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  J.D. Hollis June 24th, 2009 @ 07:15 PM

  Ok, did some digging. For some reason, rmagick is getting added to the list of @@framework_gems in GemDependency when I run 'rake test'. Best I can tell, this is because rmagick is loaded before any of the gem dependencies. Because framework_gem? returns true for rmagick, when GemDependency#add_load_paths is called for rmagick, it marks the gem as frozen (which it isn't). GemDependency#frozen? is memoized, so a proper check is never performed. By changing

  if framework_gem?
    @load_paths_added = @loaded = @frozen = true
    return
  end
  

  to

  if framework_gem?
    @load_paths_added = @loaded = true
    return
  end
  

  the problem goes away.

• You flagged this item as spam.
  

  J.D. Hollis June 24th, 2009 @ 08:09 PM

  • Tag changed from “2-3-stable, gem, gem.config, gems, gem_dependency” to “2-3-stable, gem, gem.config, gems, gem_dependency, patch”

  And here's the patch against master. (It should apply cleanly to 2-3-stable as well.)

  • dont-freeze-framework-gems-early.diff 1.7 KB

• You flagged this item as spam.
  

  J.D. Hollis June 24th, 2009 @ 08:19 PM

  • Title changed from “rmagick gem dependency is detected as unbuilt even though it isn't vendored” to “[PATCH] rmagick gem dependency is detected as unbuilt even though it isn't vendored”
• 

  Matt Jones June 24th, 2009 @ 09:06 PM

  The early freezing was marked to prevent a different situation - users were having issues with gems that were loaded early ("framework gems", as the code calls them) being frozen in vendor but not being used. In your case, even if you froze RMagick to vendor/gems, it would still need (AFAIK) to be installed in the system repository.

  The issue is with the build check - I'll take a look at it.

• You flagged this item as spam.
  

  J.D. Hollis June 24th, 2009 @ 09:24 PM

  Cool—thanks. "framework_gems" is definitely a bit confusing. Would something like "loaded_gems" work better?

• You flagged this item as spam.
  

  J.D. Hollis June 26th, 2009 @ 06:39 PM

  Looking at :built?, I can see three potential strategies (from my admittedly limited point of view). First option is to add an extra vendor_gem? check to the top of the method (essentially saying that if it isn't in vendor/gems, it's not Rails' problem whether it's built or not). Second option would be to check for the extension Makefiles in the actual path of the loaded gem (instead of assuming it's under vendor/gems). Third option would be to refactor and relabel things a bit to make the early freeze (and "framework gems") a bit more understandable.

  First option seems to be the most straight-forward (and most in spirit with what's already there).

  I'd be happy to tackle the patch for any of these.

• You flagged this item as spam.
  

  J.D. Hollis June 29th, 2009 @ 09:56 PM

  Ok—here's a patch against master for my first option described above (a vendor_gem? check in built?).

  • check_vendor_gem_in_built.diff 2.2 KB
• 

  Matt Jones June 29th, 2009 @ 10:05 PM

  The only change I'd recommend to that patch is to split out the new test as a separate method - it makes it easier to tell what's going on. Otherwise looks reasonable.

• 

  Michael Koziarski June 30th, 2009 @ 12:18 AM

  • Assigned user set to “Matt Jones”

• You flagged this item as spam.
  

  J.D. Hollis June 30th, 2009 @ 02:00 PM

  Done. This should apply cleanly to both master and 2-3-stable.

  • check-vendor-gem-in-built.diff 2.1 KB
• 

  Repository July 2nd, 2009 @ 06:30 PM

  • State changed from “new” to “resolved”

  (from [9eab435631e1b0a659126d068972ee88cff160de]) Only check for built extensions on gem dependencies that are in vendor/gems. [#2825 state:resolved]

  Signed-off-by: Yehuda Katz + Carl Lerche ykatz+clerche@engineyard.com
  http://github.com/rails/rails/commit/9eab435631e1b0a659126d068972ee...

• 

  Repository July 2nd, 2009 @ 06:52 PM

  (from [f6f24b71a41f21953cf4c934a9ece6e1ed76285c]) Only check for built extensions on gem dependencies that are in vendor/gems. [#2825 state:resolved]

  Signed-off-by: Yehuda Katz + Carl Lerche ykatz+clerche@engineyard.com
  http://github.com/rails/rails/commit/f6f24b71a41f21953cf4c934a9ece6...