From 9e0e012a565d80136b18b98f0d01305eb10b3398 Mon Sep 17 00:00:00 2001 From: Doug Hogan Date: Fri, 26 Jul 2019 22:55:16 -0700 Subject: [PATCH 1/5] cmd/upgrade: add --dry-run option. --- Library/Homebrew/cmd/upgrade.rb | 12 +++++++++--- Library/Homebrew/test/cmd/upgrade_spec.rb | 13 +++++++++++++ docs/Manpage.md | 2 ++ manpages/brew.1 | 4 ++++ 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index 8099e311d2..854035dd95 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -46,6 +46,8 @@ module Homebrew description: "Print the verification and postinstall steps." switch "--display-times", description: "Print install times for each formula at the end of the run." + switch "--dry-run", + description: "Show what would be upgraded, but do not actually upgrade anything." conflicts "--build-from-source", "--force-bottle" formula_options end @@ -104,11 +106,15 @@ module Homebrew puts formulae_upgrades.join(", ") end - upgrade_formulae(formulae_to_install) + if args.dry_run? + puts "Dry run: did not upgrade anything." + else + upgrade_formulae(formulae_to_install) - check_dependents(formulae_to_install) + check_dependents(formulae_to_install) - Homebrew.messages.display_messages + Homebrew.messages.display_messages + end end def upgrade_formulae(formulae_to_install) diff --git a/Library/Homebrew/test/cmd/upgrade_spec.rb b/Library/Homebrew/test/cmd/upgrade_spec.rb index 9f139fc059..a4e13ccb71 100644 --- a/Library/Homebrew/test/cmd/upgrade_spec.rb +++ b/Library/Homebrew/test/cmd/upgrade_spec.rb @@ -16,4 +16,17 @@ describe "brew upgrade", :integration_test do expect(HOMEBREW_CELLAR/"testball/0.1").to be_a_directory expect(HOMEBREW_CELLAR/"testball/0.0.1").not_to exist end + + it "can do a dry run upgrade" do + setup_test_formula "testball" + (HOMEBREW_CELLAR/"testball/0.0.1/foo").mkpath + + expect { brew "upgrade", "--dry-run" } + .to output(/Dry run: did not upgrade anything/).to_stdout + .and not_to_output.to_stderr + .and be_a_success + + expect(HOMEBREW_CELLAR/"testball/0.1").not_to exist + expect(HOMEBREW_CELLAR/"testball/0.0.1").to be_a_directory + end end diff --git a/docs/Manpage.md b/docs/Manpage.md index c0ea5bf305..b93fa111e9 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -557,6 +557,8 @@ upgraded formulae or, every 30 days, for all formulae. Don't delete the temporary files created during installation. * `--display-times`: Print install times for each formula at the end of the run. +* `--dry-run`: + Show what would be upgraded, but do not actually upgrade anything. ### `uses` [*`options`*] *`formula`* diff --git a/manpages/brew.1 b/manpages/brew.1 index 385a7cd678..a9f6e13488 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -690,6 +690,10 @@ Don\'t delete the temporary files created during installation\. \fB\-\-display\-times\fR Print install times for each formula at the end of the run\. . +.TP +\fB\-\-dry\-run\fR +Show what would be upgraded, but do not actually upgrade anything\. +. .SS "\fBuses\fR [\fIoptions\fR] \fIformula\fR" Show the formulae that specify \fIformula\fR as a dependency\. When given multiple formula arguments, show the intersection of formulae that use \fIformula\fR\. . From 55fcd01bed6b7dea268f8e299376dc5c83e9eca9 Mon Sep 17 00:00:00 2001 From: Doug Hogan Date: Fri, 26 Jul 2019 22:55:37 -0700 Subject: [PATCH 2/5] cask/cmd/upgrade: add --dry-run option. --- Library/Homebrew/cask/cmd/upgrade.rb | 17 +- .../Homebrew/test/cask/cmd/upgrade_spec.rb | 173 ++++++++++++++++++ manpages/brew-cask.1 | 2 +- 3 files changed, 185 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/cask/cmd/upgrade.rb b/Library/Homebrew/cask/cmd/upgrade.rb index 3afac207ea..948d97e74a 100644 --- a/Library/Homebrew/cask/cmd/upgrade.rb +++ b/Library/Homebrew/cask/cmd/upgrade.rb @@ -9,6 +9,7 @@ module Cask option "--quiet", :quiet, false option "--force", :force, false option "--skip-cask-deps", :skip_cask_deps, false + option "--dry-run", :dry_run, false def initialize(*) super @@ -41,12 +42,16 @@ module Cask .map { |(old_cask, new_cask)| "#{new_cask.full_name} #{old_cask.version} -> #{new_cask.version}" } .join(", ") - upgradable_casks.each do |(old_cask, new_cask)| - begin - upgrade_cask(old_cask, new_cask) - rescue CaskError => e - caught_exceptions << e - next + if dry_run? + puts "Dry run: did not upgrade anything." + else + upgradable_casks.each do |(old_cask, new_cask)| + begin + upgrade_cask(old_cask, new_cask) + rescue CaskError => e + caught_exceptions << e + next + end end end diff --git a/Library/Homebrew/test/cask/cmd/upgrade_spec.rb b/Library/Homebrew/test/cask/cmd/upgrade_spec.rb index 503778eb15..2c0fff3d7b 100644 --- a/Library/Homebrew/test/cask/cmd/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cmd/upgrade_spec.rb @@ -170,6 +170,179 @@ describe Cask::Cmd::Upgrade, :cask do end end + context "dry run upgrade" do + let(:installed) { + [ + "outdated/local-caffeine", + "outdated/local-transmission", + "outdated/auto-updates", + "outdated/version-latest", + ] + } + + before do + installed.each { |cask| Cask::Cmd::Install.run(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 "would update all the installed Casks when no token is provided" do + local_caffeine = Cask::CaskLoader.load("local-caffeine") + local_caffeine_path = Cask::Config.global.appdir.join("Caffeine.app") + local_transmission = Cask::CaskLoader.load("local-transmission") + local_transmission_path = Cask::Config.global.appdir.join("Transmission.app") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + + expect(local_transmission).to be_installed + expect(local_transmission_path).to be_a_directory + expect(local_transmission.versions).to include("2.60") + + described_class.run("--dry-run") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + expect(local_caffeine.versions).not_to include("1.2.3") + + expect(local_transmission).to be_installed + 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") + end + + it "would update only the Casks specified in the command line" do + local_caffeine = Cask::CaskLoader.load("local-caffeine") + local_caffeine_path = Cask::Config.global.appdir.join("Caffeine.app") + local_transmission = Cask::CaskLoader.load("local-transmission") + local_transmission_path = Cask::Config.global.appdir.join("Transmission.app") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + + expect(local_transmission).to be_installed + expect(local_transmission_path).to be_a_directory + expect(local_transmission.versions).to include("2.60") + + described_class.run("--dry-run", "local-caffeine") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + expect(local_caffeine.versions).not_to include("1.2.3") + + expect(local_transmission).to be_installed + 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") + end + + it 'would update "auto_updates" and "latest" Casks when their tokens are provided in the command line' do + local_caffeine = Cask::CaskLoader.load("local-caffeine") + local_caffeine_path = Cask::Config.global.appdir.join("Caffeine.app") + auto_updates = Cask::CaskLoader.load("auto-updates") + auto_updates_path = Cask::Config.global.appdir.join("MyFancyApp.app") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.57") + + described_class.run("--dry-run", "local-caffeine", "auto-updates") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + expect(local_caffeine.versions).not_to include("1.2.3") + + expect(auto_updates).to be_installed + 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") + end + end + + describe "with --greedy it checks additional Casks" do + it 'would include the Casks with "auto_updates true" or "version latest"' do + local_caffeine = Cask::CaskLoader.load("local-caffeine") + local_caffeine_path = Cask::Config.global.appdir.join("Caffeine.app") + auto_updates = Cask::CaskLoader.load("auto-updates") + auto_updates_path = Cask::Config.global.appdir.join("MyFancyApp.app") + local_transmission = Cask::CaskLoader.load("local-transmission") + local_transmission_path = Cask::Config.global.appdir.join("Transmission.app") + version_latest = Cask::CaskLoader.load("version-latest") + version_latest_path_1 = Cask::Config.global.appdir.join("Caffeine Mini.app") + version_latest_path_2 = Cask::Config.global.appdir.join("Caffeine Pro.app") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.57") + + expect(local_transmission).to be_installed + expect(local_transmission_path).to be_a_directory + expect(local_transmission.versions).to include("2.60") + + expect(version_latest).to be_installed + expect(version_latest_path_1).to be_a_directory + expect(version_latest.versions).to include("latest") + + described_class.run("--greedy", "--dry-run") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + expect(local_caffeine.versions).not_to include("1.2.3") + + expect(auto_updates).to be_installed + 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(local_transmission).to be_installed + 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(version_latest).to be_installed + expect(version_latest_path_2).to be_a_directory + end + + it 'does not include the Casks with "auto_updates true" when the version did not change' do + cask = Cask::CaskLoader.load("auto-updates") + cask_path = cask.config.appdir.join("MyFancyApp.app") + + expect(cask).to be_installed + expect(cask_path).to be_a_directory + expect(cask.versions).to include("2.57") + + described_class.run("--dry-run", "auto-updates", "--greedy") + + expect(cask).to be_installed + expect(cask_path).to be_a_directory + expect(cask.versions).to include("2.57") + expect(cask.versions).not_to include("2.61") + + described_class.run("--dry-run", "auto-updates", "--greedy") + + expect(cask).to be_installed + expect(cask_path).to be_a_directory + expect(cask.versions).to include("2.57") + expect(cask.versions).not_to include("2.61") + end + end + end + context "failed upgrade" do let(:installed) { [ diff --git a/manpages/brew-cask.1 b/manpages/brew-cask.1 index 23febd9eed..ae24f6e2fb 100644 --- a/manpages/brew-cask.1 +++ b/manpages/brew-cask.1 @@ -100,7 +100,7 @@ Check the given Casks for correct style using RuboCop (with custom Cask cops)\. Uninstall the given Cask\. With \fB\-\-force\fR, uninstall even if the Cask does not appear to be present\. . .TP -\fBupgrade\fR [\-\-force] [\-\-greedy] \fItoken\fR [ \fItoken\fR \.\.\. ] +\fBupgrade\fR [\-\-force] [\-\-greedy] [\-\-dry\-run] \fItoken\fR [ \fItoken\fR \.\.\. ] Without token arguments, upgrade all the installed Casks that have newer versions available in the tap; otherwise update the tokens given in the command line\. If \fB\-\-greedy\fR is given then also upgrade the Casks having \fBauto_updates true\fR or \fBversion :latest\fR\. . .TP From ac0ff9ae46dc47790a3c0d16c2c85b11bd08383d Mon Sep 17 00:00:00 2001 From: Doug Hogan Date: Sat, 27 Jul 2019 07:03:42 -0700 Subject: [PATCH 3/5] Update md and run `brew man`. --- Library/Homebrew/manpages/brew-cask.1.md | 5 ++++- manpages/brew-cask.1 | 5 ++++- manpages/brew.1 | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/manpages/brew-cask.1.md b/Library/Homebrew/manpages/brew-cask.1.md index 69af6d81e4..cec5db30ad 100644 --- a/Library/Homebrew/manpages/brew-cask.1.md +++ b/Library/Homebrew/manpages/brew-cask.1.md @@ -97,13 +97,16 @@ graphical user interface. Uninstall the given Cask. With `--force`, uninstall even if the Cask does not appear to be present. - * `upgrade` [--force] [--greedy] [ ... ]: + * `upgrade` [--force] [--greedy] [--dry-run] [ ... ]: Without token arguments, upgrade all the installed Casks that have newer versions available in the tap; otherwise update the tokens given in the command line. If `--greedy` is given then also upgrade the Casks having `auto_updates true` or `version :latest`. + If `--dry-run` is given, show what would be upgraded, but do not actually + upgrade anything. + * `zap` [--force] [ ... ]: Unconditionally remove _all_ files associated with the given Cask. With `--force`, zap even if the Cask does not appear to be currently installed. diff --git a/manpages/brew-cask.1 b/manpages/brew-cask.1 index ae24f6e2fb..93c6812b9f 100644 --- a/manpages/brew-cask.1 +++ b/manpages/brew-cask.1 @@ -1,7 +1,7 @@ .\" generated with Ronn/v0.7.3 .\" http://github.com/rtomayko/ronn/tree/0.7.3 . -.TH "BREW\-CASK" "1" "May 2019" "Homebrew" "brew-cask" +.TH "BREW\-CASK" "1" "July 2019" "Homebrew" "brew-cask" . .SH "NAME" \fBbrew\-cask\fR \- a friendly binary installer for macOS @@ -103,6 +103,9 @@ Uninstall the given Cask\. With \fB\-\-force\fR, uninstall even if the Cask does \fBupgrade\fR [\-\-force] [\-\-greedy] [\-\-dry\-run] \fItoken\fR [ \fItoken\fR \.\.\. ] Without token arguments, upgrade all the installed Casks that have newer versions available in the tap; otherwise update the tokens given in the command line\. If \fB\-\-greedy\fR is given then also upgrade the Casks having \fBauto_updates true\fR or \fBversion :latest\fR\. . +.IP +If \fB\-\-dry\-run\fR is given, show what would be upgraded, but do not actually upgrade anything\. +. .TP \fBzap\fR [\-\-force] \fItoken\fR [ \fItoken\fR \.\.\. ] Unconditionally remove \fIall\fR files associated with the given Cask\. With \fB\-\-force\fR, zap even if the Cask does not appear to be currently installed\. diff --git a/manpages/brew.1 b/manpages/brew.1 index a9f6e13488..f0325631ea 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -1,7 +1,7 @@ .\" generated with Ronn/v0.7.3 .\" http://github.com/rtomayko/ronn/tree/0.7.3 . -.TH "BREW" "1" "May 2019" "Homebrew" "brew" +.TH "BREW" "1" "July 2019" "Homebrew" "brew" . .SH "NAME" \fBbrew\fR \- The missing package manager for macOS From 7f6ef77d0e12b898a366dd907e4c4f3434fcb9a6 Mon Sep 17 00:00:00 2001 From: Doug Hogan Date: Sat, 27 Jul 2019 07:21:36 -0700 Subject: [PATCH 4/5] Address PR comments: remove test and use return ... if --- Library/Homebrew/cask/cmd/upgrade.rb | 18 ++++++++---------- Library/Homebrew/cmd/upgrade.rb | 12 +++++------- Library/Homebrew/test/cmd/upgrade_spec.rb | 13 ------------- 3 files changed, 13 insertions(+), 30 deletions(-) diff --git a/Library/Homebrew/cask/cmd/upgrade.rb b/Library/Homebrew/cask/cmd/upgrade.rb index 948d97e74a..77529012b9 100644 --- a/Library/Homebrew/cask/cmd/upgrade.rb +++ b/Library/Homebrew/cask/cmd/upgrade.rb @@ -42,16 +42,14 @@ module Cask .map { |(old_cask, new_cask)| "#{new_cask.full_name} #{old_cask.version} -> #{new_cask.version}" } .join(", ") - if dry_run? - puts "Dry run: did not upgrade anything." - else - upgradable_casks.each do |(old_cask, new_cask)| - begin - upgrade_cask(old_cask, new_cask) - rescue CaskError => e - caught_exceptions << e - next - end + return puts "Dry run: did not upgrade anything." if dry_run? + + upgradable_casks.each do |(old_cask, new_cask)| + begin + upgrade_cask(old_cask, new_cask) + rescue CaskError => e + caught_exceptions << e + next end end diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index 854035dd95..f0acf672fb 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -106,15 +106,13 @@ module Homebrew puts formulae_upgrades.join(", ") end - if args.dry_run? - puts "Dry run: did not upgrade anything." - else - upgrade_formulae(formulae_to_install) + return puts "Dry run: did not upgrade anything." if args.dry_run? - check_dependents(formulae_to_install) + upgrade_formulae(formulae_to_install) - Homebrew.messages.display_messages - end + check_dependents(formulae_to_install) + + Homebrew.messages.display_messages end def upgrade_formulae(formulae_to_install) diff --git a/Library/Homebrew/test/cmd/upgrade_spec.rb b/Library/Homebrew/test/cmd/upgrade_spec.rb index a4e13ccb71..9f139fc059 100644 --- a/Library/Homebrew/test/cmd/upgrade_spec.rb +++ b/Library/Homebrew/test/cmd/upgrade_spec.rb @@ -16,17 +16,4 @@ describe "brew upgrade", :integration_test do expect(HOMEBREW_CELLAR/"testball/0.1").to be_a_directory expect(HOMEBREW_CELLAR/"testball/0.0.1").not_to exist end - - it "can do a dry run upgrade" do - setup_test_formula "testball" - (HOMEBREW_CELLAR/"testball/0.0.1/foo").mkpath - - expect { brew "upgrade", "--dry-run" } - .to output(/Dry run: did not upgrade anything/).to_stdout - .and not_to_output.to_stderr - .and be_a_success - - expect(HOMEBREW_CELLAR/"testball/0.1").not_to exist - expect(HOMEBREW_CELLAR/"testball/0.0.1").to be_a_directory - end end From 41461b2c00cadaf4cb62ab0b85445c950c259698 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Sun, 28 Jul 2019 14:50:59 +0100 Subject: [PATCH 5/5] upgrade: tweak --dry-run wording. --- Library/Homebrew/cask/cmd/upgrade.rb | 6 +++--- Library/Homebrew/cmd/upgrade.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/cask/cmd/upgrade.rb b/Library/Homebrew/cask/cmd/upgrade.rb index 77529012b9..968e8ca95f 100644 --- a/Library/Homebrew/cask/cmd/upgrade.rb +++ b/Library/Homebrew/cask/cmd/upgrade.rb @@ -33,7 +33,8 @@ module Cask end ohai "Casks with `auto_updates` or `version :latest` will not be upgraded" if args.empty? && !greedy? - oh1 "Upgrading #{outdated_casks.count} #{"outdated package".pluralize(outdated_casks.count)}:" + verb = dry_run? ? "Would upgrade" : "Upgrading" + oh1 "#{verb} #{outdated_casks.count} #{"outdated package".pluralize(outdated_casks.count)}:" caught_exceptions = [] upgradable_casks = outdated_casks.map { |c| [CaskLoader.load(c.installed_caskfile), c] } @@ -41,8 +42,7 @@ module Cask puts upgradable_casks .map { |(old_cask, new_cask)| "#{new_cask.full_name} #{old_cask.version} -> #{new_cask.version}" } .join(", ") - - return puts "Dry run: did not upgrade anything." if dry_run? + return if dry_run? upgradable_casks.each do |(old_cask, new_cask)| begin diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index f0acf672fb..6899c2bc34 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -95,7 +95,8 @@ module Homebrew if formulae_to_install.empty? oh1 "No packages to upgrade" else - oh1 "Upgrading #{formulae_to_install.count} outdated #{"package".pluralize(formulae_to_install.count)}:" + verb = args.dry_run? ? "Would upgrade" : "Upgrading" + oh1 "#{verb} #{formulae_to_install.count} outdated #{"package".pluralize(formulae_to_install.count)}:" formulae_upgrades = formulae_to_install.map do |f| if f.optlinked? "#{f.full_specified_name} #{Keg.new(f.opt_prefix).version} -> #{f.pkg_version}" @@ -105,8 +106,7 @@ module Homebrew end puts formulae_upgrades.join(", ") end - - return puts "Dry run: did not upgrade anything." if args.dry_run? + return if args.dry_run? upgrade_formulae(formulae_to_install)