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

Set correct tap when loading installed casks #17823

Merged
merged 3 commits into from
Sep 11, 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
43 changes: 41 additions & 2 deletions Library/Homebrew/cask/cask_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -503,13 +503,26 @@
.returns(T.nilable(T.attached_class))
}
def self.try_new(ref, warn: false)
return unless ref.is_a?(String)
token = if ref.is_a?(String)
ref
elsif ref.is_a?(Pathname)
ref.basename(ref.extname).to_s
end
return unless token

possible_installed_cask = Cask.new(ref)
possible_installed_cask = Cask.new(token)
return unless (installed_caskfile = possible_installed_cask.installed_caskfile)

new(installed_caskfile)
end

sig { params(path: T.any(Pathname, String), token: String).void }
def initialize(path, token: "")
super

Check warning on line 521 in Library/Homebrew/cask/cask_loader.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/cask_loader.rb#L521

Added line #L521 was not covered by tests

installed_tap = Cask.new(@token).tab.tap

Check warning on line 523 in Library/Homebrew/cask/cask_loader.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/cask_loader.rb#L523

Added line #L523 was not covered by tests
@tap = installed_tap if installed_tap
end
end

# Pseudo-loader which raises an error when trying to load the corresponding cask.
Expand Down Expand Up @@ -598,6 +611,32 @@
end
end

sig { params(ref: String, config: T.nilable(Config), warn: T::Boolean).returns(Cask) }
def self.load_prefer_installed(ref, config: nil, warn: true)
tap, token = Tap.with_cask_token(ref)
token ||= ref
tap ||= Cask.new(ref).tab.tap

if tap.nil?
self.load(token, config:, warn:)
else
begin
self.load("#{tap}/#{token}", config:, warn:)
rescue CaskUnavailableError
# cask may be migrated to different tap. Try to search in all taps.
self.load(token, config:, warn:)
end
end
end

sig { params(path: Pathname, config: T.nilable(Config), warn: T::Boolean).returns(Cask) }
def self.load_from_installed_caskfile(path, config: nil, warn: true)
loader = FromInstalledPathLoader.try_new(path, warn:)
loader ||= NullLoader.new(path)

Check warning on line 635 in Library/Homebrew/cask/cask_loader.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/cask_loader.rb#L634-L635

Added lines #L634 - L635 were not covered by tests

loader.load(config:)

Check warning on line 637 in Library/Homebrew/cask/cask_loader.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/cask_loader.rb#L637

Added line #L637 was not covered by tests
end
Rylan12 marked this conversation as resolved.
Show resolved Hide resolved

def self.default_path(token)
find_cask_in_tap(token.to_s.downcase, CoreCaskTap.instance)
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/caskroom.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def self.ensure_caskroom_exists
sig { params(config: T.nilable(Config)).returns(T::Array[Cask]) }
def self.casks(config: nil)
tokens.sort.filter_map do |token|
CaskLoader.load(token, config:, warn: false)
CaskLoader.load_prefer_installed(token, config:, warn: false)
rescue TapCaskAmbiguityError => e
T.must(e.loaders.first).load(config:)
rescue
Expand Down
23 changes: 22 additions & 1 deletion Library/Homebrew/cli/named_args.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,18 @@
if want_keg_like_cask &&
(installed_caskfile = candidate_cask.installed_caskfile) &&
installed_caskfile.exist?
Cask::CaskLoader.load(installed_caskfile)
cask = Cask::CaskLoader.load_from_installed_caskfile(installed_caskfile)

requested_tap, requested_token = Tap.with_cask_token(name)

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

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cli/named_args.rb#L194

Added line #L194 was not covered by tests
if requested_tap && requested_token
installed_cask_tap = cask.tab.tap

if installed_cask_tap && installed_cask_tap != requested_tap
raise Cask::TapCaskUnavailableError.new(requested_tap, requested_token)
end
end

cask

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

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cli/named_args.rb#L203

Added line #L203 was not covered by tests
else
candidate_cask
end
Expand Down Expand Up @@ -405,6 +416,16 @@
rack = Formulary.to_rack(name.downcase)

kegs = rack.directory? ? rack.subdirs.map { |d| Keg.new(d) } : []

requested_tap, requested_formula = Tap.with_formula_name(name)
if requested_tap && requested_formula
kegs = kegs.select do |keg|
keg.tab.tap == requested_tap
end

raise NoSuchKegFromTapError.new(requested_formula, requested_tap) if kegs.none?
end

raise NoSuchKegError, name if kegs.none?

[rack, kegs]
Expand Down
12 changes: 12 additions & 0 deletions Library/Homebrew/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,18 @@ def initialize(name)
end
end

# Raised when a keg from a specific tap doesn't exist.
class NoSuchKegFromTapError < RuntimeError
attr_reader :name, :tap

sig { params(name: String, tap: Tap).void }
def initialize(name, tap)
@name = name
@tap = tap
super "No such keg: #{HOMEBREW_CELLAR}/#{name} from tap #{tap}"
end
end

# Raised when an invalid attribute is used in a formula.
class FormulaValidationError < StandardError
attr_reader :attr, :formula
Expand Down
56 changes: 56 additions & 0 deletions Library/Homebrew/test/cask/cask_loader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,60 @@
end
end
end

describe "::load_prefer_installed" do
let(:foo_tap) { Tap.fetch("user", "foo") }
let(:bar_tap) { Tap.fetch("user", "bar") }

let(:blank_tab) { instance_double(Cask::Tab, tap: nil) }
let(:installed_tab) { instance_double(Cask::Tab, tap: bar_tap) }

let(:cask_with_foo_tap) { instance_double(Cask::Cask, token: "test-cask", tap: foo_tap) }
let(:cask_with_bar_tap) { instance_double(Cask::Cask, token: "test-cask", tap: bar_tap) }

let(:load_args) { { config: nil, warn: true } }

before do
allow(described_class).to receive(:load).with("test-cask", load_args).and_return(cask_with_foo_tap)
allow(described_class).to receive(:load).with("user/foo/test-cask", load_args).and_return(cask_with_foo_tap)
allow(described_class).to receive(:load).with("user/bar/test-cask", load_args).and_return(cask_with_bar_tap)
end

it "returns the correct cask when no tap is specified and no tab exists" do
allow_any_instance_of(Cask::Cask).to receive(:tab).and_return(blank_tab)
expect(described_class).to receive(:load).with("test-cask", load_args)

expect(described_class.load_prefer_installed("test-cask").tap).to eq(foo_tap)
end

it "returns the correct cask when no tap is specified but a tab exists" do
allow_any_instance_of(Cask::Cask).to receive(:tab).and_return(installed_tab)
expect(described_class).to receive(:load).with("user/bar/test-cask", load_args)

expect(described_class.load_prefer_installed("test-cask").tap).to eq(bar_tap)
end

it "returns the correct cask when a tap is specified and no tab exists" do
allow_any_instance_of(Cask::Cask).to receive(:tab).and_return(blank_tab)
expect(described_class).to receive(:load).with("user/bar/test-cask", load_args)

expect(described_class.load_prefer_installed("user/bar/test-cask").tap).to eq(bar_tap)
end

it "returns the correct cask when no tap is specified and a tab exists" do
allow_any_instance_of(Cask::Cask).to receive(:tab).and_return(installed_tab)
expect(described_class).to receive(:load).with("user/foo/test-cask", load_args)

expect(described_class.load_prefer_installed("user/foo/test-cask").tap).to eq(foo_tap)
end

it "returns the correct cask when no tap is specified and the tab lists an tap that isn't installed" do
allow_any_instance_of(Cask::Cask).to receive(:tab).and_return(installed_tab)
expect(described_class).to receive(:load).with("user/bar/test-cask", load_args)
.and_raise(Cask::CaskUnavailableError.new("test-cask", bar_tap))
expect(described_class).to receive(:load).with("test-cask", load_args)

expect(described_class.load_prefer_installed("test-cask").tap).to eq(foo_tap)
end
end
end
30 changes: 30 additions & 0 deletions Library/Homebrew/test/cli/named_args_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,36 @@ def setup_unredable_cask(name)
it "when there are no matching kegs returns an empty array" do
expect(described_class.new.to_kegs).to be_empty
end

it "raises an error when a Keg is unavailable" do
expect { described_class.new("baz").to_kegs }.to raise_error NoSuchKegError
end

context "when a keg specifies a tap" do
let(:tab) { instance_double(Tab, tap: Tap.fetch("user", "repo")) }

before do
allow_any_instance_of(Keg).to receive(:tab).and_return(tab)
end

it "returns kegs if no tap is specified" do
stub_formula_loader bar, "user/repo/bar"

expect(described_class.new("bar").to_kegs.map(&:name)).to eq ["bar"]
end

it "returns kegs if the tap is specified" do
stub_formula_loader bar, "user/repo/bar"

expect(described_class.new("user/repo/bar").to_kegs.map(&:name)).to eq ["bar"]
end

it "raises an error if there is no tap match" do
stub_formula_loader bar, "other/tap/bar"

expect { described_class.new("other/tap/bar").to_kegs }.to raise_error(NoSuchKegFromTapError)
end
end
end

describe "#to_default_kegs" do
Expand Down
8 changes: 8 additions & 0 deletions Library/Homebrew/test/exceptions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
end
end

describe NoSuchKegFromTapError do
subject(:error) { described_class.new("foo", tap) }

let(:tap) { instance_double(Tap, to_s: "u/r") }

it(:to_s) { expect(error.to_s).to eq("No such keg: #{HOMEBREW_CELLAR}/foo from tap u/r") }
end

describe NoSuchKegError do
subject(:error) { described_class.new("foo") }

Expand Down