diff --git a/lib/puppet/util/execution.rb b/lib/puppet/util/execution.rb index c8673d2525..4d196f26d3 100644 --- a/lib/puppet/util/execution.rb +++ b/lib/puppet/util/execution.rb @@ -57,31 +57,19 @@ def initialize(value, exitstatus) # @see Kernel#open for `mode` values # @api public def self.execpipe(command, failonfail = true) - # Paste together an array with spaces. We used to paste directly - # together, no spaces, which made for odd invocations; the user had to - # include whitespace between arguments. - # - # Having two spaces is really not a big drama, since this passes to the - # shell anyhow, while no spaces makes for a small developer cost every - # time this is invoked. --daniel 2012-02-13 - command_str = command.respond_to?(:join) ? command.join(' ') : command - if respond_to? :debug - debug "Executing '#{command_str}'" + debug "Executing #{command.inspect}" else - Puppet.debug { "Executing '#{command_str}'" } + Puppet.debug { "Executing #{command.inspect}" } end - # force the run of the command with - # the user/system locale to "C" (via environment variables LANG and LC_*) - # it enables to have non localized output for some commands and therefore - # a predictable output - english_env = ENV.to_hash.merge({ 'LANG' => 'C', 'LC_ALL' => 'C' }) - output = Puppet::Util.withenv(english_env) do - # We are intentionally using 'pipe' with open to launch a process - open("| #{command_str} 2>&1") do |pipe| # rubocop:disable Security/Open + begin + output = IO.popen({ 'LANG' => 'C', 'LC_ALL' => 'C' }, command, err: :out) do |pipe| yield pipe end + rescue Errno::ENOENT => e + output = e.message + raise Puppet::ExecutionFailure, output if failonfail end if failonfail && exitstatus != 0 diff --git a/spec/unit/provider/package/pacman_spec.rb b/spec/unit/provider/package/pacman_spec.rb index 0971541e9b..46ad5f602e 100644 --- a/spec/unit/provider/package/pacman_spec.rb +++ b/spec/unit/provider/package/pacman_spec.rb @@ -403,13 +403,13 @@ let(:groups) { [['foo package1'], ['foo package2'], ['bar package3'], ['bar package4'], ['baz package5']] } it 'should raise an error on non-zero pacman exit without a filter' do - expect(executor).to receive(:open).with('| /usr/bin/pacman --sync -gg 2>&1').and_return('error!') + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, ['/usr/bin/pacman', '--sync', '-gg'], err: :out).and_return('error!') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(1) expect { described_class.get_installed_groups(installed_packages) }.to raise_error(Puppet::ExecutionFailure, 'error!') end it 'should return empty groups on non-zero pacman exit with a filter' do - expect(executor).to receive(:open).with('| /usr/bin/pacman --sync -gg git 2>&1').and_return('') + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, ['/usr/bin/pacman', '--sync', '-gg', 'git'], err: :out).and_return('') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(1) expect(described_class.get_installed_groups(installed_packages, 'git')).to eq({}) end @@ -417,7 +417,7 @@ it 'should return empty groups on empty pacman output' do pipe = double() expect(pipe).to receive(:each_line) - expect(executor).to receive(:open).with('| /usr/bin/pacman --sync -gg 2>&1').and_yield(pipe).and_return('') + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, ['/usr/bin/pacman', '--sync', '-gg'], err: :out).and_yield(pipe).and_return('') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(0) expect(described_class.get_installed_groups(installed_packages)).to eq({}) end @@ -427,7 +427,7 @@ pipe_expectation = receive(:each_line) groups.each { |group| pipe_expectation = pipe_expectation.and_yield(*group) } expect(pipe).to pipe_expectation - expect(executor).to receive(:open).with('| /usr/bin/pacman --sync -gg 2>&1').and_yield(pipe).and_return('') + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, ['/usr/bin/pacman', '--sync', '-gg'], err: :out).and_yield(pipe).and_return('') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(0) expect(described_class.get_installed_groups(installed_packages)).to eq({'foo' => 'package1 1.0, package2 2.0'}) end diff --git a/spec/unit/util/execution_spec.rb b/spec/unit/util/execution_spec.rb index 9c4b9f8cac..377f5c0de9 100644 --- a/spec/unit/util/execution_spec.rb +++ b/spec/unit/util/execution_spec.rb @@ -909,35 +909,35 @@ def expect_cwd_to_be(cwd) describe "#execpipe" do it "should execute a string as a string" do - expect(Puppet::Util::Execution).to receive(:open).with('| echo hello 2>&1').and_return('hello') + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'echo hello', err: :out).and_return('hello') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(0) expect(Puppet::Util::Execution.execpipe('echo hello')).to eq('hello') end it "should print meaningful debug message for string argument" do Puppet[:log_level] = 'debug' - expect(Puppet).to receive(:send_log).with(:debug, "Executing 'echo hello'") - expect(Puppet::Util::Execution).to receive(:open).with('| echo hello 2>&1').and_return('hello') + expect(Puppet).to receive(:send_log).with(:debug, 'Executing "echo hello"') + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'echo hello', err: :out).and_return('hello') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(0) Puppet::Util::Execution.execpipe('echo hello') end it "should print meaningful debug message for array argument" do Puppet[:log_level] = 'debug' - expect(Puppet).to receive(:send_log).with(:debug, "Executing 'echo hello'") - expect(Puppet::Util::Execution).to receive(:open).with('| echo hello 2>&1').and_return('hello') + expect(Puppet).to receive(:send_log).with(:debug, 'Executing ["echo", "hello"]') + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, ['echo', 'hello'], err: :out).and_return('hello') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(0) Puppet::Util::Execution.execpipe(['echo','hello']) end - it "should execute an array by pasting together with spaces" do - expect(Puppet::Util::Execution).to receive(:open).with('| echo hello 2>&1').and_return('hello') + it "should execute an array" do + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, ['echo', 'hello'], err: :out).and_return('hello') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(0) expect(Puppet::Util::Execution.execpipe(['echo', 'hello'])).to eq('hello') end it "should fail if asked to fail, and the child does" do - allow(Puppet::Util::Execution).to receive(:open).with('| echo hello 2>&1').and_return('error message') + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'echo hello', err: :out).and_return('error message') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(1) expect { Puppet::Util::Execution.execpipe('echo hello') @@ -945,7 +945,8 @@ def expect_cwd_to_be(cwd) end it "should not fail if asked not to fail, and the child does" do - allow(Puppet::Util::Execution).to receive(:open).and_return('error message') + expect(IO).to receive(:popen).with({ 'LANG' => 'C', 'LC_ALL' => 'C' }, 'echo hello', err: :out).and_return('error message') + expect(Puppet::Util::Execution).not_to receive(:exitstatus) expect(Puppet::Util::Execution.execpipe('echo hello', false)).to eq('error message') end end