Merge pull request #15980 from scpeters/keg_relocate_codesign_once

Speed up keg installation with fewer code-signing calls

Library/Homebrew/extend/os/linux/keg_relocate.rb

@@ -18,7 +18,7 @@ class Keg
   end
 
   def change_rpath(file, old_prefix, new_prefix)
-    return if !file.elf? || !file.dynamic_elf?
+    return false if !file.elf? || !file.dynamic_elf?
 
     updated = {}
     old_rpath = file.rpath
@@ -51,6 +51,7 @@ class Keg
     updated[:interpreter] = new_interpreter if old_interpreter != new_interpreter
 
     file.patch!(interpreter: updated[:interpreter], rpath: updated[:rpath])
+    true
   end
 
   def detect_cxx_stdlibs(options = {})

Library/Homebrew/extend/os/mac/keg_relocate.rb

@@ -21,20 +21,29 @@ class Keg
   def relocate_dynamic_linkage(relocation)
     mach_o_files.each do |file|
       file.ensure_writable do
+        modified = T.let(false, T::Boolean)
+        needs_codesigning = T.let(false, T::Boolean)
+
         if file.dylib?
           id = relocated_name_for(file.dylib_id, relocation)
-          change_dylib_id(id, file)
+          modified = change_dylib_id(id, file)
+          needs_codesigning ||= modified
         end
 
         each_linkage_for(file, :dynamically_linked_libraries) do |old_name|
           new_name = relocated_name_for(old_name, relocation)
-          change_install_name(old_name, new_name, file) if new_name
+          modified = change_install_name(old_name, new_name, file) if new_name
+          needs_codesigning ||= modified
         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
+          modified = change_rpath(old_name, new_name, file) if new_name
+          needs_codesigning ||= modified
         end
+
+        # codesign the file if needed
+        codesign_patched_binary(file) if needs_codesigning
       end
     end
   end
@@ -42,7 +51,11 @@ class Keg
   def fix_dynamic_linkage
     mach_o_files.each do |file|
       file.ensure_writable do
-        change_dylib_id(dylib_id_for(file), file) if file.dylib?
+        modified = T.let(false, T::Boolean)
+        needs_codesigning = T.let(false, T::Boolean)
+
+        modified = change_dylib_id(dylib_id_for(file), file) if file.dylib?
+        needs_codesigning ||= modified
 
         each_linkage_for(file, :dynamically_linked_libraries) do |bad_name|
           # Don't fix absolute paths unless they are rooted in the build directory.
@@ -52,7 +65,8 @@ class Keg
             fixed_name(file, bad_name)
           end
           loader_name = loader_name_for(file, new_name)
-          change_install_name(bad_name, loader_name, file) if loader_name != bad_name
+          modified = change_install_name(bad_name, loader_name, file) if loader_name != bad_name
+          needs_codesigning ||= modified
         end
 
         each_linkage_for(file, :rpaths) do |bad_name|
@@ -60,7 +74,8 @@ class Keg
           loader_name = loader_name_for(file, new_name)
           next if loader_name == bad_name
 
-          change_rpath(bad_name, loader_name, file)
+          modified = change_rpath(bad_name, loader_name, file)
+          needs_codesigning ||= modified
         end
 
         # Strip duplicate rpaths and rpaths rooted in the build directory.
@@ -69,8 +84,12 @@ class Keg
         each_linkage_for(file, :rpaths, resolve_variable_references: true) do |bad_name|
           next if !rooted_in_build_directory?(bad_name) && file.rpaths.count(bad_name) == 1
 
-          delete_rpath(bad_name, file)
+          modified = delete_rpath(bad_name, file)
+          needs_codesigning ||= modified
         end
+
+        # codesign the file if needed
+        codesign_patched_binary(file) if needs_codesigning
       end
     end
 

Library/Homebrew/os/mac/keg.rb

@@ -2,13 +2,14 @@
 # frozen_string_literal: true
 
 class Keg
+  sig { params(id: String, file: Pathname).returns(T::Boolean) }
   def change_dylib_id(id, file)
-    return if file.dylib_id == id
+    return false if file.dylib_id == id
 
     @require_relocation = true
     odebug "Changing dylib ID of #{file}\n  from #{file.dylib_id}\n    to #{id}"
     file.change_dylib_id(id, strict: false)
-    codesign_patched_binary(file)
+    true
   rescue MachO::MachOError
     onoe <<~EOS
       Failed changing dylib ID of #{file}
@@ -18,13 +19,14 @@ class Keg
     raise
   end
 
+  sig { params(old: String, new: String, file: Pathname).returns(T::Boolean) }
   def change_install_name(old, new, file)
-    return if old == new
+    return false if old == new
 
     @require_relocation = true
     odebug "Changing install name in #{file}\n  from #{old}\n    to #{new}"
     file.change_install_name(old, new, strict: false)
-    codesign_patched_binary(file)
+    true
   rescue MachO::MachOError
     onoe <<~EOS
       Failed changing install name in #{file}
@@ -34,13 +36,14 @@ class Keg
     raise
   end
 
+  sig { params(old: Pathname, new: Pathname, file: MachOShim).returns(T::Boolean) }
   def change_rpath(old, new, file)
-    return if old == new
+    return false if old == new
 
     @require_relocation = true
     odebug "Changing rpath in #{file}\n  from #{old}\n    to #{new}"
     file.change_rpath(old, new, strict: false)
-    codesign_patched_binary(file)
+    true
   rescue MachO::MachOError
     onoe <<~EOS
       Failed changing rpath in #{file}
@@ -50,10 +53,11 @@ class Keg
     raise
   end
 
+  sig { params(rpath: String, file: MachOShim).returns(T::Boolean) }
   def delete_rpath(rpath, file)
     odebug "Deleting rpath #{rpath} in #{file}"
     file.delete_rpath(rpath, strict: false)
-    codesign_patched_binary(file)
+    true
   rescue MachO::MachOError
     onoe <<~EOS
       Failed deleting rpath #{rpath} in #{file}