Issue with mpgen and RUNPATH #19981

issue fanquake openend this issue on September 20, 2020
  1. fanquake commented at 4:58 am on September 20, 2020: member

    Pointed out by @ryanofsky in #19685:

    Still confirming but it seems like this PR might be causing an error on travis: “/home/travis/build/bitcoin/bitcoin/depends/x86_64-pc-linux-gnu/share/../native/bin/mpgen: error while loading shared libraries: libcapnpc-0.7.0.so: cannot open shared object file: No such file or directory” error in https://travis-ci.org/github/bitcoin/bitcoin/jobs/726983595#L2545

    Running CI scripts locally, I’m able to reproduce the error. But if I revert the changes in this PR the error goes away. The difference seems to be caused by cmake no longer setting RUNPATH after this PR. With this PR, RUNPATH is empty, but if this PR is reverted, readelf shows:

    0$ readelf -d depends/x86_64-pc-linux-gnu/native/bin/mpgen | grep RUNPATH
1 0x000000000000001d (RUNPATH)            Library runpath: [/home/russ/src/bitcoin/depends/x86_64-pc-linux-gnu/native/lib]

    I need to dig in more but two questions I have are:

    1. Why did travis not catch this problem when this PR was being worked on or after this PR was merged. Is there a cache that we failed to clear? Is there a cache we should be clearing when we update depends scripts in the future? (The travis error above started happening after I pushed a PR update that bumped the depends/packages/native_libmultiprocess.mk package, so I think that accounts for why a cache might have been avoided in the failing run above).
    1. What change in this PR might be clobbering the cmake RUNPATH? If I had to guess it might be happening because the LDFLAGS environment variable was unset previously, but now it is set
  2. ryanofsky commented at 12:11 pm on September 22, 2020: member

    Here is what I’ve been able to figure out with this issue so far.

    1. I don’t think there’s a travis caching problem. Travis passing on master but failing in the 726983595 build is just a result of travis job 726983595 above executing mpgen in the bitcoin build and current master not doing that. Any changes to funcs.mk changes are also reflected in build ids so if build ids are used correctly, there probably shouldn’t be a caching issue.

    2. Confirmed setting LDFLAGS variable is the thing that causes RUNPATH not to be set. LDFLAGS environment value gets copied into CMAKE_EXE_LINKER_FLAGS cached global value, which otherwise would be empty. So prior to #19685, LDFLAGS/CMAKE_EXE_LINKER_FLAGS value was empty. After #19685 it is “-L/home/russ/src/bitcoin/depends/x86_64-pc-linux-gnu/native/lib”

    3. So far I don’t see a clean fix for this. Partially reverting #19685 and not passing LDFLAGS for native packages is probably not ideal. Overriding LDFLAGS for native_libmultiprocess in particular is hacky. I haven’t figured out exactly how setting LDFLAGS does prevent mpgen RUNPATH from being set. MPGEN target has INSTALL_RPATH_USE_LINK_PATH set to true. If LDFLAGS is empty or set to harmless value like -lm with no -L link path, RUNPATH is set. But as soon as -L is added, RUNPATH disappears. I think this might have to do with logic https://gitlab.kitware.com/cmake/cmake/-/blob/11425041f04fd0945480b8f9e9933d1549b93981/Source/cmComputeLinkInformation.cxx#L1720, but so far I don’t understand it. I might experiment with LIBRARY_PATH environment variable, but even if this works it would need to be set for the native packages, unset for native packages, so would add unappealing complication.

    4. Fastest way to test this is to hardcode capnp package build id so it doesn’t get rebuilt with every change to funcs.mk

    0--- a/depends/funcs.mk
1+++ b/depends/funcs.mk
2@@ -45,6 +45,7 @@ $(eval $(1)_all_dependencies:=$(call int_get_all_dependencies,$(1),$($($(1)_type
3 $(foreach dep,$($(1)_all_dependencies),$(eval $(1)_build_id_deps+=$(dep)-$($(dep)_version)-$($(dep)_recipe_hash)))
4 $(eval $(1)_build_id_long:=$(1)-$($(1)_version)-$($(1)_recipe_hash)-$(release_type) $($(1)_build_id_deps) $($($(1)_type)_id_string))
5 $(eval $(1)_build_id:=$(shell echo -n "$($(1)_build_id_long)" | $(build_SHA256SUM) | cut -c-$(HASH_LENGTH)))
6+$(eval native_capnp_build_id:=f5a3acc1fc3)
7 final_build_id_long+=$($(package)_build_id_long)
8 
9 #compute package-specific paths

    After that resulting RUNPATH can be checked with:

    0rm -rvf depends/x86_64-pc-linux-gnu/native depends/work/staging depends/work/build
1make -C depends MULTIPROCESS=1 native_libmultiprocess_staged
2for f in `find -name mpgen`; do echo == $f ==; readelf -d $f | grep -i path; done

    CMakeCache can be examined with:

    0make -C depends MULTIPROCESS=1 native_libmultiprocess_built
1find -name CMakeCache.txt
  3. ryanofsky commented at 6:24 pm on September 22, 2020: member

    I think I found a decent workaround setting CMAKE_INSTALL_RPATH. If it works I will incorporate it in #19160 and close this issue. Setting LIBRARY_PATH environment variable didn’t have any effect (was just ignored).

    Also posted a question about this upstream https://discourse.cmake.org/t/install-rpath-use-link-path-not-working-when-cmake-exe-linker-flags-ldflags-is-set/1892

  4. ryanofsky referenced this in commit 01a7b77360 on Sep 24, 2020
  5. ryanofsky commented at 1:15 pm on September 25, 2020: member

    Will close this issue. I confirmed 01a7b773606c7304e5ca42f6ff121d818c972361 from #19160 fixes the problem, or at least works around it: https://travis-ci.org/github/bitcoin/bitcoin/jobs/730006938.

    I could make a separate PR for the fix since it’s a build change pretty different than the other changes in #19160. On the other hand, a reason for keeping it in the same PR is that other changes in the PR are needed to verify the fix works on travis. Maybe it would be a good idea in the future for the depends build to have a test target (particularly for native packages, but checks could be done on cross-compiled packages too), so build fixes like 01a7b773606c7304e5ca42f6ff121d818c972361 could be more separable and testable.

  6. ryanofsky closed this on Sep 25, 2020
  7. ryanofsky referenced this in commit da6c9b7089 on Sep 30, 2020
  8. ryanofsky referenced this in commit 9fe5c425fd on Oct 1, 2020
  9. ryanofsky referenced this in commit 429e78a909 on Oct 1, 2020
  10. ryanofsky referenced this in commit 4e99aef80d on Oct 1, 2020
  11. ryanofsky referenced this in commit 3c51b6a5d6 on Oct 1, 2020
  12. ryanofsky referenced this in commit 321aeb0bf1 on Oct 2, 2020
  13. ryanofsky referenced this in commit 7d0271b5c3 on Oct 2, 2020
  14. ryanofsky referenced this in commit 3655f304de on Nov 25, 2020
  15. fanquake referenced this in commit 17918a987a on Dec 10, 2020
  16. sidhujag referenced this in commit 7512d370e3 on Dec 10, 2020
  17. str4d referenced this in commit 52fdab1c9b on Sep 15, 2021
  18. str4d referenced this in commit d1e25055de on Sep 15, 2021
  19. str4d referenced this in commit 26c5678fbc on Sep 29, 2021
  20. str4d referenced this in commit 02f59d7ec5 on Nov 3, 2021
  21. str4d referenced this in commit 5e7c61d80b on Jan 20, 2022
  22. DrahtBot locked this on Feb 15, 2022


fanquake ryanofsky

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 10:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me