[PATCH] rmagick gem dependency is detected as unbuilt even though it isn't vendored

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.

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.

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

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.

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

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.

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

(from [f6f24b71a41f21953cf4c934a9ece6e1ed76285c]) Only check for built extensions on gem dependencies that are in vendor/gems. [#2825 [PATCH] rmagick gem dependency is detected as unbuilt even though it isn't ve... state:resolved]

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