From 6b7ede69e8980946488bf80abb724ea8cf2b12e3 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Wed, 29 Sep 2021 12:40:14 -0500 Subject: [PATCH 1/6] Only use JITSupport on CRuby Fixes #2 --- test/test_open3.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/test_open3.rb b/test/test_open3.rb index 47d471c..699b05a 100644 --- a/test/test_open3.rb +++ b/test/test_open3.rb @@ -2,7 +2,10 @@ require 'test/unit' require 'open3' -require_relative 'lib/jit_support' + +if RUBY_ENGINE == 'ruby' + require_relative 'lib/jit_support' +end class TestOpen3 < Test::Unit::TestCase RUBY = EnvUtil.rubybin @@ -127,7 +130,11 @@ def test_popen2 i.close STDERR.reopen(old) assert_equal("zo", o.read) - assert_equal("ze", JITSupport.remove_mjit_logs(r.read)) + if defined?(JITSupport) + assert_equal("ze", JITSupport.remove_mjit_logs(r.read)) + else + assert_equal("ze", r.read) + end } } } From e79c4d8dfc0e9a143f13528c6ff0e92d00320516 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Wed, 29 Sep 2021 12:40:48 -0500 Subject: [PATCH 2/6] Add JRuby to test matrix --- .github/workflows/test-jruby.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/workflows/test-jruby.yml diff --git a/.github/workflows/test-jruby.yml b/.github/workflows/test-jruby.yml new file mode 100644 index 0000000..3d819af --- /dev/null +++ b/.github/workflows/test-jruby.yml @@ -0,0 +1,22 @@ +name: test + +on: [push, pull_request] + +jobs: + build: + name: build (${{ matrix.ruby }} / ${{ matrix.os }}) + strategy: + matrix: + ruby: [ 2.7, 2.6, head, jruby-9.3 ] + os: [ ubuntu-latest, macos-latest ] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v2 + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + - name: Install dependencies + run: bundle install + - name: Run test + run: rake test From 689da19c42a12917cbe504d580485b9a46d77afb Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Wed, 29 Sep 2021 13:21:31 -0500 Subject: [PATCH 3/6] Add JRuby's Windows (JDK non-native) Open3 support This adds JRuby's logic used on platforms where we do not have native access to posix_spawn and related posix functions needed to do fully-native subprocess launching and management. The code here instead uses the JDK ProcessBuilder logic to simulate most of the Open3 functionality. This code does not pass all tests, currently, but provides most of the key functionality on pure-Java (i.e. no native FFI) platforms. --- lib/open3.rb | 9 ++- lib/open3/jruby_windows.rb | 118 +++++++++++++++++++++++++++++++++++++ lib/open3/version.rb | 3 + open3.gemspec | 6 +- 4 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 lib/open3/jruby_windows.rb create mode 100644 lib/open3/version.rb diff --git a/lib/open3.rb b/lib/open3.rb index c574696..2f035e3 100644 --- a/lib/open3.rb +++ b/lib/open3.rb @@ -29,9 +29,14 @@ # - Open3.pipeline : run a pipeline and wait for its completion # -module Open3 - VERSION = "0.1.1" +require 'open3/version' + +if RUBY_ENGINE == 'jruby' && JRuby::Util::ON_WINDOWS + require_relative 'open3/jruby_windows' + return +end +module Open3 # Open stdin, stdout, and stderr streams and start external executable. # In addition, a thread to wait for the started process is created. # The thread has a pid method and a thread variable :pid which is the pid of diff --git a/lib/open3/jruby_windows.rb b/lib/open3/jruby_windows.rb new file mode 100644 index 0000000..24b9a1b --- /dev/null +++ b/lib/open3/jruby_windows.rb @@ -0,0 +1,118 @@ +# +# Custom implementation of Open3.popen{3,2,2e} that uses java.lang.ProcessBuilder rather than pipes and spawns. +# + +require 'jruby' # need access to runtime for RubyStatus construction + +module Open3 + + java_import java.lang.ProcessBuilder + java_import org.jruby.RubyProcess + java_import org.jruby.util.ShellLauncher + + def popen3(*cmd, &block) + if cmd.size > 0 && Hash === cmd[-1] + opts = cmd.pop + else + opts = {} + end + processbuilder_run(cmd, opts, io: IO_3, &block) + end + module_function :popen3 + + IO_3 = proc do |process| + [process.getOutputStream.to_io, process.getInputStream.to_io, process.getErrorStream.to_io] + end + + BUILD_2 = proc do |builder| + builder.redirectError(ProcessBuilder::Redirect::INHERIT) + end + + IO_2 = proc do |process| + [process.getOutputStream.to_io, process.getInputStream.to_io] + end + + def popen2(*cmd, &block) + if cmd.size > 0 && Hash === cmd[-1] + opts = cmd.pop + else + opts = {} + end + processbuilder_run(cmd, opts, build: BUILD_2, io: IO_2, &block) + end + module_function :popen2 + + BUILD_2E = proc do |builder| + builder.redirectErrorStream(true) + end + + def popen2e(*cmd, &block) + if cmd.size > 0 && Hash === cmd[-1] + opts = cmd.pop + else + opts = {} + end + processbuilder_run(cmd, opts, build: BUILD_2E, io: IO_2, &block) + end + module_function :popen2e + + def processbuilder_run(cmd, opts, build: nil, io:) + if Hash === cmd[0] + env = cmd.shift; + else + env = {} + end + + if cmd.size == 1 && (cmd[0] =~ / / || ShellLauncher.shouldUseShell(cmd[0])) + cmd = [RbConfig::CONFIG['SHELL'], JRuby::Util::ON_WINDOWS ? '/c' : '-c', cmd[0]] + end + + builder = ProcessBuilder.new(cmd.to_java(:string)) + + builder.directory(java.io.File.new(opts[:chdir] || Dir.pwd)) + + environment = builder.environment + env.each { |k, v| v.nil? ? environment.remove(k) : environment.put(k, v) } + + build.call(builder) if build + + process = builder.start + + pid = org.jruby.util.ShellLauncher.getPidFromProcess(process) + + parent_io = io.call(process) + + parent_io.each {|i| i.sync = true} + + wait_thr = DetachThread.new(pid) { RubyProcess::RubyStatus.newProcessStatus(JRuby.runtime, process.waitFor << 8, pid) } + + result = [*parent_io, wait_thr] + + if defined? yield + begin + return yield(*result) + ensure + parent_io.each(&:close) + wait_thr.join + end + end + + result + end + module_function :processbuilder_run + class << self + private :processbuilder_run + end + + class DetachThread < Thread + attr_reader :pid + + def initialize(pid) + super + + @pid = pid + self[:pid] = pid + end + end + +end diff --git a/lib/open3/version.rb b/lib/open3/version.rb new file mode 100644 index 0000000..5a8e84b --- /dev/null +++ b/lib/open3/version.rb @@ -0,0 +1,3 @@ +module Open3 + VERSION = "0.1.1" +end diff --git a/open3.gemspec b/open3.gemspec index ad9485a..89c2cbe 100644 --- a/open3.gemspec +++ b/open3.gemspec @@ -1,10 +1,8 @@ # frozen_string_literal: true name = File.basename(__FILE__, ".gemspec") -version = ["lib", Array.new(name.count("-"), "..").join("/")].find do |dir| - break File.foreach(File.join(__dir__, dir, "#{name.tr('-', '/')}.rb")) do |line| - /^\s*VERSION\s*=\s*"(.*)"/ =~ line and break $1 - end rescue nil +version = File.foreach(File.join(__dir__, "lib/open3/version.rb")) do |line| + /^\s*VERSION\s*=\s*"(.*)"/ =~ line and break $1 end Gem::Specification.new do |spec| From 7ca7f3338f34f8ca26a637214233dbea424a338b Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 30 Sep 2021 07:30:49 -0700 Subject: [PATCH 4/6] Set up bundler caching Co-authored-by: Olle Jonsson --- .github/workflows/test-jruby.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test-jruby.yml b/.github/workflows/test-jruby.yml index 3d819af..e56eab8 100644 --- a/.github/workflows/test-jruby.yml +++ b/.github/workflows/test-jruby.yml @@ -16,7 +16,6 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby }} - - name: Install dependencies - run: bundle install + bundler-cache: true # 'bundle install' and cache - name: Run test - run: rake test + run: bundle exec rake test From 73f986c23387631659e40702a24d65c6f896877d Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 30 Sep 2021 10:38:25 -0500 Subject: [PATCH 5/6] Update to match JRuby 9.4 This allows the wrapper functions in the main open3 to be defined while using our ProcessBuilder logic for the internal popen implementation. Note this adds logic to reject redirects from a numeric fd to a live IO object (or not a String or to_path object) since we cannot support direct IO redirects with ProcesBuilder. This patch allows tests to complete with the ProcessBuilder impl. Only three tests fail: * test_numeric_file_descriptor2 and test_numeric_file_descriptor2 fail due to redirecting streams to a pipe IO. * test_pid fails expecting a real PID which we cannot provide via ProcessBuilder. --- lib/open3.rb | 9 ++++----- lib/open3/jruby_windows.rb | 9 +++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/open3.rb b/lib/open3.rb index 2f035e3..9652b27 100644 --- a/lib/open3.rb +++ b/lib/open3.rb @@ -31,12 +31,8 @@ require 'open3/version' -if RUBY_ENGINE == 'jruby' && JRuby::Util::ON_WINDOWS - require_relative 'open3/jruby_windows' - return -end - module Open3 + # Open stdin, stdout, and stderr streams and start external executable. # In addition, a thread to wait for the started process is created. # The thread has a pid method and a thread variable :pid which is the pid of @@ -763,3 +759,6 @@ class << self end end + +# JRuby uses different popen logic on Windows, require it here to reuse wrapper methods above. +require 'open3/jruby_windows' if RUBY_ENGINE == 'jruby' && JRuby::Util::ON_WINDOWS diff --git a/lib/open3/jruby_windows.rb b/lib/open3/jruby_windows.rb index 24b9a1b..064c38b 100644 --- a/lib/open3/jruby_windows.rb +++ b/lib/open3/jruby_windows.rb @@ -57,6 +57,15 @@ def popen2e(*cmd, &block) module_function :popen2e def processbuilder_run(cmd, opts, build: nil, io:) + opts.each do |k, v| + if Integer === k + if IO == v || !(String === v || v.respond_to?(:to_path)) + # target is an open IO or a non-pathable object, bail out + raise NotImplementedError.new("redirect to an open IO is not implemented on this platform") + end + end + end + if Hash === cmd[0] env = cmd.shift; else From d2308040e697f28ed765cefad31ea1f0f2f36473 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 30 Sep 2021 10:42:01 -0500 Subject: [PATCH 6/6] Use RbConfig's 'host_os' RUBY_PLATFORM on JRuby is always 'java' so it does not indicate the host OS. --- test/test_open3.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_open3.rb b/test/test_open3.rb index 699b05a..36ccd4c 100644 --- a/test/test_open3.rb +++ b/test/test_open3.rb @@ -95,7 +95,7 @@ def test_numeric_file_descriptor2 end def test_numeric_file_descriptor3 - skip "passing FDs bigger than 2 is not supported on Windows" if /mswin|mingw/ =~ RUBY_PLATFORM + skip "passing FDs bigger than 2 is not supported on Windows" if /mswin|mingw/ =~ RbConfig::CONFIG['host_os'] with_pipe {|r, w| Open3.popen3(RUBY, '-e', 'IO.open(3).puts "foo"', 3 => w) {|i,o,e,t| assert_equal("foo\n", r.gets, "[GH-808] [ruby-core:67347] [Bug #10699]")