From 54ab3dd7c4328ad424f3fd50b912d430ffb03452 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 24 Oct 2019 14:55:38 +0200 Subject: [PATCH 1/2] Install `pkg` before `app`. --- Library/Homebrew/cask/artifact/abstract_artifact.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_artifact.rb b/Library/Homebrew/cask/artifact/abstract_artifact.rb index fac91f0b49..5f82ecbb08 100644 --- a/Library/Homebrew/cask/artifact/abstract_artifact.rb +++ b/Library/Homebrew/cask/artifact/abstract_artifact.rb @@ -50,6 +50,11 @@ module Cask # depend on other artifacts still being installed. Uninstall, Installer, + # `pkg` should be run before `binary`, so + # targets are created prior to linking. + # `pkg` should be run before `app`, since an `app` could + # contain a nested installer (e.g. `wireshark`). + Pkg, [ App, Suite, @@ -67,9 +72,6 @@ module Cask Vst3Plugin, ScreenSaver, ], - # `pkg` should be run before `binary`, so - # targets are created prior to linking. - Pkg, Binary, Manpage, PostflightBlock, From df3bbd0299b964e5bc759130a10fce93422a08f8 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 24 Oct 2019 15:15:40 +0200 Subject: [PATCH 2/2] Reduce need for interpolating `appdir` in casks. --- Library/Homebrew/cask/artifact/moved.rb | 4 ++++ Library/Homebrew/cask/installer.rb | 11 +++++++++-- .../Homebrew/test/cask/artifact/alt_target_spec.rb | 6 +++--- Library/Homebrew/test/cask/artifact/app_spec.rb | 12 ++++++------ .../test/cask/artifact/generic_artifact_spec.rb | 2 +- .../test/cask/artifact/two_apps_correct_spec.rb | 13 ++++++++----- 6 files changed, 31 insertions(+), 17 deletions(-) diff --git a/Library/Homebrew/cask/artifact/moved.rb b/Library/Homebrew/cask/artifact/moved.rb index 077c957a55..7774bff8ec 100644 --- a/Library/Homebrew/cask/artifact/moved.rb +++ b/Library/Homebrew/cask/artifact/moved.rb @@ -54,10 +54,14 @@ module Cask command.run!("/bin/mv", args: [source, target], sudo: true) end + FileUtils.ln_sf target, source + add_altname_metadata(target, source.basename, command: command) end def move_back(skip: false, force: false, command: nil, **options) + FileUtils.rm source if source.symlink? && source.dirname.join(source.readlink) == target + if Utils.path_occupied?(source) message = "It seems there is already #{self.class.english_article} " \ "#{self.class.english_name} at '#{source}'" diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 96005c402e..e2d7a24e2c 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -94,6 +94,8 @@ module Cask fetch uninstall_existing_cask if reinstall? + backup if force? && @cask.staged_path.exist? && @cask.metadata_versioned_path.exist? + oh1 "Installing Cask #{Formatter.identifier(@cask)}" opoo "macOS's Gatekeeper has been disabled for this Cask" unless quarantine? stage @@ -104,7 +106,12 @@ module Cask ::Utils::Analytics.report_event("cask_install", @cask.token) unless @cask.tap&.private? + purge_backed_up_versioned_files + puts summary + rescue + restore_backup + raise end def check_conflicts @@ -411,6 +418,8 @@ module Cask end def finalize_upgrade + ohai "Purging files for version #{@cask.version} of Cask #{@cask}" + purge_backed_up_versioned_files puts summary @@ -471,8 +480,6 @@ module Cask end def purge_backed_up_versioned_files - ohai "Purging files for version #{@cask.version} of Cask #{@cask}" - # versioned staged distribution gain_permissions_remove(backup_path) if backup_path&.exist? diff --git a/Library/Homebrew/test/cask/artifact/alt_target_spec.rb b/Library/Homebrew/test/cask/artifact/alt_target_spec.rb index 22d439a9ab..ed0e9d4591 100644 --- a/Library/Homebrew/test/cask/artifact/alt_target_spec.rb +++ b/Library/Homebrew/test/cask/artifact/alt_target_spec.rb @@ -24,7 +24,7 @@ describe Cask::Artifact::App, :cask do install_phase expect(target_path).to be_a_directory - expect(source_path).not_to exist + expect(source_path).to be_a_symlink end describe "when app is in a subdirectory" do @@ -45,7 +45,7 @@ describe Cask::Artifact::App, :cask do install_phase expect(target_path).to be_a_directory - expect(appsubdir.join("Caffeine.app")).not_to exist + expect(appsubdir.join("Caffeine.app")).to be_a_symlink end end @@ -56,7 +56,7 @@ describe Cask::Artifact::App, :cask do install_phase expect(target_path).to be_a_directory - expect(source_path).not_to exist + expect(source_path).to be_a_symlink expect(cask.config.appdir.join("Caffeine Deluxe.app")).not_to exist expect(cask.staged_path.join("Caffeine Deluxe.app")).to be_a_directory diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index 0b318351db..add9b9dc56 100644 --- a/Library/Homebrew/test/cask/artifact/app_spec.rb +++ b/Library/Homebrew/test/cask/artifact/app_spec.rb @@ -21,7 +21,7 @@ describe Cask::Artifact::App, :cask do install_phase expect(target_path).to be_a_directory - expect(source_path).not_to exist + expect(source_path).to be_a_symlink end describe "when app is in a subdirectory" do @@ -42,7 +42,7 @@ describe Cask::Artifact::App, :cask do install_phase expect(target_path).to be_a_directory - expect(appsubdir.join("Caffeine.app")).not_to exist + expect(appsubdir.join("Caffeine.app")).to be_a_symlink end end @@ -53,7 +53,7 @@ describe Cask::Artifact::App, :cask do install_phase expect(target_path).to be_a_directory - expect(source_path).not_to exist + expect(source_path).to be_a_symlink expect(cask.config.appdir.join("Caffeine Deluxe.app")).not_to exist expect(cask.staged_path.join("Caffeine Deluxe.app")).to exist @@ -100,7 +100,7 @@ describe Cask::Artifact::App, :cask do .to output(stdout).to_stdout .and output(stderr).to_stderr - expect(source_path).not_to exist + expect(source_path).to be_a_symlink expect(target_path).to be_a_directory contents_path = target_path.join("Contents/Info.plist") @@ -148,7 +148,7 @@ describe Cask::Artifact::App, :cask do .to output(stdout).to_stdout .and output(stderr).to_stderr - expect(source_path).not_to exist + expect(source_path).to be_a_symlink expect(target_path).to be_a_directory contents_path = target_path.join("Contents/Info.plist") @@ -191,7 +191,7 @@ describe Cask::Artifact::App, :cask do .to output(stdout).to_stdout .and output(stderr).to_stderr - expect(source_path).not_to exist + expect(source_path).to be_a_symlink expect(target_path).to be_a_directory contents_path = target_path.join("Contents/Info.plist") diff --git a/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb b/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb index c15b878510..d783a48b4c 100644 --- a/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb +++ b/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb @@ -30,7 +30,7 @@ describe Cask::Artifact::Artifact, :cask do install_phase.call expect(target_path).to be_a_directory - expect(source_path).not_to exist + expect(source_path).to be_a_symlink end it "avoids clobbering an existing artifact" do diff --git a/Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb b/Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb index 730851f699..1f63083855 100644 --- a/Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb +++ b/Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb @@ -24,23 +24,26 @@ describe Cask::Artifact::App, :cask do install_phase expect(target_path_mini).to be_a_directory - expect(source_path_mini).not_to exist + expect(source_path_mini).to be_a_symlink expect(target_path_pro).to be_a_directory - expect(source_path_pro).not_to exist + expect(source_path_pro).to be_a_symlink end describe "when apps are in a subdirectory" do let(:cask) { Cask::CaskLoader.load(cask_path("with-two-apps-subdir")) } + let(:source_path_mini) { cask.staged_path.join("Caffeines", "Caffeine Mini.app") } + let(:source_path_pro) { cask.staged_path.join("Caffeines", "Caffeine Pro.app") } + it "installs both apps using the proper target directory" do install_phase expect(target_path_mini).to be_a_directory - expect(source_path_mini).not_to exist + expect(source_path_mini).to be_a_symlink expect(target_path_pro).to be_a_directory - expect(source_path_pro).not_to exist + expect(source_path_pro).to be_a_symlink end end @@ -50,7 +53,7 @@ describe Cask::Artifact::App, :cask do install_phase expect(target_path_mini).to be_a_directory - expect(source_path_mini).not_to exist + expect(source_path_mini).to be_a_symlink expect(cask.config.appdir.join("Caffeine Deluxe.app")).not_to exist expect(cask.staged_path.join("Caffeine Deluxe.app")).to exist