From fff82dc685711a1e2e3595332e184dc0e2a5dc9c Mon Sep 17 00:00:00 2001 From: Klaas Jan Wierenga Date: Thu, 2 May 2013 11:00:47 +0200 Subject: [PATCH 1/5] Ruby 1.8 doesn't have the concept of string encodings. Compare to the string as is. --- spec/foreman/process_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/foreman/process_spec.rb b/spec/foreman/process_spec.rb index 9a189b9..f9a54e0 100644 --- a/spec/foreman/process_spec.rb +++ b/spec/foreman/process_spec.rb @@ -41,7 +41,7 @@ describe Foreman::Process do it "should output utf8 properly" do process = Foreman::Process.new(resource_path("bin/utf8")) - run(process).should == "\xFF\x03\n".force_encoding('binary') + run(process).should == (Foreman.ruby_18? ? "\xFF\x03\n" : "\xFF\x03\n".force_encoding('binary')) end end From 9b4bd10cdb19eb62ee99baa0006119a670f45d07 Mon Sep 17 00:00:00 2001 From: Klaas Jan Wierenga Date: Thu, 2 May 2013 12:37:06 +0200 Subject: [PATCH 2/5] Use pipe factory method to handle 1.8 incompatible signature for IO.pipe. --- spec/spec_helper.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 70536cd..b9f3efe 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -21,6 +21,10 @@ def mock_error(subject, message) end end +def make_pipe + IO.method(:pipe).arity.zero? ? IO.pipe : IO.pipe("BINARY") +end + def foreman(args) capture_stdout do begin @@ -31,14 +35,14 @@ def foreman(args) end def forked_foreman(args) - rd, wr = IO.pipe("BINARY") + rd, wr = make_pipe Process.spawn("bundle exec bin/foreman #{args}", :out => wr, :err => wr) wr.close rd.read end def fork_and_capture(&blk) - rd, wr = IO.pipe("BINARY") + rd, wr = make_pipe pid = fork do rd.close wr.sync = true @@ -141,7 +145,7 @@ end def capture_stdout old_stdout = $stdout.dup - rd, wr = IO.method(:pipe).arity.zero? ? IO.pipe : IO.pipe("BINARY") + rd, wr = make_pipe $stdout = wr yield wr.close From 75b782b66469419e545b8e3a26b3ed19c8cad784 Mon Sep 17 00:00:00 2001 From: Klaas Jan Wierenga Date: Thu, 2 May 2013 12:39:23 +0200 Subject: [PATCH 3/5] Use POSIX::Spawn to make foreman ruby 1.8 compatible and have all specs passing. --- Gemfile | 2 +- lib/foreman/process.rb | 10 +--------- spec/spec_helper.rb | 14 ++++++++++++-- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index 3470ea2..da92b31 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,7 @@ platform :mingw do gem "win32console", "~> 1.3.0" end -platform :jruby do +platform :jruby, :ruby_18 do gem "posix-spawn", "~> 0.3.6" end diff --git a/lib/foreman/process.rb b/lib/foreman/process.rb index 2abcb82..39cb4c5 100644 --- a/lib/foreman/process.rb +++ b/lib/foreman/process.rb @@ -52,18 +52,10 @@ class Foreman::Process Dir.chdir(cwd) do Process.spawn env, expanded_command(env), :out => output, :err => output end - elsif Foreman.jruby_18? + elsif Foreman.jruby_18? || Foreman.ruby_18? require "posix/spawn" wrapped_command = "#{Foreman.runner} -d '#{cwd}' -p -- #{command}" POSIX::Spawn.spawn env, wrapped_command, :out => output, :err => output - elsif Foreman.ruby_18? - fork do - $stdout.reopen output - $stderr.reopen output - env.each { |k,v| ENV[k] = v } - wrapped_command = "#{Foreman.runner} -d '#{cwd}' -p -- #{command}" - Kernel.exec wrapped_command - end else wrapped_command = "#{Foreman.runner} -d '#{cwd}' -p -- #{command}" Process.spawn env, wrapped_command, :out => output, :err => output diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b9f3efe..76676ee 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -36,7 +36,12 @@ end def forked_foreman(args) rd, wr = make_pipe - Process.spawn("bundle exec bin/foreman #{args}", :out => wr, :err => wr) + if Foreman.jruby_18? || Foreman.ruby_18? + require 'posix/spawn' + POSIX::Spawn.spawn({}, "bundle exec bin/foreman #{args}", :out => wr, :err => wr) + else + Process.spawn("bundle exec bin/foreman #{args}", :out => wr, :err => wr) + end wr.close rd.read end @@ -61,7 +66,12 @@ def fork_and_capture(&blk) end def fork_and_get_exitstatus(args) - pid = Process.spawn("bundle exec bin/foreman #{args}", :out => "/dev/null", :err => "/dev/null") + pid = if Foreman.jruby_18? || Foreman.ruby_18? + require 'posix/spawn' + POSIX::Spawn.spawn({}, "bundle exec bin/foreman #{args}", :out => "/dev/null", :err => "/dev/null") + else + Process.spawn("bundle exec bin/foreman #{args}", :out => "/dev/null", :err => "/dev/null") + end Process.wait(pid) $?.exitstatus end From 5c06aaaa572b84e119bf31b3a23e2cfc0c7223fc Mon Sep 17 00:00:00 2001 From: Klaas Jan Wierenga Date: Thu, 2 May 2013 12:49:05 +0200 Subject: [PATCH 4/5] Enable ruby 1.8.7 building on Travis. All specs should pass now on ruby 1.8.7. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index accae0f..b2beaa9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,6 @@ script: bundle exec rake spec matrix: allow_failures: - - rvm: 1.8.7 - rvm: jruby - rvm: rbx - rvm: ree @@ -16,6 +15,7 @@ notifications: - http://dx-helper.herokuapp.com/travis rvm: + - 1.8.7 - 1.9.2 - 1.9.3 - 2.0.0 From baf842cdd4a0addf13492423173ae60966e0b5cd Mon Sep 17 00:00:00 2001 From: Klaas Jan Wierenga Date: Thu, 2 May 2013 15:06:38 +0200 Subject: [PATCH 5/5] The wrapped_command has spaces which triggers Ruby to fork a system shell (with /bin/sh -c). This causes orphaned processes on some systems (i.e. Linux). Fix this by splitting the command using String#shellsplit and using ruby's splat operator (*) to pass discrete arguments to spawn. --- lib/foreman/process.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/foreman/process.rb b/lib/foreman/process.rb index 39cb4c5..c2838aa 100644 --- a/lib/foreman/process.rb +++ b/lib/foreman/process.rb @@ -1,4 +1,5 @@ require "foreman" +require "shellwords" class Foreman::Process @@ -54,8 +55,8 @@ class Foreman::Process end elsif Foreman.jruby_18? || Foreman.ruby_18? require "posix/spawn" - wrapped_command = "#{Foreman.runner} -d '#{cwd}' -p -- #{command}" - POSIX::Spawn.spawn env, wrapped_command, :out => output, :err => output + wrapped_command = "#{Foreman.runner} -d '#{cwd}' -p -- #{expanded_command(env)}" + POSIX::Spawn.spawn(*spawn_args(env, wrapped_command.shellsplit, {:out => output, :err => output})) else wrapped_command = "#{Foreman.runner} -d '#{cwd}' -p -- #{command}" Process.spawn env, wrapped_command, :out => output, :err => output @@ -114,4 +115,14 @@ class Foreman::Process File.expand_path(@options[:cwd] || ".") end +private + + def spawn_args(env, argv, options) + args = [] + args << env + args += argv + args << options + args + end + end