Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

My stab at fixing JRUBY-7122 #674

Merged
merged 1 commit into from

3 participants

@trejkaz

This fixes the issue for classpath:/ "relative" paths which by their very nature are always absolute. The same issue could be happening for some other URI schemes so I'm not sure if a special case for "classpath" is the right fix.

@headius
Owner

Finally dug back down to this. Reviewing.

@headius
Owner

We could probably expand this to also include jar: and jar-based file: URL forms, so we don't have to patch it in the future. Want to give that a shot?

I think the possible forms are:

jar::!

and maybe

:!

As I wrote this, it occurred to me that both of those cases will still need realpath logic for since there could be symlinks...so maybe that should be a separate issue/PR.

Anyway, I'll merge in your fix, since it's definitely correct for classpath: entries.

@headius headius merged commit 8964e43 into from
@trejkaz

I added some specs to my fork for this, but the jar:file: ones fail.

JRubyFile.create is being used to do the relativisation logic. JRubyFile did appear to work for classpath: on this computer (not Windows), but it was giving the wrong result on Windows. Give it a jar:file: path though, and it just explodes because the file doesn't exist.

So either JRubyFile.create would have to be fixed to support non-file-paths or I'd want another utility to do the double-dot removals for me which worked for non-existent paths as well as existing paths.

Then the idea would be to rewrite the method to use the following steps:

  1. Figure out if the relative path is absolute and if it isn't, resolve it relative to the base path. So special cases only when the relative path is absolute, which I assume means starts with a URI prefix or a slash.
  2. Figure out what kind of path we're dealing with:
    • For non-URI paths, use prefix="" and suffix=""
    • If it's (jar:file:).(!.), store off the prefix ($1) and suffix ($2) for later
    • If it's any other URI scheme, return the whole thing as-is immediately (skip canonicalisation)
  3. Canonicalise the bit which does refer to a real file (this method already exists, so it should be painless.)
  4. Slap the prefix and suffix back on and return it.
@trejkaz

Something else really odd I'm finding. If I try to build up a path to a jar:file URL where I specify the absolute path, I get an error about the file not existing. If I try to build it up as a relative path from a jar:file URL, that works fine. I don't get why one would work and the other wouldn't.

I improved my fork to pass two more specs and added one more spec - the symlink thing still doesn't work but I'm not entirely sure where the symlink stuff is being done, so it could be an invalid test case.

Also at least one of the tests probably fails on Windows, but I won't know until tomorrow.

@trejkaz

Finding another user here having issues with require_relative. I'm sure this is probably tied into the same mess. I initially couldn't figure out what was different from my path but then I noticed his path...

jar:file:C:/Users/Alfred/Work/Source/trunk%203/product/dependencies/jruby/

I bet it's that %20... yet another edge case I didn't expect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 29, 2013
  1. @d-noll

    My stab at fixing JRUBY-7122

    d-noll authored
This page is out of date. Refresh to see the latest.
Showing with 5 additions and 0 deletions.
  1. +5 −0 src/org/jruby/RubyFile.java
View
5 src/org/jruby/RubyFile.java
@@ -1524,6 +1524,11 @@ private static IRubyObject expandPathInternal(ThreadContext context, IRubyObject
}
if (uriParts != null) {
+ //If the path was an absolute classpath path, return it as-is.
+ if (uriParts[0].equals("classpath:")) {
+ return runtime.newString(relativePath);
+ }
+
relativePath = uriParts[1];
}
Something went wrong with that request. Please try again.