diff --git a/Library/Homebrew/cask/utils.rb b/Library/Homebrew/cask/utils.rb index 73d069fac5..877e3bde0b 100644 --- a/Library/Homebrew/cask/utils.rb +++ b/Library/Homebrew/cask/utils.rb @@ -18,23 +18,37 @@ module Cask if dir.writable? path.mkpath else - command.run!("/bin/mkdir", args: ["-p", path], sudo: true) + command.run!("/bin/mkdir", args: ["-p", "--", path], sudo: true) end end def self.gain_permissions_remove(path, command: SystemCommand) - if path.respond_to?(:rmtree) && path.exist? - gain_permissions(path, ["-R"], command) do |p| - if p.parent.writable? + directory = false + permission_flags = if path.symlink? + ["-h"] + elsif path.directory? + directory = true + ["-R"] + elsif path.exist? + [] + else + # Nothing to remove. + return + end + + gain_permissions(path, permission_flags, command) do |p| + if p.parent.writable? + if directory p.rmtree else - command.run("/bin/rm", - args: ["-r", "-f", "--", p], - sudo: true) + FileUtils.rm_f p end + else + recursive_flag = directory ? ["-R"] : [] + command.run!("/bin/rm", + args: recursive_flag + ["-f", "--", p], + sudo: true) end - elsif File.symlink?(path) - gain_permissions(path, ["-h"], command, &FileUtils.method(:rm_f)) end end @@ -51,13 +65,10 @@ module Cask # dependent on whether the file argument has a trailing # slash. This should do the right thing, but is fragile. command.run("/usr/bin/chflags", - must_succeed: false, args: command_args + ["--", "000", path]) command.run("/bin/chmod", - must_succeed: false, args: command_args + ["--", "u+rwx", path]) command.run("/bin/chmod", - must_succeed: false, args: command_args + ["-N", path]) tried_permissions = true retry # rmtree diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index f3ba605472..978045a720 100644 --- a/Library/Homebrew/test/cask/artifact/app_spec.rb +++ b/Library/Homebrew/test/cask/artifact/app_spec.rb @@ -171,21 +171,12 @@ describe Cask::Artifact::App, :cask do end it "overwrites the existing app" do - expect(command).to receive(:run).with( - "/bin/chmod", args: [ - "-R", "--", "u+rwx", target_path - ], must_succeed: false - ).and_call_original - expect(command).to receive(:run).with( - "/bin/chmod", args: [ - "-R", "-N", target_path - ], must_succeed: false - ).and_call_original - expect(command).to receive(:run).with( - "/usr/bin/chflags", args: [ - "-R", "--", "000", target_path - ], must_succeed: false - ).and_call_original + expect(command).to receive(:run).with("/usr/bin/chflags", + args: ["-R", "--", "000", target_path]).and_call_original + expect(command).to receive(:run).with("/bin/chmod", + args: ["-R", "--", "u+rwx", target_path]).and_call_original + expect(command).to receive(:run).with("/bin/chmod", + args: ["-R", "-N", target_path]).and_call_original stdout = <<~EOS ==> Removing App '#{target_path}' diff --git a/Library/Homebrew/test/cask/utils_spec.rb b/Library/Homebrew/test/cask/utils_spec.rb index cbbce2b8c6..634afaeb95 100644 --- a/Library/Homebrew/test/cask/utils_spec.rb +++ b/Library/Homebrew/test/cask/utils_spec.rb @@ -2,11 +2,11 @@ describe Cask::Utils do let(:command) { NeverSudoSystemCommand } + let(:dir) { mktmpdir } + let(:path) { dir/"a/b/c" } + let(:link) { dir/"link" } describe "::gain_permissions_mkpath" do - let(:dir) { mktmpdir } - let(:path) { dir/"a/b/c" } - it "creates a directory" do expect(path).not_to exist described_class.gain_permissions_mkpath path, command: command @@ -37,4 +37,43 @@ describe Cask::Utils do end end end + + describe "::gain_permissions_remove" do + it "removes the symlink, not the file it points to" do + path.dirname.mkpath + FileUtils.touch path + FileUtils.ln_s path, link + + expect(path).to be_a_file + expect(link).to be_a_symlink + expect(link.realpath).to eq path + + described_class.gain_permissions_remove link, command: command + + expect(path).to be_a_file + expect(link).not_to exist + + described_class.gain_permissions_remove path, command: command + + expect(path).not_to exist + end + + it "removes the symlink, not the directory it points to" do + path.mkpath + FileUtils.ln_s path, link + + expect(path).to be_a_directory + expect(link).to be_a_symlink + expect(link.realpath).to eq path + + described_class.gain_permissions_remove link, command: command + + expect(path).to be_a_directory + expect(link).not_to exist + + described_class.gain_permissions_remove path, command: command + + expect(path).not_to exist + end + end end