URL path logic in path_support #371

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

headius commented Sep 12, 2012

I will admit this change is a bit cumbersome, but I'm looking for help coming up with something better.

This code exists in JRuby's copy of RubyGems to allow loading specs, etc from URL locations like file:, jar:, and classpath:. This feature is used extensively by JRuby to allow gems to load from within package applications that may not be expanded on the filesystem (in other words, this allows us to run gem contents from within a JRuby application archive of some sort).

The code here fixes up previously-split paths back into URLs and then continues. It's not the prettiest thing, but it was written this way to avoid functionally changing any of the path-splitting logic that already existed and only doing this when there's a file: or jar:file: or classpath: URL that gets broken into pieces.

Suggestions? Thoughts?

Reinstate URL path logic in new place in path_support
Conflicts:
	lib/rubygems/path_support.rb
Owner

evanphx commented Oct 6, 2012

Could we instead use a platform hook to allow you to run this code in this same place? I'd prefer that so you can continue to own the code and don't have to update rubygems to ever change it.

Contributor

headius commented Nov 28, 2012

I am willing to wire up a hook, but I'm at a complete loss for how it should be structured. We're essentially un-mangling URLs that appear in the gem path because RubyGems knows nothing about URLs. Hooking that seems like a poor way to fix it compared to adding URL support to RubyGems. Obviously this commit doesn't do that either, but adding a hook to patch up broken URLs seems like the wrong direction.

@mpapis mpapis referenced this pull request in rvm/rvm Nov 28, 2012

Closed

Enable rubygems udating for jruby #1340

Owner

evanphx commented Nov 28, 2012

Where you have the code now, just call a platform hook (default_gempath_fixup) which is empty by default. JRuby replaces that method with a version that has the above code in it.

Contributor

enebo commented Nov 28, 2012

I have a patch I am working on which will replace the File::PATH_SEPARATOR with a file_separator method in this same class (which will default to PATH_SEPARATOR). This should allow us to use a regexp with split like:

/(?<!jar:file|jar|file|classpath):/
Contributor

enebo commented Nov 28, 2012

We have a replacement for this fix in PR #402. This PR can be closed.

@headius headius closed this Nov 29, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment