From e16bde13675120ea1e4e90bcece4953f8c20ea21 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Sun, 23 Jun 2024 19:51:12 -0700 Subject: [PATCH 1/3] fix tap migration renames with API We don't load casks and formulae that used to exist in an external tap but have been moved and renamed to an API tap when using the API. Loading works correctly when the core formula or cask tap is tapped locally though. This PR adds some logic to map tap migration renames to the API and check that when loading formulae and casks. --- Library/Homebrew/cask/cask_loader.rb | 19 +++++++++++++++---- Library/Homebrew/formulary.rb | 23 +++++++++++++++++------ Library/Homebrew/tap.rb | 26 ++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 5637aadc0e5d5..c49d74b6121a1 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -268,11 +268,13 @@ class FromAPILoader def self.try_new(ref, warn: false) return if Homebrew::EnvConfig.no_install_from_api? return unless ref.is_a?(String) - return unless (token = ref[HOMEBREW_DEFAULT_TAP_CASK_REGEX, :token]) - if !Homebrew::API::Cask.all_casks.key?(token) && - !Homebrew::API::Cask.all_renames.key?(token) - return + + token = parse_token(ref) + token ||= begin + ref = CoreCaskTap.instance.tap_migration_renames[ref] + parse_token(ref) if ref end + return unless token ref = "#{CoreCaskTap.instance}/#{token}" @@ -280,6 +282,15 @@ def self.try_new(ref, warn: false) new("#{tap}/#{token}") end + sig { params(ref: String).returns(T.nilable(String)) } + def self.parse_token(ref) + return unless (token = ref[HOMEBREW_DEFAULT_TAP_CASK_REGEX, :token]) + return token if Homebrew::API::Cask.all_casks.key?(token) + + token if Homebrew::API::Cask.all_renames.key?(token) + end + private_class_method :parse_token + sig { params(token: String, from_json: Hash, path: T.nilable(Pathname)).void } def initialize(token, from_json: T.unsafe(nil), path: nil) @token = token.sub(%r{^homebrew/(?:homebrew-)?cask/}i, "") diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index a5fae52065cea..b09e472dd4a9c 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -917,12 +917,13 @@ class FromAPILoader < FormulaLoader def self.try_new(ref, from: T.unsafe(nil), warn: false) return if Homebrew::EnvConfig.no_install_from_api? return unless ref.is_a?(String) - return unless (name = ref[HOMEBREW_DEFAULT_TAP_FORMULA_REGEX, :name]) - if !Homebrew::API::Formula.all_formulae.key?(name) && - !Homebrew::API::Formula.all_aliases.key?(name) && - !Homebrew::API::Formula.all_renames.key?(name) - return + + name = parse_name(ref) + name ||= begin + ref = CoreTap.instance.tap_migration_renames[ref] + parse_name(ref) if ref end + return unless name alias_name = name @@ -932,7 +933,7 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) name, tap, type = name_tap_type - options = if type == :alias + options = if type == :alias { alias_name: alias_name.downcase } else {} @@ -941,6 +942,16 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) new(name, tap:, **options) end + sig { params(ref: String).returns(T.nilable(String)) } + def self.parse_name(ref) + return unless (name = ref[HOMEBREW_DEFAULT_TAP_FORMULA_REGEX, :name]) + return name if Homebrew::API::Formula.all_formulae.key?(name) + return name if Homebrew::API::Formula.all_aliases.key?(name) + + name if Homebrew::API::Formula.all_renames.key?(name) + end + private_class_method :parse_name + sig { params(name: String, tap: Tap, alias_name: String).void } def initialize(name, tap: T.unsafe(nil), alias_name: T.unsafe(nil)) options = { diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index d04fdec0c7b9c..b6127a305671e 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -1284,6 +1284,19 @@ def tap_migrations end end + # External formula names to core formula renames + sig { returns(T::Hash[String, String]) } + def tap_migration_renames + @tap_migration_renames ||= Tap.each_with_object({}) do |tap, hash| + tap.tap_migrations.each do |old_name, new_name| + next unless new_name.start_with?("homebrew/core/") + + hash[old_name] = new_name + hash["#{tap}/#{old_name}"] = new_name + end + end + end + sig { returns(T::Array[String]) } def autobump @autobump ||= begin @@ -1456,6 +1469,19 @@ def tap_migrations end end + # External cask names to core cask renames + sig { returns(T::Hash[String, String]) } + def tap_migration_renames + @tap_migration_renames ||= Tap.each_with_object({}) do |tap, hash| + tap.tap_migrations.each do |old_name, new_name| + next unless new_name.start_with?("homebrew/cask/") + + hash[old_name] = new_name + hash["#{tap}/#{old_name}"] = new_name + end + end + end + sig { returns(T::Hash[String, T.untyped]) } def to_internal_api_hash casks_api_hash = cask_tokens.to_h do |token| From 18d237310aff0661010dbdb45b56c33cdc5c6e68 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Thu, 27 Jun 2024 20:51:10 -0700 Subject: [PATCH 2/3] Add tests Check for the following: - Tap migration rename to core tap can be loaded by short name - Tap migration rename to core tap can be loaded by long name - Tap migration renam that clashes with existing core tap short name is ignored in favor of loading the cask/formula from the core tap --- Library/Homebrew/tap.rb | 1 + .../cask/cask_loader/from_api_loader_spec.rb | 60 +++++++++++++++++-- Library/Homebrew/test/formulary_spec.rb | 46 ++++++++++++++ Library/Homebrew/test/spec_helper.rb | 2 +- 4 files changed, 103 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index b6127a305671e..c96a62702b477 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -236,6 +236,7 @@ def clear_cache @command_files = nil @tap_migrations = nil + @tap_migration_renames = nil @reverse_tap_migrations_renames = nil @audit_exceptions = nil 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 f6c60ff2038e3..115af7ffce111 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 @@ 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 @@ 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(described_class.try_new(token)) + .to be_a(described_class) + .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(described_class.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(described_class.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 e5293ffc7a42c..4f88d3299e646 100644 --- a/Library/Homebrew/test/formulary_spec.rb +++ b/Library/Homebrew/test/formulary_spec.rb @@ -383,6 +383,7 @@ def formula_json_contents(extra_items = {}) # 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 @@ def formula_json_contents(extra_items = {}) 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::FromAPILoader.try_new(formula_name)) + .to be_a(described_class::FromAPILoader) + .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::FromAPILoader.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::FromAPILoader.try_new("#{foo_tap}/#{old_formula_name}")) + .to be_a(described_class::FromAPILoader) + .and have_attributes(name: formula_name) + end + end end end diff --git a/Library/Homebrew/test/spec_helper.rb b/Library/Homebrew/test/spec_helper.rb index 4287f45a1c340..431f7116e2d96 100644 --- a/Library/Homebrew/test/spec_helper.rb +++ b/Library/Homebrew/test/spec_helper.rb @@ -216,7 +216,7 @@ config.around do |example| Homebrew.raise_deprecation_exceptions = true - Tap.installed.each(&:clear_cache) + Tap.all.each(&:clear_cache) Cachable::Registry.clear_all_caches FormulaInstaller.clear_attempted FormulaInstaller.clear_installed From 0c81a31a2325181cee412852cfbe5a2001fb304f Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Sat, 29 Jun 2024 15:08:10 -0700 Subject: [PATCH 3/3] show migrated from when loading packages from API This changes slightly how we load packaged migrated to core taps when they are loaded using the API. Now it should show the warning that the package has been migrated and renamed. --- Library/Homebrew/cask/cask_loader.rb | 11 +++++------ Library/Homebrew/formulary.rb | 16 ++++++++-------- Library/Homebrew/tap.rb | 24 +++++++++++++----------- Library/Homebrew/test/tap_spec.rb | 2 +- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index c49d74b6121a1..0020ce29e3001 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -269,14 +269,13 @@ def self.try_new(ref, warn: false) return if Homebrew::EnvConfig.no_install_from_api? return unless ref.is_a?(String) - token = parse_token(ref) - token ||= begin - ref = CoreCaskTap.instance.tap_migration_renames[ref] - parse_token(ref) if ref + ref = if (token = parse_token(ref)) + "#{CoreCaskTap.instance}/#{token}" + else + CoreCaskTap.instance.tap_migration_oldnames_to_full_names[ref] end - return unless token - ref = "#{CoreCaskTap.instance}/#{token}" + return unless ref token, tap, = CaskLoader.tap_cask_token_type(ref, warn:) new("#{tap}/#{token}") diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index b09e472dd4a9c..9e59d749a44ef 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -918,23 +918,23 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) return if Homebrew::EnvConfig.no_install_from_api? return unless ref.is_a?(String) - name = parse_name(ref) - name ||= begin - ref = CoreTap.instance.tap_migration_renames[ref] - parse_name(ref) if ref + ref = if (name = parse_name(ref)) + "#{CoreTap.instance}/#{name}" + else + name = ref.split("/").last + CoreTap.instance.tap_migration_oldnames_to_full_names[ref] end - return unless name - alias_name = name + return unless ref - ref = "#{CoreTap.instance}/#{name}" + alias_name = name return unless (name_tap_type = Formulary.tap_formula_name_type(ref, warn:)) name, tap, type = name_tap_type options = if type == :alias - { alias_name: alias_name.downcase } + { alias_name: T.must(alias_name).downcase } else {} end diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index c96a62702b477..cf9d567726dc6 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -236,7 +236,7 @@ def clear_cache @command_files = nil @tap_migrations = nil - @tap_migration_renames = nil + @tap_migration_oldnames_to_full_names = nil @reverse_tap_migrations_renames = nil @audit_exceptions = nil @@ -1285,15 +1285,16 @@ def tap_migrations end end - # External formula names to core formula renames + # Old formula names (that were migrated to homebrew/core) to old full names. sig { returns(T::Hash[String, String]) } - def tap_migration_renames - @tap_migration_renames ||= Tap.each_with_object({}) do |tap, hash| + def tap_migration_oldnames_to_full_names + @tap_migration_oldnames_to_full_names ||= Tap.each_with_object({}) do |tap, hash| tap.tap_migrations.each do |old_name, new_name| next unless new_name.start_with?("homebrew/core/") - hash[old_name] = new_name - hash["#{tap}/#{old_name}"] = new_name + full_name = "#{tap}/#{old_name}" + hash[old_name] = full_name + hash[full_name] = full_name end end end @@ -1470,15 +1471,16 @@ def tap_migrations end end - # External cask names to core cask renames + # Old cask names (that were migrated to homebrew/cask) to old full names. sig { returns(T::Hash[String, String]) } - def tap_migration_renames - @tap_migration_renames ||= Tap.each_with_object({}) do |tap, hash| + def tap_migration_oldnames_to_full_names + @tap_migration_oldnames_to_full_names ||= Tap.each_with_object({}) do |tap, hash| tap.tap_migrations.each do |old_name, new_name| next unless new_name.start_with?("homebrew/cask/") - hash[old_name] = new_name - hash["#{tap}/#{old_name}"] = new_name + full_name = "#{tap}/#{old_name}" + hash[old_name] = full_name + hash[full_name] = full_name end end end diff --git a/Library/Homebrew/test/tap_spec.rb b/Library/Homebrew/test/tap_spec.rb index 2f85fe93728de..259412fb53867 100644 --- a/Library/Homebrew/test/tap_spec.rb +++ b/Library/Homebrew/test/tap_spec.rb @@ -567,7 +567,7 @@ def setup_completion(link:) JSON end - describe "#reverse_tap_migration_renames" do + describe "#reverse_tap_migrations_renames" do it "returns the expected hash" do expect(homebrew_foo_tap.reverse_tap_migrations_renames).to eq({ "homebrew/cask/google-cloud-sdk" => %w[app-engine-go-32 app-engine-go-64],