From cbe347782c3eb222ac14f7b1c38d21753d9d056a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 11 Aug 2025 12:43:33 +0000 Subject: [PATCH] Implement formula conflict detection for cask binary artifacts While we're at it, update copilot instructions. Co-authored-by: MikeMcQuaid <125011+MikeMcQuaid@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- Library/Homebrew/cask/artifact/symlinked.rb | 24 +++++++ .../test/cask/artifact/symlinked_spec.rb | 62 +++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 Library/Homebrew/test/cask/artifact/symlinked_spec.rb diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 59ff03e1c5..707213fe75 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -16,7 +16,7 @@ Please follow these guidelines when contributing: ### Development Flow -- Write new code (using Sorbet `sig` type signatures and `typed: strict` files whenever possible) +- Write new code (using Sorbet `sig` type signatures and `typed: strict` files whenever possible, but not for test files) - Write new tests (avoid more than one `:integration_test` per file for speed) ## Repository Structure diff --git a/Library/Homebrew/cask/artifact/symlinked.rb b/Library/Homebrew/cask/artifact/symlinked.rb index 3ed0f956b6..348198124a 100644 --- a/Library/Homebrew/cask/artifact/symlinked.rb +++ b/Library/Homebrew/cask/artifact/symlinked.rb @@ -56,6 +56,9 @@ module Cask (target.realpath == source.realpath || target.realpath.to_s.start_with?("#{cask.caskroom_path}/")) opoo "#{message}; overwriting." Utils.gain_permissions_remove(target, command:) + elsif (formula = conflicting_formula) + opoo "#{message} from formula #{formula}; skipping link." + return else raise CaskError, "#{message}." end @@ -69,11 +72,32 @@ module Cask return unless target.symlink? ohai "Unlinking #{self.class.english_name} '#{target}'" + + if (formula = conflicting_formula) + odebug "#{target} is from formula #{formula}; skipping unlink." + return + end + Utils.gain_permissions_remove(target, command:) end sig { params(command: T.class_of(SystemCommand)).void } def create_filesystem_link(command); end + + # Check if the target file is a symlink that originates from a formula + # with the same name as this cask, indicating a potential conflict + sig { returns(T.nilable(String)) } + def conflicting_formula + if target.symlink? && target.exist? && + (match = target.realpath.to_s.match(%r{^#{HOMEBREW_CELLAR}/(?[^/]+)/}o)) + match[:formula] + end + rescue => e + # If we can't determine the realpath or any other error occurs, + # don't treat it as a conflicting formula file + odebug "Error checking for conflicting formula file: #{e}" + nil + end end end end diff --git a/Library/Homebrew/test/cask/artifact/symlinked_spec.rb b/Library/Homebrew/test/cask/artifact/symlinked_spec.rb new file mode 100644 index 0000000000..573310c14b --- /dev/null +++ b/Library/Homebrew/test/cask/artifact/symlinked_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +RSpec.describe Cask::Artifact::Symlinked, :cask do + # Test the formula conflict detection functionality that applies to all symlinked artifacts + describe "#conflicting_formula" do + let(:cask) do + Cask::CaskLoader.load(cask_path("with-binary")).tap do |cask| + InstallHelper.install_without_artifacts(cask) + end + end + + let(:binary_artifact) { cask.artifacts.find { |a| a.is_a?(Cask::Artifact::Binary) } } + let(:binarydir) { cask.config.binarydir } + let(:target_path) { binarydir.join("binary") } + + around do |example| + binarydir.mkpath + + example.run + ensure + FileUtils.rm_f target_path + FileUtils.rmdir binarydir + # Clean up the fake formula directory + FileUtils.rm_rf(HOMEBREW_CELLAR/"with-binary") if (HOMEBREW_CELLAR/"with-binary").exist? + end + + context "when target is already linked from a formula" do + it "detects the conflict and skips linking with warning" do + # Create a fake formula directory structure + formula_cellar_path = HOMEBREW_CELLAR/"with-binary/1.0.0/bin" + formula_cellar_path.mkpath + formula_binary_path = formula_cellar_path/"binary" + FileUtils.touch formula_binary_path + + # Create symlink from the expected location to the formula binary + target_path.make_symlink(formula_binary_path) + + stderr = <<~EOS + Warning: It seems there is already a Binary at '#{target_path}' from formula with-binary; skipping link. + EOS + + expect do + binary_artifact.install_phase(command: NeverSudoSystemCommand, force: false) + end.to output(stderr).to_stderr + + expect(target_path).to be_a_symlink + expect(target_path.readlink).to eq(formula_binary_path) + end + end + + context "when target doesn't exist" do + it "proceeds with normal installation" do + expect do + binary_artifact.install_phase(command: NeverSudoSystemCommand, force: false) + end.not_to raise_error + + expect(target_path).to be_a_symlink + expect(target_path.readlink).to exist + end + end + end +end