extend/os/mac/keg_relocate: fix post-bottling dylib ID relocation

[carlocab]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

Running brew bottle changes dylib IDs, install names, and rpaths into
placeholders for the bottle, creates a bottle tarball, and then changes
the placeholders back to their correct values.

With my refactoring in #11358, the behaviour of this relocation changed:
dylib IDs would no longer be changed back from placeholders into their
correct values after the creation of the bottle tarball.

[BrewTestBot]

Review period skipped due to critical label.

[Bo98]

Is this affecting users who are installing bottles?

[carlocab]

No, it isn't. It'll only affect you if you do

brew bottle foo

without subsequently trying to install from the bottle you just created. This is why this has caused no problems in CI because we get rid of the broken install immediately then install from the newly created bottle.

In particular, this should only impact users who use their own bottling infrastructure with unconventional configurations. These users should be unaffected:

1. users who install from bottles from homebrew/core
2. users who build from source without running brew bottle
3. users who build bottles using a standard brew test-bot workflow

[MikeMcQuaid]

Good catch on the fix 👏🏻

[hahnsteve]

I've run into an issue, as a "user who uses their own bottling infrastructure with unconventional configurations"... for various complicated reasons, my "bottling infrastructure" is "complicated" in the sense that the bottles for my in-house Tap are not actually built by brew but are just tarballs that are assembled with the same directory structure and filename as bottles produced by brew. A few of our packages have dylibs and they are now erroring out:

==> Changing dylib ID of <file>.dylib
  from <hardcoded path>.dylib
    to
Error: new ID must be a String

If I unzip the bottle, manually use install_name_tool to change the id to @@HOMEBREW_PREFIX@@/<etc> and zip it back up then brew is happy to install them. I might be able to get the team that builds these files to incorporate this change, but I wanted to also provide my feedback here. I'm an edge case for sure and I'm not demanding the Homebrew team fix this for me, I just thought this might be useful info.

[carlocab]

This patch should work for you:

diff --git a/Library/Homebrew/extend/os/mac/keg_relocate.rb b/Library/Homebrew/extend/os/mac/keg_relocate.rb
index c2bfdc8c8..4581d4681 100644
--- a/Library/Homebrew/extend/os/mac/keg_relocate.rb
+++ b/Library/Homebrew/extend/os/mac/keg_relocate.rb
@@ -23,7 +23,7 @@ class Keg
       file.ensure_writable do
         if file.dylib?
           id = relocated_name_for(file.dylib_id, relocation)
-          change_dylib_id(id, file)
+          change_dylib_id(id, file) if id
         end
 
         each_linkage_for(file, :dynamically_linked_libraries) do |old_name|

However, I'm not sure it's a good idea to be brew installing bottle tarballs that weren't actually built by brew. Is there any reason why you use brew to install mock bottle tarballs without actually building them from brew formulae?

Because if you do build them from brew formulae, then you wouldn't need the workaround I show in the patch above.

[MikeMcQuaid]

However, I'm not sure it's a good idea to be brew installing bottle tarballs that weren't actually built by brew. Is there any reason why you use brew to install mock bottle tarballs without actually building them from brew formulae?

It definitely is not a good idea (or at least: you should actively expect things to break when doing this and you'll need to fix these problems yourself).

[hahnsteve]

I agree 100% and I hope I've been clear I'm not demanding a fix for my completely oddball non-standard use case. Without getting too far into the weeds, our Mac clients used to run a script that would download and install our internal tools one-by-one, based on a JSON endpoint that provides download links/version numbers/etc. It was prone to error, took hours to complete, and was completely unworkable once we were all on VPN. I wrote a Homebrew external command to translate that JSON into Formulae so that brew can handle installation and updates. The tarballs we consume contain compiled code, but brew doesn't know this because the tarballs aren't bottles. This worked fine until we had a large team upgrade to Big Sur while postponing upgrading from Xcode 11 for several months. As a workaround my external command now downloads the tarballs and converts them into pseudo-bottles before handing them off to brew install, in order to head off the "your Xcode is too old" errors.

As for why on earth all of this is happening on endpoint devices instead of in a CI/CD platform... very good question. Our whole dev platform is being migrated to GitHub, at which point we'll be able to do this the right way, but in the (hopefully short) meantime the existing platform basically "is what it is" and I have to make do as best I can.