diff --git a/lib/cask/artifact/pkg.rb b/lib/cask/artifact/pkg.rb index bbf02806f4..69ec9f1b4d 100644 --- a/lib/cask/artifact/pkg.rb +++ b/lib/cask/artifact/pkg.rb @@ -84,9 +84,9 @@ class Cask::Artifact::Pkg < Cask::Artifact::Base if uninstall_options.key? :quit [*uninstall_options[:quit]].each do |id| ohai "Quitting application ID #{id}" - num_running = @command.run!('/usr/bin/osascript', :args => ['-e', "tell application \"System Events\" to count processes whose bundle identifier is \"#{id}\""], :sudo => true).to_i + num_running = @command.run!('/usr/bin/osascript', :args => ['-e', %Q{tell application "System Events" to count processes whose bundle identifier is "#{id}"}], :sudo => true).to_i if num_running > 0 - @command.run!('/usr/bin/osascript', :args => ['-e', "tell application id \"#{id}\" to quit"], :sudo => true) + @command.run!('/usr/bin/osascript', :args => ['-e', %Q{tell application id "#{id}" to quit}], :sudo => true) end end end diff --git a/lib/cask/pkg.rb b/lib/cask/pkg.rb index 6425957c5c..95a750a26f 100644 --- a/lib/cask/pkg.rb +++ b/lib/cask/pkg.rb @@ -1,6 +1,6 @@ class Cask::Pkg def self.all_matching(regexp, command) - command.run(%Q(/usr/sbin/pkgutil --pkgs="#{regexp}")).split("\n").map do |package_id| + command.run('/usr/sbin/pkgutil', :args => [%Q{--pkgs=#{regexp}}]).split("\n").map do |package_id| new(package_id.chomp, command) end end diff --git a/lib/cask/system_command.rb b/lib/cask/system_command.rb index 33292b52f7..024673615d 100644 --- a/lib/cask/system_command.rb +++ b/lib/cask/system_command.rb @@ -1,14 +1,21 @@ +require 'open3' + class Cask::SystemCommand - def self.run(command, options={}) - command = _process_options(command, options) - odebug "Executing: #{command}" + def self.run(executable, options={}) + command = _process_options(executable, options) + odebug "Executing: #{command.inspect}" output = '' - IO.popen(command, 'r+') do |pipe| + Open3.popen3(*command) do |stdin, stdout, stderr| if options[:input] - options[:input].each { |line| pipe.puts line } + options[:input].each { |line| stdin.puts line } end - pipe.close_write - while line = pipe.gets + stdin.close_write + while line = stdout.gets + output << line + ohai line.chomp if options[:print] + end + while line = stderr.gets + next if options[:stderr] == :silence output << line ohai line.chomp if options[:print] end @@ -25,33 +32,20 @@ class Cask::SystemCommand run(command, options.merge(:must_succeed => true)) end - def self._process_options(command, options) + def self._process_options(executable, options) + command = [executable] if options[:sudo] - command = "/usr/bin/sudo -E -- #{_quote(command)}" + command.unshift('/usr/bin/sudo', '-E', '--') end - if options[:args] - command = "#{command} #{options[:args].map { |arg| _quote(arg) }.join(' ')}" - end - case options[:stderr] - when :silence then - command = "#{command} 2>/dev/null" - when :merge, nil then - command = "#{command} 2>&1" + if ! options[:args].empty? + command.concat options[:args] end command end def self._assert_success(status, command, output) unless status.success? - raise CaskCommandFailedError.new(command, output) - end - end - - def self._quote(string) - if %r{^(['"]).*\1$}.match(string) - string - else - %Q('#{string}') + raise CaskCommandFailedError.new(command.inspect, output) end end @@ -62,7 +56,7 @@ class Cask::SystemCommand raise CaskError.new(<<-ERRMSG) Error parsing plist output from command. command was: - #{command} + #{command.inspect} output we attempted to parse: #{output} ERRMSG diff --git a/test/cask/artifact/pkg_test.rb b/test/cask/artifact/pkg_test.rb index c35c304963..128e02208b 100644 --- a/test/cask/artifact/pkg_test.rb +++ b/test/cask/artifact/pkg_test.rb @@ -12,14 +12,11 @@ describe Cask::Artifact::Pkg do it 'runs the system installer on the specified pkgs' do pkg = Cask::Artifact::Pkg.new(@cask, Cask::FakeSystemCommand) - expected_command = "/usr/bin/sudo -E -- '/usr/sbin/installer' '-pkg' '#{@cask.destination_path/'MyFancyPkg'/'Fancy.pkg'}' '-target' '/' 2>&1" - Cask::FakeSystemCommand.stubs_command(expected_command) + Cask::FakeSystemCommand.expects_command(['/usr/bin/sudo', '-E', '--', '/usr/sbin/installer', '-pkg', @cask.destination_path/'MyFancyPkg'/'Fancy.pkg', '-target', '/']) shutup do pkg.install end - - Cask::FakeSystemCommand.system_calls[expected_command].must_equal 1 end end @@ -27,17 +24,14 @@ describe Cask::Artifact::Pkg do it 'runs the specified uninstaller for the cask' do pkg = Cask::Artifact::Pkg.new(@cask, Cask::FakeSystemCommand) - Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/bin/osascript' '-e' 'tell application "System Events" to count processes whose bundle identifier is "my.fancy.package.app"' 2>&1), '1') - Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/bin/osascript' '-e' 'tell application id "my.fancy.package.app" to quit' 2>&1)) + Cask::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application "System Events" to count processes whose bundle identifier is "my.fancy.package.app"'], '1') + Cask::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application id "my.fancy.package.app" to quit']) - expected_command = "/usr/bin/sudo -E -- '#{@cask.destination_path/'MyFancyPkg'/'FancyUninstaller.tool'}' '--please' 2>&1" - Cask::FakeSystemCommand.stubs_command(expected_command) + Cask::FakeSystemCommand.expects_command(['/usr/bin/sudo', '-E', '--', @cask.destination_path/'MyFancyPkg'/'FancyUninstaller.tool', '--please']) shutup do pkg.uninstall end - - Cask::FakeSystemCommand.system_calls[expected_command].must_equal 1 end it 'can uninstall using pkgutil, launchctl, and file lists' do @@ -45,7 +39,7 @@ describe Cask::Artifact::Pkg do pkg = Cask::Artifact::Pkg.new(cask, Cask::FakeSystemCommand) Cask::FakeSystemCommand.stubs_command( - %Q(/usr/sbin/pkgutil --pkgs="my.fancy.package.*" 2>&1), + ['/usr/sbin/pkgutil', '--pkgs=my.fancy.package.*'], [ 'my.fancy.package.main', 'my.fancy.package.agent', @@ -53,14 +47,14 @@ describe Cask::Artifact::Pkg do ) Cask::FakeSystemCommand.stubs_command( - %Q(/usr/sbin/pkgutil '--only-files' '--files' 'my.fancy.package.main' 2>&1), + ['/usr/sbin/pkgutil', '--only-files', '--files', 'my.fancy.package.main'], [ 'fancy/bin/fancy.exe', 'fancy/var/fancy.data', ].join("\n") ) Cask::FakeSystemCommand.stubs_command( - %Q(/usr/sbin/pkgutil '--only-dirs' '--files' 'my.fancy.package.main' 2>&1), + ['/usr/sbin/pkgutil', '--only-dirs', '--files', 'my.fancy.package.main'], [ 'fancy', 'fancy/bin', @@ -68,7 +62,7 @@ describe Cask::Artifact::Pkg do ].join("\n") ) Cask::FakeSystemCommand.stubs_command( - %Q(/usr/sbin/pkgutil '--pkg-info-plist' 'my.fancy.package.main' 2>&1), + ['/usr/sbin/pkgutil', '--pkg-info-plist', 'my.fancy.package.main'], <<-PLIST @@ -84,11 +78,11 @@ describe Cask::Artifact::Pkg do ) Cask::FakeSystemCommand.stubs_command( - %Q(/bin/launchctl 'list' '-x' 'my.fancy.package.service' 2>&1), + ['/bin/launchctl', 'list', '-x', 'my.fancy.package.service'], "launchctl list returned unknown response\n" ) Cask::FakeSystemCommand.stubs_command( - %Q(/usr/bin/sudo -E -- '/bin/launchctl' 'list' '-x' 'my.fancy.package.service' 2>&1), + ['/usr/bin/sudo', '-E', '--', '/bin/launchctl', 'list', '-x', 'my.fancy.package.service'], <<-PLIST @@ -112,14 +106,15 @@ describe Cask::Artifact::Pkg do PLIST ) - Cask::FakeSystemCommand.expects_command(%Q(/usr/bin/sudo -E -- '/bin/launchctl' 'remove' '--' 'my.fancy.package.service' 2>&1)) - Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/sbin/kextstat' '-l' '-b' 'my.fancy.package.kernelextension' 2>&1), 'loaded') - Cask::FakeSystemCommand.expects_command(%Q(/usr/bin/sudo -E -- '/sbin/kextunload' '-b' '--' 'my.fancy.package.kernelextension' 2>&1)) - Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/sbin/pkgutil' '--forget' 'my.fancy.package.main' 2>&1)) + Cask::FakeSystemCommand.expects_command(['/usr/bin/sudo', '-E', '--', '/bin/launchctl', 'remove', '--', 'my.fancy.package.service']) + + Cask::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/sbin/kextstat', '-l', '-b', 'my.fancy.package.kernelextension'], 'loaded') + Cask::FakeSystemCommand.expects_command(['/usr/bin/sudo', '-E', '--', '/sbin/kextunload', '-b', '--', 'my.fancy.package.kernelextension']) + Cask::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/sbin/pkgutil', '--forget', 'my.fancy.package.main']) Cask::FakeSystemCommand.stubs_command( - %Q(/usr/sbin/pkgutil '--only-files' '--files' 'my.fancy.package.agent' 2>&1), + ['/usr/sbin/pkgutil', '--only-files', '--files', 'my.fancy.package.agent'], [ 'fancy/agent/fancy-agent.exe', 'fancy/agent/fancy-agent.pid', @@ -127,14 +122,14 @@ describe Cask::Artifact::Pkg do ].join("\n") ) Cask::FakeSystemCommand.stubs_command( - %Q(/usr/sbin/pkgutil '--only-dirs' '--files' 'my.fancy.package.agent' 2>&1), + ['/usr/sbin/pkgutil', '--only-dirs', '--files', 'my.fancy.package.agent'], [ 'fancy', 'fancy/agent', ].join("\n") ) Cask::FakeSystemCommand.stubs_command( - %Q(/usr/sbin/pkgutil '--pkg-info-plist' 'my.fancy.package.agent' 2>&1), + ['/usr/sbin/pkgutil', '--pkg-info-plist', 'my.fancy.package.agent'], <<-PLIST @@ -155,13 +150,18 @@ describe Cask::Artifact::Pkg do /tmp/fancy/bin /tmp/fancy/var ].each do |dir| - Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/bin/chmod' '--' '777' '#{dir}' 2>&1)) + Cask::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/bin/chmod', '--', '777', '#{dir}']) end - Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/sbin/pkgutil' '--forget' 'my.fancy.package.agent' 2>&1)) + Cask::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/sbin/pkgutil', '--forget', 'my.fancy.package.agent']) - Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/bin/rm' '-f' '--' '/tmp/fancy/bin/fancy.exe' '/tmp/fancy/var/fancy.data' 2>&1)) - Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/bin/rm' '-f' '--' '/tmp/fancy/agent/fancy-agent.exe' '/tmp/fancy/agent/fancy-agent.pid' '/tmp/fancy/agent/fancy-agent.log' 2>&1)) + Cask::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/bin/rm', '-f', '--', + Pathname.new('/tmp/fancy/bin/fancy.exe'), + Pathname.new('/tmp/fancy/var/fancy.data')]) + Cask::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/bin/rm', '-f', '--', + Pathname.new('/tmp/fancy/agent/fancy-agent.exe'), + Pathname.new('/tmp/fancy/agent/fancy-agent.pid'), + Pathname.new('/tmp/fancy/agent/fancy-agent.log')]) # No assertions after call since all assertions are implicit from the interactions setup above. # TODO: verify rmdir commands (requires setting up actual file tree or faking out .exists? diff --git a/test/cask/cli/alfred_test.rb b/test/cask/cli/alfred_test.rb index c72191e7c5..430d0d6007 100644 --- a/test/cask/cli/alfred_test.rb +++ b/test/cask/cli/alfred_test.rb @@ -1,7 +1,7 @@ require 'test_helper' def fake_alfred_preference(key, response) - Cask::FakeSystemCommand.stubs_command("/usr/bin/defaults 'read' 'com.runningwithcrayons.Alfred-Preferences' '#{key}' 2>&1", response) + Cask::FakeSystemCommand.stubs_command(['/usr/bin/defaults', 'read', 'com.runningwithcrayons.Alfred-Preferences', key], response) end def fake_alfred_installed(installed=true) @@ -78,7 +78,7 @@ describe Cask::CLI::Alfred do SCOPE_RESPONSE Cask::FakeSystemCommand.stubs_command( - %Q(/usr/bin/defaults 'write' 'com.runningwithcrayons.Alfred-Preferences' 'features.defaultresults.scope' "('/Applications','/Library/PreferencePanes','/System/Library/PreferencePanes','#{Cask.caskroom}')" 2>&1) + ['/usr/bin/defaults', 'write', 'com.runningwithcrayons.Alfred-Preferences', 'features.defaultresults.scope', %Q{"('/Applications','/Library/PreferencePanes','/System/Library/PreferencePanes','#{Cask.caskroom}')"}] ) TestHelper.must_output(self, lambda { @@ -95,7 +95,7 @@ describe Cask::CLI::Alfred do expected_scopes = (Cask::CLI::Alfred::DEFAULT_SCOPES + [Cask.caskroom]).map { |s| "'#{s}'" } Cask::FakeSystemCommand.stubs_command( - %Q(/usr/bin/defaults 'write' 'com.runningwithcrayons.Alfred-Preferences' 'features.defaultresults.scope' "(#{expected_scopes.join(',')})" 2>&1) + ['/usr/bin/defaults', 'write', 'com.runningwithcrayons.Alfred-Preferences', 'features.defaultresults.scope', %Q{"(#{expected_scopes.join(',')})"}] ) TestHelper.must_output(self, lambda { @@ -140,7 +140,7 @@ describe Cask::CLI::Alfred do SCOPE_RESPONSE Cask::FakeSystemCommand.stubs_command( - %Q(/usr/bin/defaults 'write' 'com.runningwithcrayons.Alfred-Preferences' 'features.defaultresults.scope' "('/Applications','/Library/PreferencePanes','/System/Library/PreferencePanes')" 2>&1) + ['/usr/bin/defaults', 'write', 'com.runningwithcrayons.Alfred-Preferences', 'features.defaultresults.scope', %Q{"('/Applications','/Library/PreferencePanes','/System/Library/PreferencePanes')"}] ) TestHelper.must_output(self, lambda { diff --git a/test/cask/container/naked_test.rb b/test/cask/container/naked_test.rb index 11af3cf790..6a6dda34f0 100644 --- a/test/cask/container/naked_test.rb +++ b/test/cask/container/naked_test.rb @@ -11,7 +11,7 @@ describe Cask::Container::Naked do cask = SpaceyCask.new path = '/tmp/downloads/kevin-spacey-1.2.pkg' expected_destination = cask.destination_path.join('kevin spacey.pkg') - expected_command = %Q(/usr/bin/ditto '--' '#{path}' '#{expected_destination}' 2>&1) + expected_command = ['/usr/bin/ditto', '--', path, expected_destination] Cask::FakeSystemCommand.stubs_command(expected_command) container = Cask::Container::Naked.new(cask, path, Cask::FakeSystemCommand) diff --git a/test/cask/pkg_test.rb b/test/cask/pkg_test.rb index c4acee8300..e6238d1d13 100644 --- a/test/cask/pkg_test.rb +++ b/test/cask/pkg_test.rb @@ -23,14 +23,14 @@ describe Cask::Pkg do pkg = Cask::Pkg.new('my.fake.pkg', Cask::FakeSystemCommand) Cask::FakeSystemCommand.stubs_command( - "/usr/sbin/pkgutil '--only-files' '--files' 'my.fake.pkg' 2>&1" + ['/usr/sbin/pkgutil', '--only-files', '--files', 'my.fake.pkg'] ) Cask::FakeSystemCommand.stubs_command( - "/usr/sbin/pkgutil '--only-dirs' '--files' 'my.fake.pkg' 2>&1" + ['/usr/sbin/pkgutil', '--only-dirs', '--files', 'my.fake.pkg'] ) Cask::FakeSystemCommand.expects_command( - %q(/usr/bin/sudo -E -- '/usr/sbin/pkgutil' '--forget' 'my.fake.pkg' 2>&1) + ['/usr/bin/sudo', '-E', '--', '/usr/sbin/pkgutil', '--forget', 'my.fake.pkg'] ) pkg.uninstall diff --git a/test/support/fake_system_command.rb b/test/support/fake_system_command.rb index 4bf7c32cad..22381a42df 100644 --- a/test/support/fake_system_command.rb +++ b/test/support/fake_system_command.rb @@ -29,7 +29,7 @@ class Cask::FakeSystemCommand def self.verify_expectations! expectations.each do |command, times| unless system_calls[command] == times - fail("expected #{command} to be run #{times} times, but got #{system_calls[command]}") + fail("expected #{command.inspect} to be run #{times} times, but got #{system_calls[command]}") end end end