Fix cgo rpath computation for prebuilt dynamic libraries#2869
Fix cgo rpath computation for prebuilt dynamic libraries#2869jayconrod merged 1 commit intobazel-contrib:masterfrom TimothyGu:cgo-rpath
Conversation
|
Sorry for the slow review on this. The fix looks good. The one thing I'm nervous about is checking in binaries as part of the tests. We have no way of reviewing that code, and if it stops working, we have no way of fixing it. I'm guessing we can't test this fix with binaries that are generated at build time, since they'll be in generated directories? If that's the case, I'd be fine with marking the test with cc @steeve also, who knows this code better than I do at this point. |
Previously, we assumed that any dynamically linked libraries are in execroot. This may not be the case if the library is NOT generated. However, the library should always be in runfiles. Fixes #2867
|
@jayconrod Done. Created a |
jayconrod
left a comment
There was a problem hiding this comment.
Looks good, thanks for fixing this.
I think the RBE failure was already corrected on master, so I'll go ahead and submit this.
… required (bazel-contrib#2869) An upcoming change in Bazel makes the test toolchain required, which means a compatible exec platform amongst toolchains must be found (bazelbuild/bazel@2780393). Some analysis tests of `py_test` force the target platform to a specific platform, but before this change didn't register a compatible exec platform. This can be fixed by registering the target platform as an exec platform. Since Python targets currently depend on a C++ toolchain through Bazel's `launcher` and `launcher_maker` and the default toolchain can't cross-compile to Linux, the host platform still needs to be kept at highest priority to ensure that cross-compilation isn't needed on macOS. Work towards bazel-contrib#2850
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
Previously, we assumed that any dynamically linked library exists in execroot. This is the case if the library is NOT generated (e.g., prebuilt third-party SDKs). On the other hand, the library should always be symlinked in runfiles of the executable, so use that as the basis for rpath computation.
Which issues(s) does this PR fix?
Fixes #2867
Other notes for review