diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 5637aadc0e..e8a8303163 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -84,8 +84,8 @@ module Cask # Loads a cask from a path. class FromPathLoader < AbstractContentLoader sig { - params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(T.attached_class)) + overridable.params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) + .returns(T.nilable(T.attached_class)) } def self.try_new(ref, warn: false) path = case ref @@ -152,8 +152,8 @@ module Cask # Loads a cask from a URI. class FromURILoader < FromPathLoader sig { - params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(T.attached_class)) + override.params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) + .returns(T.nilable(T.attached_class)) } def self.try_new(ref, warn: false) # Cache compiled regex @@ -199,16 +199,22 @@ module Cask attr_reader :tap sig { - params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(T.attached_class)) + override(allow_incompatible: true) # rubocop:todo Sorbet/AllowIncompatibleOverride + .params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) + .returns(T.nilable(T.any(T.attached_class, FromAPILoader))) } def self.try_new(ref, warn: false) ref = ref.to_s return unless (token_tap_type = CaskLoader.tap_cask_token_type(ref, warn:)) - token, tap, = token_tap_type - new("#{tap}/#{token}") + token, tap, type = token_tap_type + + if type == :migration && tap.core_cask_tap? && (loader = FromAPILoader.try_new(token)) + loader + else + new("#{tap}/#{token}") + end end sig { params(tapped_token: String).void } @@ -441,8 +447,8 @@ module Cask # if the same token exists in multiple taps. class FromNameLoader < FromTapLoader sig { - params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(T.attached_class)) + override.params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) + .returns(T.nilable(T.any(T.attached_class, FromAPILoader))) } def self.try_new(ref, warn: false) return unless ref.is_a?(String) @@ -452,7 +458,7 @@ module Cask # If it exists in the default tap, never treat it as ambiguous with another tap. if (core_cask_tap = CoreCaskTap.instance).installed? && - (loader= super("#{core_cask_tap}/#{token}", warn:))&.path&.exist? + (loader = super("#{core_cask_tap}/#{token}", warn:))&.path&.exist? return loader end @@ -473,8 +479,8 @@ module Cask # Loader which loads a cask from the installed cask file. class FromInstalledPathLoader < FromPathLoader sig { - params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(T.attached_class)) + override.params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) + .returns(T.nilable(T.attached_class)) } def self.try_new(ref, warn: false) return unless ref.is_a?(String) @@ -489,8 +495,8 @@ module Cask # Pseudo-loader which raises an error when trying to load the corresponding cask. class NullLoader < FromPathLoader sig { - params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(T.attached_class)) + override.params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) + .returns(T.nilable(T.attached_class)) } def self.try_new(ref, warn: false) return if ref.is_a?(Cask) diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index a5fae52065..8d86f13e72 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -759,7 +759,7 @@ module Formulary sig { params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) - .returns(T.nilable(T.attached_class)) + .returns(T.nilable(FormulaLoader)) } def self.try_new(ref, from: T.unsafe(nil), warn: false) ref = ref.to_s @@ -776,7 +776,11 @@ module Formulary {} end - new(name, path, tap:, **options) + if type == :migration && tap.core_tap? && (loader = FromAPILoader.try_new(name)) + loader + else + new(name, path, tap:, **options) + end end sig { params(name: String, path: Pathname, tap: Tap, alias_name: String).void } @@ -811,7 +815,7 @@ module Formulary class FromNameLoader < FromTapLoader sig { params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) - .returns(T.nilable(T.attached_class)) + .returns(T.nilable(FormulaLoader)) } def self.try_new(ref, from: T.unsafe(nil), warn: false) return unless ref.is_a?(String) diff --git a/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb b/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb index f6c60ff203..624c1f3cbf 100644 --- a/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb +++ b/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb @@ -14,12 +14,14 @@ RSpec.describe Cask::CaskLoader::FromAPILoader, :cask do before do allow(Homebrew::API::Cask) - .to receive(:all_casks) - .and_return(casks_from_api_hash) + .to receive_messages(all_casks: casks_from_api_hash, all_renames: {}) + + # The call to `Cask::CaskLoader.load` above sets the Tap cache prematurely. + Tap.clear_cache end end - describe ".can_load?" do + describe ".try_new" do include_context "with API setup", "test-opera" context "when not using the API", :no_api do @@ -34,16 +36,64 @@ RSpec.describe Cask::CaskLoader::FromAPILoader, :cask do end it "returns a loader for valid token" do - expect(described_class.try_new(token)).not_to be_nil + expect(described_class.try_new(token)) + .to be_a(described_class) + .and have_attributes(token:) end it "returns a loader for valid full name" do - expect(described_class.try_new("homebrew/cask/#{token}")).not_to be_nil + expect(described_class.try_new("homebrew/cask/#{token}")) + .to be_a(described_class) + .and have_attributes(token:) end it "returns nil for full name with invalid tap" do expect(described_class.try_new("homebrew/foo/#{token}")).to be_nil end + + context "with core tap migration renames" do + let(:foo_tap) { Tap.fetch("homebrew", "foo") } + + before do + foo_tap.path.mkpath + end + + after do + FileUtils.rm_rf foo_tap.path + end + + it "returns the core cask if the short name clashes with a tap migration rename" do + (foo_tap.path/"tap_migrations.json").write <<~JSON + { "#{token}": "homebrew/cask/#{token}-v2" } + JSON + + expect(Cask::CaskLoader::FromNameLoader.try_new(token)) + .to be_a(Cask::CaskLoader::FromNameLoader) + .and have_attributes(token:) + end + + it "returns the tap migration rename by old token" do + old_token = "#{token}-old" + (foo_tap.path/"tap_migrations.json").write <<~JSON + { "#{old_token}": "homebrew/cask/#{token}" } + JSON + + expect(Cask::CaskLoader::FromNameLoader.try_new(old_token)) + .to be_a(described_class) + .and have_attributes(token:) + end + + it "returns the tap migration rename by old full name" do + old_token = "#{token}-old" + (foo_tap.path/"tap_migrations.json").write <<~JSON + { "#{old_token}": "homebrew/cask/#{token}" } + JSON + + expect(Cask::CaskLoader::FromTapLoader.try_new("#{foo_tap}/#{old_token}")) + .to be_a(described_class) + .and have_attributes(token:) + end + end end end diff --git a/Library/Homebrew/test/formulary_spec.rb b/Library/Homebrew/test/formulary_spec.rb index e5293ffc7a..43fb416bd1 100644 --- a/Library/Homebrew/test/formulary_spec.rb +++ b/Library/Homebrew/test/formulary_spec.rb @@ -383,6 +383,7 @@ RSpec.describe Formulary do # avoid unnecessary network calls allow(Homebrew::API::Formula).to receive_messages(all_aliases: {}, all_renames: {}) allow(CoreTap.instance).to receive(:tap_migrations).and_return({}) + allow(CoreCaskTap.instance).to receive(:tap_migrations).and_return({}) # don't try to load/fetch gcc/glibc allow(DevelopmentTools).to receive_messages(needs_libc_formula?: false, needs_compiler_formula?: false) @@ -482,6 +483,51 @@ RSpec.describe Formulary do expect(formula.deps.count).to eq 5 expect(formula.deps.map(&:name).include?("uses_from_macos_dep")).to be true end + + context "with core tap migration renames" do + let(:foo_tap) { Tap.fetch("homebrew", "foo") } + + before do + allow(Homebrew::API::Formula).to receive(:all_formulae).and_return formula_json_contents + foo_tap.path.mkpath + end + + after do + FileUtils.rm_rf foo_tap.path + end + + it "returns the core formula if the short name clashes with a tap migration rename" do + (foo_tap.path/"tap_migrations.json").write <<~JSON + { "#{formula_name}": "homebrew/core/#{formula_name}-v2" } + JSON + + expect(described_class::FromNameLoader.try_new(formula_name)) + .to be_a(described_class::FromNameLoader) + .and have_attributes(name: formula_name) + end + + it "returns the tap migration rename by old formula_name" do + old_formula_name = "#{formula_name}-old" + (foo_tap.path/"tap_migrations.json").write <<~JSON + { "#{old_formula_name}": "homebrew/core/#{formula_name}" } + JSON + + expect(described_class::FromNameLoader.try_new(old_formula_name)) + .to be_a(described_class::FromAPILoader) + .and have_attributes(name: formula_name) + end + + it "returns the tap migration rename by old full name" do + old_formula_name = "#{formula_name}-old" + (foo_tap.path/"tap_migrations.json").write <<~JSON + { "#{old_formula_name}": "homebrew/core/#{formula_name}" } + JSON + + expect(described_class::FromTapLoader.try_new("#{foo_tap}/#{old_formula_name}")) + .to be_a(described_class::FromAPILoader) + .and have_attributes(name: formula_name) + end + end end end