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 965e9c3c8b742..f30644744c83a 100644 --- a/Library/Homebrew/livecheck/livecheck.rb +++ b/Library/Homebrew/livecheck/livecheck.rb @@ -241,29 +241,23 @@ def run_checks( 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 2ee449ed88b69..dc19de5916c1a 100644 --- a/Library/Homebrew/livecheck/skip_conditions.rb +++ b/Library/Homebrew/livecheck/skip_conditions.rb @@ -151,6 +151,27 @@ def cask_disabled(cask, livecheckable, full_name: false, verbose: false) 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 @@ def cask_url_unversioned(cask, livecheckable, full_name: false, verbose: false) :cask_discontinued, :cask_deprecated, :cask_disabled, + :cask_extract_plist, :cask_version_latest, :cask_url_unversioned, ].freeze @@ -211,9 +233,10 @@ def cask_url_unversioned(cask, livecheckable, full_name: false, verbose: false) 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 @@ def skip_information(package_or_resource, full_name: false, verbose: false) 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 @@ def skip_information(package_or_resource, full_name: false, verbose: false) 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 e174fbe3e781b..493b8b92e9c23 100644 --- a/Library/Homebrew/test/livecheck/skip_conditions_spec.rb +++ b/Library/Homebrew/test/livecheck/skip_conditions_spec.rb @@ -127,6 +127,18 @@ 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 @@ 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 @@ 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 @@ 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 6cad963d55040..4868097656111 100644 --- a/Library/Homebrew/dev-cmd/livecheck.rb +++ b/Library/Homebrew/dev-cmd/livecheck.rb @@ -37,7 +37,7 @@ class LivecheckCmd < AbstractCommand 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 f30644744c83a..13a987339c095 100644 --- a/Library/Homebrew/livecheck/livecheck.rb +++ b/Library/Homebrew/livecheck/livecheck.rb @@ -218,7 +218,7 @@ def run_checks( ) 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 13a987339c095..190c92e4475cb 100644 --- a/Library/Homebrew/livecheck/livecheck.rb +++ b/Library/Homebrew/livecheck/livecheck.rb @@ -202,7 +202,6 @@ def run_checks( 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 493b8b92e9c23..33e40cf543aca 100644 --- a/Library/Homebrew/test/livecheck/skip_conditions_spec.rb +++ b/Library/Homebrew/test/livecheck/skip_conditions_spec.rb @@ -31,11 +31,6 @@ 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 @@ 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