From 57fae524de41a7e9f149f9c87d6cdfd633d0556c Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Tue, 18 Jan 2022 15:16:01 +0800 Subject: [PATCH 1/2] extend/os/mac/keg_relocate: fix duplicate RPATH handling ruby-macho chokes on changing duplicate RPATHs, so we need to strip the duplicates before trying to relocate them. This continues #11405. We need this to unblock Homebrew/homebrew-core#91224. While we're here, let's get rid of `HOMEBREW_RELOCATE_RPATHS`. We've been using it for nearly a year with essentially no problems (barring `pdnsrec`), so I think it is safe to do unconditionally. --- .../Homebrew/extend/os/mac/keg_relocate.rb | 26 ++++++++++++------- Library/Homebrew/formula_installer.rb | 6 +---- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/keg_relocate.rb b/Library/Homebrew/extend/os/mac/keg_relocate.rb index 7161346eda..bb0b726493 100644 --- a/Library/Homebrew/extend/os/mac/keg_relocate.rb +++ b/Library/Homebrew/extend/os/mac/keg_relocate.rb @@ -31,11 +31,9 @@ class Keg change_install_name(old_name, new_name, file) if new_name end - if ENV["HOMEBREW_RELOCATE_RPATHS"] - each_linkage_for(file, :rpaths) do |old_name| - new_name = relocated_name_for(old_name, relocation) - change_rpath(old_name, new_name, file) if new_name - end + each_linkage_for(file, :rpaths) do |old_name| + new_name = relocated_name_for(old_name, relocation) + change_rpath(old_name, new_name, file) if new_name end end end @@ -56,11 +54,21 @@ class Keg change_install_name(bad_name, new_name, file) unless new_name == bad_name end - each_linkage_for(file, :rpaths) do |bad_name| - # Strip rpaths rooted in the build directory - next if !bad_name.start_with?(HOMEBREW_TEMP.to_s) && - !bad_name.start_with?(HOMEBREW_TEMP.realpath.to_s) + # Count duplicate rpaths. We need to keep track of this ourselves + # because the MachO data is cached and this cache is not updated + # after modification with #delete_rpath. + rpath_dupe_count = Hash.new { |h, k| h[k] = -1 } + file.rpaths.each do |rpath| + rpath_dupe_count[rpath] += 1 + end + each_linkage_for(file, :rpaths) do |bad_name| + # Strip duplicate rpaths and rpaths rooted in the build directory + next if !bad_name.start_with?(HOMEBREW_TEMP.to_s) && + !bad_name.start_with?(HOMEBREW_TEMP.realpath.to_s) && + (rpath_dupe_count[bad_name] <= 0) + + rpath_dupe_count[bad_name] -= 1 delete_rpath(bad_name, file) end end diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 45163ad2d2..f9dce3fbf5 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -1229,11 +1229,7 @@ class FormulaInstaller keg = Keg.new(formula.prefix) skip_linkage = formula.bottle_specification.skip_relocation? - # TODO: Remove `with_env` when bottles are built with RPATH relocation enabled - # https://github.com/Homebrew/brew/issues/11329 - with_env(HOMEBREW_RELOCATE_RPATHS: "1") do - keg.replace_placeholders_with_locations tab.changed_files, skip_linkage: skip_linkage - end + keg.replace_placeholders_with_locations tab.changed_files, skip_linkage: skip_linkage end sig { params(output: T.nilable(String)).void } From ffb3c9cff9bf56b9f920f6b8ae84d38c2bc34a2b Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Tue, 18 Jan 2022 19:29:58 +0800 Subject: [PATCH 2/2] Fuse the `rpath` loops. We previously looped twice over the `rpath`s, but we actually only need to do that once. --- .../Homebrew/extend/os/mac/keg_relocate.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/keg_relocate.rb b/Library/Homebrew/extend/os/mac/keg_relocate.rb index bb0b726493..25eae0a6cf 100644 --- a/Library/Homebrew/extend/os/mac/keg_relocate.rb +++ b/Library/Homebrew/extend/os/mac/keg_relocate.rb @@ -54,21 +54,20 @@ class Keg change_install_name(bad_name, new_name, file) unless new_name == bad_name end - # Count duplicate rpaths. We need to keep track of this ourselves - # because the MachO data is cached and this cache is not updated - # after modification with #delete_rpath. - rpath_dupe_count = Hash.new { |h, k| h[k] = -1 } - file.rpaths.each do |rpath| - rpath_dupe_count[rpath] += 1 - end + # Keep track of the rpath counts for deletion of duplicates. + # We need to track this here since we cache the MachO data [0] + # and this cache is not updated after modification with #delete_rpath. + # + # [0] See os/mac/mach.rb. + rpath_count = Hash.new { |h, k| h[k] = file.rpaths.count(k) } each_linkage_for(file, :rpaths) do |bad_name| - # Strip duplicate rpaths and rpaths rooted in the build directory + # Strip duplicate rpaths and rpaths rooted in the build directory. next if !bad_name.start_with?(HOMEBREW_TEMP.to_s) && !bad_name.start_with?(HOMEBREW_TEMP.realpath.to_s) && - (rpath_dupe_count[bad_name] <= 0) + (rpath_count[bad_name] == 1) - rpath_dupe_count[bad_name] -= 1 + rpath_count[bad_name] -= 1 delete_rpath(bad_name, file) end end