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

build: There should be a warning when there are missing links in the workspace dependency graph #257

Closed
jbohren opened this issue Dec 21, 2015 · 6 comments

Comments

@jbohren
Copy link
Contributor

jbohren commented Dec 21, 2015

Currently, a user could mistakenly try to overlay system package X by putting its source code into his or her workspace, in order to build package Z which depends on it. However, if package Z depends on package Y which also depends on X, and package Y is not in the workspace, it could lead to package Z being built against the system package X instead of the workspace package X.

catkin build could easily check for this by constructing the dependency graph including the workspace and $CMAKE_PREFIX_PATH, and then determining if there are any paths between workspace packages which include system packages. If so, the user should be warned about the problem.

@jbohren jbohren added this to the untargeted milestone Dec 21, 2015
@wjwwood
Copy link
Member

wjwwood commented Dec 22, 2015

If I understand you correctly this is captured here: #30 and in #90.

@jbohren
Copy link
Contributor Author

jbohren commented Dec 22, 2015

If I understand you correctly this is captured here: #30 and in #90.

This doesn't entirely capture it. These (#30 and #90) will make sure packages in this workspace are ordered correctly, but in the case where an interim dependency exists, the build might be broken. This is really apparent when building c++ libraries that are linked together.

@wjwwood
Copy link
Member

wjwwood commented Dec 22, 2015

I see what you mean, Z' links against Y which is linked against X and not X' as it should (where ' denotes the version in the overlaid workspace)? In some cases the change to the LD_LIBRARY_PATH should be sufficient, but it might fail at linkedit if there are new symbols in the headers of X' which the library of X does not have and you're building with an isolated devel or install setup.

@jbohren
Copy link
Contributor Author

jbohren commented Dec 23, 2015

Yeah, it's not handled by LD_LIBRARY_PATH if the absolute path is given by the CMake module, and this is what happens with catkin packages.

@jbohren jbohren changed the title catkin could warn when there are missing links in the workspace dependency graph build: catkin could warn when there are missing links in the workspace dependency graph Dec 23, 2015
@jbohren jbohren changed the title build: catkin could warn when there are missing links in the workspace dependency graph build: There should be a warning when there are missing links in the workspace dependency graph Dec 23, 2015
@jbohren
Copy link
Contributor Author

jbohren commented Jan 23, 2016

Also note that now that #249 is merged, users are presented with CMake warnings about conflicting paths like the following:

CMake Warning at /home/jbohren/ws/ascent_sim/src/moveit_ros/planning/plan_execution/CMakeLists.txt:3 (add_library):
  Cannot generate a safe runtime search path for target moveit_plan_execution
  because there is a cycle in the constraint graph:

    dir 0 is [/home/jbohren/ws/ascent_sim/devel/lib]
      dir 1 must precede it due to runtime library [liborocos-kdl.so.1.3]
    dir 1 is [/opt/ros/indigo/lib]
      dir 0 must precede it due to runtime library [libmoveit_planning_pipeline.so]

  Some of these libraries may not be found correctly.

@jbohren
Copy link
Contributor Author

jbohren commented Jan 27, 2016

closing in favor of #30

@jbohren jbohren closed this as completed Jan 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants