From a62d3b577a8989e90c95cd840afb5efce73f940a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Sep 2025 07:50:26 +0000 Subject: [PATCH] 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> --- Library/Homebrew/keg_relocate.rb | 71 ++++++++++ .../test/keg_relocate/homebrew_files_spec.rb | 127 ++++++++++++++++++ 2 files changed, 198 insertions(+) create mode 100644 Library/Homebrew/test/keg_relocate/homebrew_files_spec.rb diff --git a/Library/Homebrew/keg_relocate.rb b/Library/Homebrew/keg_relocate.rb index 6b333dc39d..e01ac7aa9c 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -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 diff --git a/Library/Homebrew/test/keg_relocate/homebrew_files_spec.rb b/Library/Homebrew/test/keg_relocate/homebrew_files_spec.rb new file mode 100644 index 0000000000..551a406192 --- /dev/null +++ b/Library/Homebrew/test/keg_relocate/homebrew_files_spec.rb @@ -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 \ No newline at end of file