process_manager: Incompatible subprocess cmd parsing behaviour on 1.7 (ENV variables) #3642

Closed
kml opened this Issue Feb 4, 2016 · 2 comments

Comments

Projects
None yet
2 participants
@kml

kml commented Feb 4, 2016

Problem since 1.7.21.

1.7:

$ jruby -e 'puts RUBY_DESCRIPTION; puts `LANG=en.UTF-8 date`'
jruby 1.7.24 (1.9.3p551) 2016-01-20 bd68d85 on Java HotSpot(TM) 64-Bit Server VM 1.8.0_66-b17 [darwin-x86_64]
Errno::ENOENT: No such file or directory - LANG=en.UTF-8
       ` at file:~/.rvm/rubies/jruby-1.7.24/lib/jruby.jar!/jruby/kernel/jruby/process_manager.rb:28
       ` at file:~/.rvm/rubies/jruby-1.7.24/lib/jruby.jar!/jruby/kernel/jruby/process_manager.rb:55
  (root) at -e:1

9k:

$ jruby -e 'puts RUBY_DESCRIPTION; puts `LANG=en.UTF-8 date`'
jruby 9.0.5.0 (2.2.3) 2016-01-26 7bee00d Java HotSpot(TM) 64-Bit Server VM 25.66-b17 on 1.8.0_66-b17 [darwin-x86_64]
Thu Feb  4 14:25:15 CET 2016

$ jruby -e 'puts RUBY_DESCRIPTION; puts `LANG=pl_PL.UTF-8 date`'
jruby 9.0.5.0 (2.2.3) 2016-01-26 7bee00d Java HotSpot(TM) 64-Bit Server VM 25.66-b17 on 1.8.0_66-b17 [darwin-x86_64]
czw lut  4 14:25:37 CET 2016

MRI:

$ ruby -e 'puts RUBY_DESCRIPTION; puts `LANG=en.UTF-8 date`'
ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-darwin14]
Thu Feb  4 14:17:15 CET 2016

$ ruby -e 'puts RUBY_DESCRIPTION; puts `LANG=pl_PL.UTF-8 date`'
ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-darwin14]
czw lut  4 14:17:21 CET 2016

@enebo enebo added this to the JRuby 1.7.25 milestone Feb 4, 2016

@enebo enebo added the core label Feb 4, 2016

@enebo enebo added the regression label Apr 8, 2016

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Apr 8, 2016

Member

I bisected this to: 0505de3. This change swapped only Windows to use shell wrapping if on windows. Whereas before it would just a config option. I think least invasive fix here might be to look for '=' on first first word and go back to shell wrapping in that case. At least we know that what this commit fix could never be used by someone also passing ENV vars in or they would be reporting the same issue.

Member

enebo commented Apr 8, 2016

I bisected this to: 0505de3. This change swapped only Windows to use shell wrapping if on windows. Whereas before it would just a config option. I think least invasive fix here might be to look for '=' on first first word and go back to shell wrapping in that case. At least we know that what this commit fix could never be used by someone also passing ENV vars in or they would be reporting the same issue.

enebo added a commit that referenced this issue Apr 8, 2016

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Apr 8, 2016

Member

So I went with a more conservative fix here. I basically look for the presence of ENV Key=value as first word and then fall back to shell-launch. For reasons given in last comment I think this is least likely to produce additional regressions and 9k is seemingly working so I am taking least risky fix.

Member

enebo commented Apr 8, 2016

So I went with a more conservative fix here. I basically look for the presence of ENV Key=value as first word and then fall back to shell-launch. For reasons given in last comment I think this is least likely to produce additional regressions and 9k is seemingly working so I am taking least risky fix.

@enebo enebo closed this Apr 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment