Been a while since I've last looked at this formula, and I see there have been quite a few changes. A few questions came up while reading through the new formula:
otool, but I don't have code that can test that.My apologies if this seems accusatory; my wording isn't the greatest...
You'll probably want to chat to @maxim-belkin. A lot of my work recently was built on top of their's rather than more fundamental changes.
Regarding your fix: I'm trying it now (It'll take me about an hour). Looks like it should allow to get rid of the necessity to use -Wl,-rpath,#{lib} when using LLVM's libc++, right? I'll reply here.
So, the change seems fine to me: with it, the test for LLVM's libc++ seems to pass fine without -Wl,-rpath,#{lib}. I can file a PR but first I'd like to get an opinion from @UniqMartin
Also a question for @MikeMcQuaid: will we be able to patch resources in the (near) future?
Awesome! Glad to know it appears to work for other people
Also a question for @MikeMcQuaid: will we be able to patch resources in the (near) future?
Unless you submit a PR (which you should because you've done great work in the past): I don't think so, no, sorry.
Please let me know if I misunderstood what is being asked, but my understanding is that the fix (workaround) will address the RPATH problem, but there are still two problems remaining:
LC_RPATH command that is now useless (minor).libc++abi from the system and from the llvm formula (major).Ideally, I'd like to see someone passionate about this work with upstream on solving this, so that we can install an unpatched and unhacked LLVM in the future. Or if this double linking isn't a problem (contrary to what the upstream libc++ TODO says), I'd like to understand why it's fine.
Hmmm, those are good points.
I asked why RPATH was hardcoded some time ago on the LLVM dev mailing list, but never got a response. Might ask again once I grok what's going on a bit more.
Is RPATH even necessary, given we know exactly where we're putting the library?
I'll look at the double linking too, but I'm not sure how much I can help. Long build times help, too...
Oh, and I guess I should ask: Which libc++abi is the "correct" one to link against? My first guess would be the Homebrew one, as it was built at the same time.
Is RPATH even necessary, given we know exactly where we're putting the library?
It's not needed at all if all libraries are linked using absolute paths, or more precisely if none of them are referenced with @rpath/<some-library>.dylib.
Which libc++abi is the "correct" one to link against?
The one provided by the formula, if we're building it at all and want to keep it optional, otherwise the system one. Both are valid choices as far as I understand this and both should be supported by libc++, just not at the same time.
Might the RPATH issue be a dupe of brew issue #371? Looks like it's about executables, though, so not sure whether that would call for significantly different changes. Makes me wonder why LLVM still deals with RPATHS if they haven't been used much for a while.
Looks like something in the llvm build can do something with the RPATH though? This is from a build where CMAKE_INSTALL_WITH_INSTALL_RPATH was set to OFF instead of ON:
Changing dylib ID of /usr/local/Cellar/llvm/HEAD-c61cf90/lib/libLTO.dylib
from @rpath/libLTO.dylib
to /usr/local/opt/llvm/lib/libLTO.dylib
Changing dylib ID of /usr/local/Cellar/llvm/HEAD-c61cf90/lib/libc++.1.0.dylib
from @rpath/libc++.1.dylib
to /usr/local/opt/llvm/lib/libc++.1.dylib
Changing dylib ID of /usr/local/Cellar/llvm/HEAD-c61cf90/lib/libc++abi.1.0.dylib
from @rpath/libc++abi.1.dylib
to /usr/local/opt/llvm/lib/libc++abi.1.dylib
Changing dylib ID of /usr/local/Cellar/llvm/HEAD-c61cf90/lib/libclang.dylib
from @rpath/libclang.dylib
to /usr/local/opt/llvm/lib/libclang.dylib
Don't remember seeing this for previous installs, but I could easily have missed it. Unfortunately, changing the option didn't help with rpaths either...
Might the RPATH issue be a dupe of brew issue #371? Looks like it's about executables, though, so not sure whether that would call for significantly different changes.
No, that's about how Homebrew handles (or currently doesn't handle) RPATHs. But that's just a future post-processing step that wouldn't be able to fix up the LLVM binaries afterwards. It applies equally to all Mach-O binaries, not just executables …
Makes me wonder why LLVM still deals with RPATHS if they haven't been used much for a while.
RPATHs have valid use cases, though mostly in scenarios where a bunch of binaries should be easily relocatable, which is not very relevant for a typical Homebrew installation.
Looks like something in the llvm build can do something with the RPATH though? This is from a build where
CMAKE_INSTALL_WITH_INSTALL_RPATHwas set toOFFinstead ofON:
That's output from Homebrew you will only see when building with --debug. It's fixing up those install names and dylib IDs that it can fix safely, which is mostly relevant with software that hasn't been fully ported to macOS and thus isn't aware of the somewhat different handling of dynamic linking for this operating system.
LLVM shouldn't be among those, as it's low-level enough to be properly adjusted to the different operating systems it supports officially. I'm not even saying that LLVM is doing it wrong (I'm certainly not above all those compiler and linker experts working on LLVM on a daily basis), it's just that it seems to be suboptimal for how we would like to distribute it via Homebrew. (And the double linking is objectively wrong as acknowledged in the TODO.txt of the libcxxabi project.)
RPATHs have valid use cases, though mostly in scenarios where a bunch of binaries should be easily relocatable, which is not very relevant for a typical Homebrew installation.
Guess it makes sense why LLVM is using it, then. In that case it seems easiest to go with LLVM's setup and figure out how to get rid of that extra LC_RPATH.
That's output from Homebrew you will only see when building with
--debug.
I use --debug as a hack to edit CMakeLists.txt before the build starts, so that would explain why I haven't seen the dylib ID fixes before.
It's fixing up those install names and dylib IDs that it can fix safely, which is mostly relevant with software that hasn't been fully ported to macOS and thus isn't aware of the somewhat different handling of dynamic linking for this operating system.
Isn't fixing install names/dylib IDs something for install_name_tool?
Isn't fixing install names/dylib IDs something for
install_name_tool?
Yes and that's what is used by Homebrew behind the scenes to perform the changes, once it determines a change is necessary. (The reality is a bit more complicated as we're in the process of transitioning away from install_name_tool/otool to a pure-Ruby version as part of one of our Google Summer of Code projects this year, but that's just an implementation detail.)
Hm, and that mechanic can't be easily extended to fixing rpaths for other dylibs being referenced by the dylibs already being altered?
I'm really not familiar enough with the Homebrew codebase to be doing this kind of thing :(
Hm, and that mechanic can't be easily extended to fixing rpaths for other dylibs being referenced by the dylibs already being altered?
Unfortunately no, because the Homebrew code only performs modifications that can be done safely just by looking at the binaries. RPATHs and library references that involve @rpath/ are expanded at run time and multiple binaries can contribute to the list of RPATHs that will be considered by the dynamic linker, thus this cannot be (safely) analyzed statically and thus not automatically fixed without an understanding of what the package author actually intended.
I'm really not familiar enough with the Homebrew codebase to be doing this kind of thing :(
You don't have to. I'm happy to supply all the relevant information that is needed. What we really need is someone who's passionate about LLVM, ideally has already established upstream contact, has sufficient understanding of static and dynamic linking on macOS, and is somewhat familiar with CMake and how it is used by LLVM.
Ideally, the LLVM build system would be flexible enough to either create an installation that is relocatable and that uses RPATHs or to allow installation in a fixed location with all binaries and libraries linked using absolute paths. The latter is what would be useful for Homebrew and should (in my understanding) be somewhat similar to how LLVM gets installed as a system-level tool on Linux distributions. In my opinion, someone needs to discuss this with upstream, find out how upstream feels about this and how this can be made a reality or find out if there are already other means to achieve our goal, etc. (This someone could be me, if I had enough free time—sadly not a reality at the moment.)
RPATHs and library references that involve @rpath/ are expanded at run time and multiple binaries can contribute to the list of RPATHs that will be considered by the dynamic linker, thus this cannot be (safely) analyzed statically and thus not automatically fixed without an understanding of what the package author actually intended.
Makes sense.
Might go for mailing the LLVM devs if my schedule turns out to be ok. Don't quite fit your criteria, but hopefully it's better than nothing :)
I found a solution but it depends on the ability to patch resources. how ironic...
You can do limited patching of resources with inreplace.
You could also vendor a patch as a resource, install it into another resource's stage & use system "patch", "-i", patchfile. It's not pretty & kind of grim, but if it's resolving a heinous enough problem it's not the worst thing in the world.
Thanks! Will use this approach! It doesn't look that grim to me, actually ;)
Wait, inreplace works on binaries too?
inreplace is effectively a slightly fancy gsub. Whatever gsub can do, inreplace should be able to do.
My local test is finishing up but it looks like it will be successful: libc++.dylib will refer only to LLVM's libc++abi.dylib. However, libc++abi.dylib itself still refers to system's libc++abi.dylib.
argh
Wed Jul 20 | 14:16:33 $ otool -L /usr/local/Cellar/llvm/3.8.1/lib/libc++.dylib
/usr/local/Cellar/llvm/3.8.1/lib/libc++.dylib:
/usr/local/opt/llvm/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1.0.0)
@rpath/libc++abi.1.dylib (compatibility version 1.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
@rpath/libunwind.1.dylib (compatibility version 1.0.0, current version 1.0.0)
Testing my solution with -DCMAKE_MACOSX_RPATH=0
didn't help
If you haven't already, might want to grep through all the CMakeLists. It's possible CMAKE_MACOSX_RPATH or something related to that is hardcoded
I think this discussion has died off for now, so closing.
Most helpful comment
I found a solution but it depends on the ability to patch resources. how ironic...