Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generated setup_cached.sh file excludes LD_LIBRARY_PATH of current package #1158

Open
mikepurvis opened this issue Nov 12, 2021 · 1 comment

Comments

@mikepurvis
Copy link
Member

mikepurvis commented Nov 12, 2021

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
Copy link
Member Author

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant