Macos cc_wrapper: Use otool to read install name #13313
Conversation
jlaxson
force-pushed
the
osx-cc-wrapper-read-install-name
branch
from
0f7caf7
to
06bddbf
Compare
|
I'm not terribly familiar with this script or its history; maybe @trybka has some insight. |
|
note this logic will have to be duplicated in https://github.com/bazelbuild/bazel/blob/master/tools/cpp/osx_cc_wrapper.sh |
|
I'm not super familiar with this TBH, but the change looks reasonable to me. |
|
@keith's point about duplicating the changes to the other script still stand though. :-) |
jlaxson
force-pushed
the
osx-cc-wrapper-read-install-name
branch
from
06bddbf
to
ae64118
Compare
|
Rebased and added the change to the non-tpl copy of the script |
There was a problem hiding this comment.
works for our use case with swift-syntax, here's the diff of otool -L in that case:
bazel-bin/tools/deadcode/deadcode:
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 905.6.0)
- @rpath/lib_InternalSwiftSyntaxParser.dylib (compatibility version 1.0.0, current version 1300.0.31)
+ @loader_path/../../_solib_darwin_x86_64/_U@com_Ugithub_Ukeith_Uswift_Usyntax_S_S_Clibrary___U/lib_InternalSwiftSyntaxParser.dylib (compatibility version 1.0.0, current version 1300.0.31)
|
@trybka can you take another look at this one? |
|
Shouldn't the OTOOL var appear in the .sh as well as the .sh.tpl file? |
| # Get the path of a lib inside a tool | ||
| function get_otool_path() { | ||
| # the lib path is the path of the original lib relative to the workspace | ||
| get_realpath $1 | sed 's|^.*/bazel-out/|bazel-out/|' | ||
| ${OTOOL} -D "$1" | awk 'NR==2{print $1}' | sed 's|^.*/bazel-out/|bazel-out/|' |
There was a problem hiding this comment.
| ${OTOOL} -D "$1" | awk 'NR==2{print $1}' | sed 's|^.*/bazel-out/|bazel-out/|' | |
| /usr/bin/otool -D "$1" | awk 'NR==2{print $1}' | sed 's|^.*/bazel-out/|bazel-out/|' |
sgowroji
added
team-Rules-ObjC
|
Hello @jlaxson, Could you please resolve the above branch conflicts and reply to the given review comments. Thanks! |
|
I submitted a rebased version here #16542 |
|
Hi there! Thank you for contributing to the Bazel repository. We appreciate your time and effort. We're doing a clean up of old PRs and will be closing this since a new one has been submitted. Please feel free to reopen if you'd like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so. |
|
should be fine, #16542 should be landed instead |
When linking an executable on darwin, cc_wrapper.sh contains functionality to ensure rpath entries and shared library install names are relative and match the intended paths that will be used in downstream actions. This depends on being able to traverse symlinks from the _solibs path (like
_solib_darwin_x86_64/_U_S_S_Cgenlib_Uimport___U/libgenlib.so) to the actual file (likebazel-out/darwin-opt-exec-31C2CD59/bin/libgenlib.so). This also assumes that the library has its internal install name equal to that latter path - this seems to be generally true right now, but is maybe a bit brittle.This system breaks down when doing linking on a remote executor, because Bazel does not encode symlinks, and so the file shows up in
_solib_darwin_x86_64as a real file, and the wrapper picks the wrong install name. The resulting executable works when run directly from bazel-bin/bazel-out on a local machine, but will not work viabazel run, or when used in remote actions.This change uses otool to read the install name of the target library, which is guaranteed to match what was embedded into the executable during link time. This result will be immune to the peculiarities of how symlinks are used and what install_name is encoded into the shared library during its creation.