From 682d35a1d711df75f5f17b435c2a500081ee18d2 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Fri, 12 Jul 2024 04:20:54 +0100 Subject: [PATCH] cli/named_args: raise priority of core casks --- Library/Homebrew/cli/named_args.rb | 86 +++++++++++++------ Library/Homebrew/test/cli/named_args_spec.rb | 30 ++++++- .../Homebrew/test/support/helper/formula.rb | 4 +- 3 files changed, 92 insertions(+), 28 deletions(-) diff --git a/Library/Homebrew/cli/named_args.rb b/Library/Homebrew/cli/named_args.rb index c92693a87ea459..394eec05e0995f 100644 --- a/Library/Homebrew/cli/named_args.rb +++ b/Library/Homebrew/cli/named_args.rb @@ -138,9 +138,9 @@ def load_formula_or_cask(name, only: nil, method: nil, warn: nil) Homebrew.with_no_api_env_if_needed(@without_api) do unreadable_error = nil - if only != :cask + formula_or_keg = if only != :cask begin - formula = case method + case method when nil, :factory options = { warn:, force_bottle: @force_bottle, flags: @flags }.compact Formulary.factory(name, *@override_spec, **options) @@ -156,27 +156,32 @@ def load_formula_or_cask(name, only: nil, method: nil, warn: nil) else raise end - - warn_if_cask_conflicts(name, "formula") if only != :formula - return formula rescue FormulaUnreadableError, FormulaClassUnavailableError, TapFormulaUnreadableError, TapFormulaClassUnavailableError, FormulaSpecificationError => e # Need to rescue before `FormulaUnavailableError` (superclass of this) # The formula was found, but there's a problem with its implementation unreadable_error ||= e + nil rescue NoSuchKegError, FormulaUnavailableError => e raise e if only == :formula + + nil end end - if only != :formula + if only == :formula + return formula_or_keg if formula_or_keg + elsif formula_or_keg.is_a?(Keg) || formula_or_keg&.tap&.core_tap? + warn_if_cask_conflicts(name, "formula") + return formula_or_keg + else want_keg_like_cask = [:latest_kegs, :default_kegs, :kegs].include?(method) - begin + cask = begin config = Cask::Config.from_args(@parent) if @cask_options options = { warn: }.compact - cask = Cask::CaskLoader.load(name, config:, **options) + candidate_cask = Cask::CaskLoader.load(name, config:, **options) if unreadable_error.present? onoe <<~EOS @@ -188,27 +193,40 @@ def load_formula_or_cask(name, only: nil, method: nil, warn: nil) # If we're trying to get a keg-like Cask, do our best to use the same cask # file that was used for installation, if possible. - if want_keg_like_cask && (installed_caskfile = cask.installed_caskfile) && installed_caskfile.exist? - cask = Cask::CaskLoader.load(installed_caskfile) + if want_keg_like_cask && + (installed_caskfile = candidate_cask.installed_caskfile) && + installed_caskfile.exist? + Cask::CaskLoader.load(installed_caskfile) + else + candidate_cask end - - return cask rescue Cask::CaskUnreadableError, Cask::CaskInvalidError => e # If we're trying to get a keg-like Cask, do our best to handle it # not being readable and return something that can be used. if want_keg_like_cask cask_version = Cask::Cask.new(name, config:).installed_version - cask = Cask::Cask.new(name, config:) do + Cask::Cask.new(name, config:) do version cask_version if cask_version end - return cask + else + # Need to rescue before `CaskUnavailableError` (superclass of this) + # The cask was found, but there's a problem with its implementation + unreadable_error ||= e + nil end - - # Need to rescue before `CaskUnavailableError` (superclass of this) - # The cask was found, but there's a problem with its implementation - unreadable_error ||= e rescue Cask::CaskUnavailableError => e raise e if only == :cask + + nil + end + + # Prioritise formulae unless it's a core tap cask (we already prioritised core tap formulae above) + if formula_or_keg && !cask&.tap&.core_cask_tap? + opoo package_conflicts_message(name, "formula", cask) if cask + return formula_or_keg + elsif cask + opoo package_conflicts_message(name, "cask", formula_or_keg) if formula_or_keg + return cask end end @@ -438,13 +456,29 @@ def resolve_default_keg(name) end end - def warn_if_cask_conflicts(ref, loaded_type) + def package_conflicts_message(ref, loaded_type, package) message = "Treating #{ref} as a #{loaded_type}." - begin - cask = Cask::CaskLoader.load(ref, warn: false) + case package + when Formula, Keg + message += " For the formula, " + if package.is_a?(Formula) && (tap = package.tap) + message += "use #{tap.name}/#{package.name} or " + end + message += "specify the `--formula` flag." + when Cask::Cask message += " For the cask, " - message += "use #{cask.tap.name}/#{cask.token} or " if cask.tap + if (tap = package.tap) + message += "use #{tap.name}/#{package.token} or " + end message += "specify the `--cask` flag." + end + message.freeze + end + + def warn_if_cask_conflicts(ref, loaded_type) + available = true + cask = begin + Cask::CaskLoader.load(ref, warn: false) rescue Cask::CaskUnreadableError => e # Need to rescue before `CaskUnavailableError` (superclass of this) # The cask was found, but there's a problem with its implementation @@ -452,11 +486,15 @@ def warn_if_cask_conflicts(ref, loaded_type) Failed to load cask: #{ref} #{e} EOS + nil rescue Cask::CaskUnavailableError # No ref conflict with a cask, do nothing - return + available = false + nil end - opoo message.freeze + return unless available + + opoo package_conflicts_message(ref, loaded_type, cask) end end end diff --git a/Library/Homebrew/test/cli/named_args_spec.rb b/Library/Homebrew/test/cli/named_args_spec.rb index 0d70647a3fb6a6..0da0436384eeb6 100644 --- a/Library/Homebrew/test/cli/named_args_spec.rb +++ b/Library/Homebrew/test/cli/named_args_spec.rb @@ -31,7 +31,7 @@ def setup_unredable_cask(name) end let(:baz) do - Cask::CaskLoader.load(+<<~RUBY) + Cask::CaskLoader::FromContentLoader.new(+<<~RUBY, tap: CoreCaskTap.instance).load(config: nil) cask "baz" do version "1.0" end @@ -39,7 +39,7 @@ def setup_unredable_cask(name) end let(:foo_cask) do - Cask::CaskLoader.load(+<<~RUBY) + Cask::CaskLoader::FromContentLoader.new(+<<~RUBY, tap: CoreCaskTap.instance).load(config: nil) cask "foo" do version "1.0" end @@ -90,6 +90,32 @@ def setup_unredable_cask(name) end end + context "when a non-core formula and a core cask are present" do + let(:non_core_formula) do + formula "foo", tap: Tap.fetch("some/tap") do + url "https://brew.sh" + version "1.0" + end + end + + before do + stub_formula_loader non_core_formula, "foo" + stub_cask_loader foo_cask + end + + it "returns the cask by default" do + expect(described_class.new("foo").to_formulae_and_casks).to eq [foo_cask] + end + + it "returns formula if loading formula only" do + expect(described_class.new("foo").to_formulae_and_casks(only: :formula)).to eq [non_core_formula] + end + + it "returns cask if loading cask only" do + expect(described_class.new("foo").to_formulae_and_casks(only: :cask)).to eq [foo_cask] + end + end + context "when both formula and cask are unreadable" do before do setup_unredable_formula "foo" diff --git a/Library/Homebrew/test/support/helper/formula.rb b/Library/Homebrew/test/support/helper/formula.rb index a3c042666ebf2a..fb24c36b26fd9f 100644 --- a/Library/Homebrew/test/support/helper/formula.rb +++ b/Library/Homebrew/test/support/helper/formula.rb @@ -5,8 +5,8 @@ module Test module Helper module Formula - def formula(name = "formula_name", path: Formulary.core_path(name), spec: :stable, alias_path: nil, tap: nil, - &block) + def formula(name = "formula_name", path: nil, spec: :stable, alias_path: nil, tap: nil, &block) + path ||= Formulary.find_formula_in_tap(name, tap || CoreTap.instance) Class.new(::Formula, &block).new(name, path, spec, alias_path:, tap:) end