From b85f407e959d0420eaaaca25882335078aa70799 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Sat, 7 May 2022 20:19:54 -0700 Subject: [PATCH] Only upgrade :latest casks when --greedy and the cask has been updated A sha256 hash of the previous download is stored and compared with new downloads before updating :latest casks. This prevents unnecessary reinstalls when the cask hasn't been updated. Move download path to cask from installer to prevent unnecessary redownloads of casks. --- Library/Homebrew/cask/cask.rb | 37 +++++++-- Library/Homebrew/cask/installer.rb | 10 ++- Library/Homebrew/test/cask/cask_spec.rb | 24 ++++-- .../Homebrew/test/cask/cmd/upgrade_spec.rb | 76 ++++++++++++++++--- Library/Homebrew/test/cask/installer_spec.rb | 16 ++++ 5 files changed, 133 insertions(+), 30 deletions(-) diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index 4b612bd0c6..277518ea55 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -21,6 +21,8 @@ module Cask attr_reader :token, :sourcefile_path, :source, :config, :default_config + attr_accessor :download + def self.all Tap.flat_map(&:cask_files).map do |f| CaskLoader::FromTapPathLoader.new(f).load(config: nil) @@ -127,6 +129,23 @@ module Cask metadata_main_container_path/"config.json" end + def download_sha_path + metadata_main_container_path/"LATEST_DOWNLOAD_SHA256" + end + + def new_download_sha + require "cask/installer" + + @new_download_sha ||= Installer.new(self, verify_download_integrity: false) + .download(quiet: true) + .instance_eval { |x| Digest::SHA256.file(x).hexdigest } + end + + def outdated_download_sha? + current_download_sha = download_sha_path.read if download_sha_path.exist? + current_download_sha.blank? || current_download_sha != new_download_sha + end + def caskroom_path @caskroom_path ||= Caskroom.path.join(token) end @@ -140,14 +159,6 @@ module Cask # special case: tap version is not available return [] if version.nil? - if greedy || (greedy_latest && greedy_auto_updates) || (greedy_auto_updates && auto_updates) - return versions if version.latest? - elsif greedy_latest && version.latest? - return versions - elsif auto_updates - return [] - end - latest_version = if Homebrew::EnvConfig.install_from_api? && (latest_cask_version = Homebrew::API::Versions.latest_cask_version(token)) DSL::Version.new latest_cask_version.to_s @@ -155,6 +166,16 @@ module Cask version end + if greedy || greedy_latest || (greedy_auto_updates && auto_updates) + if latest_version.latest? + return versions if outdated_download_sha? + + return [] + end + elsif auto_updates + return [] + end + installed = versions current = installed.last diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 14be48b6c7..ac9de8fba0 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -181,8 +181,9 @@ module Cask sig { params(quiet: T.nilable(T::Boolean), timeout: T.nilable(T.any(Integer, Float))).returns(Pathname) } def download(quiet: nil, timeout: nil) - @download ||= downloader.fetch(quiet: quiet, verify_download_integrity: @verify_download_integrity, - timeout: timeout) + # Store cask download path in cask to prevent multiple downloads in a row when checking if it's outdated + @cask.download ||= downloader.fetch(quiet: quiet, verify_download_integrity: @verify_download_integrity, + timeout: timeout) end def verify_has_sha @@ -248,6 +249,7 @@ module Cask end save_config_file + save_download_sha if @cask.version.latest? rescue => e begin already_installed_artifacts.each do |artifact| @@ -394,6 +396,10 @@ module Cask @cask.config_path.atomic_write(@cask.config.to_json) end + def save_download_sha + @cask.download_sha_path.atomic_write(@cask.new_download_sha) + end + def uninstall oh1 "Uninstalling Cask #{Formatter.identifier(@cask)}" uninstall_artifacts(clear: true) diff --git a/Library/Homebrew/test/cask/cask_spec.rb b/Library/Homebrew/test/cask/cask_spec.rb index 50298bbb28..8bda74354b 100644 --- a/Library/Homebrew/test/cask/cask_spec.rb +++ b/Library/Homebrew/test/cask/cask_spec.rb @@ -131,15 +131,17 @@ describe Cask::Cask, :cask do describe ":latest casks" do let(:cask) { described_class.new("basic-cask") } - shared_examples ":latest cask" do |greedy, tap_version, expectations| + shared_examples ":latest cask" do |greedy, outdated_sha, tap_version, expectations| expectations.each do |installed_version, expected_output| context "when versions #{installed_version} are installed and the " \ - "tap version is #{tap_version}, #{"not" unless greedy} greedy" do + "tap version is #{tap_version}, #{"not " unless greedy}greedy" \ + "and sha is #{"not " unless outdated_sha}outdated" do subject { cask.outdated_versions(greedy: greedy) } it { allow(cask).to receive(:versions).and_return(installed_version) allow(cask).to receive(:version).and_return(Cask::DSL::Version.new(tap_version)) + allow(cask).to receive(:outdated_download_sha?).and_return(outdated_sha) expect(cask).to receive(:outdated_versions).and_call_original expect(subject).to eq expected_output } @@ -148,23 +150,29 @@ describe Cask::Cask, :cask do end describe ":latest version installed, :latest version in tap" do - include_examples ":latest cask", false, "latest", + include_examples ":latest cask", false, false, "latest", ["latest"] => [] - include_examples ":latest cask", true, "latest", + include_examples ":latest cask", true, false, "latest", + ["latest"] => [] + include_examples ":latest cask", true, true, "latest", ["latest"] => ["latest"] end describe "numbered version installed, :latest version in tap" do - include_examples ":latest cask", false, "latest", + include_examples ":latest cask", false, false, "latest", ["1.2.3"] => ["1.2.3"] - include_examples ":latest cask", true, "latest", + include_examples ":latest cask", true, false, "latest", + ["1.2.3"] => [] + include_examples ":latest cask", true, true, "latest", ["1.2.3"] => ["1.2.3"] end describe "latest version installed, numbered version in tap" do - include_examples ":latest cask", false, "1.2.3", + include_examples ":latest cask", false, false, "1.2.3", ["latest"] => ["latest"] - include_examples ":latest cask", true, "1.2.3", + include_examples ":latest cask", true, false, "1.2.3", + ["latest"] => ["latest"] + include_examples ":latest cask", true, true, "1.2.3", ["latest"] => ["latest"] end end diff --git a/Library/Homebrew/test/cask/cmd/upgrade_spec.rb b/Library/Homebrew/test/cask/cmd/upgrade_spec.rb index 70cdd4c510..ec4e0fac2d 100644 --- a/Library/Homebrew/test/cask/cmd/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cmd/upgrade_spec.rb @@ -106,8 +106,10 @@ describe Cask::Cmd::Upgrade, :cask do expect(version_latest).to be_installed expect(version_latest_path_1).to be_a_directory - expect(version_latest_path_2).to be_a_directory expect(version_latest.versions).to include("latest") + # Change download sha so that :latest cask decides to update itself + version_latest.download_sha_path.write("fake download sha") + expect(version_latest.outdated_download_sha?).to be(true) described_class.run("--greedy") @@ -124,12 +126,12 @@ describe Cask::Cmd::Upgrade, :cask do expect(local_transmission.versions).to include("2.61") expect(version_latest).to be_installed - expect(version_latest_path_1).to be_a_directory expect(version_latest_path_2).to be_a_directory expect(version_latest.versions).to include("latest") + expect(version_latest.outdated_download_sha?).to be(false) end - it 'does not include the Casks with "auto_updates true" when the version did not change' do + it 'does not include the Casks with "auto_updates true" or "version latest" when the version did not change' do expect(auto_updates).to be_installed expect(auto_updates_path).to be_a_directory expect(auto_updates.versions).to include("2.57") @@ -146,6 +148,32 @@ describe Cask::Cmd::Upgrade, :cask do expect(auto_updates_path).to be_a_directory expect(auto_updates.versions).to include("2.61") end + + it 'does not include the Casks with "version latest" when the version did not change' do + expect(version_latest).to be_installed + expect(version_latest_path_1).to be_a_directory + expect(version_latest_path_2).to be_a_directory + expect(version_latest.versions).to include("latest") + # Change download sha so that :latest cask decides to update itself + version_latest.download_sha_path.write("fake download sha") + expect(version_latest.outdated_download_sha?).to be(true) + + described_class.run("version-latest", "--greedy") + + expect(version_latest).to be_installed + expect(version_latest_path_1).to be_a_directory + expect(version_latest_path_2).to be_a_directory + expect(version_latest.versions).to include("latest") + expect(version_latest.outdated_download_sha?).to be(false) + + described_class.run("version-latest", "--greedy") + + expect(version_latest).to be_installed + expect(version_latest_path_1).to be_a_directory + expect(version_latest_path_2).to be_a_directory + expect(version_latest.versions).to include("latest") + expect(version_latest.outdated_download_sha?).to be(false) + end end end @@ -167,6 +195,8 @@ describe Cask::Cmd::Upgrade, :cask do 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 + expect(described_class).not_to receive(:upgrade_cask) + expect(local_caffeine).to be_installed expect(local_caffeine_path).to be_a_directory expect(local_caffeine.versions).to include("1.2.2") @@ -189,6 +219,8 @@ describe Cask::Cmd::Upgrade, :cask do end it "would update only the Casks specified in the command line" do + expect(described_class).not_to receive(:upgrade_cask) + expect(local_caffeine).to be_installed expect(local_caffeine_path).to be_a_directory expect(local_caffeine.versions).to include("1.2.2") @@ -211,6 +243,8 @@ describe Cask::Cmd::Upgrade, :cask do end it 'would update "auto_updates" and "latest" Casks when their tokens are provided in the command line' do + expect(described_class).not_to receive(:upgrade_cask) + expect(local_caffeine).to be_installed expect(local_caffeine_path).to be_a_directory expect(local_caffeine.versions).to include("1.2.2") @@ -235,6 +269,8 @@ describe Cask::Cmd::Upgrade, :cask do describe "with --greedy it checks additional Casks" do it 'would include the Casks with "auto_updates true" or "version latest"' do + expect(described_class).not_to receive(:upgrade_cask) + expect(local_caffeine).to be_installed expect(local_caffeine_path).to be_a_directory expect(local_caffeine.versions).to include("1.2.2") @@ -248,8 +284,9 @@ describe Cask::Cmd::Upgrade, :cask do 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") + # Change download sha so that :latest cask decides to update itself + version_latest.download_sha_path.write("fake download sha") + expect(version_latest.outdated_download_sha?).to be(true) described_class.run("--greedy", "--dry-run") @@ -269,10 +306,12 @@ describe Cask::Cmd::Upgrade, :cask do expect(local_transmission.versions).not_to include("2.61") expect(version_latest).to be_installed - expect(version_latest_path_2).to be_a_directory + expect(version_latest.outdated_download_sha?).to be(true) end - it 'does not include the Casks with "auto_updates true" when the version did not change' do + it 'would update outdated Casks with "auto_updates true"' do + expect(described_class).not_to receive(:upgrade_cask) + expect(auto_updates).to be_installed expect(auto_updates_path).to be_a_directory expect(auto_updates.versions).to include("2.57") @@ -283,13 +322,26 @@ describe Cask::Cmd::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") + end - described_class.run("--dry-run", "auto-updates", "--greedy") + it 'would update outdated Casks with "version latest"' do + expect(described_class).not_to receive(:upgrade_cask) - 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(version_latest).to be_installed + expect(version_latest_path_1).to be_a_directory + expect(version_latest_path_2).to be_a_directory + expect(version_latest.versions).to include("latest") + # Change download sha so that :latest cask decides to update itself + version_latest.download_sha_path.write("fake download sha") + expect(version_latest.outdated_download_sha?).to be(true) + + described_class.run("--dry-run", "version-latest", "--greedy") + + expect(version_latest).to be_installed + expect(version_latest_path_1).to be_a_directory + expect(version_latest_path_2).to be_a_directory + expect(version_latest.versions).to include("latest") + expect(version_latest.outdated_download_sha?).to be(true) end end end diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb index 928a98278b..ac65fd7cf0 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -223,6 +223,22 @@ describe Cask::Installer, :cask do described_class.new(caffeine, quiet: true).install }.to output(nil).to_stdout end + + it "does NOT generate LATEST_DOWNLOAD_SHA256 file for installed Cask without version :latest" do + caffeine = Cask::CaskLoader.load(cask_path("local-caffeine")) + + described_class.new(caffeine).install + + expect(caffeine.download_sha_path).not_to be_a_file + end + + it "generates and finds LATEST_DOWNLOAD_SHA256 file for installed Cask with version :latest" do + latest_cask = Cask::CaskLoader.load(cask_path("version-latest")) + + described_class.new(latest_cask).install + + expect(latest_cask.download_sha_path).to be_a_file + end end describe "uninstall" do