diff --git a/Library/Homebrew/cask/lib/hbc/system_command.rb b/Library/Homebrew/cask/lib/hbc/system_command.rb index c21e87d8e8..48d57689ca 100644 --- a/Library/Homebrew/cask/lib/hbc/system_command.rb +++ b/Library/Homebrew/cask/lib/hbc/system_command.rb @@ -47,7 +47,7 @@ module Hbc @must_succeed = must_succeed options.extend(HashValidator).assert_valid_keys(:chdir) @options = options - @env = { "PATH" => ENV["PATH"] }.merge(env) + @env = env @env.keys.grep_v(/^[\w&&\D]\w*$/) do |name| raise ArgumentError, "Invalid variable name: '#{name}'" @@ -55,7 +55,7 @@ module Hbc end def command - [*sudo_prefix, executable, *args] + [*sudo_prefix, *env_args, executable, *args] end private @@ -64,18 +64,22 @@ module Hbc attr_predicate :sudo?, :print_stdout?, :print_stderr?, :must_succeed? + def env_args + return [] if env.empty? + + variables = env.map do |name, value| + sanitized_name = Shellwords.escape(name) + sanitized_value = Shellwords.escape(value) + "#{sanitized_name}=#{sanitized_value}" + end + + ["env", *variables] + end + def sudo_prefix return [] unless sudo? askpass_flags = ENV.key?("SUDO_ASKPASS") ? ["-A"] : [] - prefix = ["/usr/bin/sudo", *askpass_flags, "-E"] - - env.each do |name, value| - sanitized_name = Shellwords.escape(name) - sanitized_value = Shellwords.escape(value) - prefix << "#{sanitized_name}=#{sanitized_value}" - end - - prefix << "--" + ["/usr/bin/sudo", *askpass_flags, "-E", "--"] end def assert_success @@ -97,11 +101,7 @@ module Hbc executable, *args = expanded_command raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = - # We need to specifically use `with_env` for `PATH`, otherwise - # Ruby itself will not look for the executable in `PATH`. - with_env "PATH" => env["PATH"] do - Open3.popen3(env, [executable, executable], *args, **options) - end + Open3.popen3([executable, executable], *args, **options) write_input_to(raw_stdin) raw_stdin.close_write diff --git a/Library/Homebrew/test/cask/system_command_spec.rb b/Library/Homebrew/test/cask/system_command_spec.rb index 1c139a167f..f08193e652 100644 --- a/Library/Homebrew/test/cask/system_command_spec.rb +++ b/Library/Homebrew/test/cask/system_command_spec.rb @@ -15,10 +15,10 @@ describe Hbc::SystemCommand, :cask do its("run!.stdout") { is_expected.to eq("123") } describe "the resulting command line" do - it "does not include the given variables" do + it "includes the given variables explicitly" do expect(Open3) .to receive(:popen3) - .with(a_hash_including("PATH"), ["env"] * 2, *env_args, {}) + .with(["env", "env"], "A=1", "B=2", "C=3", "env", *env_args, {}) .and_call_original subject.run! @@ -41,9 +41,8 @@ describe Hbc::SystemCommand, :cask do it "includes the given variables explicitly" do expect(Open3) .to receive(:popen3) - .with(an_instance_of(Hash), ["/usr/bin/sudo"] * 2, - "-E", a_string_starting_with("PATH="), - "A=1", "B=2", "C=3", "--", "env", *env_args, {}) + .with(["/usr/bin/sudo", "/usr/bin/sudo"], "-E", "--", + "env", "A=1", "B=2", "C=3", "env", *env_args, {}) .and_wrap_original do |original_popen3, *_, &block| original_popen3.call("/usr/bin/true", &block) end @@ -197,7 +196,7 @@ describe Hbc::SystemCommand, :cask do end end - it "looks for executables in custom PATH" do + it "looks for executables in a custom PATH" do mktmpdir do |path| (path/"tool").write <<~SH #!/bin/sh diff --git a/Library/Homebrew/test/support/helper/cask/fake_system_command.rb b/Library/Homebrew/test/support/helper/cask/fake_system_command.rb index 211b081888..93e1c194f1 100644 --- a/Library/Homebrew/test/support/helper/cask/fake_system_command.rb +++ b/Library/Homebrew/test/support/helper/cask/fake_system_command.rb @@ -1,5 +1,5 @@ def sudo(*args) - ["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--"] + args.flatten + ["/usr/bin/sudo", "-E", "--"] + args.flatten end module Hbc diff --git a/Library/Homebrew/test/support/helper/spec/shared_examples/hbc_staged.rb b/Library/Homebrew/test/support/helper/spec/shared_examples/hbc_staged.rb index 1e29456f31..b69c116fdb 100644 --- a/Library/Homebrew/test/support/helper/spec/shared_examples/hbc_staged.rb +++ b/Library/Homebrew/test/support/helper/spec/shared_examples/hbc_staged.rb @@ -80,7 +80,7 @@ shared_examples Hbc::Staged do allow(staged).to receive(:Pathname).and_return(fake_pathname) Hbc::FakeSystemCommand.expects_command( - ["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--", "/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname], + sudo("/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname), ) staged.set_ownership(fake_pathname.to_s) @@ -93,7 +93,7 @@ shared_examples Hbc::Staged do allow(staged).to receive(:Pathname).and_return(fake_pathname) Hbc::FakeSystemCommand.expects_command( - ["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--", "/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname, fake_pathname], + sudo("/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname, fake_pathname), ) staged.set_ownership([fake_pathname.to_s, fake_pathname.to_s]) @@ -105,7 +105,7 @@ shared_examples Hbc::Staged do allow(staged).to receive(:Pathname).and_return(fake_pathname) Hbc::FakeSystemCommand.expects_command( - ["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--", "/usr/sbin/chown", "-R", "--", "other_user:other_group", fake_pathname], + sudo("/usr/sbin/chown", "-R", "--", "other_user:other_group", fake_pathname), ) staged.set_ownership(fake_pathname.to_s, user: "other_user", group: "other_group")