Implement keg relocation fix for Homebrew-created files
- Add homebrew_created_file? method to identify service files - Add prepare_relocation_to_placeholders_for_homebrew_files method - Modify replace_text_in_files to handle Homebrew files separately - Homebrew-created files now get full path relocation instead of selective relocation Co-authored-by: MikeMcQuaid <125011+MikeMcQuaid@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
parent
3d3e256a2b
commit
a62d3b577a
@ -157,6 +157,39 @@ class Keg
|
||||
relocation
|
||||
end
|
||||
|
||||
sig { params(file: Pathname).returns(T::Boolean) }
|
||||
def homebrew_created_file?(file)
|
||||
# Service files created by Homebrew
|
||||
return true if file.extname == ".plist" && file.basename.to_s.start_with?("homebrew.")
|
||||
return true if file.extname == ".service" && file.basename.to_s.start_with?("homebrew.")
|
||||
return true if file.extname == ".timer" && file.basename.to_s.start_with?("homebrew.")
|
||||
|
||||
false
|
||||
end
|
||||
|
||||
sig { returns(Relocation) }
|
||||
def prepare_relocation_to_placeholders_for_homebrew_files
|
||||
relocation = Relocation.new
|
||||
|
||||
# For Homebrew-created files, always use full prefix replacement
|
||||
# These files are generated by Homebrew so they should always be relocatable
|
||||
relocation.add_replacement_pair(:prefix, HOMEBREW_PREFIX.to_s, PREFIX_PLACEHOLDER, path: true)
|
||||
relocation.add_replacement_pair(:cellar, HOMEBREW_CELLAR.to_s, CELLAR_PLACEHOLDER, path: true)
|
||||
|
||||
# when HOMEBREW_PREFIX == HOMEBREW_REPOSITORY we should use HOMEBREW_PREFIX for all relocations to avoid
|
||||
# being unable to differentiate between them.
|
||||
if HOMEBREW_PREFIX != HOMEBREW_REPOSITORY
|
||||
relocation.add_replacement_pair(:repository, HOMEBREW_REPOSITORY.to_s, REPOSITORY_PLACEHOLDER, path: true)
|
||||
end
|
||||
relocation.add_replacement_pair(:library, HOMEBREW_LIBRARY.to_s, LIBRARY_PLACEHOLDER, path: true)
|
||||
relocation.add_replacement_pair(:perl,
|
||||
%r{\A#![ \t]*(?:/usr/bin/perl\d\.\d+|#{HOMEBREW_PREFIX}/opt/perl/bin/perl)( |$)}o,
|
||||
"#!#{PERL_PLACEHOLDER}\\1")
|
||||
relocation.add_replacement_pair(:java, JAVA_REGEX, JAVA_PLACEHOLDER)
|
||||
|
||||
relocation
|
||||
end
|
||||
|
||||
sig { returns(T::Array[Pathname]) }
|
||||
def replace_locations_with_placeholders
|
||||
relocation = prepare_relocation_to_placeholders.freeze
|
||||
@ -200,7 +233,22 @@ class Keg
|
||||
files ||= text_files | libtool_files
|
||||
|
||||
changed_files = T.let([], T::Array[Pathname])
|
||||
|
||||
# Separate Homebrew-created files from regular files
|
||||
homebrew_files = []
|
||||
regular_files = []
|
||||
|
||||
files.map { path.join(_1) }.group_by { |f| f.stat.ino }.each_value do |first, *rest|
|
||||
file_group = [first, *rest]
|
||||
if homebrew_created_file?(first)
|
||||
homebrew_files << file_group
|
||||
else
|
||||
regular_files << file_group
|
||||
end
|
||||
end
|
||||
|
||||
# Process regular files with normal relocation
|
||||
regular_files.each do |first, *rest|
|
||||
s = first.open("rb", &:read)
|
||||
|
||||
next unless relocation.replace_text!(s)
|
||||
@ -217,6 +265,29 @@ class Keg
|
||||
rest.each { |file| FileUtils.ln(first, file, force: true) }
|
||||
end
|
||||
end
|
||||
|
||||
# Process Homebrew-created files with full relocation
|
||||
unless homebrew_files.empty?
|
||||
homebrew_relocation = prepare_relocation_to_placeholders_for_homebrew_files.freeze
|
||||
homebrew_files.each do |first, *rest|
|
||||
s = first.open("rb", &:read)
|
||||
|
||||
next unless homebrew_relocation.replace_text!(s)
|
||||
|
||||
changed_files += [first, *rest].map { |file| file.relative_path_from(path) }
|
||||
|
||||
begin
|
||||
first.atomic_write(s)
|
||||
rescue SystemCallError
|
||||
first.ensure_writable do
|
||||
first.open("wb") { |f| f.write(s) }
|
||||
end
|
||||
else
|
||||
rest.each { |file| FileUtils.ln(first, file, force: true) }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
changed_files
|
||||
end
|
||||
|
||||
|
||||
127
Library/Homebrew/test/keg_relocate/homebrew_files_spec.rb
Normal file
127
Library/Homebrew/test/keg_relocate/homebrew_files_spec.rb
Normal file
@ -0,0 +1,127 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "keg_relocate"
|
||||
require "keg"
|
||||
|
||||
RSpec.describe Keg, "#homebrew_created_file?" do
|
||||
let(:keg) { instance_double(described_class) }
|
||||
let(:path) { Pathname.new("/tmp/test") }
|
||||
|
||||
before do
|
||||
allow(keg).to receive(:path).and_return(path)
|
||||
allow(keg).to receive(:homebrew_created_file?).and_call_original
|
||||
end
|
||||
|
||||
it "identifies Homebrew service files correctly" do
|
||||
plist_file = instance_double(Pathname, extname: ".plist", basename: Pathname.new("homebrew.foo.plist"))
|
||||
service_file = instance_double(Pathname, extname: ".service", basename: Pathname.new("homebrew.foo.service"))
|
||||
timer_file = instance_double(Pathname, extname: ".timer", basename: Pathname.new("homebrew.foo.timer"))
|
||||
regular_file = instance_double(Pathname, extname: ".txt", basename: Pathname.new("readme.txt"))
|
||||
non_homebrew_plist = instance_double(Pathname, extname: ".plist", basename: Pathname.new("com.example.foo.plist"))
|
||||
|
||||
allow(plist_file.basename).to receive(:to_s).and_return("homebrew.foo.plist")
|
||||
allow(service_file.basename).to receive(:to_s).and_return("homebrew.foo.service")
|
||||
allow(timer_file.basename).to receive(:to_s).and_return("homebrew.foo.timer")
|
||||
allow(regular_file.basename).to receive(:to_s).and_return("readme.txt")
|
||||
allow(non_homebrew_plist.basename).to receive(:to_s).and_return("com.example.foo.plist")
|
||||
|
||||
expect(keg.homebrew_created_file?(plist_file)).to be true
|
||||
expect(keg.homebrew_created_file?(service_file)).to be true
|
||||
expect(keg.homebrew_created_file?(timer_file)).to be true
|
||||
expect(keg.homebrew_created_file?(regular_file)).to be false
|
||||
expect(keg.homebrew_created_file?(non_homebrew_plist)).to be false
|
||||
end
|
||||
end
|
||||
|
||||
RSpec.describe Keg, "#prepare_relocation_to_placeholders_for_homebrew_files" do
|
||||
include FileUtils
|
||||
|
||||
let(:keg_path) { Pathname.new(Dir.mktmpdir) }
|
||||
let(:keg) { described_class.new(keg_path) }
|
||||
|
||||
after do
|
||||
rmtree keg_path
|
||||
end
|
||||
|
||||
it "creates relocation with full prefix replacement for Homebrew files" do
|
||||
relocation = keg.prepare_relocation_to_placeholders_for_homebrew_files
|
||||
|
||||
# Verify that the relocation includes full prefix replacement
|
||||
prefix_old, prefix_new = relocation.replacement_pair_for(:prefix)
|
||||
|
||||
expect(prefix_new).to eq("@@HOMEBREW_PREFIX@@")
|
||||
expect(prefix_old).to be_a(Regexp)
|
||||
expect("#{HOMEBREW_PREFIX}/bin/foo").to match(prefix_old)
|
||||
end
|
||||
end
|
||||
|
||||
RSpec.describe Keg, "#replace_text_in_files with Homebrew-created files" do
|
||||
include FileUtils
|
||||
|
||||
let(:keg_path) { Pathname.new(Dir.mktmpdir) }
|
||||
let(:keg) { described_class.new(keg_path) }
|
||||
|
||||
before do
|
||||
allow(keg).to receive(:name).and_return("test-formula")
|
||||
allow(keg).to receive(:new_usr_local_relocation?).and_return(true)
|
||||
end
|
||||
|
||||
after do
|
||||
rmtree keg_path
|
||||
end
|
||||
|
||||
it "applies full relocation to Homebrew-created files" do
|
||||
# Create a mock service file
|
||||
service_file = keg_path/"homebrew.test-formula.service"
|
||||
service_content = <<~SERVICE
|
||||
[Unit]
|
||||
Description=Test Service
|
||||
|
||||
[Service]
|
||||
ExecStart=#{HOMEBREW_PREFIX}/bin/test-command
|
||||
Environment="PATH=#{HOMEBREW_PREFIX}/bin:#{HOMEBREW_PREFIX}/sbin:/usr/bin"
|
||||
WorkingDirectory=#{HOMEBREW_PREFIX}/var/test
|
||||
|
||||
[Install]
|
||||
WantedBy=multi-user.target
|
||||
SERVICE
|
||||
|
||||
service_file.write(service_content)
|
||||
|
||||
# Create a regular file that should get selective relocation
|
||||
regular_file = keg_path/"test.txt"
|
||||
regular_content = <<~CONTENT
|
||||
This is a test file.
|
||||
It references #{HOMEBREW_PREFIX}/opt/something
|
||||
And also #{HOMEBREW_PREFIX}/bin/something
|
||||
CONTENT
|
||||
|
||||
regular_file.write(regular_content)
|
||||
|
||||
# Mock the text_files method to return our test files
|
||||
allow(keg).to receive(:text_files).and_return([service_file.basename, regular_file.basename])
|
||||
allow(keg).to receive(:libtool_files).and_return([])
|
||||
|
||||
# Create a regular relocation that would only replace specific paths
|
||||
relocation = keg.prepare_relocation_to_placeholders
|
||||
|
||||
# Call replace_text_in_files
|
||||
changed_files = keg.replace_text_in_files(relocation)
|
||||
|
||||
# Check results
|
||||
expect(changed_files).to include(service_file.basename)
|
||||
expect(changed_files).to include(regular_file.basename)
|
||||
|
||||
# Service file should have full relocation (including /bin path)
|
||||
service_result = service_file.read
|
||||
expect(service_result).to include("@@HOMEBREW_PREFIX@@/bin/test-command")
|
||||
expect(service_result).to include("PATH=@@HOMEBREW_PREFIX@@/bin:@@HOMEBREW_PREFIX@@/sbin:/usr/bin")
|
||||
expect(service_result).to include("WorkingDirectory=@@HOMEBREW_PREFIX@@/var/test")
|
||||
|
||||
# Regular file should have selective relocation (only /opt path replaced)
|
||||
regular_result = regular_file.read
|
||||
expect(regular_result).to include("@@HOMEBREW_PREFIX@@/opt/something")
|
||||
# bin path should NOT be replaced in regular files when using new usr local relocation
|
||||
expect(regular_result).to include("#{HOMEBREW_PREFIX}/bin/something")
|
||||
end
|
||||
end
|
||||
Loading…
x
Reference in New Issue
Block a user