Issue with prefixing "system" commands with ENV variables in Dir.chdir

[perlun]

Okay, this is a weird one. https://github.com/perlun/jruby-system-chdir-bug is a repro repo that has everything in place to illustrate the issue.

Basically, a Rakefile that looks like this:

def run_command(command_line)
  result = system(command_line)

  raise 'Command execution failed' if result.nil?
end

desc 'A wonderful task'
task :foo do
  Dir.chdir 'bar' do
    run_command 'BAZ=zot rake -T'
  end
end

...behaves differently on MRI and JRuby.

On MRI:

$ rvm use 2.5.1
Using /Users/plundberg/.rvm/gems/ruby-2.5.1
$ bundle exec rake foo
(in /Users/plundberg/tmp)
rake foo  # A wonderful task

On JRuby:

$ java -version
java version "9.0.4"
Java(TM) SE Runtime Environment (build 9.0.4+11)
Java HotSpot(TM) 64-Bit Server VM (build 9.0.4+11, mixed mode)
$ bundle exec rake foo
rake aborted!
Command execution failed
/Users/plundberg/tmp/Rakefile:4:in `run_command'
/Users/plundberg/tmp/Rakefile:10:in `block in (root)'
/Users/plundberg/tmp/Rakefile:9:in `block in (root)'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/rake-12.3.1/exe/rake:27:in `<main>'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli/exec.rb:1:in `(root)'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli/exec.rb:75:in `kernel_load'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli/exec.rb:28:in `run'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli.rb:424:in `exec'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli.rb:27:in `dispatch'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/vendor/thor/lib/thor/base.rb:466:in `block in start'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli.rb:18:in `start'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/exe/bundle:30:in `<main>'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/bin/bundle:23:in `<main>'

Caused by:
Errno::ENOENT: No such file or directory - BAZ=zot
/Users/plundberg/tmp/Rakefile:2:in `run_command'
/Users/plundberg/tmp/Rakefile:10:in `block in (root)'
/Users/plundberg/tmp/Rakefile:9:in `block in (root)'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/rake-12.3.1/exe/rake:27:in `<main>'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli/exec.rb:1:in `(root)'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli/exec.rb:75:in `kernel_load'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli/exec.rb:28:in `run'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli.rb:424:in `exec'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli.rb:27:in `dispatch'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/vendor/thor/lib/thor/base.rb:466:in `block in start'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/lib/bundler/cli.rb:18:in `start'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/gems/bundler-1.16.1/exe/bundle:30:in `<main>'
/Users/plundberg/.rvm/gems/jruby-9.1.16.0/bin/bundle:23:in `<main>'
Tasks: TOP => foo
(See full trace by running task with --trace)

jruby-head unfortunately doesn't work either, and doesn't even give me the error details:

$ bundle exec rake foo
rake aborted!
Command execution failed
/Users/plundberg/tmp/Rakefile:4:in `run_command'
/Users/plundberg/tmp/Rakefile:10:in `block in (root)'
/Users/plundberg/tmp/Rakefile:9:in `block in (root)'
/Users/plundberg/.rvm/gems/jruby-head/bin/jruby_executable_hooks:15:in `<main>'
Tasks: TOP => foo
(See full trace by running task with --trace)
$ jruby -v
jruby 9.2.0.0-SNAPSHOT (2.5.0) 2018-04-19 a3661da Java HotSpot(TM) 64-Bit Server VM 9.0.4+11 on 9.0.4+11 +jit [darwin-x86_64]

Observations

1. Removing the Dir.chdir call & block makes it work as expected, so it seems to be specifically this circumstance (being in a different folder or something else that chdir changes in the context.)
2. Removing the FOO=bar prefix also makes it work, but is a bit bad because I need the prefix in certain scenarios.
3. The failure has been seen on Linux (Travis CI) and on macOS locally.

[perlun]

(FWIW, I noted that MRI has problems with these commands also, but "different problems". It fails to populate $CHILD_STATUS.termsig correctly in these cases, when the command is prefixed by an env variable. $CHILD_STATUS has a string value of pid 18410 exit 143 or similar. I will consider filing that bug upstream to MRI also.)

[headius]

This is a known issue. The main reason it fails is that we maintain a notion of "cwd" that is not tied to the system level current directory. We do this to allow running multiple JRuby instances in the same process with their own cwd's. However this means that we must always simulate cwd when opening files, running subprocesses and so on. For processes, this usually means that we run the command as a sh script that calls chdir first. We have not expanded that logic to also insert ENV, for various reasons.

In dealing with some cwd-related failures today, I started to wonder if we should just go ahead and try a real system-level chdir call. We'd still need to have support for multi-tenant cwd, but since most folks using chdir will be running a single JRuby instance from a command line, giving them a real chdir might be nice.

@enebo What sayeth you?

[enebo]

@headius will that only effect the current thread? We could detect multiple runtimes in one vm as well so not exactly sure if we then toggle back to emulating or not? I do like the idea of it though. What we are doing is never quite the right fit.

[headius]

@enebo No, the native chdir changes process-global state. All threads would see the new cwd. Unfortunately the only other option is what we do now.*

(*There is another option but it would require us to have native code that forks, chdirs, and then execs. MRI does this if it can fork, or chdirs before/after a spawn call if fork is not available.)

[headius]

There have been other fixes to better support chdir in the context of system, but without a specific reproduction case I can't say whether this issue has been resolved. In general, there are still problems with chdir and process-launching that can't be easily fixed due to the thread-unsafety of the native chdir function.

We'll call this one invalid/duplicate. If there's a specific case that doesn't work, ask us about it or open a new issue and we can deal with these on a case-by-case basis.