admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

Load tap migration renames from API with short names

Browse Source 
This is a follow-up to 484498e. I added loading for tap migration
renames from the API but it apparently only worked for full names.

There was a bug in both the code and the tests which prevented
loading by short names. This fixes those bugs so everything should
be good now.
This commit is contained in:
apainintheneck 2024-07-01 18:41:53 -07:00
parent 3baf9f2054
commit 16e605e056
4 changed files with 245 additions and 259 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
Library/Homebrew/cask/cask_loader.rb
View File

@ -458,14 +458,14 @@ 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?
return loader
(core_cask_loader = super("#{core_cask_tap}/#{token}", warn:))&.path&.exist?
return core_cask_loader
end
loaders = Tap.select { |tap| tap.installed? && !tap.core_cask_tap? }
.filter_map { |tap| super("#{tap}/#{token}", warn:) }
.uniq(&:path)
.select { |tap| tap.path.exist? }
.select { |loader| loader.is_a?(FromAPILoader) || loader.path.exist? }
case loaders.count
when 1

6
Library/Homebrew/formulary.rb
View File

@ -825,14 +825,14 @@ module Formulary
# 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:))&.path&.exist?
return loader
(core_loader = super("#{core_tap}/#{name}", warn:))&.path&.exist?
return core_loader
end
loaders = Tap.select { |tap| tap.installed? && !tap.core_tap? }
.filter_map { |tap| super("#{tap}/#{name}", warn:) }
.uniq(&:path)
.select { |tap| tap.path.exist? }
.select { |loader| loader.is_a?(FromAPILoader) || loader.path.exist? }
case loaders.count
when 1

82
Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb
View File

@ -1,16 +1,16 @@
# frozen_string_literal: true
RSpec.describe Cask::CaskLoader::FromAPILoader, :cask do
shared_context "with API setup" do |new_token|
let(:token) { new_token }
let(:cask_from_source) { Cask::CaskLoader.load(token) }
shared_context "with API setup" do |local_token|
let(:api_token) { "#{local_token}-api" }
let(:cask_from_source) { Cask::CaskLoader.load(local_token) }
let(:cask_json) do
hash = cask_from_source.to_hash_with_variations
json = JSON.pretty_generate(hash)
JSON.parse(json)
end
let(:casks_from_api_hash) { { cask_json["token"] => cask_json.except("token") } }
let(:api_loader) { described_class.new(token, from_json: cask_json) }
let(:casks_from_api_hash) { { api_token => cask_json.except("token") } }
let(:api_loader) { described_class.new(api_token, from_json: cask_json) }
before do
allow(Homebrew::API::Cask)
@ -26,7 +26,7 @@ RSpec.describe Cask::CaskLoader::FromAPILoader, :cask do
context "when not using the API", :no_api do
it "returns false" do
expect(described_class.try_new(token)).to be_nil
expect(described_class.try_new(api_token)).to be_nil
end
end
@ -36,19 +36,19 @@ RSpec.describe Cask::CaskLoader::FromAPILoader, :cask do
end
it "returns a loader for valid token" do
expect(described_class.try_new(token))
expect(described_class.try_new(api_token))
.to be_a(described_class)
.and have_attributes(token:)
.and have_attributes(token: api_token)
end
it "returns a loader for valid full name" do
expect(described_class.try_new("homebrew/cask/#{token}"))
expect(described_class.try_new("homebrew/cask/#{api_token}"))
.to be_a(described_class)
.and have_attributes(token:)
.and have_attributes(token: api_token)
end
it "returns nil for full name with invalid tap" do
expect(described_class.try_new("homebrew/foo/#{token}")).to be_nil
expect(described_class.try_new("homebrew/foo/#{api_token}")).to be_nil
end
context "with core tap migration renames" do
@ -62,100 +62,92 @@ RSpec.describe Cask::CaskLoader::FromAPILoader, :cask 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"
old_token = "#{api_token}-old"
(foo_tap.path/"tap_migrations.json").write <<~JSON
{ "#{old_token}": "homebrew/cask/#{token}" }
{ "#{old_token}": "homebrew/cask/#{api_token}" }
JSON
expect(Cask::CaskLoader::FromNameLoader.try_new(old_token))
.to be_a(described_class)
.and have_attributes(token:)
loader = Cask::CaskLoader::FromNameLoader.try_new(old_token)
expect(loader).to be_a(described_class)
expect(loader.token).to eq api_token
expect(loader.path).not_to exist
end
it "returns the tap migration rename by old full name" do
old_token = "#{token}-old"
old_token = "#{api_token}-old"
(foo_tap.path/"tap_migrations.json").write <<~JSON
{ "#{old_token}": "homebrew/cask/#{token}" }
{ "#{old_token}": "homebrew/cask/#{api_token}" }
JSON
expect(Cask::CaskLoader::FromTapLoader.try_new("#{foo_tap}/#{old_token}"))
.to be_a(described_class)
.and have_attributes(token:)
loader = Cask::CaskLoader::FromTapLoader.try_new("#{foo_tap}/#{old_token}")
expect(loader).to be_a(described_class)
expect(loader.token).to eq api_token
expect(loader.path).not_to exist
end
end
end
end
describe "#load" do
shared_examples "loads from API" do |cask_token, caskfile_only|
shared_examples "loads from API" do |cask_token, caskfile_only:|
include_context "with API setup", cask_token
let(:cask_from_api) { api_loader.load(config: nil) }
it "loads from JSON API" do
expect(cask_from_api).to be_a(Cask::Cask)
expect(cask_from_api.token).to eq(cask_token)
expect(cask_from_api.token).to eq(api_token)
expect(cask_from_api.loaded_from_api?).to be(true)
expect(cask_from_api.caskfile_only?).to be(caskfile_only)
end
end
context "with a binary stanza" do
include_examples "loads from API", "with-binary", false
include_examples "loads from API", "with-binary", caskfile_only: false
end
context "with cask dependencies" do
include_examples "loads from API", "with-depends-on-cask-multiple", false
include_examples "loads from API", "with-depends-on-cask-multiple", caskfile_only: false
end
context "with formula dependencies" do
include_examples "loads from API", "with-depends-on-formula-multiple", false
include_examples "loads from API", "with-depends-on-formula-multiple", caskfile_only: false
end
context "with macos dependencies" do
include_examples "loads from API", "with-depends-on-macos-array", false
include_examples "loads from API", "with-depends-on-macos-array", caskfile_only: false
end
context "with an installer stanza" do
include_examples "loads from API", "with-installer-script", false
include_examples "loads from API", "with-installer-script", caskfile_only: false
end
context "with uninstall stanzas" do
include_examples "loads from API", "with-uninstall-multi", false
include_examples "loads from API", "with-uninstall-multi", caskfile_only: false
end
context "with a zap stanza" do
include_examples "loads from API", "with-zap", false
include_examples "loads from API", "with-zap", caskfile_only: false
end
context "with a preflight stanza" do
include_examples "loads from API", "with-preflight", true
include_examples "loads from API", "with-preflight", caskfile_only: true
end
context "with an uninstall-preflight stanza" do
include_examples "loads from API", "with-uninstall-preflight", true
include_examples "loads from API", "with-uninstall-preflight", caskfile_only: true
end
context "with a postflight stanza" do
include_examples "loads from API", "with-postflight", true
include_examples "loads from API", "with-postflight", caskfile_only: true
end
context "with an uninstall-postflight stanza" do
include_examples "loads from API", "with-uninstall-postflight", true
include_examples "loads from API", "with-uninstall-postflight", caskfile_only: true
end
context "with a language stanza" do
include_examples "loads from API", "with-languages", true
include_examples "loads from API", "with-languages", caskfile_only: true
end
end
end

410
Library/Homebrew/test/formulary_spec.rb
View File

@ -55,37 +55,10 @@ RSpec.describe Formulary do
end
describe "::factory" do
before do
formula_path.dirname.mkpath
formula_path.write formula_content
end
it "returns a Formula" do
expect(described_class.factory(formula_name)).to be_a(Formula)
end
it "returns a Formula when given a fully qualified name" do
expect(described_class.factory("homebrew/core/#{formula_name}")).to be_a(Formula)
end
it "raises an error if the Formula cannot be found" do
expect do
described_class.factory("not_existed_formula")
end.to raise_error(FormulaUnavailableError)
end
it "raises an error if ref is nil" do
expect do
described_class.factory(nil)
end.to raise_error(TypeError)
end
context "with sharded Formula directory" do
let(:formula_name) { "testball_sharded" }
let(:formula_path) do
core_tap = CoreTap.instance
(core_tap.formula_dir/formula_name[0]).mkpath
core_tap.new_formula_path(formula_name)
context "without the API" do
before do
formula_path.dirname.mkpath
formula_path.write formula_content
end
it "returns a Formula" do
@ -95,180 +68,209 @@ RSpec.describe Formulary do
it "returns a Formula when given a fully qualified name" do
expect(described_class.factory("homebrew/core/#{formula_name}")).to be_a(Formula)
end
end
context "when the Formula has the wrong class" do
let(:formula_name) { "giraffe" }
let(:formula_content) do
<<~RUBY
class Wrong#{described_class.class_s(formula_name)} < Formula
end
RUBY
end
it "raises an error" do
it "raises an error if the Formula cannot be found" do
expect do
described_class.factory(formula_name)
end.to raise_error(TapFormulaClassUnavailableError)
end
end
it "returns a Formula when given a path" do
expect(described_class.factory(formula_path)).to be_a(Formula)
end
it "returns a Formula when given a URL", :needs_utils_curl, :no_api do
formula = described_class.factory("file://#{formula_path}")
expect(formula).to be_a(Formula)
end
context "when given a bottle" do
subject(:formula) { described_class.factory(bottle) }
it "returns a Formula" do
expect(formula).to be_a(Formula)
described_class.factory("not_existed_formula")
end.to raise_error(FormulaUnavailableError)
end
it "calling #local_bottle_path on the returned Formula returns the bottle path" do
expect(formula.local_bottle_path).to eq(bottle.realpath)
end
end
context "when given an alias" do
subject(:formula) { described_class.factory("foo") }
let(:alias_dir) { CoreTap.instance.alias_dir }
let(:alias_path) { alias_dir/"foo" }
before do
alias_dir.mkpath
FileUtils.ln_s formula_path, alias_path
end
it "returns a Formula" do
expect(formula).to be_a(Formula)
end
it "calling #alias_path on the returned Formula returns the alias path" do
expect(formula.alias_path).to eq(alias_path)
end
end
context "with installed Formula" do
before do
# don't try to load/fetch gcc/glibc
allow(DevelopmentTools).to receive_messages(needs_libc_formula?: false, needs_compiler_formula?: false)
end
let(:installed_formula) { described_class.factory(formula_path) }
let(:installer) { FormulaInstaller.new(installed_formula) }
it "returns a Formula when given a rack" do
installer.fetch
installer.install
f = described_class.from_rack(installed_formula.rack)
expect(f).to be_a(Formula)
end
it "returns a Formula when given a Keg" do
installer.fetch
installer.install
keg = Keg.new(installed_formula.prefix)
f = described_class.from_keg(keg)
expect(f).to be_a(Formula)
end
end
context "when migrating from a Tap" do
let(:tap) { Tap.fetch("homebrew", "foo") }
let(:another_tap) { Tap.fetch("homebrew", "bar") }
let(:tap_migrations_path) { tap.path/"tap_migrations.json" }
let(:another_tap_formula_path) { another_tap.path/"Formula/#{formula_name}.rb" }
before do
tap.path.mkpath
another_tap_formula_path.dirname.mkpath
another_tap_formula_path.write formula_content
end
after do
FileUtils.rm_rf tap.path
FileUtils.rm_rf another_tap.path
end
it "returns a Formula that has gone through a tap migration into homebrew/core" do
tap_migrations_path.write <<~EOS
{
"#{formula_name}": "homebrew/core"
}
EOS
formula = described_class.factory("#{tap}/#{formula_name}")
expect(formula).to be_a(Formula)
expect(formula.tap).to eq(CoreTap.instance)
expect(formula.path).to eq(formula_path)
end
it "returns a Formula that has gone through a tap migration into another tap" do
tap_migrations_path.write <<~EOS
{
"#{formula_name}": "#{another_tap}"
}
EOS
formula = described_class.factory("#{tap}/#{formula_name}")
expect(formula).to be_a(Formula)
expect(formula.tap).to eq(another_tap)
expect(formula.path).to eq(another_tap_formula_path)
end
end
context "when loading from Tap" do
let(:tap) { Tap.fetch("homebrew", "foo") }
let(:another_tap) { Tap.fetch("homebrew", "bar") }
let(:formula_path) { tap.path/"Formula/#{formula_name}.rb" }
let(:alias_name) { "bar" }
let(:alias_dir) { tap.alias_dir }
let(:alias_path) { alias_dir/alias_name }
before do
alias_dir.mkpath
FileUtils.ln_s formula_path, alias_path
end
it "returns a Formula when given a name" do
expect(described_class.factory(formula_name)).to be_a(Formula)
end
it "returns a Formula from an Alias path" do
expect(described_class.factory(alias_name)).to be_a(Formula)
end
it "returns a Formula from a fully qualified Alias path" do
expect(described_class.factory("#{tap.name}/#{alias_name}")).to be_a(Formula)
end
it "raises an error when the Formula cannot be found" do
it "raises an error if ref is nil" do
expect do
described_class.factory("#{tap}/not_existed_formula")
end.to raise_error(TapFormulaUnavailableError)
described_class.factory(nil)
end.to raise_error(TypeError)
end
it "returns a Formula when given a fully qualified name" do
expect(described_class.factory("#{tap}/#{formula_name}")).to be_a(Formula)
context "with sharded Formula directory" do
let(:formula_name) { "testball_sharded" }
let(:formula_path) do
core_tap = CoreTap.instance
(core_tap.formula_dir/formula_name[0]).mkpath
core_tap.new_formula_path(formula_name)
end
it "returns a Formula" do
expect(described_class.factory(formula_name)).to be_a(Formula)
end
it "returns a Formula when given a fully qualified name" do
expect(described_class.factory("homebrew/core/#{formula_name}")).to be_a(Formula)
end
end
it "raises an error if a Formula is in multiple Taps" do
(another_tap.path/"Formula").mkpath
(another_tap.path/"Formula/#{formula_name}.rb").write formula_content
context "when the Formula has the wrong class" do
let(:formula_name) { "giraffe" }
let(:formula_content) do
<<~RUBY
class Wrong#{described_class.class_s(formula_name)} < Formula
end
RUBY
end
expect do
described_class.factory(formula_name)
end.to raise_error(TapFormulaAmbiguityError)
it "raises an error" do
expect do
described_class.factory(formula_name)
end.to raise_error(TapFormulaClassUnavailableError)
end
end
it "returns a Formula when given a path" do
expect(described_class.factory(formula_path)).to be_a(Formula)
end
it "returns a Formula when given a URL", :needs_utils_curl, :no_api do
formula = described_class.factory("file://#{formula_path}")
expect(formula).to be_a(Formula)
end
context "when given a bottle" do
subject(:formula) { described_class.factory(bottle) }
it "returns a Formula" do
expect(formula).to be_a(Formula)
end
it "calling #local_bottle_path on the returned Formula returns the bottle path" do
expect(formula.local_bottle_path).to eq(bottle.realpath)
end
end
context "when given an alias" do
subject(:formula) { described_class.factory("foo") }
let(:alias_dir) { CoreTap.instance.alias_dir }
let(:alias_path) { alias_dir/"foo" }
before do
alias_dir.mkpath
FileUtils.ln_s formula_path, alias_path
end
it "returns a Formula" do
expect(formula).to be_a(Formula)
end
it "calling #alias_path on the returned Formula returns the alias path" do
expect(formula.alias_path).to eq(alias_path)
end
end
context "with installed Formula" do
before do
# don't try to load/fetch gcc/glibc
allow(DevelopmentTools).to receive_messages(needs_libc_formula?: false, needs_compiler_formula?: false)
end
let(:installed_formula) { described_class.factory(formula_path) }
let(:installer) { FormulaInstaller.new(installed_formula) }
it "returns a Formula when given a rack" do
installer.fetch
installer.install
f = described_class.from_rack(installed_formula.rack)
expect(f).to be_a(Formula)
end
it "returns a Formula when given a Keg" do
installer.fetch
installer.install
keg = Keg.new(installed_formula.prefix)
f = described_class.from_keg(keg)
expect(f).to be_a(Formula)
end
end
context "when migrating from a Tap" do
let(:tap) { Tap.fetch("homebrew", "foo") }
let(:another_tap) { Tap.fetch("homebrew", "bar") }
let(:tap_migrations_path) { tap.path/"tap_migrations.json" }
let(:another_tap_formula_path) { another_tap.path/"Formula/#{formula_name}.rb" }
before do
tap.path.mkpath
another_tap_formula_path.dirname.mkpath
another_tap_formula_path.write formula_content
end
after do
FileUtils.rm_rf tap.path
FileUtils.rm_rf another_tap.path
end
it "returns a Formula that has gone through a tap migration into homebrew/core" do
tap_migrations_path.write <<~EOS
{
"#{formula_name}": "homebrew/core"
}
EOS
formula = described_class.factory("#{tap}/#{formula_name}")
expect(formula).to be_a(Formula)
expect(formula.tap).to eq(CoreTap.instance)
expect(formula.path).to eq(formula_path)
end
it "returns a Formula that has gone through a tap migration into another tap" do
tap_migrations_path.write <<~EOS
{
"#{formula_name}": "#{another_tap}"
}
EOS
formula = described_class.factory("#{tap}/#{formula_name}")
expect(formula).to be_a(Formula)
expect(formula.tap).to eq(another_tap)
expect(formula.path).to eq(another_tap_formula_path)
end
end
context "when loading from Tap" do
let(:tap) { Tap.fetch("homebrew", "foo") }
let(:another_tap) { Tap.fetch("homebrew", "bar") }
let(:formula_path) { tap.path/"Formula/#{formula_name}.rb" }
let(:alias_name) { "bar" }
let(:alias_dir) { tap.alias_dir }
let(:alias_path) { alias_dir/alias_name }
before do
alias_dir.mkpath
FileUtils.ln_s formula_path, alias_path
end
it "returns a Formula when given a name" do
expect(described_class.factory(formula_name)).to be_a(Formula)
end
it "returns a Formula from an Alias path" do
expect(described_class.factory(alias_name)).to be_a(Formula)
end
it "returns a Formula from a fully qualified Alias path" do
expect(described_class.factory("#{tap.name}/#{alias_name}")).to be_a(Formula)
end
it "raises an error when the Formula cannot be found" do
expect do
described_class.factory("#{tap}/not_existed_formula")
end.to raise_error(TapFormulaUnavailableError)
end
it "returns a Formula when given a fully qualified name" do
expect(described_class.factory("#{tap}/#{formula_name}")).to be_a(Formula)
end
it "raises an error if a Formula is in multiple Taps" do
(another_tap.path/"Formula").mkpath
(another_tap.path/"Formula/#{formula_name}.rb").write formula_content
expect do
described_class.factory(formula_name)
end.to raise_error(TapFormulaAmbiguityError)
end
end
end
context "when loading from the API" do
context "with the API" do
def formula_json_contents(extra_items = {})
{
formula_name => {
@ -496,25 +498,16 @@ RSpec.describe Formulary 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)
loader = described_class::FromNameLoader.try_new(old_formula_name)
expect(loader).to be_a(described_class::FromAPILoader)
expect(loader.name).to eq formula_name
expect(loader.path).not_to exist
end
it "returns the tap migration rename by old full name" do
@ -523,9 +516,10 @@ RSpec.describe Formulary do
{ "#{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)
loader = described_class::FromTapLoader.try_new("#{foo_tap}/#{old_formula_name}")
expect(loader).to be_a(described_class::FromAPILoader)
expect(loader.name).to eq formula_name
expect(loader.path).not_to exist
end
end
end