Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli/named_args: raise priority of core casks #17681

Merged
merged 1 commit into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 68 additions & 24 deletions Library/Homebrew/cli/named_args.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@
Homebrew.with_no_api_env_if_needed(@without_api) do
unreadable_error = nil

if only != :cask
formula_or_kegs = 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)
Expand All @@ -156,27 +156,32 @@
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_kegs if formula_or_kegs
elsif formula_or_kegs && (!formula_or_kegs.is_a?(Formula) || formula_or_kegs.tap&.core_tap?)
warn_if_cask_conflicts(name, "formula")
return formula_or_kegs
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
Expand All @@ -188,27 +193,46 @@

# 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

Check warning on line 208 in Library/Homebrew/cli/named_args.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cli/named_args.rb#L208

Added line #L208 was not covered by tests
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_kegs && !cask&.tap&.core_cask_tap?
if cask || unreadable_error
onoe <<~EOS if unreadable_error
Failed to load cask: #{name}
#{unreadable_error}
EOS
opoo package_conflicts_message(name, "formula", cask)
end
return formula_or_kegs
elsif cask
opoo package_conflicts_message(name, "cask", formula_or_kegs) if formula_or_kegs
return cask
end
end

Expand Down Expand Up @@ -438,25 +462,45 @@
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, Array
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
onoe <<~EOS
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
Expand Down
30 changes: 28 additions & 2 deletions Library/Homebrew/test/cli/named_args_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ 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
RUBY
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
Expand Down Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/test/support/helper/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down