depends: Set CMAKE_INSTALL_RPATH for native packages#20046
depends: Set CMAKE_INSTALL_RPATH for native packages#20046fanquake merged 1 commit intobitcoin:masterfrom
Conversation
After bitcoin#19685 started setting LDFLAGS, the INSTALL_RPATH_USE_LINK_PATH cmake option used in the libmultiprocess build no longer works, so it is neccessary to set CMAKE_INSTALL_RPATH as a fallback. It's unclear currently whether the bad interaction between INSTALL_RPATH_USE_LINK_PATH and LDFLAGS is a bug, but the issue is reported: bitcoin#19981 (comment) https://discourse.cmake.org/t/install-rpath-use-link-path-not-working-when-cmake-exe-linker-flags-ldflags-is-set/1892 Commands useful for building / testing this change make -C depends MULTIPROCESS=1 print-libmultiprocess_cmake make -C depends MULTIPROCESS=1 print-native_libmultiprocess_cmake make -C depends MULTIPROCESS=1 HOST=x86_64-apple-darwin16 print-libmultiprocess_cmake rm -rvf depends/x86_64-pc-linux-gnu/native depends/work/staging depends/work/build make -C depends MULTIPROCESS=1 V=1 native_libmultiprocess_staged for f in `find -name mpgen`; do echo == $f ==; readelf -d $f | grep -i path; done make -C depends MULTIPROCESS=1 V=1 native_libmultiprocess_built find -name CMakeCache.txt Fixes bitcoin#19981
|
This commit was originally part of #19160 but I'm making it a standalone PR because it seems to be causing travis timeouts on the macOS 10.12 build that I don't know how to fix: |
|
Reproduced the travis macOS 10.12 timeout again: @MarcoFalke do you know any way to debug or fix this? I don't think this PR is actually changing the build results. I think maybe all it is doing is invalidating the travis cache, and that macOS 10.12 travis build is too slow to update the cache. Does this sound right? |
|
The build is probably failing altogether. For temporary debugging, you can remove the |
|
Thanks @MarcoFalke! It still looks to me that this PR might be uncovering a pre-existing problem with the macOS 10.12 build, and not actually causing the failure. Removing >/dev/null in d9dc091 shows the error "configure: error: Qt5Core >= 5.5.1 not found" (https://travis-ci.org/github/bitcoin/bitcoin/jobs/731743605#L2010) with debug output: (https://travis-ci.org/github/bitcoin/bitcoin/jobs/731743605#L5266) It's good to have a starting point for debugging this, but am I wrong that this problem could be happening in master but masked by travis caching? @dongcarl's PR #19683 also has a failing macOS 10.12 build https://travis-ci.org/github/bitcoin/bitcoin/jobs/730372585, but it seems to be failing with a different problem a little earlier so it's unclear if this Qt5Core error would happen there |
|
FWIW I'm fairly certain the failure in #19683 is unrelated. As it's due to over-specifying |
I'm also sure it's unrelated. But since it happens before my error, if you fix it you might see the same problem there |
|
It does appear the Qt5Core error is not related to the change in this PR. If I revert the original commit da6c9b7, and just add a comment to funcs.mk 7472ee7, the error still happens:
However, it turns out the Qt5Core error isn't limited to the macOS 10.12 build, but also happens on the CentOS 7 build: Which suggests the Qt5Core error is not related to the original macOS 10.12 timeouts #20046 (comment) and #20046 (comment), but maybe is caused by removing >/dev/null in d9dc091, or maybe is caused by more recent changes to master, or caused by other changes on travis. I guess I will try reverting the >/dev/null change d9dc091 to see if the timeout returns. |
…(comment)" This reverts commit d9dc091.
|
I think I see why the Qt5Core error is happening. I was trying to debug in d9dc091 by replacing |
ryanofsky
force-pushed
the
pr/ipc-rpath
branch
2 times, most recently
from
e5d6dd0 to
9fe5c42
Compare
|
After pushing e5d6dd0, was able to reproduce the original macOS 10.12 error and get debug information: https://travis-ci.org/github/bitcoin/bitcoin/jobs/731786312#L4517. The error was Added 1 commits da6c9b7 -> d9dc091 ( |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
ryanofsky
force-pushed
the
pr/ipc-rpath
branch
from
9fe5c42 to
3c51b6a
Compare
ryanofsky
force-pushed
the
pr/ipc-rpath
branch
from
3c51b6a to
7d0271b
Compare
Guix builds
|
|
ACK 7d0271b - I haven't looked in depth, but I've re-read through #19981 and checked the failure by testing #19160 (with this reverted): CXX bitcoin_node-bitcoind.o
CXX init/bitcoin_node-bitcoin-node.o
GEN ipc/capnp/echo.capnp.c++
/home/ubuntu/bitcoin/depends/x86_64-pc-linux-gnu/native/bin/mpgen: error while loading shared libraries: libcapnpc-0.7.0.so: cannot open shared object file: No such file or directory
make[2]: *** [/home/ubuntu/bitcoin/depends/x86_64-pc-linux-gnu/native/include/mpgen.mk:4: ipc/capnp/echo.capnp.c++] Error 127Hopefully our upstream issue will get a reply at some point. Will wait for Carl to comment. |
|
Thanks for looking fanquake. Note original version of this PR was more complicated and ugly. Now that it's is just setting CMAKE_INSTALL_RPATH for native packages, I think it's pretty clean and straightforward. It may even a be good thing to set rpath explicitly and be clear the build looks for depends libraries ahead of system libraries |
|
ACK 7d0271b Looked into this a bit, it makes sense that for the things we build in depends, we want the library search to start in depends. It seems reasonable to expect this to happen automatically when I looked into the CMake codebase a bit, but couldn't find a definite culprit for the weird CMAKE_INSTALL_RPATH_USE_LINK_PATH behaviour. It seems unintended, but we'll see what the forums say I guess. |
…ages 7d0271b depends: Set CMAKE_INSTALL_RPATH for native packages (Russell Yanofsky) Pull request description: This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). --- After bitcoin#19685 started setting `LDFLAGS`, the `INSTALL_RPATH_USE_LINK_PATH` cmake option used in the libmultiprocess build no longer works, so it is neccessary to set `CMAKE_INSTALL_RPATH` as a fallback. It's unclear currently whether the bad interaction between `INSTALL_RPATH_USE_LINK_PATH` and `LDFLAGS` is a bug, but the issue is reported: - bitcoin#19981 (comment) - https://discourse.cmake.org/t/install-rpath-use-link-path-not-working-when-cmake-exe-linker-flags-ldflags-is-set/1892 Fixes bitcoin#19981 ACKs for top commit: fanquake: ACK 7d0271b - I haven't looked in depth, but I've re-read through bitcoin#19981 and checked the failure by testing bitcoin#19160 (with this reverted): dongcarl: ACK 7d0271b Looked into this a bit, it makes sense that for the things we build in depends, we want the library search to start in depends. It seems reasonable to expect this to happen automatically when `CMAKE_INSTALL_PREFIX` and `INSTALL_RPATH_USE_LINK_PATH` are set, but oh well... Tree-SHA512: 97cc5801c3204c14cd33004423631456ca0701e2127ee5146810a76e2f4aac9de1f4b5437402a4329cda54e022dc99270fee7e38c2995765f36b3848215fa78e
5 participants
This PR is part of the process separation project.
After #19685 started setting
LDFLAGS, theINSTALL_RPATH_USE_LINK_PATHcmake option used in the libmultiprocess build no longer works, so it is neccessary to setCMAKE_INSTALL_RPATHas a fallback.It's unclear currently whether the bad interaction between
INSTALL_RPATH_USE_LINK_PATHandLDFLAGSis a bug, but the issue is reported:Fixes #19981