Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
perlun opened this issue Apr 23, 2018 · 5 comments
Closed

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

perlun opened this issue Apr 23, 2018 · 5 comments
Milestone
Invalid or Duplicate

Comments

@perlun
Copy link
Contributor

perlun commented Apr 23, 2018

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
Copy link
Contributor Author

perlun commented Apr 23, 2018

(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
Copy link
Member

headius commented Apr 25, 2018

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?

headius added a commit that referenced this issue Apr 25, 2018
@headius
Exclude this test since it reflects broken chdir+env logic. #5145
9b1e93b
@enebo
Copy link
Member

enebo commented Apr 25, 2018

@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
Copy link
Member

headius commented May 14, 2018

@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 headius mentioned this issue May 15, 2018
Open
MaxLap referenced this issue in deep-cover/deep-cover Jan 29, 2019
@MaxLap
truffle workaround #1492
bf7043b 
oracle/truffleruby#1492
@ahorek ahorek mentioned this issue May 22, 2019
Closed
@headius
Copy link
Member

headius commented Jul 18, 2020

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.

@headius headius closed this as completed Jul 18, 2020
@headius headius added this to the Invalid or Duplicate milestone Jul 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Invalid or Duplicate
Development

No branches or pull requests
3 participants
@headius @enebo @perlun