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

Passing input through with Open3 to a subprocess using Reline.readline crashes #66

Closed
deivid-rodriguez opened this issue Nov 6, 2019 · 6 comments · Fixed by ruby/ruby#4796

Comments

@deivid-rodriguez
Copy link

Take the following script named test.rb:

# frozen_string_literal: true

require "open3"

if ENV["SUBPROCESS"]
  if ENV["RELINE"]
    require "reline"
    readline_module = Reline
  else
    require "readline"
    readline_module = Readline
  end

  readline_module.readline("y or n? ")
else
  res, _ = Open3.capture2e({ "SUBPROCESS" => "true" }, "ruby test.rb", stdin_data: "y\n")

  puts res
end

If I run it with ruby test.rb so that it uses the builtin readline, it does what I expect and prints "y or no? y" to the script.

However, if I run it through RELINE=true ruby test.rb so that it uses reline, it prints the following:

$ RELINE=true ruby test.rb 
stty: 'standard input': Inappropriate ioctl for device
stty: 'standard input': Inappropriate ioctl for device
stty: 'standard input': Inappropriate ioctl for device
▽/home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline/ansi.rb:70:in `pos': Illegal seek @ rb_io_tell - <STDOUT> (Errno::ESPIPE)
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline/ansi.rb:70:in `rescue in cursor_pos'
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline/ansi.rb:57:in `cursor_pos'
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline.rb:325:in `may_req_ambiguous_char_width'
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline.rb:199:in `inner_readline'
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline.rb:179:in `readline'
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/2.6.0/forwardable.rb:230:in `readline'
	from test.rb:14:in `<main>'
/home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline/ansi.rb:59:in `raw': Inappropriate ioctl for device (Errno::ENOTTY)
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline/ansi.rb:59:in `cursor_pos'
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline.rb:325:in `may_req_ambiguous_char_width'
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline.rb:199:in `inner_readline'
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/reline-0.0.4/lib/reline.rb:179:in `readline'
	from /home/deivid/.rbenv/versions/2.6.5/lib/ruby/2.6.0/forwardable.rb:230:in `readline'
	from test.rb:14:in `<main>'
@MSP-Greg
Copy link
Contributor

MSP-Greg commented Nov 6, 2019

Replacing the bottom code of lib/reline.rb with the following seems to work on Windows, but not sure about other OS's and using tty to determine IO type...

Reline::IOGate = begin
  t = if Reline::Core::IS_WINDOWS
    require 'reline/windows'
    Reline::Windows
  else
    require 'reline/ansi'
    Reline::ANSI
  end
  require 'reline/general_io'
  STDOUT.tty? && STDIN.tty? ? t : Reline::GeneralIO
end

Reline.input = $stdin if Reline::IOGate == Reline::GeneralIO

FYI, with a few patches, byebug passes on Windows using Reline.

@deivid-rodriguez
Copy link
Author

That sounds like a snippet we could perfectly add to byebug's test helper, right?

Would you like to open a PR there? We shouldn't merge it straight away since we'll need to verify that the real user experience is unchanged, but a green CI would be a great starting point.

Not sure if there's anything to be changed in reline? Certainly the "testing mode" could be easier to setup, but this is something!

@MSP-Greg
Copy link
Contributor

MSP-Greg commented Nov 7, 2019

That sounds like a snippet we could perfectly add to byebug's test helper, right?

Is the test helper loaded when Open3 is used? Don't recall...

need to verify that the real user experience is unchanged

I'm not sure what the benchmark is for a 'correct' user experience.

@deivid-rodriguez
Copy link
Author

No benchmark, I meant that I'll use byebug with reline for a bit and verify that there's no obvious user visible changes/gotchas/bugs.

Is the test helper loaded when Open3 is used? Don't recall...

I don't think so, but we can load the snippet when needed, I think we do something similar with a simplecov helper file to make sure that coverage is tracked inside subprocesses.

Anyways, this is getting out of topic here, let's move the discussion to deivid-rodriguez/byebug#510.

@Nakilon
Copy link

Nakilon commented Sep 1, 2021

Hi guys, your issue is the only one result google gives me about this:

$ ruby -e "p STDOUT.pos"
17800074
$ ruby -ropen3 -e "p Open3.capture2e 'ruby -e \"p STDOUT.pos\"'"
["-e:1:in `pos': Illegal seek @ rb_io_tell - <STDOUT> (Errno::ESPIPE)\n\tfrom -e:1:in `<main>'\n", #<Process::Status: pid 83602 exit 1>]

I face this issue in tests that launch my gem bin/ executable. It's the same in Ruby 2.3 .. 3.0

UPD: the same with backticks:

$ irb
irb(main):001:0> `ruby -e "p STDOUT.pos"`
-e:1:in `pos': Illegal seek @ rb_io_tell - <STDOUT> (Errno::ESPIPE)
	from -e:1:in `<main>'

and with IO.popen

@Nakilon
Copy link

Nakilon commented Sep 1, 2021

I tried to reuse the #4796 patch. With no success.

$ cat use_ext_readline.rb 
require "readline.so"
ReadlineSo = Readline
  Object.send(:remove_const, :Readline) if Object.const_defined?(:Readline)
  Object.const_set(:Readline, ReadlineSo)
$ ruby -ropen3 -e "p Open3.capture2e 'ruby -rreline -r./use_ext_readline.rb -e \"p STDOUT.pos\"'"
["-e:1:in `pos': Illegal seek @ rb_io_tell - <STDOUT> (Errno::ESPIPE)\n\tfrom -e:1:in `<main>'\n", #<Process::Status: pid 85906 exit 1>]

$ cat use_lib_reline.rb 
    Reline.send(:remove_const, 'IOGate') if Reline.const_defined?('IOGate')
    Reline.const_set('IOGate', Reline::GeneralIO)
    Reline.send(:core).config.instance_variable_set(:@test_mode, true)
    Reline.send(:core).config.reset
    Object.send(:remove_const, :Readline) if Object.const_defined?(:Readline)
    Object.const_set(:Readline, Reline)
$ ruby -ropen3 -e "p Open3.capture2e 'ruby -rreline -r./use_lib_reline.rb -e \"p STDOUT.pos\"'"
["-e:1:in `pos': Illegal seek @ rb_io_tell - <STDOUT> (Errno::ESPIPE)\n\tfrom -e:1:in `<main>'\n", #<Process::Status: pid 85916 exit 1>]

ruby 3.0.1

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

Successfully merging a pull request may close this issue.

3 participants