catkin icon indicating copy to clipboard operation
catkin copied to clipboard

Generated setup_cached.sh file excludes LD_LIBRARY_PATH of current package

Open mikepurvis opened this issue 3 years ago • 1 comments

We started noticing this recently as part of some experiments in migrating to colcon as our primary build tool, which defaults to isolated installspaces. A trivial package that builds one lib:

project(catkin_test_issue)
find_package(catkin REQUIRED)
catkin_package(LIBRARIES ${PROJECT_NAME})
add_library(${PROJECT_NAME} src/lib.cpp)

On first build of this (just catkin_make), we have:

$ cat build/catkin_generated/setup_cached.sh
#!/usr/bin/env sh
# generated from catkin/python/catkin/environment_cache.py

# based on a snapshot of the environment before and after calling the setup script
# it emulates the modifications of the setup script without recurring computations

# new environment variables

# modified environment variables
export CMAKE_PREFIX_PATH="/home/administrator/ws/devel:$CMAKE_PREFIX_PATH"
export PATH='/opt/clearpath/x.y/bin:/home/administrator/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/usr/share/clearpath/bin'
export PWD='/home/administrator/ws/build'
export ROS_PACKAGE_PATH="/home/administrator/ws/src:$ROS_PACKAGE_PATH"

However, if we make a small change and rebuild, then suddenly the cached environment is aware of a develspace lib directory:

$ cat build/catkin_generated/setup_cached.sh
....
# modified environment variables
export CMAKE_PREFIX_PATH="/home/administrator/ws/devel:$CMAKE_PREFIX_PATH"
export LD_LIBRARY_PATH="/home/administrator/ws/devel/lib:$LD_LIBRARY_PATH"
export PATH='/opt/clearpath/x.y/bin:/home/administrator/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/usr/share/clearpath/bin'
export PKG_CONFIG_PATH="/home/administrator/ws/devel/lib/pkgconfig:$PKG_CONFIG_PATH"
export PWD='/home/administrator/ws/build'
export ROS_PACKAGE_PATH="/home/administrator/ws/src:$ROS_PACKAGE_PATH"

I think the reason this has flown under the radar for so long is that the case where it mostly matters (make run_tests) doesn't really see the issue, since test binaries have the proper devel/lib location in their RUNPATH. However, it's a pretty big problem for systems like Nix, which rely on LD_LIBRARY_PATH to properly set up environments— this manifested for us as a package's tests trying to link at runtime against the version of a library in an underlay, rather than the modified one in the current workspace.

Some possible options:

  • Move the generation of setup_cached to later in the process.
  • Rerun the generation of setup_cached to after the "main" build is complete (whatever that even means?)
  • Speculatively add lib to LD_LIBRARY_PATH in case it comes to exist later on in the build.
  • Add a new setup_cached_post which generates later and upon which the run_tests targets depend.
  • Avoid env_cached altogether and just switch the tests to run with devel/env.sh.

I don't think I'm enough of a catkin expert to really soberly evaluate these, but I'm willing to collaborate on a solution if I can get the required input/guidance. My instinct is toward the final one just for its implementation simplicity, but I'd love to know if a "better" solution exists.

mikepurvis avatar Nov 12 '21 22:11 mikepurvis

Here's a first-pass attempt at option 3:

https://github.com/ros/catkin/compare/noetic-devel...mikepurvis:issue-1158

I briefly tried the final option, but it became apparent that there are other cases where including devel/lib isn't enough— sometimes you need to link against build/gtest/lib/libgtest.so, hence manually (and speculatively) inserting both these paths into setup_cached.sh.

mikepurvis avatar Nov 17 '21 02:11 mikepurvis