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.

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

JRuby 1.7.8 (1.9) Open3 `capture3` bug #1290

Closed
slippycheeze opened this Issue Dec 2, 2013 · 9 comments

Comments

Projects
None yet
10 participants
@slippycheeze
Copy link
Contributor

slippycheeze commented Dec 2, 2013

The newest version of JRuby fixes the old issue where popen3 would not pass a status object to the block, which is awesome. Unfortunately, while this makes capture3 run, it is not quite correct:

jruby-1.7.8 :001 > require 'open3'
 => true
jruby-1.7.8 :002 > Open3.capture3('echo')
 => ["{}\n", "", #]
jruby-1.7.8 :003 > Open3.capture3('echo', :foo => :bar)
 => ["{:foo=>:bar}\n", "", #]

As you will observe, the output includes a stringified hash, which is the "options" part of handling the thing. It seems like your IO::popen3 implementation should either handle those appropriately, or you should move the "RUBY_ENGINE" part down below the "if Hash === cmd.last" part to handle (by ignoring) the options passed. :)

Another option, perhaps better, might be to go down and implement popen_run instead of popen3, which is invoked by the rest of the methods to handle the actual I/O process.

@randycoulman

This comment has been minimized.

Copy link

randycoulman commented Mar 27, 2014

+1. I just ran into this with a gem I'm working on. In my case, I can handle the extra argument to work around it, but if I was trying to run a program that couldn't handle it, I'm not sure what I'd do.

randycoulman added a commit to randycoulman/jujube that referenced this issue Mar 27, 2014

Remove trailing '{}' from ARGV if present
This gets the acceptance tests passing on JRuby, which has a bug in Open3.capture3.

See jruby/jruby#1290 for details.

@atambo atambo added this to the JRuby 1.7.13 milestone Apr 27, 2014

@atambo atambo self-assigned this Apr 27, 2014

@enebo enebo modified the milestones: JRuby 1.7.14, JRuby 1.7.13 Jun 24, 2014

@enebo enebo modified the milestones: JRuby 1.7.14, JRuby 1.7.15 Aug 27, 2014

@gucki

This comment has been minimized.

Copy link

gucki commented Sep 9, 2014

+1 Just hit this bug, which just makes this method completely useless. Please fix asap. Thank you.

@flavorjones

This comment has been minimized.

Copy link
Contributor

flavorjones commented Oct 2, 2014

This bug makes capture3 and popen3 useless for most conventional use cases.

Additional example, if one is needed:

#!/usr/bin/env ruby

require 'open3'

def run_test *args
  puts "------\n- args: `#{args.inspect}`"
  begin
    o, e, s = Open3.capture3(*args)
    puts "- stdout:"
    puts o
    puts "- stderr:"
    puts e
    puts "- status: #{s.exitstatus}"
  rescue Exception => e
    puts e.inspect
  end
end

puts ::RUBY_DESCRIPTION

# from the test/mri/test_open3.rb, which passes
run_test "ruby", '-e', 'i=STDIN.read; print i+"o"; STDOUT.flush; STDERR.print i+"e"', :stdin_data=>"i"

# but you can see that additional args are being included in the executed command ...
run_test "cat", "-n", :stdin_data => "asdf\nqwer\nzxcv\n"

On MRI outputs:

ruby 2.1.2p95 (2014-05-08 revision 45877) [x86_64-linux]
------
- args: `["ruby", "-e", "i=STDIN.read; print i+\"o\"; STDOUT.flush; STDERR.print i+\"e\"", {:stdin_data=>"i"}]`
- stdout:
io
- stderr:
ie
- status: 0
------
- args: `["cat", "-n", {:stdin_data=>"asdf\nqwer\nzxcv\n"}]`
- stdout:
     1  asdf
     2  qwer
     3  zxcv
- stderr:

- status: 0

On JRuby outputs:

jruby 1.7.15 (1.9.3p392) 2014-09-03 82b5cc3 on OpenJDK 64-Bit Server VM 1.7.0_65-b32 +jit [linux-amd64]
------
- args: `["ruby", "-e", "i=STDIN.read; print i+\"o\"; STDOUT.flush; STDERR.print i+\"e\"", {:stdin_data=>"i"}]`
- stdout:
io
- stderr:
ie
- status: 0
------
- args: `["cat", "-n", {:stdin_data=>"asdf\nqwer\nzxcv\n"}]`
- stdout:

- stderr:
cat: {}: No such file or directory
- status: 1

(you'll notice that there is an additional argument of {} being passed to cat on the command line).

@nadavshatz

This comment has been minimized.

Copy link

nadavshatz commented Nov 11, 2014

+1

@mgomes

This comment has been minimized.

Copy link

mgomes commented Nov 19, 2014

+1 for this. This bug makes JRuby unable to run the latest mini_magick (minimagick/minimagick#254).

@retoo

This comment has been minimized.

Copy link
Contributor

retoo commented Dec 2, 2014

+1, painful

headius added a commit that referenced this issue Dec 2, 2014

@headius headius assigned headius and unassigned atambo Dec 2, 2014

@headius

This comment has been minimized.

Copy link
Member

headius commented Dec 2, 2014

Fixed for 1.7.17. I will add a test to MRI, since open3 is no longer part of RubySpec proper and we don't run the rubysl specs.

@headius headius closed this Dec 2, 2014

@flavorjones

This comment has been minimized.

Copy link
Contributor

flavorjones commented Dec 2, 2014

Thanks so much, @headius.

headius added a commit that referenced this issue Dec 2, 2014

Less broken but more grosser fix for #1290 and #1547.
MRI manages the frame's self differently, just saving it into the
binding and then putting it in the sole global state field that
represents the current thread's active frame. In our case, we use
the entire Frame object as a carrier for frame data, so we need
to keep the object the same for mutable frame fields like backref
to propagate. As a result, the only fix possible for this on 1.7
is to change the self back after instance_eval.

We may be able to fix this better on master, where the runtime
has been restructured and may support a more correct notion of the
current frame's self.
@nadavshatz

This comment has been minimized.

Copy link

nadavshatz commented Dec 3, 2014

Yay @headius !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.