Did this ever work?

@headius I did try one or two jruby-1.7.* versions and it did not work there. so I assume it never worked.

I guess my next question is: how should it work? Obviously we can't shell out to a command with a classloader current directory. Perhaps it should simply default to actual (JRuby) current directory if the current dir is a non-file URL?

It also begs whether this creates an interesting security issue. I think people will be super disappointed that they classfile ruby scripts can no longer execute, but do they really realize they may be exposing a directory they didn't bargain for being in the scope of the system'd command?

A less popular option may be an error unless they bother to set up a proper location to execute. So special error if they execute from a non-real CWD. Too draconian?

so the problem showed up when I executed a gzip which is on the $PATH with an absolute path argument (see maven/jruby-dist/pom.rn) and in this situation it does not really matter which is the CWD of the shell and it does not matter what the CWD of JRuby is.

so I expected it to work like from any real directory.

of course the moment the real CWD plays a role for the shell script things fall apart.

I looked into the code and did not understand what really goes wrong, if this is just the cd uri:classloader:/ bit, then I just would skip this and let the CWD untouched.

regarding the potential security issue, yes things might be unexpected for the script which could be exploited. the question is how to give Kernel.system the location of the directory to execute in ? Dir.chdir("$HOME") do system('...'); end ?

@mkristian yeah I don't know. Error could be thrown but how that would change existing code in the wild and expectation of users is probably a big issue with not throwing an Error. As you said, you were using some command where you did not care which directory it was executed in. In that case you will angrily (just making this up), "what the hell I don't care about CWD in this case just let me do it!". That fictional outburst may be enough justification to just ignore CWD in this case?

@enebo I will not be angry but I would not know how to fix the build then ;)
best to ignore the CWD in case we have Dir.pwd.start_with?('uri:classloader:/').

@mkristian yeah I am starting to think so too now.

@mkristian @enebo Dunno if we have made any progress here...

@headius @mkristian I think we have not reached consensus totally. If you bundle a script and run it you get a CWD which cannot work. Ignoring that bogus CWD would allow it to work and probably be what an user would expect...but...then you have to pick some CWD. What if there are files and something in that executed script happens to be sensitive to the contents of that CWD? My best attempt at dealing with that would be to make a new directory where nothing is in it...but of course that directory will reside in a directory so it may be prone. Also the person may expect to require or find files based on the CWD which would execute but then not work in a weird way. Admittedly, this last case probably only really matters for ruby invoke ruby scenarios. So I am still on the fence...anyone else have new opinions?

@enebo Perhaps when we load completely from a jar, with no other flags or properties, we should just use user.home as CWD? That would reflect the real CWD, and I can't think of any cases where CWD shouldn't actually return the system path from which JRuby was run.

@headius It is easy to explain at least and may even be what they actually want vs the inside of jar file (even if we could make that work). Not sure we have a better outcome here either. Not running stuff is going to just keep getting reported. I only linger on how we can best document this somewhere.

@enebo @headius the only case where things do not work is when JRuby has Dir.pwd.start_with?('uri:classloader:/') and then any solution seems to be OK, i.e. use user.home or user.dir. actually user.dir seems to be right as this is what the JRuby process already sees: Dir.pwd is inside the classloader but user.dir is something else.

So then I guess the question is whether we should tweak JRuby's notion of pwd to point at user.dir when it would be a URI. Right now it appears to just look at user.dir in RubyInstanceConfig. Where does the URI come from?

@headius the uri-path you can either be set via -C option of the commandline or with the IsolatedScriptingContainer this is default https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java#L75
I use this IsolatedScriptingContainer whenever JRuby is inside classloader 'heavy' setup like maven or osgi or webservers.

Weirdly we do have logic that tries to do some special things for a uri CWD:

This logic will use the "user.dir" location as the system-level directory for the command, but only if we are launching a new JRuby instance (and even weirder, only if we're launching a new JRuby instance via org.jruby.Main on the command line).

The logic for system, at least, still falls back on the old ShellLauncher since we can't safely chdir before doing the native process logic (due to a process-wide race to change the dir back). For the moment, this is where we could make a change to detect the uri CWD and use the user's actual current directory.

@enebo @mkristian What about this patch?

diff --git a/core/src/main/java/org/jruby/util/ShellLauncher.java b/core/src/main/java/org/jruby/util/ShellLauncher.java
index 517cb11d62..38dd37b51e 100644
--- a/core/src/main/java/org/jruby/util/ShellLauncher.java
+++ b/core/src/main/java/org/jruby/util/ShellLauncher.java
@@ -1366,13 +1366,16 @@ public class ShellLauncher {
                     cfg.verifyExecutableForDirect();
                 }
                 String[] args = cfg.getExecArgs();
-                // only if we inside a jar and spawning org.jruby.Main we
-                // change to the current directory inside the jar
-                if (runtime.getCurrentDirectory().startsWith("uri:classloader:") &&
-                        args[args.length - 1].contains("org.jruby.Main")) {
+                if (runtime.getCurrentDirectory().startsWith("uri:classloader:")) {
+                    // system commands can't run with a URI for the current dir, so the best we can use is user.dir
                     pwd = new File(System.getProperty("user.dir"));
-                    args[args.length - 1] = args[args.length - 1].replace("org.jruby.Main",
-                            "org.jruby.Main -C " + runtime.getCurrentDirectory());
+
+                    // only if we inside a jar and spawning org.jruby.Main we
+                    // change to the current directory inside the jar
+                    if (args[args.length - 1].contains("org.jruby.Main")) {
+                        args[args.length - 1] = args[args.length - 1].replace("org.jruby.Main",
+                                "org.jruby.Main -C " + runtime.getCurrentDirectory());
+                    }
                 }
                 aProcess = buildProcess(runtime, args, getCurrentEnv(runtime), pwd);
             }

Targetting 9.2.9.0 since @headius has a possible patch

+1 for the above patch. as long executing Main remains within the uri:classloader: realm, i.e. stays inside the classloaders.

I'll go with that patch and try to come up with a simple test.

Patch and test are in. Patch also includes a similar change for the native popen logic, so that should be working now as well (needed to be able to test with backquote, which uses native popen).