Skip to content

Implement brew cask upgrade #3396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Nov 27, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions Library/Homebrew/cask/lib/hbc/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
require "hbc/cli/search"
require "hbc/cli/style"
require "hbc/cli/uninstall"
require "hbc/cli/upgrade"
require "hbc/cli/--version"
require "hbc/cli/zap"

Expand Down
69 changes: 69 additions & 0 deletions Library/Homebrew/cask/lib/hbc/cli/upgrade.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
module Hbc
class CLI
class Upgrade < AbstractCommand
option "--greedy", :greedy, false
option "--quiet", :quiet, false
option "--force", :force, false
option "--force-update", :force_update, false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flag isn't used anywhere.

option "--skip-cask-deps", :skip_cask_deps, false

def initialize(*)
super
self.verbose = ($stdout.tty? || verbose?) && !quiet?
end

def run
outdated_casks = casks(alternative: -> { Hbc.installed }).find_all { |cask| cask.outdated?(greedy?) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use select instead of find_all.


if outdated_casks.empty?
oh1 "No packages to upgrade"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should just return here.

else
oh1 "Upgrading #{Formatter.pluralize(outdated_casks.length, "outdated package")}, with result:"
puts outdated_casks.map { |f| "#{f.full_name} #{f.version}" } * ", "
end

outdated_casks.each do |old_cask|
odebug "Uninstalling Cask #{old_cask}"

raise CaskNotInstalledError, old_cask unless old_cask.installed? || force?

unless old_cask.installed_caskfile.nil?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should error here if there is no old cask file.

# use the same cask file that was used for installation, if possible
old_cask = CaskLoader.load(old_cask.installed_caskfile) if old_cask.installed_caskfile.exist?
end

old_cask_installer = Installer.new(old_cask, binaries: binaries?, verbose: verbose?, force: force?, upgrade: true)

new_cask = CaskLoader.load(old_cask.to_s)

new_cask_installer =
Installer.new(new_cask, binaries: binaries?,
verbose: verbose?,
force: force?,
skip_cask_deps: skip_cask_deps?,
require_sha: require_sha?,
upgrade: true)

begin
# purge artifacts BUT keep metadata aside
old_cask_installer.start_upgrade

# install BUT do not yet save metadata

new_cask_installer.install

# if successful, remove old metadata and install new
old_cask_installer.finalize_upgrade
rescue CaskError => e
opoo e.message
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead outputting the error message here, re-raise the exception at the end. Otherwise it will exit with 0 when it fails.

old_cask_installer.revert_upgrade
end
end
end

def self.help
"upgrades all outdated casks"
end
end
end
end
33 changes: 27 additions & 6 deletions Library/Homebrew/cask/lib/hbc/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class Installer

PERSISTENT_METADATA_SUBDIRS = ["gpg"].freeze

def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false, binaries: true, verbose: false, require_sha: false)
def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false, binaries: true, verbose: false, require_sha: false, upgrade: false)
@cask = cask
@command = command
@force = force
Expand All @@ -28,6 +28,7 @@ def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false
@verbose = verbose
@require_sha = require_sha
@reinstall = false
@upgrade = upgrade
end

attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?, :verbose?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add :upgrade? here and use upgrade? instead of @upgrade in other methods.

Expand Down Expand Up @@ -83,7 +84,7 @@ def install
odebug "Hbc::Installer#install"

if @cask.installed? && !force? && !@reinstall
raise CaskAlreadyInstalledError, @cask
raise CaskAlreadyInstalledError, @cask unless @upgrade
end

check_conflicts
Expand Down Expand Up @@ -129,7 +130,7 @@ def uninstall_existing_cask
installed_cask = installed_caskfile.exist? ? CaskLoader.load(installed_caskfile) : @cask

# Always force uninstallation, ignore method parameter
Installer.new(installed_cask, binaries: binaries?, verbose: verbose?, force: true).uninstall
Installer.new(installed_cask, binaries: binaries?, verbose: verbose?, force: true, upgrade: @upgrade).uninstall
end

def summary
Expand Down Expand Up @@ -372,6 +373,26 @@ def uninstall
purge_caskroom_path if force?
end

def start_upgrade
return unless @upgrade
oh1 "Starting upgrade for Cask #{@cask}"

disable_accessibility_access
uninstall_artifacts
end

def revert_upgrade
return unless @upgrade
opoo "Reverting upgrade for Cask #{@cask}"
reinstall
end

def finalize_upgrade
return unless @upgrade
purge_versioned_files(upgrade: true)
oh1 "Cask #{@cask} was successfully upgraded!"
end

def uninstall_artifacts
odebug "Un-installing artifacts"
artifacts = @cask.artifacts
Expand Down Expand Up @@ -404,7 +425,7 @@ def gain_permissions_remove(path)
Utils.gain_permissions_remove(path, command: @command)
end

def purge_versioned_files
def purge_versioned_files(upgrade: false)
odebug "Purging files for version #{@cask.version} of Cask #{@cask}"

# versioned staged distribution
Expand All @@ -420,10 +441,10 @@ def purge_versioned_files
end
end
@cask.metadata_versioned_path.rmdir_if_possible
@cask.metadata_master_container_path.rmdir_if_possible
@cask.metadata_master_container_path.rmdir_if_possible unless upgrade

# toplevel staged distribution
@cask.caskroom_path.rmdir_if_possible
@cask.caskroom_path.rmdir_if_possible unless upgrade
end

def purge_caskroom_path
Expand Down
78 changes: 78 additions & 0 deletions Library/Homebrew/test/cask/cli/upgrade_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
require_relative "shared_examples/invalid_option"

describe Hbc::CLI::Upgrade, :cask do
let(:installed) do
[
Hbc::CaskLoader.load(cask_path("outdated/local-caffeine")),
Hbc::CaskLoader.load(cask_path("outdated/local-transmission")),
Hbc::CaskLoader.load(cask_path("outdated/auto-updates")),
]
end

it_behaves_like "a command that handles invalid options"

before(:example) do
installed.each { |cask| InstallHelper.install_with_caskfile(cask) }

allow_any_instance_of(described_class).to receive(:verbose?).and_return(true)
end

describe 'without --greedy it ignores the Casks with "version latest" or "auto_updates true"' do
it "updates all the installed Casks when no token is provided" do
described_class.run

expect(Hbc::CaskLoader.load("local-caffeine")).to be_installed
expect(Hbc.appdir.join("Caffeine.app")).to be_a_directory
expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.3")

expect(Hbc::CaskLoader.load("local-transmission")).to be_installed
expect(Hbc.appdir.join("Transmission.app")).to be_a_directory
expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.61")
end

it "updates only the Casks specified in the command line" do
expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.2")
expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60")

described_class.run("local-caffeine")

expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.3")
expect(Hbc::CaskLoader.load("local-caffeine").versions).to_not include("1.2.2")
expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60")
end

it 'ignores "auto_updates" and "latest" Casks even when their tokens are provided in the command line' do
expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.2")
expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57")

described_class.run("local-caffeine", "auto-updates", "version-latest-string")

expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.3")
expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57")
end
end

describe "with --greedy it checks additional Casks" do
it 'includes the Casks with "auto_updates true" or "version latest" with --greedy' do
expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57")
expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.2")
expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60")

described_class.run("--greedy")

expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61")
expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.3")
expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.61")
end

it 'does not include the Casks with "auto_updates true" when the version did not change' do
cask = Hbc::CaskLoader.load(cask_path("auto-updates"))
InstallHelper.install_with_caskfile(cask)
expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61")

described_class.run("auto-updates", "--greedy")

expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61")
end
end
end