Only keep empty directories when the new version also has them.

This commit is contained in:
JBYoshi 2023-04-08 16:13:12 -05:00
parent dcc4ae76e8
commit 9fedaee462
No known key found for this signature in database
GPG Key ID: AE4430116622D05D
14 changed files with 125 additions and 34 deletions

View File

@ -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 } }

View File

@ -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|

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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

View File

@ -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