Skip to content

Commit 19a79a7

Browse files
committed
formula: imply link_overwrite for related formulae
Treat Homebrew-owned conflicts from related formula families as implied `link_overwrite` paths when linking versioned or -full formulae. This restores the version-switching behavior that regressed for cases like installing `node@22` after uninstalling `node` left `npm` files behind in the prefix. While we're here, add some `AGENTS.md` changes based on feedback. References Homebrew/homebrew-core#271139
1 parent 9605dd1 commit 19a79a7

4 files changed

Lines changed: 262 additions & 34 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ When running commands in this repository, use `./bin/brew` (not a system `brew`
3131
- Write new code (using Sorbet `sig` type signatures and `typed: strict` for new files, but never for RSpec/test/`*_spec.rb` files)
3232
- Write new tests (avoid more than one `:integration_test` per file for speed).
3333
Write fast tests by preferring a single `expect` per unit test and combine expectations in a single test when it is an integration test or has non-trivial `before` for test setup.
34+
- When adding or tightening tests, verify them with a red/green cycle using the exact `--only=file:line` target for the example you changed.
35+
- Formula classes created in specs may be frozen; avoid stubbing class methods on them with RSpec mocks and prefer instance-level stubs or test setup that does not require class-method stubbing.
3436
- Keep comments minimal; prefer self-documenting code through strings, variable names, etc. over more comments.
3537

3638
## Repository Structure

Library/Homebrew/formula.rb

Lines changed: 78 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -630,18 +630,14 @@ def versioned_formulae_names
630630

631631
versioned_names = if (formula_tap = tap)
632632
formula_tap.prefix_to_versioned_formulae_names.fetch(name_prefix, [])
633-
elsif path.exist?
633+
else
634634
versioned_formula_glob = if name_prefix.end_with?("-full")
635635
"#{name_prefix.delete_suffix("-full")}@*-full.rb"
636636
else
637637
"#{name_prefix}@*.rb"
638638
end
639639

640-
Pathname.glob((path.dirname/versioned_formula_glob).to_s)
641-
.map { |path| path.basename(".rb").to_s }
642-
.sort
643-
else
644-
raise "Either tap or path is required to list versioned formulae"
640+
formula_names_for_glob(versioned_formula_glob)
645641
end
646642

647643
versioned_names.reject do |versioned_name|
@@ -666,16 +662,36 @@ def unversioned_formula_name
666662
name.sub(/@[\d.]+(?=-full$|$)/, "")
667663
end
668664

665+
sig { params(glob: String).returns(T::Array[String]) }
666+
def formula_names_for_glob(glob)
667+
@formula_names_for_glob ||= T.let({}, T.nilable(T::Hash[String, T::Array[String]]))
668+
@formula_names_for_glob[glob] ||= if (formula_tap = tap)
669+
formula_name = File.basename(glob, ".rb")
670+
if formula_tap.formula_files_by_name.key?(formula_name)
671+
[formula_name]
672+
else
673+
[]
674+
end
675+
elsif path.exist?
676+
Pathname.glob((path.dirname/glob).to_s)
677+
.map { |path| path.basename(".rb").to_s }
678+
.sort
679+
else
680+
raise "Either tap or path is required to list sibling formulae"
681+
end
682+
end
683+
private :formula_names_for_glob
684+
669685
# Returns the sibling `-full` or non-`-full` formula names for any Formula.
670686
sig { returns(T::Array[String]) }
671687
def full_formulae_names
672-
[
673-
if name.end_with?("-full")
674-
name.delete_suffix("-full")
675-
else
676-
"#{name}-full"
677-
end,
678-
]
688+
sibling_name = if name.end_with?("-full")
689+
name.delete_suffix("-full")
690+
else
691+
"#{name}-full"
692+
end
693+
694+
formula_names_for_glob("#{sibling_name}.rb")
679695
end
680696

681697
# Returns sibling `-full` or non-`-full` Formula objects for any Formula.
@@ -747,6 +763,38 @@ def link_overwrite_formulae
747763
end
748764
end
749765

766+
sig { params(path: Pathname).returns(T.nilable(T.any(String, Symbol))) }
767+
def link_overwrite_keg_name(path)
768+
# Don't overwrite files not created by Homebrew.
769+
return if path.stat.uid != HOMEBREW_ORIGINAL_BREW_FILE.stat.uid
770+
771+
keg = Keg.for(path)
772+
# This keg doesn't belong to any current core/tap formula, most likely coming from a DIY install.
773+
return if keg.tab.tap.nil?
774+
775+
keg.name
776+
rescue NotAKegError, Errno::ENOENT
777+
# File doesn't belong to any keg.
778+
:missing
779+
end
780+
781+
sig {
782+
params(keg_name: T.nilable(T.any(String, Symbol)), overwrite_formulae: T::Array[Formula]).returns(T::Boolean)
783+
}
784+
def implied_link_overwrite?(keg_name, overwrite_formulae)
785+
return false if overwrite_formulae.empty?
786+
return false if keg_name.nil?
787+
788+
case keg_name
789+
when :missing
790+
# File doesn't belong to any keg, so implied overwrites do not apply.
791+
false
792+
else
793+
overwrite_formulae.any? do |formula|
794+
formula.possible_names.include?(keg_name)
795+
end
796+
end
797+
end
750798
# Whether this {Formula} is version-synced with other formulae.
751799
sig { returns(T::Boolean) }
752800
def synced_with_other_formulae?
@@ -1599,39 +1647,37 @@ def skip_clean?(path)
15991647
end
16001648

16011649
# @see .link_overwrite
1650+
# Explicit `link_overwrite` paths may also be implied for related formula families.
16021651
sig { params(path: Pathname).returns(T::Boolean) }
16031652
def link_overwrite?(path)
1604-
# Don't overwrite files not created by Homebrew.
1605-
return false if path.stat.uid != HOMEBREW_ORIGINAL_BREW_FILE.stat.uid
1606-
1607-
# Don't overwrite files belong to other keg except when that
1653+
# Don't overwrite files that belong to another keg except when that
16081654
# keg's formula is deleted.
1609-
begin
1610-
keg = Keg.for(path)
1611-
rescue NotAKegError, Errno::ENOENT
1612-
# file doesn't belong to any keg.
1613-
else
1614-
tab_tap = keg.tab.tap
1615-
# this keg doesn't below to any core/tap formula, most likely coming from a DIY install.
1616-
return false if tab_tap.nil?
1617-
1655+
case keg_name = link_overwrite_keg_name(path)
1656+
when String
16181657
begin
1619-
f = Formulary.factory(keg.name)
1658+
f = Formulary.factory(keg_name)
16201659
rescue FormulaUnavailableError
16211660
# formula for this keg is deleted, so defer to allowlist
16221661
rescue TapFormulaAmbiguityError
16231662
return false # this keg belongs to another formula
16241663
else
1625-
# this keg belongs to another unrelated formula
1626-
return false unless f.possible_names.include?(keg.name)
1664+
# Ensure `keg_name` maps cleanly to the resolved formula via `possible_names`.
1665+
return false unless f.possible_names.include?(keg_name)
16271666
end
1667+
when :missing
1668+
# File doesn't belong to any keg, so defer to overwrite checks below.
1669+
else
1670+
return false
16281671
end
1672+
16291673
to_check = path.relative_path_from(HOMEBREW_PREFIX).to_s
1630-
T.must(self.class.link_overwrite_paths).any? do |p|
1674+
return true if T.must(self.class.link_overwrite_paths).any? do |p|
16311675
p.to_s == to_check ||
1632-
to_check.start_with?("#{p.to_s.chomp("/")}/") ||
1633-
/^#{Regexp.escape(p.to_s).gsub('\*', ".*?")}$/.match?(to_check)
1676+
to_check.start_with?("#{p.to_s.chomp("/")}/") ||
1677+
/^#{Regexp.escape(p.to_s).gsub('\*', ".*?")}$/.match?(to_check)
16341678
end
1679+
1680+
implied_link_overwrite?(keg_name, link_overwrite_formulae)
16351681
end
16361682

16371683
# Whether this {Formula} is deprecated (i.e. warns on installation).

Library/Homebrew/test/cmd/link_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,23 @@
66
RSpec.describe Homebrew::Cmd::Link do
77
it_behaves_like "parseable arguments"
88

9+
it "uses formula-aware conflict handling when linking a Formula" do
10+
formula = formula "testball" do
11+
url "foo-1.0"
12+
end
13+
keg = instance_double(Keg, rack: HOMEBREW_CELLAR/"testball", linked?: false, name: "testball")
14+
15+
cmd = described_class.new(["testball"])
16+
allow(cmd.args.named).to receive(:to_latest_kegs).and_return([keg])
17+
allow(Formulary).to receive(:keg_only?).with(keg.rack).and_return(false)
18+
allow(keg).to receive(:to_formula).and_return(formula)
19+
expect(Homebrew::Unlink).to receive(:unlink_link_overwrite_formulae).with(formula, verbose: false)
20+
allow(keg).to receive(:lock).and_yield
21+
expect(keg).to receive(:link).with(dry_run: false, verbose: false, overwrite: false).and_return(1)
22+
23+
expect { cmd.run }.to output(/Linking .*1 symlinks created\./).to_stdout
24+
end
25+
926
it "links a given Formula", :integration_test do
1027
setup_test_formula "testball", tab_attributes: { installed_on_request: true }
1128
Formula["testball"].any_installed_keg.unlink

Library/Homebrew/test/formula_spec.rb

Lines changed: 165 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,33 @@
220220
end
221221
end
222222

223-
it "returns sibling full and non-full names" do
223+
let(:f_versioned_full) do
224+
formula "foo@2.0-full" do
225+
url "foo-full-2.0"
226+
end
227+
end
228+
229+
before do
230+
[f, f_full, f_versioned].each do |formula|
231+
allow(formula).to receive(:tap).and_return(nil)
232+
FileUtils.touch formula.path
233+
end
234+
end
235+
236+
it "returns only existing sibling full and non-full names" do
224237
expect(f.full_formulae_names).to eq ["foo-full"]
225238
expect(f_full.full_formulae_names).to eq ["foo"]
226-
expect(f_versioned.full_formulae_names).to eq ["foo@2.0-full"]
239+
expect(f_versioned.full_formulae_names).to eq []
240+
241+
allow(f_versioned_full).to receive(:tap).and_return(nil)
242+
FileUtils.touch f_versioned_full.path
243+
f_versioned_with_full = formula "foo@2.0" do
244+
url "foo-2.0"
245+
end
246+
allow(f_versioned_with_full).to receive(:tap).and_return(nil)
247+
FileUtils.touch f_versioned_with_full.path
248+
249+
expect(f_versioned_with_full.full_formulae_names).to eq ["foo@2.0-full"]
227250
end
228251
end
229252

@@ -340,6 +363,146 @@
340363
end
341364
end
342365

366+
describe "#link_overwrite?" do
367+
let(:versioned_formula) do
368+
formula "foo@22" do
369+
url "foo-22.0"
370+
end
371+
end
372+
373+
let(:related_formula) do
374+
formula "foo" do
375+
url "foo-1.0"
376+
end
377+
end
378+
379+
let(:conflict_file) { HOMEBREW_PREFIX/"lib/formula_spec/node_modules/npm/LICENSE" }
380+
381+
before do
382+
allow(versioned_formula).to receive(:link_overwrite_formulae).and_return([related_formula])
383+
conflict_file.dirname.mkpath
384+
FileUtils.touch conflict_file
385+
end
386+
387+
after do
388+
FileUtils.rm_f conflict_file
389+
conflict_file.dirname.rmdir_if_possible
390+
conflict_file.dirname.parent.rmdir_if_possible
391+
conflict_file.dirname.parent.parent.rmdir_if_possible
392+
end
393+
394+
it "does not allow untracked conflicts for related formula families" do
395+
expect(versioned_formula.link_overwrite?(conflict_file)).to be false
396+
end
397+
398+
it "returns false when the conflict is not Homebrew-managed" do
399+
allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return(nil)
400+
401+
expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/foo")).to be false
402+
end
403+
404+
it "returns false for ambiguous keg names" do
405+
allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return("foo")
406+
ambiguity_loaders = [
407+
instance_double(Formulary::FormulaLoader, tap: instance_double(Tap, to_s: "homebrew/core")),
408+
instance_double(Formulary::FormulaLoader, tap: instance_double(Tap, to_s: "homebrew/other")),
409+
]
410+
allow(Formulary).to receive(:factory).with("foo")
411+
.and_raise(TapFormulaAmbiguityError.new("foo", ambiguity_loaders))
412+
413+
expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/foo")).to be false
414+
end
415+
416+
it "returns false for unrelated keg names" do
417+
unrelated_formula = formula "bar" do
418+
url "bar-1.0"
419+
end
420+
allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return("bar")
421+
allow(Formulary).to receive(:factory).with("bar").and_return(unrelated_formula)
422+
allow(unrelated_formula).to receive(:possible_names).and_return(["baz"])
423+
424+
expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/bar")).to be false
425+
end
426+
427+
it "allows explicit link_overwrite paths" do
428+
formula_with_explicit_overwrite = formula "baz" do
429+
url "baz-1.0"
430+
link_overwrite "bin/baz"
431+
end
432+
allow(formula_with_explicit_overwrite).to receive(:link_overwrite_keg_name).and_return("baz")
433+
allow(Formulary).to receive(:factory).with("baz").and_return(formula_with_explicit_overwrite)
434+
435+
expect(formula_with_explicit_overwrite.link_overwrite?(HOMEBREW_PREFIX/"bin/baz")).to be true
436+
end
437+
438+
it "allows existing related keg names through implied overwrites" do
439+
allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return("foo")
440+
allow(Formulary).to receive(:factory).with("foo").and_return(related_formula)
441+
442+
expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/foo")).to be true
443+
end
444+
445+
it "allows deleted related keg names through implied overwrites" do
446+
allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return("foo-old")
447+
allow(Formulary).to receive(:factory).with("foo-old").and_raise(FormulaUnavailableError.new("foo-old"))
448+
allow(related_formula).to receive_messages(oldnames: ["foo-old"], aliases: [])
449+
450+
expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/foo")).to be true
451+
end
452+
453+
it "returns false for missing conflicts without explicit or implied overwrites" do
454+
formula_without_overwrites = formula "qux" do
455+
url "qux-1.0"
456+
end
457+
allow(formula_without_overwrites).to receive_messages(link_overwrite_keg_name: :missing,
458+
link_overwrite_formulae: [])
459+
460+
expect(formula_without_overwrites.link_overwrite?(HOMEBREW_PREFIX/"bin/qux")).to be false
461+
end
462+
end
463+
464+
describe "#implied_link_overwrite?" do
465+
let(:versioned_formula) do
466+
formula "foo@22" do
467+
url "foo-22.0"
468+
end
469+
end
470+
471+
let(:related_formula) do
472+
formula "foo" do
473+
url "foo-1.0"
474+
end
475+
end
476+
477+
before do
478+
allow(related_formula).to receive_messages(oldnames: ["foo-old"], aliases: ["foo-alias"])
479+
end
480+
481+
it "does not allow missing conflicts without actual related formulae" do
482+
expect(versioned_formula.implied_link_overwrite?(:missing, [])).to be false
483+
end
484+
485+
it "does not allow non-Homebrew conflicts" do
486+
expect(versioned_formula.implied_link_overwrite?(nil, [related_formula])).to be false
487+
end
488+
489+
it "does not allow missing conflicts even when related formulae exist" do
490+
expect(versioned_formula.implied_link_overwrite?(:missing, [related_formula])).to be false
491+
end
492+
493+
it "allows related keg names via oldnames" do
494+
expect(versioned_formula.implied_link_overwrite?("foo-old", [related_formula])).to be true
495+
end
496+
497+
it "allows related keg names via aliases" do
498+
expect(versioned_formula.implied_link_overwrite?("foo-alias", [related_formula])).to be true
499+
end
500+
501+
it "does not allow unrelated keg names" do
502+
expect(versioned_formula.implied_link_overwrite?("bar", [related_formula])).to be false
503+
end
504+
end
505+
343506
example "installed alias with core" do
344507
f = formula do
345508
url "foo-1.0"

0 commit comments

Comments
 (0)