diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 66d4e572ae..cb03d6a6b2 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -464,12 +464,19 @@ module Cask } def self.try_new(ref, warn: false) return unless ref.is_a?(String) - return if ref.include?("/") + return unless ref.match?(/\A#{HOMEBREW_TAP_CASK_TOKEN_REGEX}\Z/o) token = ref - loaders = Tap.filter_map { |tap| super("#{tap}/#{token}", warn: warn) } - .select { _1.path.exist? } + # 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: warn))&.path&.exist? + return loader + end + + loaders = Tap.select { |tap| tap.installed? && !tap.core_cask_tap? } + .filter_map { |tap| super("#{tap}/#{token}", warn: warn) } + .select { |tap| tap.path.exist? } case loaders.count when 1 @@ -570,7 +577,6 @@ module Cask FromURILoader, FromAPILoader, FromTapLoader, - FromDefaultNameLoader, FromNameLoader, FromPathLoader, FromInstalledPathLoader, diff --git a/Library/Homebrew/diagnostic.rb b/Library/Homebrew/diagnostic.rb index 244c935383..7ad243f70b 100644 --- a/Library/Homebrew/diagnostic.rb +++ b/Library/Homebrew/diagnostic.rb @@ -838,7 +838,6 @@ module Homebrew loadable = [ Formulary::FromAPILoader, - Formulary::FromDefaultNameLoader, Formulary::FromNameLoader, ].any? do |loader_class| loader = begin diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index a0d45bd333..4c78c8850b 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -765,22 +765,6 @@ module Formulary end end - class FromDefaultNameLoader < FromTapLoader - sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) - .returns(T.nilable(T.attached_class)) - } - def self.try_new(ref, from: T.unsafe(nil), warn: false) - return unless ref.is_a?(String) - return unless (name = ref[HOMEBREW_DEFAULT_TAP_FORMULA_REGEX, :name]) - return unless (tap = CoreTap.instance).installed? - - return unless (loader = super("#{tap}/#{name}", warn: warn)) - - loader if loader.path.exist? - end - end - # Loads a formula from a name, as long as it exists only in a single tap. class FromNameLoader < FromTapLoader sig { @@ -789,11 +773,19 @@ module Formulary } def self.try_new(ref, from: T.unsafe(nil), warn: false) return unless ref.is_a?(String) - return if ref.include?("/") + return unless ref.match?(/\A#{HOMEBREW_TAP_FORMULA_NAME_REGEX}\Z/o) name = ref - loaders = Tap.filter_map { |tap| super("#{tap}/#{name}", warn: warn) }.select { _1.path.exist? } + # If it exists in the default tap, never treat it as ambiguous with another tap. + if (core_tap = CoreTap.instance).installed? && + (loader = super("#{core_tap}/#{name}", warn: warn))&.path&.exist? + return loader + end + + loaders = Tap.select { |tap| tap.installed? && !tap.core_tap? } + .filter_map { |tap| super("#{tap}/#{name}", warn: warn) } + .select { |tap| tap.path.exist? } case loaders.count when 1 @@ -1182,7 +1174,6 @@ module Formulary FromAPILoader, FromTapLoader, FromPathLoader, - FromDefaultNameLoader, FromNameLoader, FromKegLoader, FromCacheLoader, diff --git a/Library/Homebrew/test/.rubocop.yml b/Library/Homebrew/test/.rubocop.yml index 85cbaa2444..47f0ac2690 100644 --- a/Library/Homebrew/test/.rubocop.yml +++ b/Library/Homebrew/test/.rubocop.yml @@ -14,3 +14,4 @@ RSpec/ContextWording: - unless - for - which + - to diff --git a/Library/Homebrew/test/cask/cask_loader_spec.rb b/Library/Homebrew/test/cask/cask_loader_spec.rb index 52823dd643..e7fcc9696e 100644 --- a/Library/Homebrew/test/cask/cask_loader_spec.rb +++ b/Library/Homebrew/test/cask/cask_loader_spec.rb @@ -72,52 +72,88 @@ RSpec.describe Cask::CaskLoader, :cask do ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" end - context "when a cask is migrated to the default tap" do + context "when a cask is migrated" do let(:token) { "local-caffeine" } + + let(:core_tap) { CoreTap.instance } + let(:core_cask_tap) { CoreCaskTap.instance } + let(:tap_migrations) do { - token => default_tap.name, + token => new_tap.name, } end - let(:old_tap) { CoreTap.instance } - let(:default_tap) { CoreCaskTap.instance } before do + old_tap.path.mkpath + new_tap.path.mkpath (old_tap.path/"tap_migrations.json").write tap_migrations.to_json end - it "does not warn when loading the short token" do - expect do - described_class.for(token) - end.not_to output.to_stderr + context "to a formula in the default tap" do + let(:old_tap) { core_cask_tap } + let(:new_tap) { core_tap } + + let(:formula_file) { new_tap.formula_dir/"#{token}.rb" } + + before do + new_tap.formula_dir.mkpath + FileUtils.touch formula_file + end + + it "warn only once" do + expect do + described_class.for(token) + end.to output( + a_string_including("Warning: Cask #{token} was renamed to #{new_tap}/#{token}.").once, + ).to_stderr + end end - it "does not warn when loading the full token in the default tap" do - expect do - described_class.for("#{default_tap}/#{token}") - end.not_to output.to_stderr - end + context "to the default tap" do + let(:old_tap) { core_tap } + let(:new_tap) { core_cask_tap } - it "warns when loading the full token in the old tap" do - expect do - described_class.for("#{old_tap}/#{token}") - end.to output(%r{Cask #{old_tap}/#{token} was renamed to #{token}\.}).to_stderr - end + let(:cask_file) { new_tap.cask_dir/"#{token}.rb" } - # FIXME - # context "when there is an infinite tap migration loop" do - # before do - # (default_tap.path/"tap_migrations.json").write({ - # token => old_tap.name, - # }.to_json) - # end - # - # it "stops recursing" do - # expect do - # described_class.for("#{default_tap}/#{token}") - # end.not_to output.to_stderr - # end - # end + before do + new_tap.cask_dir.mkpath + FileUtils.touch cask_file + end + + it "does not warn when loading the short token" do + expect do + described_class.for(token) + end.not_to output.to_stderr + end + + it "does not warn when loading the full token in the default tap" do + expect do + described_class.for("#{new_tap}/#{token}") + end.not_to output.to_stderr + end + + it "warns when loading the full token in the old tap" do + expect do + described_class.for("#{old_tap}/#{token}") + end.to output(%r{Cask #{old_tap}/#{token} was renamed to #{token}\.}).to_stderr + end + + # FIXME + # context "when there is an infinite tap migration loop" do + # before do + # (new_tap.path/"tap_migrations.json").write({ + # token => old_tap.name, + # }.to_json) + # end + # + # it "stops recursing" do + # expect do + # described_class.for("#{new_tap}/#{token}") + # end.not_to output.to_stderr + # end + # end + end end end end diff --git a/Library/Homebrew/test/formulary_spec.rb b/Library/Homebrew/test/formulary_spec.rb index bf43eedf6f..86631a3a66 100644 --- a/Library/Homebrew/test/formulary_spec.rb +++ b/Library/Homebrew/test/formulary_spec.rb @@ -556,54 +556,90 @@ RSpec.describe Formulary do ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" end - context "when a formula is migrated to the default tap" do + context "when a formula is migrated" do let(:token) { "foo" } + + let(:core_tap) { CoreTap.instance } + let(:core_cask_tap) { CoreCaskTap.instance } + let(:tap_migrations) do { - token => default_tap.name, + token => new_tap.name, } end - let(:old_tap) { CoreCaskTap.instance } - let(:default_tap) { CoreTap.instance } before do old_tap.path.mkpath + new_tap.path.mkpath (old_tap.path/"tap_migrations.json").write tap_migrations.to_json - FileUtils.touch default_tap.formula_dir/"foo.rb" end - it "does not warn when loading the short token" do - expect do - described_class.loader_for(token) - end.not_to output.to_stderr + context "to a cask in the default tap" do + let(:old_tap) { core_tap } + let(:new_tap) { core_cask_tap } + + let(:cask_file) { new_tap.cask_dir/"#{token}.rb" } + + before do + new_tap.cask_dir.mkpath + FileUtils.touch cask_file + end + + it "warn only once" do + expect do + described_class.loader_for(token) + end.to output( + a_string_including("Warning: Formula #{token} was renamed to #{new_tap}/#{token}.").once, + ).to_stderr + end end - it "does not warn when loading the full token in the default tap" do - expect do - described_class.loader_for("#{default_tap}/#{token}") - end.not_to output.to_stderr - end + context "to the default tap" do + let(:old_tap) { core_cask_tap } + let(:new_tap) { core_tap } - it "warns when loading the full token in the old tap" do - expect do - described_class.loader_for("#{old_tap}/#{token}") - end.to output(%r{Formula #{old_tap}/#{token} was renamed to #{token}\.}).to_stderr - end + let(:formula_file) { new_tap.formula_dir/"#{token}.rb" } - # FIXME - # context "when there is an infinite tap migration loop" do - # before do - # (default_tap.path/"tap_migrations.json").write({ - # token => old_tap.name, - # }.to_json) - # end - # - # it "stops recursing" do - # expect do - # described_class.loader_for("#{default_tap}/#{token}") - # end.not_to output.to_stderr - # end - # end + before do + new_tap.formula_dir.mkpath + FileUtils.touch formula_file + end + + it "does not warn when loading the short token" do + expect do + described_class.loader_for(token) + end.not_to output.to_stderr + end + + it "does not warn when loading the full token in the default tap" do + expect do + described_class.loader_for("#{new_tap}/#{token}") + end.not_to output.to_stderr + end + + it "warns when loading the full token in the old tap" do + expect do + described_class.loader_for("#{old_tap}/#{token}") + end.to output( + a_string_including("Formula #{old_tap}/#{token} was renamed to #{token}.").once, + ).to_stderr + end + + # FIXME + # context "when there is an infinite tap migration loop" do + # before do + # (new_tap.path/"tap_migrations.json").write({ + # token => old_tap.name, + # }.to_json) + # end + # + # it "stops recursing" do + # expect do + # described_class.loader_for("#{new_tap}/#{token}") + # end.not_to output.to_stderr + # end + # end + end end end end