From 875aa60d22f412eb8969668ebc7446d863f3134f Mon Sep 17 00:00:00 2001 From: Randy Stauner Date: Wed, 25 Jun 2025 16:11:08 -0700 Subject: [PATCH 1/3] Fix Open3 documentation about blocking behavior and pipeline examples 1. Fixed incorrect documentation in popen3, popen2, and popen2e that claimed these methods always wait for child processes to exit. Actually, they only wait when used with a block. Without a block, they return immediately and the caller must wait manually. 2. Updated pipeline method examples to clarify that when used with a block, the methods automatically wait for all processes to exit. Removed misleading code that showed manual thread joining in the block form. --- lib/open3.rb | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/lib/open3.rb b/lib/open3.rb index 3c725b3..00622b5 100644 --- a/lib/open3.rb +++ b/lib/open3.rb @@ -137,8 +137,10 @@ module Open3 # if called with untrusted input; # see {Command Injection}[https://docs.ruby-lang.org/en/master/command_injection_rdoc.html#label-Command+Injection]. # - # Unlike Process.spawn, this method waits for the child process to exit - # before returning, so the caller need not do so. + # Blocking behavior: + # - With a block: waits for the child process to exit before returning. + # - Without a block: returns immediately without waiting for the child process; + # the caller must call +wait_thread.join+ to wait for the process to exit. # # If the first argument is a hash, it becomes leading argument +env+ # in the call to Process.spawn; @@ -290,8 +292,10 @@ def popen3(*cmd, &block) # if called with untrusted input; # see {Command Injection}[https://docs.ruby-lang.org/en/master/command_injection_rdoc.html#label-Command+Injection]. # - # Unlike Process.spawn, this method waits for the child process to exit - # before returning, so the caller need not do so. + # Blocking behavior: + # - With a block: waits for the child process to exit before returning. + # - Without a block: returns immediately without waiting for the child process; + # the caller must call +wait_thread.join+ to wait for the process to exit. # # If the first argument is a hash, it becomes leading argument +env+ # in the call to Process.spawn; @@ -434,8 +438,10 @@ def popen2(*cmd, &block) # if called with untrusted input; # see {Command Injection}[https://docs.ruby-lang.org/en/master/command_injection_rdoc.html#label-Command+Injection]. # - # Unlike Process.spawn, this method waits for the child process to exit - # before returning, so the caller need not do so. + # Blocking behavior: + # - With a block: waits for the child process to exit before returning. + # - Without a block: returns immediately without waiting for the child process; + # the caller must call +wait_thread.join+ to wait for the process to exit. # # If the first argument is a hash, it becomes leading argument +env+ # in the call to Process.spawn; @@ -972,15 +978,14 @@ def capture2e(*cmd) # # With a block given, calls the block with the +stdin+ stream of the first child, # the +stdout+ stream of the last child, - # and an array of the wait processes: + # and an array of the wait processes. + # The method automatically waits for all processes to exit after the block returns. # # Open3.pipeline_rw('sort', 'cat -n') do |first_stdin, last_stdout, wait_threads| # first_stdin.puts "foo\nbar\nbaz" # first_stdin.close # send EOF to sort. # puts last_stdout.read - # wait_threads.each do |wait_thread| - # wait_thread.join - # end + # # No need to join wait_threads - the method handles it automatically. # end # # Output: @@ -1065,13 +1070,12 @@ def pipeline_rw(*cmds, &block) # # With a block given, calls the block with the +stdout+ stream # of the last child process, - # and an array of the wait processes: + # and an array of the wait processes. + # The method automatically waits for all processes to exit after the block returns. # # Open3.pipeline_r('ls', 'grep R') do |last_stdout, wait_threads| # puts last_stdout.read - # wait_threads.each do |wait_thread| - # wait_thread.join - # end + # # No need to join wait_threads - the method handles it automatically. # end # # Output: @@ -1154,14 +1158,13 @@ def pipeline_r(*cmds, &block) # # With a block given, calls the block with the +stdin+ stream # of the first child process, - # and an array of the wait processes: + # and an array of the wait processes. + # The method automatically waits for all processes to exit after the block returns. # # Open3.pipeline_w('sort', 'cat -n') do |first_stdin, wait_threads| # first_stdin.puts("foo\nbar\nbaz") # first_stdin.close # Send EOF to sort. - # wait_threads.each do |wait_thread| - # wait_thread.join - # end + # # No need to join wait_threads - the method handles it automatically. # end # # Output: @@ -1234,12 +1237,11 @@ def pipeline_w(*cmds, &block) # Rakefile # README.md # - # With a block given, calls the block with an array of the wait processes: + # With a block given, calls the block with an array of the wait processes. + # The method automatically waits for all processes to exit after the block returns. # # Open3.pipeline_start('ls', 'grep R') do |wait_threads| - # wait_threads.each do |wait_thread| - # wait_thread.join - # end + # # No need to join wait_threads - the method handles it automatically. # end # # Output: From aa93f2e4c166807e2cfd946b1c81ad98f1727a6f Mon Sep 17 00:00:00 2001 From: Randy Stauner Date: Wed, 25 Jun 2025 16:08:24 -0700 Subject: [PATCH 2/3] Add notes about ignored redirection options in Open3 methods Each Open3 method that sets up pipes now documents which redirection options will be ignored. This clarifies that when methods create pipes for stdin, stdout, or stderr, the corresponding redirection options in the execution options hash will have no effect. --- lib/open3.rb | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/open3.rb b/lib/open3.rb index 00622b5..0183552 100644 --- a/lib/open3.rb +++ b/lib/open3.rb @@ -78,6 +78,10 @@ # - An optional hash of execution options; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # +# Note: When using methods that set up pipes for I/O streams, +# the corresponding redirection options in the execution options +# will be ignored for those streams. +# module Open3 # :call-seq: @@ -150,6 +154,9 @@ module Open3 # in the call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: The redirection options :in, :out, and :err will be ignored + # because popen3 sets up pipes for stdin, stdout, and stderr. + # # The single required argument is one of the following: # # - +command_line+ if it is a string, @@ -305,6 +312,9 @@ def popen3(*cmd, &block) # in the call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: The redirection options :in and :out will be ignored + # because popen2 sets up pipes for stdin and stdout. + # # The single required argument is one of the following: # # - +command_line+ if it is a string, @@ -451,6 +461,9 @@ def popen2(*cmd, &block) # in the call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: The redirection options :in and [:out, :err] will be ignored + # because popen2e sets up pipes for stdin and merged stdout/stderr. + # # The single required argument is one of the following: # # - +command_line+ if it is a string, @@ -589,6 +602,9 @@ class << self # in the call to Open3.popen3; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: The redirection options :in, :out, and :err will be ignored + # because capture3 manages stdin, stdout, and stderr internally. + # # The hash +options+ is given; # two options have local effect in method Open3.capture3: # @@ -715,6 +731,9 @@ def capture3(*cmd) # in the call to Open3.popen3; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: The redirection options :in and :out will be ignored + # because capture2 manages stdin and stdout internally. + # # The hash +options+ is given; # two options have local effect in method Open3.capture2: # @@ -843,6 +862,9 @@ def capture2(*cmd) # in the call to Open3.popen3; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: The redirection options :in and [:out, :err] will be ignored + # because capture2e manages stdin and merged stdout/stderr internally. + # # The hash +options+ is given; # two options have local effect in method Open3.capture2e: # @@ -1006,6 +1028,10 @@ def capture2e(*cmd) # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: The redirection options for the first process's stdin (:in) + # and the last process's stdout (:out) will be ignored + # because pipeline_rw sets up pipes for these. + # # Each remaining argument in +cmds+ is one of: # # - A +command_line+: a string that begins with a shell reserved word @@ -1095,6 +1121,9 @@ def pipeline_rw(*cmds, &block) # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: The redirection option for the last process's stdout (:out) + # will be ignored because pipeline_r sets up a pipe for it. + # # Each remaining argument in +cmds+ is one of: # # - A +command_line+: a string that begins with a shell reserved word @@ -1185,6 +1214,9 @@ def pipeline_r(*cmds, &block) # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: The redirection option for the first process's stdin (:in) + # will be ignored because pipeline_w sets up a pipe for it. + # # Each remaining argument in +cmds+ is one of: # # - A +command_line+: a string that begins with a shell reserved word @@ -1261,6 +1293,9 @@ def pipeline_w(*cmds, &block) # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: All redirection options are honored because pipeline_start + # does not set up any pipes automatically. + # # Each remaining argument in +cmds+ is one of: # # - A +command_line+: a string that begins with a shell reserved word @@ -1320,9 +1355,12 @@ def pipeline_start(*cmds, &block) # see {Execution Environment}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Environment]. # # If the last argument is a hash, it becomes trailing argument +options+ - # in each call to Process.spawn' + # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # + # Note: Redirection options are generally honored, but pipes between + # commands are automatically managed. + # # Each remaining argument in +cmds+ is one of: # # - A +command_line+: a string that begins with a shell reserved word From f976c2b765d6e90da800da5c4f326afaae6e81fa Mon Sep 17 00:00:00 2001 From: Randy Stauner Date: Thu, 3 Jul 2025 08:43:53 -0700 Subject: [PATCH 3/3] Generalize redirection option warnings in Open3 documentation Replace specific implementation details about which redirection options are ignored with a more general user-friendly message. This avoids exposing internal implementation details and provides clearer guidance to users. --- lib/open3.rb | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/lib/open3.rb b/lib/open3.rb index 0183552..b9bed1d 100644 --- a/lib/open3.rb +++ b/lib/open3.rb @@ -154,8 +154,8 @@ module Open3 # in the call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: The redirection options :in, :out, and :err will be ignored - # because popen3 sets up pipes for stdin, stdout, and stderr. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # The single required argument is one of the following: # @@ -312,8 +312,8 @@ def popen3(*cmd, &block) # in the call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: The redirection options :in and :out will be ignored - # because popen2 sets up pipes for stdin and stdout. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # The single required argument is one of the following: # @@ -461,8 +461,8 @@ def popen2(*cmd, &block) # in the call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: The redirection options :in and [:out, :err] will be ignored - # because popen2e sets up pipes for stdin and merged stdout/stderr. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # The single required argument is one of the following: # @@ -602,8 +602,8 @@ class << self # in the call to Open3.popen3; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: The redirection options :in, :out, and :err will be ignored - # because capture3 manages stdin, stdout, and stderr internally. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # The hash +options+ is given; # two options have local effect in method Open3.capture3: @@ -731,8 +731,8 @@ def capture3(*cmd) # in the call to Open3.popen3; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: The redirection options :in and :out will be ignored - # because capture2 manages stdin and stdout internally. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # The hash +options+ is given; # two options have local effect in method Open3.capture2: @@ -862,8 +862,8 @@ def capture2(*cmd) # in the call to Open3.popen3; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: The redirection options :in and [:out, :err] will be ignored - # because capture2e manages stdin and merged stdout/stderr internally. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # The hash +options+ is given; # two options have local effect in method Open3.capture2e: @@ -1028,9 +1028,8 @@ def capture2e(*cmd) # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: The redirection options for the first process's stdin (:in) - # and the last process's stdout (:out) will be ignored - # because pipeline_rw sets up pipes for these. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # Each remaining argument in +cmds+ is one of: # @@ -1121,8 +1120,8 @@ def pipeline_rw(*cmds, &block) # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: The redirection option for the last process's stdout (:out) - # will be ignored because pipeline_r sets up a pipe for it. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # Each remaining argument in +cmds+ is one of: # @@ -1214,8 +1213,8 @@ def pipeline_r(*cmds, &block) # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: The redirection option for the first process's stdin (:in) - # will be ignored because pipeline_w sets up a pipe for it. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # Each remaining argument in +cmds+ is one of: # @@ -1293,8 +1292,8 @@ def pipeline_w(*cmds, &block) # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: All redirection options are honored because pipeline_start - # does not set up any pipes automatically. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # Each remaining argument in +cmds+ is one of: # @@ -1358,8 +1357,8 @@ def pipeline_start(*cmds, &block) # in each call to Process.spawn; # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. # - # Note: Redirection options are generally honored, but pipes between - # commands are automatically managed. + # Note: Redirection options are managed by the method and should not be + # provided as they may be ignored or lead to errors. # # Each remaining argument in +cmds+ is one of: #