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>
This commit is contained in:
		
							parent
							
								
									b8c82b44b8
								
							
						
					
					
						commit
						cbe347782c
					
				
							
								
								
									
										2
									
								
								.github/copilot-instructions.md
									
									
									
									
										vendored
									
									
								
							
							
						
						
									
										2
									
								
								.github/copilot-instructions.md
									
									
									
									
										vendored
									
									
								
							@ -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
 | 
			
		||||
 | 
			
		||||
@ -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}/(?<formula>[^/]+)/}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
 | 
			
		||||
 | 
			
		||||
							
								
								
									
										62
									
								
								Library/Homebrew/test/cask/artifact/symlinked_spec.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										62
									
								
								Library/Homebrew/test/cask/artifact/symlinked_spec.rb
									
									
									
									
									
										Normal file
									
								
							@ -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
 | 
			
		||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user