Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Library/Homebrew/cask/artifact/uninstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ class Uninstall < AbstractUninstall
params(
upgrade: T::Boolean,
reinstall: T::Boolean,
quit: T::Boolean,
options: T.anything,
).void
}
def uninstall_phase(upgrade: false, reinstall: false, **options)
def uninstall_phase(upgrade: false, reinstall: false, quit: true, **options)
raw_on_upgrade = directives[:on_upgrade]
on_upgrade_syms =
case raw_on_upgrade
Expand All @@ -31,6 +32,7 @@ def uninstall_phase(upgrade: false, reinstall: false, **options)

filtered_directives = ORDERED_DIRECTIVES.filter do |directive_sym|
next false if directive_sym == :rmdir
next false if directive_sym == :quit && !quit

if (upgrade || reinstall) &&
UPGRADE_REINSTALL_SKIP_DIRECTIVES.include?(directive_sym) &&
Expand Down
16 changes: 9 additions & 7 deletions Library/Homebrew/cask/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -587,9 +587,9 @@ def remove_download_sha
@cask.download_sha_path.parent.rmdir_if_possible
end

sig { params(successor: T.nilable(Cask)).void }
def start_upgrade(successor:)
uninstall_artifacts(successor:)
sig { params(successor: T.nilable(Cask), quit: T::Boolean).void }
def start_upgrade(successor:, quit: true)
uninstall_artifacts(successor:, quit:)
backup
end

Expand Down Expand Up @@ -638,8 +638,8 @@ def finalize_upgrade
puts summary
end

sig { params(clear: T::Boolean, successor: T.nilable(Cask)).void }
def uninstall_artifacts(clear: false, successor: nil)
sig { params(clear: T::Boolean, successor: T.nilable(Cask), quit: T::Boolean).void }
def uninstall_artifacts(clear: false, successor: nil, quit: true)
odebug "Uninstalling artifacts"
odebug "#{::Utils.pluralize("artifact", artifacts.length, include_count: true)} defined", artifacts

Expand All @@ -659,15 +659,17 @@ def uninstall_artifacts(clear: false, successor: nil)
)

odebug "Uninstalling artifact of class #{artifact.class}"
artifact.uninstall_phase(
uninstall_options = {
command: @command,
verbose: verbose?,
skip: clear,
force: force?,
successor:,
upgrade: upgrade?,
reinstall: reinstall?,
)
}
uninstall_options[:quit] = quit if artifact.is_a?(Artifact::Uninstall)
artifact.uninstall_phase(**uninstall_options)
end

next unless artifact.respond_to?(:post_uninstall_phase)
Expand Down
13 changes: 8 additions & 5 deletions Library/Homebrew/cask/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ def self.show_upgrade_summary(cask_upgrades, dry_run: false)
binaries: T.nilable(T::Boolean),
quarantine: T.nilable(T::Boolean),
require_sha: T.nilable(T::Boolean),
quit: T::Boolean,
skip_prefetch: T::Boolean,
show_upgrade_summary: T::Boolean,
download_queue: T.nilable(Homebrew::DownloadQueue),
Expand All @@ -135,6 +136,7 @@ def self.upgrade_casks!(
binaries: nil,
quarantine: nil,
require_sha: nil,
quit: true,
skip_prefetch: false,
show_upgrade_summary: true,
download_queue: nil,
Expand Down Expand Up @@ -261,7 +263,7 @@ def self.upgrade_casks!(
upgrade_cask(
old_cask, new_cask,
binaries:, force:, skip_cask_deps:, verbose:,
quarantine:, require_sha:, download_queue:
quarantine:, require_sha:, quit:, download_queue:
)
rescue => e
new_exception = e.exception("#{new_cask.full_name}: #{e}")
Expand Down Expand Up @@ -331,14 +333,15 @@ def self.reopen_apps_after_upgrade(old_cask)
force: T.nilable(T::Boolean),
quarantine: T.nilable(T::Boolean),
require_sha: T.nilable(T::Boolean),
quit: T::Boolean,
skip_cask_deps: T.nilable(T::Boolean),
verbose: T.nilable(T::Boolean),
download_queue: Homebrew::DownloadQueue,
).void
}
def self.upgrade_cask(
old_cask, new_cask,
binaries:, force:, quarantine:, require_sha:, skip_cask_deps:, verbose:, download_queue:
binaries:, force:, quarantine:, require_sha:, quit:, skip_cask_deps:, verbose:, download_queue:
)
require "cask/installer"

Expand Down Expand Up @@ -402,7 +405,7 @@ def self.upgrade_cask(
end

# Move the old cask's artifacts back to staging
old_cask_installer.start_upgrade(successor: new_cask)
old_cask_installer.start_upgrade(successor: new_cask, quit:)
# And flag it so in case of error
started_upgrade = true

Expand All @@ -423,9 +426,9 @@ def self.upgrade_cask(
# If successful, wipe the old cask from staging.
old_cask_installer.finalize_upgrade

reopen_apps_after_upgrade(old_cask)
reopen_apps_after_upgrade(old_cask) if quit
rescue => e
new_cask_installer.uninstall_artifacts(successor: old_cask) if new_artifacts_installed
new_cask_installer.uninstall_artifacts(successor: old_cask, quit:) if new_artifacts_installed
new_cask_installer.purge_versioned_files
old_cask_installer.revert_upgrade(predecessor: new_cask) if started_upgrade
raise e
Expand Down
5 changes: 5 additions & 0 deletions Library/Homebrew/cmd/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ class FinalUpgradeSummary < T::Struct
[:switch, "--skip-cask-deps", {
description: "Skip installing cask dependencies.",
}],
[:switch, "--no-quit", {
description: "Prevent running cask applications from being quit during upgrade.",
env: :no_upgrade_quit_casks,
}],
[:switch, "-g", "--greedy", {
description: "Also include casks with `version :latest` and `auto_updates true` casks " \
"that would otherwise be skipped.",
Expand Down Expand Up @@ -778,6 +782,7 @@ def upgrade_outdated_casks!(casks, skip_prefetch: false, show_upgrade_summary: t
quarantine: args.quarantine?,
require_sha: args.require_sha?,
skip_cask_deps: args.skip_cask_deps?,
quit: !args.no_quit?,
verbose: args.verbose?,
quiet: args.quiet?,
skip_prefetch:,
Expand Down
4 changes: 4 additions & 0 deletions Library/Homebrew/env_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,10 @@ module EnvConfig
boolean: true,
hidden: true, # odeprecated: remove in 5.2.0
},
HOMEBREW_NO_UPGRADE_QUIT_CASKS: {
description: "If set, `brew upgrade` will not quit running applications for casks during upgrades.",
boolean: true,
},
HOMEBREW_NO_VERIFY_ATTESTATIONS: {
description: "If set, Homebrew will not verify cryptographic attestations of build provenance for bottles " \
"from homebrew-core.",
Expand Down
3 changes: 3 additions & 0 deletions Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/upgrade_cmd.rbi

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions Library/Homebrew/test/cask/artifact/uninstall_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@
expect(quit_called).to be true
end

it "skips :quit during upgrade when quit is false" do
quit_called = false
allow(artifact).to receive(:dispatch_uninstall_directive) do |directive, **options|
quit_called ||= directive == :quit && options[:command] == fake_system_command
end

artifact.uninstall_phase(upgrade: true, quit: false, command: fake_system_command)

expect(quit_called).to be false
end

it "invokes :quit during reinstall" do
quit_called = false
allow(artifact).to receive(:dispatch_uninstall_directive) do |directive, **options|
Expand Down
14 changes: 14 additions & 0 deletions Library/Homebrew/test/cask/upgrade_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,20 @@ def write_info_plist(path, short_version:, bundle_version:)
expect(summary_deprecated).to include("local-caffeine")
end

it "passes the quit option to cask upgrades" do
expect(Cask::Upgrade).to receive(:upgrade_cask) do |_, _, **options|
expect(options[:quit]).to be(false)
end

Cask::Upgrade.upgrade_casks!(
local_caffeine,
quit: false,
skip_prefetch: true,
show_upgrade_summary: false,
args:,
)
end

it "excludes pinned Casks" do
local_caffeine.pin
summary_pinned = []
Expand Down
24 changes: 24 additions & 0 deletions Library/Homebrew/test/cmd/upgrade_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,30 @@ def install_formula_version(name, version, optlinked: false)
cmd.send(:upgrade_outdated_casks!, [])
end

it "passes --no-quit to cask upgrades" do
cmd = Homebrew::Cmd::UpgradeCmd.new(["--cask", "--no-quit"])

expect(Cask::Upgrade).to receive(:upgrade_casks!) do |*_, **kwargs|
expect(kwargs[:quit]).to be(false)
true
end

cmd.send(:upgrade_outdated_casks!, [])
Comment thread
MikeMcQuaid marked this conversation as resolved.
end

it "passes HOMEBREW_NO_UPGRADE_QUIT_CASKS to cask upgrades" do
with_env("HOMEBREW_NO_UPGRADE_QUIT_CASKS" => "1") do
cmd = Homebrew::Cmd::UpgradeCmd.new(["--cask"])

expect(Cask::Upgrade).to receive(:upgrade_casks!) do |*_, **kwargs|
expect(kwargs[:quit]).to be(false)
true
end

cmd.send(:upgrade_outdated_casks!, [])
end
end

it "prints formula and cask ask plans before upgrading" do
cmd = klass.new(["--ask"])

Expand Down
Loading