Unable to run scripts using -S if they aren't in the primary jruby.jar

Hiro Asari added a comment - 11/Sep/10 1:56 PM Eric, could you add a test case, please?

Eric Anderson added a comment - 11/Sep/10 2:12 PM Sure, I will provide two tests: Ensure that the existing scripts that jruby bundles are still executable Ensure that other scripts on the classpath (separate jar) are executable as well

Eric Anderson added a comment - 11/Sep/10 2:51 PM The test code for this project has me very lost. Perhaps you could point me in the general direction for where I would add this test?

Hiro Asari added a comment - 12/Sep/10 9:21 PM Eric, if (true) return "classpath:/META-INF/jruby.home"; This makes little sense, since there is code following this line. So, for brevity, it would look something like: diff --git a/src/org/jruby/RubyInstanceConfig.java b/src/org/jruby/RubyInstanceConfig.java index 5445ca6..7527e6a 100644 --- a/src/org/jruby/RubyInstanceConfig.java +++ b/src/org/jruby/RubyInstanceConfig.java @@ -819,27 +819,7 @@ public class RubyInstanceConfig { // verify it if it's there jrubyHome = verifyHome(jrubyHome); } else { - try { - // try loading from classloader resources - URI jrubyHomeURI = getClass().getResource("/META-INF/jruby.home").toURI(); - String scheme = jrubyHomeURI.getScheme(); - String path = jrubyHomeURI.getSchemeSpecificPart(); - if ("jar".equals(scheme) && path.startsWith("file:")) { - // special case for jar:file (most typical case) - jrubyHome = path; - } else { - jrubyHome = "classpath:/META-INF/jruby.home"; - return jrubyHome; - } - } catch (Exception e) {} - - if (jrubyHome != null) { - // verify it if it's there - jrubyHome = verifyHome(jrubyHome); - } else { - // otherwise fall back on system temp location - jrubyHome = SafePropertyAccessor.getProperty("java.io.tmpdir"); - } + return "classpath:/META-INF/jruby.home"; } } return jrubyHome; However… The case that apparently gets dropped with your patch is on line 829, when the scheme is jar and the path starts with file: . How does that work?

Charles Oliver Nutter added a comment - 22/Oct/10 5:08 PM Eric: Thanks for looking into this! Can you answer Hiro's question above, so we can try to get your patch incorporated?

Eric Anderson added a comment - 22/Oct/10 6:00 PM Yup. So, the if statement was just to short java's compiler while I made sure my change worked. Basically, you don't need to search into the exact jar to get a URL for the script you're looking for. jrubyHome can just be that location in the classpath.

Charles Oliver Nutter added a comment - 04/Nov/10 12:40 PM So I believe we need to keep the code and drop the "if (true)", since Eric was just using it to test his fixes. I'm for rolling it into 1.5.4 (and master, of course).

Charles Oliver Nutter added a comment - 04/Nov/10 2:24 PM So here's the patch I came up with. Eliminates the "cleverness" and goes straight to classpath: URL if there's no jruby home set. Ideally "classpath:" URL should cover anything in classpath, so the file/jar stuff isn't needed. Right? Not sure if there's something I'm breaking with this... https://gist.github.com/663018

Nick Sieger added a comment - 06/Jan/11 4:08 PM It does break the new "test-jar-complete" ant target I made. The pre-installed rake is not found in the complete jar. I think we need to keep the existing behavior, but perhaps add some logic to allow -S to be smarter about bin scripts in jars.