From a8d506fdda553428d281ce606f440319d3f530c3 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Fri, 29 Mar 2024 09:22:08 -0400 Subject: [PATCH 1/3] livecheck: Add ExtractPlist skip to SkipConditions When the `--extract-plist` option was added to livecheck, conditions were added in `#run_checks` to skip casks using `ExtractPlist` if the `--extract-plist` isn't used and the run involves multiple formulae/casks. This integrates the skip into the `SkipConditions` class. --- Library/Homebrew/livecheck/livecheck.rb | 22 ++++------- Library/Homebrew/livecheck/skip_conditions.rb | 37 +++++++++++++++++-- .../test/livecheck/skip_conditions_spec.rb | 36 ++++++++++++++++++ 3 files changed, 78 insertions(+), 17 deletions(-) diff --git a/Library/Homebrew/livecheck/livecheck.rb b/Library/Homebrew/livecheck/livecheck.rb index 965e9c3c8b..f30644744c 100644 --- a/Library/Homebrew/livecheck/livecheck.rb +++ b/Library/Homebrew/livecheck/livecheck.rb @@ -241,29 +241,23 @@ module Homebrew puts end - if cask && !extract_plist && formula_or_cask.livecheck.strategy == :extract_plist - skip_info = { - cask: cask.token, - status: "skipped", - messages: ["Livecheck skipped due to the ExtractPlist strategy"], - meta: { livecheckable: true }, - } - - SkipConditions.print_skip_information(skip_info) if !newer_only && !quiet - next - end - # Check skip conditions for a referenced formula/cask if referenced_formula_or_cask skip_info = SkipConditions.referenced_skip_information( referenced_formula_or_cask, name, - full_name: use_full_name, + full_name: use_full_name, verbose:, + extract_plist:, ) end - skip_info ||= SkipConditions.skip_information(formula_or_cask, full_name: use_full_name, verbose:) + skip_info ||= SkipConditions.skip_information( + formula_or_cask, + full_name: use_full_name, + verbose:, + extract_plist:, + ) if skip_info.present? next skip_info if json && !newer_only diff --git a/Library/Homebrew/livecheck/skip_conditions.rb b/Library/Homebrew/livecheck/skip_conditions.rb index 2ee449ed88..dc19de5916 100644 --- a/Library/Homebrew/livecheck/skip_conditions.rb +++ b/Library/Homebrew/livecheck/skip_conditions.rb @@ -151,6 +151,27 @@ module Homebrew Livecheck.status_hash(cask, "disabled", full_name:, verbose:) end + sig { + params( + cask: Cask::Cask, + _livecheckable: T::Boolean, + full_name: T::Boolean, + verbose: T::Boolean, + extract_plist: T::Boolean, + ).returns(Hash) + } + def cask_extract_plist(cask, _livecheckable, full_name: false, verbose: false, extract_plist: false) + return {} if extract_plist || cask.livecheck.strategy != :extract_plist + + Livecheck.status_hash( + cask, + "skipped", + ["Use `--extract-plist` to enable checking multiple casks with ExtractPlist strategy"], + full_name:, + verbose:, + ) + end + sig { params( cask: Cask::Cask, @@ -194,6 +215,7 @@ module Homebrew :cask_discontinued, :cask_deprecated, :cask_disabled, + :cask_extract_plist, :cask_version_latest, :cask_url_unversioned, ].freeze @@ -211,9 +233,10 @@ module Homebrew package_or_resource: T.any(Formula, Cask::Cask, Resource), full_name: T::Boolean, verbose: T::Boolean, + extract_plist: T::Boolean, ).returns(Hash) } - def skip_information(package_or_resource, full_name: false, verbose: false) + def skip_information(package_or_resource, full_name: false, verbose: false, extract_plist: true) livecheckable = package_or_resource.livecheckable? checks = case package_or_resource @@ -227,7 +250,12 @@ module Homebrew return {} unless checks checks.each do |method_name| - skip_hash = send(method_name, package_or_resource, livecheckable, full_name:, verbose:) + skip_hash = case method_name + when :cask_extract_plist + send(method_name, package_or_resource, livecheckable, full_name:, verbose:, extract_plist:) + else + send(method_name, package_or_resource, livecheckable, full_name:, verbose:) + end return skip_hash if skip_hash.present? end @@ -244,18 +272,21 @@ module Homebrew original_package_or_resource_name: String, full_name: T::Boolean, verbose: T::Boolean, + extract_plist: T::Boolean, ).returns(T.nilable(Hash)) } def referenced_skip_information( livecheck_package_or_resource, original_package_or_resource_name, full_name: false, - verbose: false + verbose: false, + extract_plist: true ) skip_info = SkipConditions.skip_information( livecheck_package_or_resource, full_name:, verbose:, + extract_plist:, ) return if skip_info.blank? diff --git a/Library/Homebrew/test/livecheck/skip_conditions_spec.rb b/Library/Homebrew/test/livecheck/skip_conditions_spec.rb index e174fbe3e7..493b8b92e9 100644 --- a/Library/Homebrew/test/livecheck/skip_conditions_spec.rb +++ b/Library/Homebrew/test/livecheck/skip_conditions_spec.rb @@ -127,6 +127,18 @@ RSpec.describe Homebrew::Livecheck::SkipConditions do disable! date: "2020-06-25", because: :discontinued end, + extract_plist: Cask::Cask.new("test_extract_plist_skip") do + version "0.0.1" + + url "https://brew.sh/test-0.0.1.tgz" + name "Test ExtractPlist Skip" + desc "Skipped test cask" + homepage "https://brew.sh" + + livecheck do + strategy :extract_plist + end + end, latest: Cask::Cask.new("test_latest") do version :latest sha256 :no_check @@ -267,6 +279,14 @@ RSpec.describe Homebrew::Livecheck::SkipConditions do livecheckable: false, }, }, + extract_plist: { + cask: "test_extract_plist_skip", + status: "skipped", + messages: ["Use `--extract-plist` to enable checking multiple casks with ExtractPlist strategy"], + meta: { + livecheckable: true, + }, + }, latest: { cask: "test_latest", status: "latest", @@ -381,6 +401,13 @@ RSpec.describe Homebrew::Livecheck::SkipConditions do end end + context "when a cask has a `livecheck` block using `ExtractPlist` and `--extract-plist` is not used" do + it "skips" do + expect(skip_conditions.skip_information(casks[:extract_plist], extract_plist: false)) + .to eq(status_hashes[:cask][:extract_plist]) + end + end + context "when a cask without a livecheckable has `version :latest`" do it "skips" do expect(skip_conditions.skip_information(casks[:latest])) @@ -497,6 +524,15 @@ RSpec.describe Homebrew::Livecheck::SkipConditions do end end + context "when a cask has a `livecheck` block using `ExtractPlist` and `--extract-plist` is not used" do + it "skips" do + expect do + skip_conditions.referenced_skip_information(casks[:extract_plist], original_name, extract_plist: false) + end + .to raise_error(RuntimeError, "Referenced cask (test_extract_plist_skip) is automatically skipped") + end + end + context "when a cask without a livecheckable has `version :latest`" do it "errors" do expect { skip_conditions.referenced_skip_information(casks[:latest], original_name) } From a4134125f20327e5081d1cdae3822d433001dfd9 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Fri, 29 Mar 2024 09:25:36 -0400 Subject: [PATCH 2/3] livecheck: Clarify --extract-plist behavior From the description of the `--extract-plist` option, it would seem that the `ExtractPlist` strategy is only enabled when the option is used. Instead, livecheck automatically enables the strategy if the command is run on only one cask. This rewords descriptions of the option to clarify the behavior. --- Library/Homebrew/dev-cmd/livecheck.rb | 2 +- Library/Homebrew/livecheck/livecheck.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/dev-cmd/livecheck.rb b/Library/Homebrew/dev-cmd/livecheck.rb index 6cad963d55..4868097656 100644 --- a/Library/Homebrew/dev-cmd/livecheck.rb +++ b/Library/Homebrew/dev-cmd/livecheck.rb @@ -37,7 +37,7 @@ module Homebrew switch "--cask", "--casks", description: "Only check casks." switch "--extract-plist", - description: "Include casks using the ExtractPlist livecheck strategy." + description: "Enable checking multiple casks with ExtractPlist strategy." conflicts "--debug", "--json" conflicts "--tap=", "--eval-all", "--installed" diff --git a/Library/Homebrew/livecheck/livecheck.rb b/Library/Homebrew/livecheck/livecheck.rb index f30644744c..13a987339c 100644 --- a/Library/Homebrew/livecheck/livecheck.rb +++ b/Library/Homebrew/livecheck/livecheck.rb @@ -218,7 +218,7 @@ module Homebrew ) end - # If only one formula/cask is being checked, we enable extract-plist + # Allow ExtractPlist strategy if only one formula/cask is being checked. extract_plist = true if formulae_and_casks_total == 1 formulae_checked = formulae_and_casks_to_check.map.with_index do |formula_or_cask, i| From 111ac5810cc41b30a9a9be4c69de290d300faffd Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Fri, 29 Mar 2024 09:26:14 -0400 Subject: [PATCH 3/3] livecheck: Clean up whitespace, ordering --- Library/Homebrew/livecheck/livecheck.rb | 1 - .../Homebrew/test/livecheck/skip_conditions_spec.rb | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/livecheck/livecheck.rb b/Library/Homebrew/livecheck/livecheck.rb index 13a987339c..190c92e447 100644 --- a/Library/Homebrew/livecheck/livecheck.rb +++ b/Library/Homebrew/livecheck/livecheck.rb @@ -202,7 +202,6 @@ module Homebrew has_a_newer_upstream_version = T.let(false, T::Boolean) formulae_and_casks_total = formulae_and_casks_to_check.count - if json && !quiet && $stderr.tty? Tty.with($stderr) do |stderr| stderr.puts Formatter.headline("Running checks", color: :blue) diff --git a/Library/Homebrew/test/livecheck/skip_conditions_spec.rb b/Library/Homebrew/test/livecheck/skip_conditions_spec.rb index 493b8b92e9..33e40cf543 100644 --- a/Library/Homebrew/test/livecheck/skip_conditions_spec.rb +++ b/Library/Homebrew/test/livecheck/skip_conditions_spec.rb @@ -31,11 +31,6 @@ RSpec.describe Homebrew::Livecheck::SkipConditions do url "https://brew.sh/test-0.0.1.tgz" disable! date: "2020-06-25", because: :unmaintained end, - versioned: formula("test@0.0.1") do - desc "Versioned test formula" - homepage "https://brew.sh" - url "https://brew.sh/test-0.0.1.tgz" - end, head_only: formula("test_head_only") do desc "HEAD-only test formula" homepage "https://brew.sh" @@ -74,6 +69,11 @@ RSpec.describe Homebrew::Livecheck::SkipConditions do skip "Not maintained" end end, + versioned: formula("test@0.0.1") do + desc "Versioned test formula" + homepage "https://brew.sh" + url "https://brew.sh/test-0.0.1.tgz" + end, } end