Ignore duplicate rpaths in #change_rpath (#438)

This causes a deviation in behaviour from `install_name_tool`, which will happily change an existing rpath to one that already exists. Closes #436. Co-authored-by: William Woodruff <[email protected]>
Homebrew · Mar 3, 2022 · e9eaa75 · e9eaa75
1 parent 680949d
commit e9eaa75
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 6 deletions.
diff --git a/lib/macho/macho_file.rb b/lib/macho/macho_file.rb
@@ -381,11 +381,9 @@ def rpaths
     #  rpaths simultaneously.
     # @return [void]
     # @raise [RpathUnknownError] if no such old runtime path exists
-    # @raise [RpathExistsError] if the new runtime path already exists
     def change_rpath(old_path, new_path, options = {})
       old_lc = command(:LC_RPATH).find { |r| r.path.to_s == old_path }
       raise RpathUnknownError, old_path if old_lc.nil?
-      raise RpathExistsError, new_path if rpaths.include?(new_path)
 
       new_lc = LoadCommands::LoadCommand.create(:LC_RPATH, new_path)
 

diff --git a/test/test_macho.rb b/test/test_macho.rb
@@ -415,6 +415,14 @@ def test_change_rpath
       # there should be at least one rpath in each binary
       refute_empty rpaths
 
+      # We should ignore errors when changing to an existing rpath
+      # This is the same behaviour as `install_name_tool`
+      file.change_rpath(rpaths.first, rpaths.first)
+      new_rpaths = file.rpaths
+
+      assert_equal new_rpaths.first, rpaths.first
+      refute_empty new_rpaths.first, rpaths.first
+
       file.change_rpath(rpaths.first, "/usr/lib")
       new_rpaths = file.rpaths
 
@@ -574,10 +582,6 @@ def test_rpath_exceptions
       file.change_rpath("/this/rpath/doesn't/exist", "/lib")
     end
 
-    assert_raises MachO::RpathExistsError do
-      file.change_rpath(file.rpaths.first, file.rpaths.first)
-    end
-
     assert_raises MachO::RpathExistsError do
       file.add_rpath(file.rpaths.first)
     end