From 833e5aa5be5c21dd253fae92fc0184a93b8bd366 Mon Sep 17 00:00:00 2001 From: James Couball Date: Sat, 17 Dec 2022 15:39:40 -0800 Subject: [PATCH 1/3] Spawn should call #to_io on non-IO file descriptor objects --- spec/ruby/core/process/spawn_spec.rb | 46 +++++++++++++++++++ .../core/truffle/process_operations.rb | 6 ++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/spec/ruby/core/process/spawn_spec.rb b/spec/ruby/core/process/spawn_spec.rb index ad4800d4f8a3..9adefea9f6fa 100644 --- a/spec/ruby/core/process/spawn_spec.rb +++ b/spec/ruby/core/process/spawn_spec.rb @@ -477,6 +477,52 @@ def child_pids(pid) # redirection + context "redirects STDOUT to the IO returned by obj.to_io if out: obj" do + # When an instance of @wrapped_io_class is passed as a value for Process#spawn + # redirection (e.g. as the value for out: or err:), Process#spawn is + # expected to call #to_io to get the wrapped IO object. + # + @wrapped_io_class = Class.new do + def initialize(io) + @io = io + @to_io_called = false + end + + def to_io + @to_io_called = true + @io + end + + def to_io_called? + @to_io_called + end + end + + it 'should not raise an error' do + out = @wrapped_io_class.new(STDOUT) + # Since 'exit' is a shell builtin, the quotes around 0 are necessary to + # force spawn to use a shell + -> { Process.wait Process.spawn('exit "0"', out: out) }.should_not raise_error + end + + it 'should call #to_io to get the wrapped IO object' do + out = @wrapped_io_class.new(STDOUT) + # Since 'exit' is a shell builtin, the quotes around 0 are necessary to + # force spawn to use a shell + Process.wait Process.spawn('exit "0"', out: out) + out.to_io_called?.should == true + end + + it 'should redirect stdout of the subprocess to the wrapped IO object' do + File.open(@name, 'w') do |file| + -> do + out = @wrapped_io_class.new(file) + Process.wait Process.spawn('echo "Hello World"', out: out) + end.should output_to_fd("Hello World\n", file) + end + end + end + it "redirects STDOUT to the given file descriptor if out: Integer" do File.open(@name, 'w') do |file| -> do diff --git a/src/main/ruby/truffleruby/core/truffle/process_operations.rb b/src/main/ruby/truffleruby/core/truffle/process_operations.rb index 84595bf4e35d..9464dfbdc4e9 100644 --- a/src/main/ruby/truffleruby/core/truffle/process_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/process_operations.rb @@ -377,7 +377,11 @@ def convert_to_fd(obj, target) open_file_for_child(obj[0], convert_file_mode(obj[1]), obj[2]) end else - raise ArgumentError, "wrong exec redirect: #{obj.inspect}" + if obj.respond_to?(:to_io) + obj.to_io.fileno + else + raise ArgumentError, "wrong exec redirect: #{obj.inspect}" + end end end From 69cd98dbf3c49ce715874ee3c92e2e78158a5e5b Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Mon, 19 Dec 2022 13:01:04 +0100 Subject: [PATCH 2/3] Simplify specs * Avoid `should_not raise_error` * Use mock() instead of manually doing it --- spec/ruby/core/process/spawn_spec.rb | 50 ++++------------------------ 1 file changed, 7 insertions(+), 43 deletions(-) diff --git a/spec/ruby/core/process/spawn_spec.rb b/spec/ruby/core/process/spawn_spec.rb index 9adefea9f6fa..7a280ccdf4cd 100644 --- a/spec/ruby/core/process/spawn_spec.rb +++ b/spec/ruby/core/process/spawn_spec.rb @@ -477,49 +477,13 @@ def child_pids(pid) # redirection - context "redirects STDOUT to the IO returned by obj.to_io if out: obj" do - # When an instance of @wrapped_io_class is passed as a value for Process#spawn - # redirection (e.g. as the value for out: or err:), Process#spawn is - # expected to call #to_io to get the wrapped IO object. - # - @wrapped_io_class = Class.new do - def initialize(io) - @io = io - @to_io_called = false - end - - def to_io - @to_io_called = true - @io - end - - def to_io_called? - @to_io_called - end - end - - it 'should not raise an error' do - out = @wrapped_io_class.new(STDOUT) - # Since 'exit' is a shell builtin, the quotes around 0 are necessary to - # force spawn to use a shell - -> { Process.wait Process.spawn('exit "0"', out: out) }.should_not raise_error - end - - it 'should call #to_io to get the wrapped IO object' do - out = @wrapped_io_class.new(STDOUT) - # Since 'exit' is a shell builtin, the quotes around 0 are necessary to - # force spawn to use a shell - Process.wait Process.spawn('exit "0"', out: out) - out.to_io_called?.should == true - end - - it 'should redirect stdout of the subprocess to the wrapped IO object' do - File.open(@name, 'w') do |file| - -> do - out = @wrapped_io_class.new(file) - Process.wait Process.spawn('echo "Hello World"', out: out) - end.should output_to_fd("Hello World\n", file) - end + it 'redirects to the wrapped IO using wrapped_io.to_io if out: wrapped_io' do + File.open(@name, 'w') do |file| + -> do + wrapped_io = mock('wrapped IO') + wrapped_io.should_receive(:to_io).and_return(file) + Process.wait Process.spawn('echo "Hello World"', out: wrapped_io) + end.should output_to_fd("Hello World\n", file) end end From 938790044ffd398b52fa2918d99a7fc8bba2b13f Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Mon, 19 Dec 2022 13:08:04 +0100 Subject: [PATCH 3/3] Add ChangeLog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f2aa4ef247e..5e252eada404 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Compatibility: * Fix argument implicit convertion in `IO#pos=` and `IO#seek` methods (#2787, @andrykonchin). * Warn about unknown directive passed to `Array#pack` in verbose mode (#2791, @andrykonchin). * Added constants `IO::SEEK_DATE` and `IO::SEEK_HOLE` (#2792, @andrykonchin). +* `Process#spawn` should call `#to_io` on non-IO file descriptor objects (#2809, @jcouball). Performance: