From dcc4ae76e8f1eadeff7d6f8fd36c2724d67f4db0 Mon Sep 17 00:00:00 2001 From: JBYoshi <12983479+JBYoshi@users.noreply.github.com> Date: Tue, 4 Apr 2023 11:36:27 -0500 Subject: [PATCH] Also keep app directories with "brew reinstall". --- .../cask/artifact/abstract_uninstall.rb | 4 +-- Library/Homebrew/cask/artifact/moved.rb | 9 ++++--- Library/Homebrew/cask/cmd/reinstall.rb | 3 ++- Library/Homebrew/cask/installer.rb | 27 ++++++++++--------- .../Homebrew/test/cask/artifact/app_spec.rb | 4 +-- 5 files changed, 26 insertions(+), 21 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 7b4d522a7a..2f7f17595f 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: false, **_) - return if upgrade + def uninstall_login_item(*login_items, command: nil, upgrade_or_reinstall: false, **_) + return if upgrade_or_reinstall 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 fa1915364a..f3fa07be6e 100644 --- a/Library/Homebrew/cask/artifact/moved.rb +++ b/Library/Homebrew/cask/artifact/moved.rb @@ -35,13 +35,14 @@ module Cask private - def move(adopt: false, force: false, verbose: false, upgrade: false, command: nil, **options) + def move(adopt: false, force: false, verbose: false, upgrade_or_reinstall: false, 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 && target.directory? && target.children.empty? + if upgrade_or_reinstall && target.directory? && target.children.empty? # An upgrade removed the directory contents but left the directory itself (see below). unless source.directory? if target.parent.writable? && !force @@ -148,13 +149,13 @@ module Cask delete(target, force: force, command: command, **options) end - def delete(target, force: false, upgrade: false, command: nil, **_) + def delete(target, force: false, upgrade_or_reinstall: false, 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 && target.directory? + if upgrade_or_reinstall && target.directory? # 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/cmd/reinstall.rb b/Library/Homebrew/cask/cmd/reinstall.rb index 9637805450..2812c055b2 100644 --- a/Library/Homebrew/cask/cmd/reinstall.rb +++ b/Library/Homebrew/cask/cmd/reinstall.rb @@ -41,6 +41,7 @@ module Cask force: force, skip_cask_deps: skip_cask_deps, require_sha: require_sha, + reinstall: true, quarantine: quarantine, zap: zap, }.compact @@ -48,7 +49,7 @@ module Cask options[:quarantine] = true if options[:quarantine].nil? casks.each do |cask| - Installer.new(cask, **options).reinstall + Installer.new(cask, **options).install end end end diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index f92469e8e1..96ddcce544 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -22,7 +22,7 @@ module Cask def initialize(cask, command: SystemCommand, force: false, adopt: false, skip_cask_deps: false, binaries: true, verbose: false, - zap: false, require_sha: false, upgrade: false, + zap: false, require_sha: false, upgrade: false, reinstall: false, installed_as_dependency: false, quarantine: true, verify_download_integrity: true, quiet: false) @cask = cask @@ -34,7 +34,7 @@ module Cask @verbose = verbose @zap = zap @require_sha = require_sha - @reinstall = false + @reinstall = reinstall @upgrade = upgrade @installed_as_dependency = installed_as_dependency @quarantine = quarantine @@ -143,17 +143,11 @@ on_request: true) end end - def reinstall - odebug "Cask::Installer#reinstall" - @reinstall = true - install - end - def uninstall_existing_cask return unless @cask.installed? # Always force uninstallation, ignore method parameter - cask_installer = Installer.new(@cask, verbose: verbose?, force: true, upgrade: upgrade?) + cask_installer = Installer.new(@cask, verbose: verbose?, force: true, upgrade: upgrade?, reinstall: true) zap? ? cask_installer.zap : cask_installer.uninstall end @@ -234,7 +228,8 @@ on_request: true) next if artifact.is_a?(Artifact::Binary) && !binaries? - artifact.install_phase(command: @command, verbose: verbose?, adopt: adopt?, force: force?, upgrade: upgrade?) + artifact.install_phase(command: @command, verbose: verbose?, adopt: adopt?, force: force?, + upgrade_or_reinstall: upgrade? || reinstall?) already_installed_artifacts.unshift(artifact) end @@ -461,7 +456,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: upgrade?, + command: @command, + verbose: verbose?, + skip: clear, + force: force?, + upgrade_or_reinstall: upgrade? || reinstall?, ) end @@ -469,7 +468,11 @@ on_request: true) odebug "Post-uninstalling artifact of class #{artifact.class}" artifact.post_uninstall_phase( - command: @command, verbose: verbose?, skip: clear, force: force?, upgrade: upgrade?, + command: @command, + verbose: verbose?, + skip: clear, + force: force?, + upgrade_or_reinstall: upgrade? || reinstall?, ) end end diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index 47bcd14a59..a172bd7bc6 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: true) + app.uninstall_phase(command: command, force: force, upgrade_or_reinstall: true) 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: true) + app.install_phase(command: command, adopt: adopt, force: force, upgrade_or_reinstall: true) expect(target_path).to exist expect(target_path.stat.ino).to eq(inode)