diff --git a/Library/Homebrew/cask/lib/hbc/artifact/installer.rb b/Library/Homebrew/cask/lib/hbc/artifact/installer.rb index 8fa54c01ec..a3a84164e0 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/installer.rb @@ -14,16 +14,32 @@ module Hbc To complete the installation of Cask #{cask}, you must also run the installer at - '#{path}' + '#{cask.staged_path.join(path)}' EOS end end module ScriptInstaller def install_phase(command: nil, **_) - ohai "Running #{self.class.dsl_key} script '#{path.relative_path_from(cask.staged_path)}'" - FileUtils.chmod "+x", path unless path.executable? - command.run(path, **args, path: PATH.new(HOMEBREW_PREFIX/"bin", HOMEBREW_PREFIX/"sbin", ENV["PATH"])) + ohai "Running #{self.class.dsl_key} script '#{path}'" + + absolute_path = if path.absolute? + path + else + cask.staged_path.join(path) + end + + if absolute_path.exist? && !absolute_path.executable? + FileUtils.chmod "+x", absolute_path + end + + executable = if absolute_path.exist? + absolute_path + else + path + end + + command.run!(executable, **args, env: { "PATH" => PATH.new(HOMEBREW_PREFIX/"bin", HOMEBREW_PREFIX/"sbin", ENV["PATH"]) }) end end @@ -54,7 +70,7 @@ module Hbc super(cask) if args.key?(:manual) - @path = cask.staged_path.join(args[:manual]) + @path = Pathname(args[:manual]) @args = [] extend(ManualInstaller) return @@ -65,17 +81,16 @@ module Hbc ) raise CaskInvalidError.new(cask, "#{self.class.dsl_key} missing executable") if path.nil? - path = Pathname(path) - @path = path.absolute? ? path : cask.staged_path.join(path) + @path = Pathname(path) extend(ScriptInstaller) end def summarize - path.relative_path_from(cask.staged_path).to_s + path.to_s end def to_h - { path: path.relative_path_from(cask.staged_path).to_s }.tap do |h| + { path: path }.tap do |h| h[:args] = args unless is_a?(ManualInstaller) end end diff --git a/Library/Homebrew/cask/lib/hbc/system_command.rb b/Library/Homebrew/cask/lib/hbc/system_command.rb index 2da4d2c677..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,7 +101,7 @@ module Hbc executable, *args = expanded_command raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = - Open3.popen3(env, [executable, executable], *args, **options) + Open3.popen3([executable, executable], *args, **options) write_input_to(raw_stdin) raw_stdin.close_write diff --git a/Library/Homebrew/cask/lib/hbc/utils/hash_validator.rb b/Library/Homebrew/cask/lib/hbc/utils/hash_validator.rb index 6c8f95c3a4..f43bf173f4 100644 --- a/Library/Homebrew/cask/lib/hbc/utils/hash_validator.rb +++ b/Library/Homebrew/cask/lib/hbc/utils/hash_validator.rb @@ -2,6 +2,6 @@ module HashValidator def assert_valid_keys(*valid_keys) unknown_keys = keys - valid_keys return if unknown_keys.empty? - raise CaskError, %Q(Unknown keys: #{unknown_keys.inspect}. Running "#{UPDATE_CMD}" will likely fix it.) + raise %Q(Unknown keys: #{unknown_keys.inspect}. Running "brew update" will likely fix it.) end end diff --git a/Library/Homebrew/test/cask/artifact/installer_spec.rb b/Library/Homebrew/test/cask/artifact/installer_spec.rb new file mode 100644 index 0000000000..7e4c551125 --- /dev/null +++ b/Library/Homebrew/test/cask/artifact/installer_spec.rb @@ -0,0 +1,40 @@ +describe Hbc::Artifact::Installer, :cask do + let(:staged_path) { mktmpdir } + let(:cask) { instance_double("Cask", staged_path: staged_path, config: nil) } + subject(:installer) { described_class.new(cask, **args) } + let(:command) { Hbc::SystemCommand } + + let(:args) { {} } + + describe "#install_phase" do + context "when given a manual installer" do + let(:args) { { manual: "installer" } } + + it "shows a message prompting to run the installer manually" do + expect { + installer.install_phase(command: command) + }.to output(%r{run the installer at\s+'#{staged_path}/installer'}).to_stdout + end + end + + context "when given a script installer" do + let(:executable) { staged_path/"executable" } + let(:args) { { script: { executable: "executable" } } } + + before(:each) do + FileUtils.touch executable + end + + it "looks for the executable in HOMEBREW_PREFIX" do + expect(command).to receive(:run!).with( + executable, + a_hash_including( + env: { "PATH" => PATH.new("#{HOMEBREW_PREFIX}/bin", "#{HOMEBREW_PREFIX}/sbin", ENV["PATH"]) }, + ), + ) + + installer.install_phase(command: command) + end + end + end +end diff --git a/Library/Homebrew/test/cask/dsl_spec.rb b/Library/Homebrew/test/cask/dsl_spec.rb index 2500a53fa3..6ce40502a6 100644 --- a/Library/Homebrew/test/cask/dsl_spec.rb +++ b/Library/Homebrew/test/cask/dsl_spec.rb @@ -520,7 +520,7 @@ describe Hbc::DSL, :cask do it "allows installer manual to be specified" do installer = cask.artifacts.first expect(installer).to be_a(Hbc::Artifact::Installer::ManualInstaller) - expect(installer.path).to eq(cask.staged_path.join("Caffeine.app")) + expect(installer.path).to eq(Pathname("Caffeine.app")) end end end diff --git a/Library/Homebrew/test/cask/system_command_spec.rb b/Library/Homebrew/test/cask/system_command_spec.rb index 4a55310788..f08193e652 100644 --- a/Library/Homebrew/test/cask/system_command_spec.rb +++ b/Library/Homebrew/test/cask/system_command_spec.rb @@ -2,7 +2,7 @@ describe Hbc::SystemCommand, :cask do describe "#initialize" do let(:env_args) { ["bash", "-c", 'printf "%s" "${A?}" "${B?}" "${C?}"'] } - describe "given some environment variables" do + context "when given some environment variables" do subject { described_class.new( "env", @@ -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! @@ -26,7 +26,7 @@ describe Hbc::SystemCommand, :cask do end end - describe "given some environment variables and sudo: true" do + context "when given some environment variables and sudo: true" do subject { described_class.new( "env", @@ -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 @@ -54,7 +53,7 @@ describe Hbc::SystemCommand, :cask do end end - describe "when the exit code is 0" do + context "when the exit code is 0" do describe "its result" do subject { described_class.run("/usr/bin/true") } @@ -63,10 +62,10 @@ describe Hbc::SystemCommand, :cask do end end - describe "when the exit code is 1" do + context "when the exit code is 1" do let(:command) { "/usr/bin/false" } - describe "and the command must succeed" do + context "and the command must succeed" do it "throws an error" do expect { described_class.run!(command) @@ -74,7 +73,7 @@ describe Hbc::SystemCommand, :cask do end end - describe "and the command does not have to succeed" do + context "and the command does not have to succeed" do describe "its result" do subject { described_class.run(command) } @@ -84,7 +83,7 @@ describe Hbc::SystemCommand, :cask do end end - describe "given a pathname" do + context "when given a pathname" do let(:command) { "/bin/ls" } let(:path) { Pathname(Dir.mktmpdir) } @@ -100,7 +99,7 @@ describe Hbc::SystemCommand, :cask do end end - describe "with both STDOUT and STDERR output from upstream" do + context "with both STDOUT and STDERR output from upstream" do let(:command) { "/bin/bash" } let(:options) { { args: [ @@ -119,7 +118,7 @@ describe Hbc::SystemCommand, :cask do end end - describe "with default options" do + context "with default options" do it "echoes only STDERR" do expected = [2, 4, 6].map { |i| "#{i}\n" }.join expect { @@ -130,7 +129,7 @@ describe Hbc::SystemCommand, :cask do include_examples("it returns '1 2 3 4 5 6'") end - describe "with print_stdout" do + context "with print_stdout" do before do options.merge!(print_stdout: true) end @@ -144,7 +143,7 @@ describe Hbc::SystemCommand, :cask do include_examples("it returns '1 2 3 4 5 6'") end - describe "without print_stderr" do + context "without print_stderr" do before do options.merge!(print_stderr: false) end @@ -158,7 +157,7 @@ describe Hbc::SystemCommand, :cask do include_examples("it returns '1 2 3 4 5 6'") end - describe "with print_stdout but without print_stderr" do + context "with print_stdout but without print_stderr" do before do options.merge!(print_stdout: true, print_stderr: false) end @@ -174,7 +173,7 @@ describe Hbc::SystemCommand, :cask do end end - describe "with a very long STDERR output" do + context "with a very long STDERR output" do let(:command) { "/bin/bash" } let(:options) { { args: [ @@ -190,10 +189,23 @@ describe Hbc::SystemCommand, :cask do end end - describe "given an invalid variable name" do + context "when given an invalid variable name" do it "raises an ArgumentError" do expect { described_class.run("true", env: { "1ABC" => true }) } .to raise_error(ArgumentError, /variable name/) end end + + it "looks for executables in a custom PATH" do + mktmpdir do |path| + (path/"tool").write <<~SH + #!/bin/sh + echo Hello, world! + SH + + FileUtils.chmod "+x", path/"tool" + + expect(described_class.run("tool", env: { "PATH" => path }).stdout).to include "Hello, world!" + end + end end 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")