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

fix tap migration renames with API #17555

Closed
wants to merge 3 commits into from
Closed
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
20 changes: 15 additions & 5 deletions Library/Homebrew/cask/cask_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,18 +268,28 @@ 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

ref = if (token = parse_token(ref))
"#{CoreCaskTap.instance}/#{token}"
else
CoreCaskTap.instance.tap_migration_oldnames_to_full_names[ref]
end

ref = "#{CoreCaskTap.instance}/#{token}"
return unless ref

token, tap, = CaskLoader.tap_cask_token_type(ref, warn:)
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, "")
Expand Down
29 changes: 20 additions & 9 deletions Library/Homebrew/formulary.rb
Original file line number Diff line number Diff line change
Expand Up @@ -917,30 +917,41 @@ 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

ref = if (name = parse_name(ref))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on naming this something like:

Suggested change
ref = if (name = parse_name(ref))
ref = if (name = parse_homebrew_core_name(ref))

Or something like that? That way it's clear that the "parsing" that's happening is only within the homebrew/core tap?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe parse_core_name here and parse_core_token in Cask::CaskLoader?

"#{CoreTap.instance}/#{name}"
else
name = ref.split("/").last
CoreTap.instance.tap_migration_oldnames_to_full_names[ref]
end

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 }
options = if type == :alias
{ alias_name: T.must(alias_name).downcase }
else
{}
end

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 = {
Expand Down
29 changes: 29 additions & 0 deletions Library/Homebrew/tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ def clear_cache
@command_files = nil

@tap_migrations = nil
@tap_migration_oldnames_to_full_names = nil
@reverse_tap_migrations_renames = nil

@audit_exceptions = nil
Expand Down Expand Up @@ -1284,6 +1285,20 @@ def tap_migrations
end
end

# Old formula names (that were migrated to homebrew/core) to old full names.
sig { returns(T::Hash[String, String]) }
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/")

full_name = "#{tap}/#{old_name}"
hash[old_name] = full_name
hash[full_name] = full_name
end
end
end

sig { returns(T::Array[String]) }
def autobump
@autobump ||= begin
Expand Down Expand Up @@ -1456,6 +1471,20 @@ def tap_migrations
end
end

# Old cask names (that were migrated to homebrew/cask) to old full names.
sig { returns(T::Hash[String, String]) }
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/")

full_name = "#{tap}/#{old_name}"
hash[old_name] = full_name
hash[full_name] = full_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|
Expand Down
60 changes: 55 additions & 5 deletions Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
46 changes: 46 additions & 0 deletions Library/Homebrew/test/formulary_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/tap_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down