change_rpath differs in behaviour from install_name_tool when handling duplicates

[carlocab]

Suppose I compile a dylib with a duplicate RPATH:

❯ clang -xc /dev/null -shared -rpath dupe -rpath dupe -o libdupes.dylib
❯ otool -l libdupes.dylib | rg -A2 LC_RPATH
          cmd LC_RPATH
      cmdsize 24
         path dupe (offset 12)
--
          cmd LC_RPATH
      cmdsize 24
         path dupe (offset 12)

(Admittedly, I don't know why one would do this, but pdnsrec still does.)

change_rpath handles changing one RPATH just fine, but chokes on the second:

❯ brew ruby -e 'MachO::Tools.change_rpath("libdupes.dylib", "dupe", "notdupe")'
❯ otool -l libdupes.dylib | rg -A2 LC_RPATH
          cmd LC_RPATH
      cmdsize 24
         path notdupe (offset 12)
--
          cmd LC_RPATH
      cmdsize 24
         path dupe (offset 12)
❯ brew ruby -e 'MachO::Tools.change_rpath("libdupes.dylib", "dupe", "notdupe")'
/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/ruby-macho-3.0.0/lib/macho/macho_file.rb:388:in `change_rpath': notdupe already exists (MachO::RpathExistsError)
        from /usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/ruby-macho-3.0.0/lib/macho/tools.rb:60:in `change_rpath'
        from -e:1:in `<main>'

install_name_tool chugs along just fine, though:

❯ install_name_tool -rpath dupe notdupe libdupes.dylib
❯ otool -l libdupes.dylib | rg -A2 LC_RPATH
          cmd LC_RPATH
      cmdsize 24
         path notdupe (offset 12)
--
          cmd LC_RPATH
      cmdsize 24
         path notdupe (offset 12)

This is slightly related to changes we made in #362 and #366, and I think I realised this shortly after, but this slipped off my radar.

It might be that this is intentional and nothing needs changing here, but I just wanted to bring this up just in case you want ruby-macho to behave more look install_name_tool here.

[woodruffw]

Yeah, I think #362 caused the deviation.

That change_rpath behavior is definitely not ideal, IMO: we should either change exactly one RPATH always (if possible) or change all of them at once, and never fail if some change is possible.

The MachO::Tool namespace is intended to imply some level of compatibility with otool and install_name_tool, so we should probably do what install_name_tool does in this case (even though I think that changing all RPATHs at once is more reasonable behavior).

[carlocab]

To be fair, before #362 there was a different deviation.

Edit: No, I think the below is not true. I was misremembering how change_rpath works.

---

Note that emulating install_name_tool here might require rethinking how change_rpath works: currently, change_rpath deletes an rpath and then adds one. ruby-macho fails at the second step because it refuses to add an rpath that already exists.

This is also how install_name_tool behaves:

❯ otool -l libdupes.dylib | rg -A2 LC_RPATH
          cmd LC_RPATH
      cmdsize 24
         path dupe3 (offset 12)
--
          cmd LC_RPATH
      cmdsize 24
         path dupe (offset 12)
❯ install_name_tool -add_rpath dupe libdupes.dylib
error: /Library/Developer/CommandLineTools/usr/bin/install_name_tool: for: libdupes.dylib (for architecture x86_64) option "-add_rpath dupe" would duplicate path, file already has LC_RPATH for: dupe

I suppose you could have a special ignore_duplicates flag for add_rpath that's used inside change_rpath.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.