diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 2f7f17595f..7fff5cfe31 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -270,8 +270,8 @@ module Cask end end - def uninstall_login_item(*login_items, command: nil, upgrade_or_reinstall: false, **_) - return if upgrade_or_reinstall + def uninstall_login_item(*login_items, command: nil, successor: nil, **_) + return unless successor.nil? apps = cask.artifacts.select { |a| a.class.dsl_key == :app } derived_login_items = apps.map { |a| { path: a.target } } diff --git a/Library/Homebrew/cask/artifact/moved.rb b/Library/Homebrew/cask/artifact/moved.rb index f3fa07be6e..e97a65b6f4 100644 --- a/Library/Homebrew/cask/artifact/moved.rb +++ b/Library/Homebrew/cask/artifact/moved.rb @@ -35,14 +35,16 @@ module Cask private - def move(adopt: false, force: false, verbose: false, upgrade_or_reinstall: false, reinstall: false, + def move(adopt: false, force: false, verbose: false, predecessor: nil, reinstall: false, command: nil, **options) unless source.exist? raise CaskError, "It seems the #{self.class.english_name} source '#{source}' is not there." end if Utils.path_occupied?(target) - if upgrade_or_reinstall && target.directory? && target.children.empty? + if !predecessor.nil? && target.directory? && target.children.empty? && predecessor.artifacts.any? do |a| + a.class.dsl_key == self.class.dsl_key && a.target == target + end # An upgrade removed the directory contents but left the directory itself (see below). unless source.directory? if target.parent.writable? && !force @@ -149,13 +151,15 @@ module Cask delete(target, force: force, command: command, **options) end - def delete(target, force: false, upgrade_or_reinstall: false, command: nil, **_) + def delete(target, force: false, successor: nil, command: nil, **_) ohai "Removing #{self.class.english_name} '#{target}'" raise CaskError, "Cannot remove undeletable #{self.class.english_name}." if MacOS.undeletable?(target) return unless Utils.path_occupied?(target) - if upgrade_or_reinstall && target.directory? + if !successor.nil? && target.directory? && successor.artifacts.any? do |a| + a.class.dsl_key == self.class.dsl_key && a.target == self.target + end # If an app folder is deleted, macOS considers the app uninstalled and removes some data. # Remove only the contents to handle this case. target.children.each do |child| diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 96ddcce544..d289e30795 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -95,6 +95,7 @@ module Cask raise CaskAlreadyInstalledError, @cask end + predecessor = (reinstall? && @cask.installed?) ? @cask : nil check_conflicts @@ -110,7 +111,7 @@ module Cask @cask.config = @cask.default_config.merge(old_config) - install_artifacts + install_artifacts(predecessor: predecessor) if (tap = @cask.tap) && tap.should_report_analytics? ::Utils::Analytics.report_event(:cask_install, package_name: @cask.token, tap_name: tap.name, @@ -148,7 +149,7 @@ on_request: true) # Always force uninstallation, ignore method parameter cask_installer = Installer.new(@cask, verbose: verbose?, force: true, upgrade: upgrade?, reinstall: true) - zap? ? cask_installer.zap : cask_installer.uninstall + zap? ? cask_installer.zap(successor: @cask) : cask_installer.uninstall(successor: @cask) end sig { returns(String) } @@ -215,7 +216,7 @@ on_request: true) Quarantine.propagate(from: primary_container.path, to: to) end - def install_artifacts + def install_artifacts(predecessor: nil) artifacts = @cask.artifacts already_installed_artifacts = [] @@ -229,7 +230,7 @@ on_request: true) next if artifact.is_a?(Artifact::Binary) && !binaries? artifact.install_phase(command: @command, verbose: verbose?, adopt: adopt?, force: force?, - upgrade_or_reinstall: upgrade? || reinstall?) + predecessor: predecessor) already_installed_artifacts.unshift(artifact) end @@ -391,10 +392,10 @@ on_request: true) @cask.download_sha_path.atomic_write(@cask.new_download_sha) if @cask.checksumable? end - def uninstall + def uninstall(successor: nil) load_installed_caskfile! oh1 "Uninstalling Cask #{Formatter.identifier(@cask)}" - uninstall_artifacts(clear: true) + uninstall_artifacts(clear: true, successor: successor) if !reinstall? && !upgrade? remove_download_sha remove_config_file @@ -412,8 +413,8 @@ on_request: true) FileUtils.rm_f @cask.download_sha_path if @cask.download_sha_path.exist? end - def start_upgrade - uninstall_artifacts + def start_upgrade(successor:) + uninstall_artifacts(successor: successor) backup end @@ -432,10 +433,10 @@ on_request: true) backup_metadata_path.rename @cask.metadata_versioned_path end - def revert_upgrade + def revert_upgrade(predecessor) opoo "Reverting upgrade for Cask #{@cask}" restore_backup - install_artifacts + install_artifacts(predecessor: predecessor) end def finalize_upgrade @@ -446,7 +447,7 @@ on_request: true) puts summary end - def uninstall_artifacts(clear: false) + def uninstall_artifacts(clear: false, successor: nil) artifacts = @cask.artifacts odebug "Uninstalling artifacts" @@ -456,11 +457,11 @@ on_request: true) if artifact.respond_to?(:uninstall_phase) odebug "Uninstalling artifact of class #{artifact.class}" artifact.uninstall_phase( - command: @command, - verbose: verbose?, - skip: clear, - force: force?, - upgrade_or_reinstall: upgrade? || reinstall?, + command: @command, + verbose: verbose?, + skip: clear, + force: force?, + successor: successor, ) end @@ -468,16 +469,16 @@ on_request: true) odebug "Post-uninstalling artifact of class #{artifact.class}" artifact.post_uninstall_phase( - command: @command, - verbose: verbose?, - skip: clear, - force: force?, - upgrade_or_reinstall: upgrade? || reinstall?, + command: @command, + verbose: verbose?, + skip: clear, + force: force?, + successor: successor, ) end end - def zap + def zap(successor: nil) load_installed_caskfile! ohai "Implied `brew uninstall --cask #{@cask}`" uninstall_artifacts diff --git a/Library/Homebrew/cask/upgrade.rb b/Library/Homebrew/cask/upgrade.rb index 17c38f43be..eab3a2342f 100644 --- a/Library/Homebrew/cask/upgrade.rb +++ b/Library/Homebrew/cask/upgrade.rb @@ -180,22 +180,22 @@ module Cask new_cask_installer.fetch # Move the old cask's artifacts back to staging - old_cask_installer.start_upgrade + old_cask_installer.start_upgrade(successor: new_cask) # And flag it so in case of error started_upgrade = true # Install the new cask new_cask_installer.stage - new_cask_installer.install_artifacts + new_cask_installer.install_artifacts(predecessor: old_cask) new_artifacts_installed = true # If successful, wipe the old cask from staging old_cask_installer.finalize_upgrade rescue => e - new_cask_installer.uninstall_artifacts if new_artifacts_installed + new_cask_installer.uninstall_artifacts(successor: old_cask) if new_artifacts_installed new_cask_installer.purge_versioned_files - old_cask_installer.revert_upgrade if started_upgrade + old_cask_installer.revert_upgrade(predecessor: new_cask) if started_upgrade raise e end diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index a172bd7bc6..aa401e345e 100644 --- a/Library/Homebrew/test/cask/artifact/app_spec.rb +++ b/Library/Homebrew/test/cask/artifact/app_spec.rb @@ -323,13 +323,13 @@ describe Cask::Artifact::App, :cask do inode = target_path.stat.ino expect(contents_path).to exist - app.uninstall_phase(command: command, force: force, upgrade_or_reinstall: true) + app.uninstall_phase(command: command, force: force, successor: cask) expect(target_path).to exist expect(target_path.children).to be_empty expect(contents_path).not_to exist - app.install_phase(command: command, adopt: adopt, force: force, upgrade_or_reinstall: true) + app.install_phase(command: command, adopt: adopt, force: force, predecessor: cask) expect(target_path).to exist expect(target_path.stat.ino).to eq(inode) diff --git a/Library/Homebrew/test/cask/upgrade_spec.rb b/Library/Homebrew/test/cask/upgrade_spec.rb index 5647f3eceb..d10b53be54 100644 --- a/Library/Homebrew/test/cask/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/upgrade_spec.rb @@ -13,6 +13,9 @@ describe Cask::Upgrade, :cask do let(:local_transmission) { Cask::CaskLoader.load("local-transmission") } let(:local_caffeine_path) { local_caffeine.config.appdir.join("Caffeine.app") } let(:local_caffeine) { Cask::CaskLoader.load("local-caffeine") } + let(:renamed_app) { Cask::CaskLoader.load("renamed-app") } + let(:renamed_app_old_path) { renamed_app.config.appdir.join("OldApp.app") } + let(:renamed_app_new_path) { renamed_app.config.appdir.join("NewApp.app") } let(:args) { Homebrew::CLI::Args.new } context "when the upgrade is successful" do @@ -22,6 +25,7 @@ describe Cask::Upgrade, :cask do "outdated/local-transmission", "outdated/auto-updates", "outdated/version-latest", + "outdated/renamed-app", ] end @@ -41,6 +45,11 @@ describe Cask::Upgrade, :cask do expect(local_transmission_path).to be_a_directory expect(local_transmission.versions).to include("2.60") + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") + described_class.upgrade_casks(args: args) expect(local_caffeine).to be_installed @@ -50,6 +59,11 @@ describe Cask::Upgrade, :cask do expect(local_transmission).to be_installed expect(local_transmission_path).to be_a_directory expect(local_transmission.versions).to include("2.61") + + expect(renamed_app).to be_installed + expect(renamed_app_old_path).not_to be_a_directory + expect(renamed_app_new_path).to be_a_directory + expect(renamed_app.versions).to include("2.0.0") end it "updates only the Casks specified in the command line" do @@ -61,6 +75,11 @@ describe Cask::Upgrade, :cask do expect(local_transmission_path).to be_a_directory expect(local_transmission.versions).to include("2.60") + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") + described_class.upgrade_casks(local_caffeine, args: args) expect(local_caffeine).to be_installed @@ -70,6 +89,11 @@ describe Cask::Upgrade, :cask do expect(local_transmission).to be_installed expect(local_transmission_path).to be_a_directory expect(local_transmission.versions).to include("2.60") + + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") end it 'updates "auto_updates" and "latest" Casks when their tokens are provided in the command line' do @@ -107,6 +131,11 @@ describe Cask::Upgrade, :cask do expect(local_transmission_path).to be_a_directory expect(local_transmission.versions).to include("2.60") + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") + expect(version_latest).to be_installed expect(version_latest_path_1).to be_a_directory expect(version_latest.versions).to include("latest") @@ -128,6 +157,11 @@ describe Cask::Upgrade, :cask do expect(local_transmission_path).to be_a_directory expect(local_transmission.versions).to include("2.61") + expect(renamed_app).to be_installed + expect(renamed_app_old_path).not_to be_a_directory + expect(renamed_app_new_path).to be_a_directory + expect(renamed_app.versions).to include("2.0.0") + expect(version_latest).to be_installed expect(version_latest_path_2).to be_a_directory expect(version_latest.versions).to include("latest") @@ -187,6 +221,7 @@ describe Cask::Upgrade, :cask do "outdated/local-transmission", "outdated/auto-updates", "outdated/version-latest", + "outdated/renamed-app", ] end @@ -208,6 +243,11 @@ describe Cask::Upgrade, :cask do expect(local_transmission_path).to be_a_directory expect(local_transmission.versions).to include("2.60") + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") + described_class.upgrade_casks(dry_run: true, args: args) expect(local_caffeine).to be_installed @@ -219,6 +259,12 @@ describe Cask::Upgrade, :cask do expect(local_transmission_path).to be_a_directory expect(local_transmission.versions).to include("2.60") expect(local_transmission.versions).not_to include("2.61") + + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") + expect(renamed_app.versions).not_to include("2.0.0") end it "would update only the Casks specified in the command line" do @@ -256,6 +302,11 @@ describe Cask::Upgrade, :cask do expect(auto_updates_path).to be_a_directory expect(auto_updates.versions).to include("2.57") + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") + described_class.upgrade_casks(local_caffeine, auto_updates, dry_run: true, args: args) expect(local_caffeine).to be_installed @@ -267,6 +318,12 @@ describe Cask::Upgrade, :cask do expect(auto_updates_path).to be_a_directory expect(auto_updates.versions).to include("2.57") expect(auto_updates.versions).not_to include("2.61") + + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") + expect(renamed_app.versions).not_to include("2.0.0") end end @@ -286,6 +343,11 @@ describe Cask::Upgrade, :cask do expect(local_transmission_path).to be_a_directory expect(local_transmission.versions).to include("2.60") + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") + expect(version_latest).to be_installed # Change download sha so that :latest cask decides to update itself version_latest.download_sha_path.write("fake download sha") @@ -308,6 +370,12 @@ describe Cask::Upgrade, :cask do expect(local_transmission.versions).to include("2.60") expect(local_transmission.versions).not_to include("2.61") + expect(renamed_app).to be_installed + expect(renamed_app_old_path).to be_a_directory + expect(renamed_app_new_path).not_to be_a_directory + expect(renamed_app.versions).to include("1.0.0") + expect(renamed_app.versions).not_to include("2.0.0") + expect(version_latest).to be_installed expect(version_latest.outdated_download_sha?).to be(true) end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/renamed-app.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/renamed-app.rb new file mode 100644 index 0000000000..e93b90cb8e --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/renamed-app.rb @@ -0,0 +1,9 @@ +cask "renamed-app" do + version "1.0.0" + sha256 "cf001ed6c81820e049dc7a353957dab8936b91f1956ee74ff0b3eb59791f1ad9" + + url "file://#{TEST_FIXTURE_DIR}/cask/old-app.tar.gz" + homepage "https://brew.sh/" + + app "OldApp.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/renamed-app.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/renamed-app.rb new file mode 100644 index 0000000000..3e1f367fa5 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/renamed-app.rb @@ -0,0 +1,9 @@ +cask "renamed-app" do + version "2.0.0" + sha256 "9f88a6f3d8a7977cd3c116c56ee7a20a3c69e838a1d4946f815a926a57883299" + + url "file://#{TEST_FIXTURE_DIR}/cask/new-app.tar.gz" + homepage "https://brew.sh/" + + app "NewApp.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/NewApp.app/Contents/Info.plist b/Library/Homebrew/test/support/fixtures/cask/NewApp.app/Contents/Info.plist new file mode 100644 index 0000000000..e69de29bb2 diff --git a/Library/Homebrew/test/support/fixtures/cask/NewApp.app/Contents/MacOS/NewApp b/Library/Homebrew/test/support/fixtures/cask/NewApp.app/Contents/MacOS/NewApp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/Library/Homebrew/test/support/fixtures/cask/NewApp.app/Contents/PkgInfo b/Library/Homebrew/test/support/fixtures/cask/NewApp.app/Contents/PkgInfo new file mode 100644 index 0000000000..e69de29bb2 diff --git a/Library/Homebrew/test/support/fixtures/cask/NewApp.app/Contents/Resources/Caffeine.icns b/Library/Homebrew/test/support/fixtures/cask/NewApp.app/Contents/Resources/Caffeine.icns new file mode 100644 index 0000000000..e69de29bb2 diff --git a/Library/Homebrew/test/support/fixtures/cask/new-app.tar.gz b/Library/Homebrew/test/support/fixtures/cask/new-app.tar.gz new file mode 100644 index 0000000000..a07fc7c8fe Binary files /dev/null and b/Library/Homebrew/test/support/fixtures/cask/new-app.tar.gz differ diff --git a/Library/Homebrew/test/support/fixtures/cask/old-app.tar.gz b/Library/Homebrew/test/support/fixtures/cask/old-app.tar.gz new file mode 100644 index 0000000000..d02fd7e111 Binary files /dev/null and b/Library/Homebrew/test/support/fixtures/cask/old-app.tar.gz differ