diff --git a/Library/Homebrew/cask/artifact/moved.rb b/Library/Homebrew/cask/artifact/moved.rb index 48861a0f61..e12d0160f7 100644 --- a/Library/Homebrew/cask/artifact/moved.rb +++ b/Library/Homebrew/cask/artifact/moved.rb @@ -92,9 +92,9 @@ module Cask if target.directory? if target.writable? - source.children.each { |child| FileUtils.move(child, target + child.basename) } + source.children.each { |child| FileUtils.move(child, target/child.basename) } else - command.run!("/bin/cp", args: ["-pR", "#{source}/*", "#{source}/.*", "#{target}/"], + command.run!("/bin/cp", args: ["-pR", *source.children, target], sudo: true) end Quarantine.copy_xattrs(source, target) diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index 796f03f19f..f3ba605472 100644 --- a/Library/Homebrew/test/cask/artifact/app_spec.rb +++ b/Library/Homebrew/test/cask/artifact/app_spec.rb @@ -2,7 +2,7 @@ describe Cask::Artifact::App, :cask do let(:cask) { Cask::CaskLoader.load(cask_path("local-caffeine")) } - let(:command) { SystemCommand } + let(:command) { NeverSudoSystemCommand } let(:adopt) { false } let(:force) { false } let(:app) { cask.artifacts.find { |a| a.is_a?(described_class) } } @@ -334,5 +334,29 @@ describe Cask::Artifact::App, :cask do expect(contents_path).to exist end + + it "properly handles non-writable directories" do + install_phase + + source_contents_path = source_path.join("Contents") + target_contents_path = target_path.join("Contents") + + allow(app.target).to receive(:writable?).and_return false + allow(command).to receive(:run!).with(any_args).and_call_original + + expect(command).to receive(:run!) + .with("/bin/cp", args: ["-pR", source_contents_path, target_path], + sudo: true) + .and_call_original + expect(FileUtils).not_to receive(:move).with(source_contents_path, an_instance_of(Pathname)) + + app.uninstall_phase(command: command, force: force, successor: cask) + expect(target_contents_path).not_to exist + expect(target_path).to exist + expect(source_contents_path).to exist + + app.install_phase(command: command, adopt: adopt, force: force, predecessor: cask) + expect(target_contents_path).to exist + end end end