Merge pull request #21689 from Homebrew/implied_link_overwrite

formula: imply `link_overwrite` for related formulae

Author: MikeMcQuaid

AGENTS.md

@@ -31,6 +31,8 @@ When running commands in this repository, use `./bin/brew` (not a system `brew`
 - Write new code (using Sorbet `sig` type signatures and `typed: strict` for new files, but never for RSpec/test/`*_spec.rb` files)
 - Write new tests (avoid more than one `:integration_test` per file for speed).
   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.
+- When adding or tightening tests, verify them with a red/green cycle using the exact `--only=file:line` target for the example you changed.
+- 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.
 - Keep comments minimal; prefer self-documenting code through strings, variable names, etc. over more comments.
 
 ## Repository Structure

Library/Homebrew/formula.rb

@@ -630,18 +630,14 @@ def versioned_formulae_names
 
     versioned_names = if (formula_tap = tap)
       formula_tap.prefix_to_versioned_formulae_names.fetch(name_prefix, [])
-    elsif path.exist?
+    else
       versioned_formula_glob = if name_prefix.end_with?("-full")
         "#{name_prefix.delete_suffix("-full")}@*-full.rb"
       else
         "#{name_prefix}@*.rb"
       end
 
-      Pathname.glob((path.dirname/versioned_formula_glob).to_s)
-              .map { |path| path.basename(".rb").to_s }
-              .sort
-    else
-      raise "Either tap or path is required to list versioned formulae"
+      formula_names_for_glob(versioned_formula_glob)
     end
 
     versioned_names.reject do |versioned_name|
@@ -666,16 +662,36 @@ def unversioned_formula_name
     name.sub(/@[\d.]+(?=-full$|$)/, "")
   end
 
+  sig { params(glob: String).returns(T::Array[String]) }
+  def formula_names_for_glob(glob)
+    @formula_names_for_glob ||= T.let({}, T.nilable(T::Hash[String, T::Array[String]]))
+    @formula_names_for_glob[glob] ||= if (formula_tap = tap)
+      formula_name = File.basename(glob, ".rb")
+      if formula_tap.formula_files_by_name.key?(formula_name)
+        [formula_name]
+      else
+        []
+      end
+    elsif path.exist?
+      Pathname.glob((path.dirname/glob).to_s)
+              .map { |path| path.basename(".rb").to_s }
+              .sort
+    else
+      raise "Either tap or path is required to list sibling formulae"
+    end
+  end
+  private :formula_names_for_glob
+
   # Returns the sibling `-full` or non-`-full` formula names for any Formula.
   sig { returns(T::Array[String]) }
   def full_formulae_names
-    [
-      if name.end_with?("-full")
-        name.delete_suffix("-full")
-      else
-        "#{name}-full"
-      end,
-    ]
+    sibling_name = if name.end_with?("-full")
+      name.delete_suffix("-full")
+    else
+      "#{name}-full"
+    end
+
+    formula_names_for_glob("#{sibling_name}.rb")
   end
 
   # Returns sibling `-full` or non-`-full` Formula objects for any Formula.
@@ -747,6 +763,38 @@ def link_overwrite_formulae
     end
   end
 
+  sig { params(path: Pathname).returns(T.nilable(T.any(String, Symbol))) }
+  def link_overwrite_keg_name(path)
+    # Don't overwrite files not created by Homebrew.
+    return if path.stat.uid != HOMEBREW_ORIGINAL_BREW_FILE.stat.uid
+
+    keg = Keg.for(path)
+    # This keg doesn't belong to any current core/tap formula, most likely coming from a DIY install.
+    return if keg.tab.tap.nil?
+
+    keg.name
+  rescue NotAKegError, Errno::ENOENT
+    # File doesn't belong to any keg.
+    :missing
+  end
+
+  sig {
+    params(keg_name: T.nilable(T.any(String, Symbol)), overwrite_formulae: T::Array[Formula]).returns(T::Boolean)
+  }
+  def implied_link_overwrite?(keg_name, overwrite_formulae)
+    return false if overwrite_formulae.empty?
+    return false if keg_name.nil?
+
+    case keg_name
+    when :missing
+      # File doesn't belong to any keg, so implied overwrites do not apply.
+      false
+    else
+      overwrite_formulae.any? do |formula|
+        formula.possible_names.include?(keg_name)
+      end
+    end
+  end
   # Whether this {Formula} is version-synced with other formulae.
   sig { returns(T::Boolean) }
   def synced_with_other_formulae?
@@ -1599,39 +1647,37 @@ def skip_clean?(path)
   end
 
   # @see .link_overwrite
+  # Explicit `link_overwrite` paths may also be implied for related formula families.
   sig { params(path: Pathname).returns(T::Boolean) }
   def link_overwrite?(path)
-    # Don't overwrite files not created by Homebrew.
-    return false if path.stat.uid != HOMEBREW_ORIGINAL_BREW_FILE.stat.uid
-
-    # Don't overwrite files belong to other keg except when that
+    # Don't overwrite files that belong to another keg except when that
     # keg's formula is deleted.
-    begin
-      keg = Keg.for(path)
-    rescue NotAKegError, Errno::ENOENT
-      # file doesn't belong to any keg.
-    else
-      tab_tap = keg.tab.tap
-      # this keg doesn't below to any core/tap formula, most likely coming from a DIY install.
-      return false if tab_tap.nil?
-
+    case keg_name = link_overwrite_keg_name(path)
+    when String
       begin
-        f = Formulary.factory(keg.name)
+        f = Formulary.factory(keg_name)
       rescue FormulaUnavailableError
         # formula for this keg is deleted, so defer to allowlist
       rescue TapFormulaAmbiguityError
         return false # this keg belongs to another formula
       else
-        # this keg belongs to another unrelated formula
-        return false unless f.possible_names.include?(keg.name)
+        # Ensure `keg_name` maps cleanly to the resolved formula via `possible_names`.
+        return false unless f.possible_names.include?(keg_name)
       end
+    when :missing
+      # File doesn't belong to any keg, so defer to overwrite checks below.
+    else
+      return false
     end
+
     to_check = path.relative_path_from(HOMEBREW_PREFIX).to_s
-    T.must(self.class.link_overwrite_paths).any? do |p|
+    return true if T.must(self.class.link_overwrite_paths).any? do |p|
       p.to_s == to_check ||
-        to_check.start_with?("#{p.to_s.chomp("/")}/") ||
-        /^#{Regexp.escape(p.to_s).gsub('\*', ".*?")}$/.match?(to_check)
+      to_check.start_with?("#{p.to_s.chomp("/")}/") ||
+      /^#{Regexp.escape(p.to_s).gsub('\*', ".*?")}$/.match?(to_check)
     end
+
+    implied_link_overwrite?(keg_name, link_overwrite_formulae)
   end
 
   # Whether this {Formula} is deprecated (i.e. warns on installation).

Library/Homebrew/test/cmd/link_spec.rb

@@ -6,6 +6,23 @@
 RSpec.describe Homebrew::Cmd::Link do
   it_behaves_like "parseable arguments"
 
+  it "uses formula-aware conflict handling when linking a Formula" do
+    formula = formula "testball" do
+      url "foo-1.0"
+    end
+    keg = instance_double(Keg, rack: HOMEBREW_CELLAR/"testball", linked?: false, name: "testball")
+
+    cmd = described_class.new(["testball"])
+    allow(cmd.args.named).to receive(:to_latest_kegs).and_return([keg])
+    allow(Formulary).to receive(:keg_only?).with(keg.rack).and_return(false)
+    allow(keg).to receive(:to_formula).and_return(formula)
+    expect(Homebrew::Unlink).to receive(:unlink_link_overwrite_formulae).with(formula, verbose: false)
+    allow(keg).to receive(:lock).and_yield
+    expect(keg).to receive(:link).with(dry_run: false, verbose: false, overwrite: false).and_return(1)
+
+    expect { cmd.run }.to output(/Linking .*1 symlinks created\./).to_stdout
+  end
+
   it "links a given Formula", :integration_test do
     setup_test_formula "testball", tab_attributes: { installed_on_request: true }
     Formula["testball"].any_installed_keg.unlink

Library/Homebrew/test/formula_spec.rb

@@ -220,10 +220,33 @@
       end
     end
 
-    it "returns sibling full and non-full names" do
+    let(:f_versioned_full) do
+      formula "foo@2.0-full" do
+        url "foo-full-2.0"
+      end
+    end
+
+    before do
+      [f, f_full, f_versioned].each do |formula|
+        allow(formula).to receive(:tap).and_return(nil)
+        FileUtils.touch formula.path
+      end
+    end
+
+    it "returns only existing sibling full and non-full names" do
       expect(f.full_formulae_names).to eq ["foo-full"]
       expect(f_full.full_formulae_names).to eq ["foo"]
-      expect(f_versioned.full_formulae_names).to eq ["foo@2.0-full"]
+      expect(f_versioned.full_formulae_names).to eq []
+
+      allow(f_versioned_full).to receive(:tap).and_return(nil)
+      FileUtils.touch f_versioned_full.path
+      f_versioned_with_full = formula "foo@2.0" do
+        url "foo-2.0"
+      end
+      allow(f_versioned_with_full).to receive(:tap).and_return(nil)
+      FileUtils.touch f_versioned_with_full.path
+
+      expect(f_versioned_with_full.full_formulae_names).to eq ["foo@2.0-full"]
     end
   end
 
@@ -340,6 +363,146 @@
     end
   end
 
+  describe "#link_overwrite?" do
+    let(:versioned_formula) do
+      formula "foo@22" do
+        url "foo-22.0"
+      end
+    end
+
+    let(:related_formula) do
+      formula "foo" do
+        url "foo-1.0"
+      end
+    end
+
+    let(:conflict_file) { HOMEBREW_PREFIX/"lib/formula_spec/node_modules/npm/LICENSE" }
+
+    before do
+      allow(versioned_formula).to receive(:link_overwrite_formulae).and_return([related_formula])
+      conflict_file.dirname.mkpath
+      FileUtils.touch conflict_file
+    end
+
+    after do
+      FileUtils.rm_f conflict_file
+      conflict_file.dirname.rmdir_if_possible
+      conflict_file.dirname.parent.rmdir_if_possible
+      conflict_file.dirname.parent.parent.rmdir_if_possible
+    end
+
+    it "does not allow untracked conflicts for related formula families" do
+      expect(versioned_formula.link_overwrite?(conflict_file)).to be false
+    end
+
+    it "returns false when the conflict is not Homebrew-managed" do
+      allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return(nil)
+
+      expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/foo")).to be false
+    end
+
+    it "returns false for ambiguous keg names" do
+      allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return("foo")
+      ambiguity_loaders = [
+        instance_double(Formulary::FormulaLoader, tap: instance_double(Tap, to_s: "homebrew/core")),
+        instance_double(Formulary::FormulaLoader, tap: instance_double(Tap, to_s: "homebrew/other")),
+      ]
+      allow(Formulary).to receive(:factory).with("foo")
+                                           .and_raise(TapFormulaAmbiguityError.new("foo", ambiguity_loaders))
+
+      expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/foo")).to be false
+    end
+
+    it "returns false for unrelated keg names" do
+      unrelated_formula = formula "bar" do
+        url "bar-1.0"
+      end
+      allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return("bar")
+      allow(Formulary).to receive(:factory).with("bar").and_return(unrelated_formula)
+      allow(unrelated_formula).to receive(:possible_names).and_return(["baz"])
+
+      expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/bar")).to be false
+    end
+
+    it "allows explicit link_overwrite paths" do
+      formula_with_explicit_overwrite = formula "baz" do
+        url "baz-1.0"
+        link_overwrite "bin/baz"
+      end
+      allow(formula_with_explicit_overwrite).to receive(:link_overwrite_keg_name).and_return("baz")
+      allow(Formulary).to receive(:factory).with("baz").and_return(formula_with_explicit_overwrite)
+
+      expect(formula_with_explicit_overwrite.link_overwrite?(HOMEBREW_PREFIX/"bin/baz")).to be true
+    end
+
+    it "allows existing related keg names through implied overwrites" do
+      allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return("foo")
+      allow(Formulary).to receive(:factory).with("foo").and_return(related_formula)
+
+      expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/foo")).to be true
+    end
+
+    it "allows deleted related keg names through implied overwrites" do
+      allow(versioned_formula).to receive(:link_overwrite_keg_name).and_return("foo-old")
+      allow(Formulary).to receive(:factory).with("foo-old").and_raise(FormulaUnavailableError.new("foo-old"))
+      allow(related_formula).to receive_messages(oldnames: ["foo-old"], aliases: [])
+
+      expect(versioned_formula.link_overwrite?(HOMEBREW_PREFIX/"bin/foo")).to be true
+    end
+
+    it "returns false for missing conflicts without explicit or implied overwrites" do
+      formula_without_overwrites = formula "qux" do
+        url "qux-1.0"
+      end
+      allow(formula_without_overwrites).to receive_messages(link_overwrite_keg_name: :missing,
+                                                            link_overwrite_formulae: [])
+
+      expect(formula_without_overwrites.link_overwrite?(HOMEBREW_PREFIX/"bin/qux")).to be false
+    end
+  end
+
+  describe "#implied_link_overwrite?" do
+    let(:versioned_formula) do
+      formula "foo@22" do
+        url "foo-22.0"
+      end
+    end
+
+    let(:related_formula) do
+      formula "foo" do
+        url "foo-1.0"
+      end
+    end
+
+    before do
+      allow(related_formula).to receive_messages(oldnames: ["foo-old"], aliases: ["foo-alias"])
+    end
+
+    it "does not allow missing conflicts without actual related formulae" do
+      expect(versioned_formula.implied_link_overwrite?(:missing, [])).to be false
+    end
+
+    it "does not allow non-Homebrew conflicts" do
+      expect(versioned_formula.implied_link_overwrite?(nil, [related_formula])).to be false
+    end
+
+    it "does not allow missing conflicts even when related formulae exist" do
+      expect(versioned_formula.implied_link_overwrite?(:missing, [related_formula])).to be false
+    end
+
+    it "allows related keg names via oldnames" do
+      expect(versioned_formula.implied_link_overwrite?("foo-old", [related_formula])).to be true
+    end
+
+    it "allows related keg names via aliases" do
+      expect(versioned_formula.implied_link_overwrite?("foo-alias", [related_formula])).to be true
+    end
+
+    it "does not allow unrelated keg names" do
+      expect(versioned_formula.implied_link_overwrite?("bar", [related_formula])).to be false
+    end
+  end
+
   example "installed alias with core" do
     f = formula do
       url "foo-1.0"