catkin_make_isolated not isolated

[tkruse]

So, the behavior is weird, hopefully the cause is simple to fix. This occured using both catkin 0.5.63 as well as latest groovy-devel (19/02/2013).

The shell code below produces 2 c++ packages, foo_msgs and foo. foo_msgs defines a message Foo.msg.
foo has a c++ executable depends on foo_msgs/Foo.h
however, the CMakeLists.txt of foo does not find_package foo_msgs

To reproduce:

. /opt/ros/groovy/setup.bash 
mkdir issue_ws
cd issue_ws/
mkdir src
cd src/
catkin_create_pkg foo_msgs message_generation message_runtime std_msgs
cd foo_msgs/
mkdir msg
echo "#an array of cells in a 2D grid
Header header
float32 cell_width
float32 cell_height" > msg/Foo.msg
rm CMakeLists.txt 
echo "cmake_minimum_required(VERSION 2.8.3)
project(foo_msgs)
find_package(catkin REQUIRED COMPONENTS message_generation std_msgs)

add_message_files(
  FILES
  Foo.msg
)

generate_messages(
  DEPENDENCIES
  std_msgs
)

catkin_package(
 CATKIN_DEPENDS message_runtime std_msgs
)" >CMakeLists.txt

cd -
catkin_create_pkg foo roscpp foo_msgs
cd foo
echo "#include <foo_msgs/Foo.h>


int main(int argc, char **argv)
{
    return 0;
}
" > src/foo.cpp
rm CMakeLists.txt 
echo "cmake_minimum_required(VERSION 2.8.3)
project(foo)
find_package(catkin REQUIRED COMPONENTS roscpp)
include_directories(include \${catkin_INCLUDE_DIRS})

add_executable(foo_node src/foo.cpp)
add_dependencies(foo_node foo_msgs_generate_messages_cpp)
target_link_libraries(foo_node
  \${catkin_LIBRARIES}
)

catkin_package()
" > CMakeLists.txt
cd ../..
catkin_make_isolated

This build should fail, as project foo does not declare find_package(foo_msgs), but it does not fail, making isolated builds useless to detect missing find_package calls. it seems to me that catkin_INCLUDE_DIRS in this case contains an entry it should not contain, but maybe something else causes this.

[dirk-thomas]

This has nothing to do with catkin_make_isolated. It happens exactly the same way if you invoke cmake/make on these two packages.

The environment of your first package exports CPATH which contains the include folder of that workspace. Therefore the included message file is found.

[tkruse]

I don't hink it does:

EDIT: printed CPATH after calling env, of course.

kruset:~ $ . devel_isolated/foo_msgs/env.sh 
Entering environment at '/tmp/issue_ws/devel_isolated/foo_msgs', type 'exit' to leave
kruset:~ $ env | grep CPATH
CPATH=/home/kruset/local/ubuntu-amd64/include:/home/kruset/local/ubuntu-amd64/openrobots/include:/usr/include/tcl8.4
kruset:~ $ env | grep tmp
GPG_AGENT_INFO=/tmp/keyring-li99PO/gpg:0:1
ROS_PACKAGE_PATH=/tmp/issue_ws/src/foo_msgs:/opt/ros/groovy/share:/opt/ros/groovy/stacks
GNOME_KEYRING_CONTROL=/tmp/keyring-li99PO
OLDPWD=/tmp/issue_ws
LD_LIBRARY_PATH=/home/kruset/local/ubuntu-amd64/openrobots/lib:/home/kruset/local/all/lib:/home/kruset/local/ubuntu-amd64/lib:/tmp/issue_ws/devel_isolated/foo_msgs/lib:/opt/ros/groovy/lib:/home/kruset/local/ubuntu-amd64/openrobots/lib:/home/kruset/local/all/lib:/home/kruset/local/ubuntu-amd64/lib
SSH_AUTH_SOCK=/tmp/keyring-li99PO/ssh
CATKIN_TEST_RESULTS_DIR=/tmp/issue_ws/build_isolated/foo_msgs/test_results
SESSION_MANAGER=local/kruset-GT70:@/tmp/.ICE-unix/2003,unix/kruset-GT70:/tmp/.ICE-unix/2003
ROS_TEST_RESULTS_DIR=/tmp/issue_ws/build_isolated/foo_msgs/test_results
PATH=/usr/lib/jvm/default-java/bin:/home/kruset/local/ubuntu-amd64/bin:/home/kruset/local/ubuntu-amd64/bin:/home/kruset/local/all/bin:/tmp/issue_ws/devel_isolated/foo_msgs/bin:/opt/ros/groovy/bin:/usr/lib/jvm/default-java/bin:/home/kruset/local/ubuntu-amd64/bin:/home/kruset/local/ubuntu-amd64/bin:/home/kruset/local/all/bin:/usr/lib/lightdm/lightdm:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/home/kruset/local/ubuntu-amd64/openrobots/sbin:/home/kruset/local/ubuntu-amd64/openrobots/bin:/home/kruset/local/ubuntu-amd64/openrobots/sbin:/home/kruset/local/ubuntu-amd64/openrobots/bin
DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-DtDXxRcira,guid=e8eff13560ff5ab49b3c28510000000a
kruset:~ $ env | grep foo
ROS_PACKAGE_PATH=/tmp/issue_ws/src/foo_msgs:/opt/ros/groovy/share:/opt/ros/groovy/stacks
LD_LIBRARY_PATH=/home/kruset/local/ubuntu-amd64/openrobots/lib:/home/kruset/local/all/lib:/home/kruset/local/ubuntu-amd64/lib:/tmp/issue_ws/devel_isolated/foo_msgs/lib:/opt/ros/groovy/lib:/home/kruset/local/ubuntu-amd64/openrobots/lib:/home/kruset/local/all/lib:/home/kruset/local/ubuntu-amd64/lib
CATKIN_TEST_RESULTS_DIR=/tmp/issue_ws/build_isolated/foo_msgs/test_results
ROS_TEST_RESULTS_DIR=/tmp/issue_ws/build_isolated/foo_msgs/test_results
PATH=/usr/lib/jvm/default-java/bin:/home/kruset/local/ubuntu-amd64/bin:/home/kruset/local/ubuntu-amd64/bin:/home/kruset/local/all/bin:/tmp/issue_ws/devel_isolated/foo_msgs/bin:/opt/ros/groovy/bin:/usr/lib/jvm/default-java/bin:/home/kruset/local/ubuntu-amd64/bin:/home/kruset/local/ubuntu-amd64/bin:/home/kruset/local/all/bin:/usr/lib/lightdm/lightdm:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/home/kruset/local/ubuntu-amd64/openrobots/sbin:/home/kruset/local/ubuntu-amd64/openrobots/bin:/home/kruset/local/ubuntu-amd64/openrobots/sbin:/home/kruset/local/ubuntu-amd64/openrobots/bin

So the env does export things (which is also a bug), but not the CPATH.

The only other reference to the first packeg I see is here:

/tmp/issue_ws/devel_isolated/foo_msgs/env.sh cmake /tmp/issue_ws/src/foo -DCATKIN_DEVEL_PREFIX=/tmp/issue_ws/devel_isolated/foo -DCMAKE_INSTALL_PREFIX=/tmp/issue_ws/install_isolated
-- Using CATKIN_DEVEL_PREFIX: /tmp/issue_ws/devel_isolated/foo
-- Using CMAKE_PREFIX_PATH: /tmp/issue_ws/devel_isolated/foo_msgs;/opt/ros/groovy;/home/kruset/local/ubuntu-amd64/
-- This workspace overlays: /tmp/issue_ws/devel_isolated/foo_msgs;/opt/ros/groovy

In any case, so is the current design of catkin_make_isolated that of a strict chain of environments, rather than using only an env that contains only the build_depend packages listed in a package's manifest? That would also be a bug to me.

[dirk-thomas]

Yes, it does - CPATH is responsible that the header is found. If you remove that it fails as you expect it to be.

catkin_make_isolated processes each package separately with cmake and make and than sources the resulting environment. That is as clean as it can get - that is definitely not a bug. The build_depend stuff from the package.xml has nothing to do with the resulting environment.

The only arguable thing is if CPATH should be set in the environment or not.

[tkruse]

Hm, I get:

$ devel_isolated/foo_msgs/env.sh env | grep CPATH
CPATH=/tmp/issue_ws/devel_isolated/foo_msgs/include:...

But.

$ devel_isolated/foo_msgs/env.sh
Entering environment at '/tmp/issue_ws/devel_isolated/foo_msgs', type 'exit' to leave
$ env | grep CPATH
CPATH=/home/kruset/local/ubuntu-amd64/include:/home/kruset/local/ubuntu-amd64/openrobots/include:/usr/include/tcl8.4
$ exit
$ . devel_isolated/foo_msgs/env.sh
Entering environment at '/tmp/issue_ws/devel_isolated/foo_msgs', type 'exit' to leave
$ env | grep CPATH
CPATH=/home/kruset/local/ubuntu-amd64/include:/home/kruset/local/ubuntu-amd64/openrobots/include:/usr/include/tcl8.4

So i guess you are right, but still just running env.sh, or sourcing it, seems to do something else to the environment that running something with it. Is that a bug? If not, I suggest chaning the the "Entering ..." message to tell users that they don't get the whole truth.

Regarding sourcing the previous env regardless of what is in the package.xml, I'll discuss this in buildsystem SIG.

[tkruse]

https://groups.google.com/d/topic/ros-sig-buildsystem/xE0gHE1jRuM/discussion

[wjwwood]

There is a simple situation where this occurs:

Consider a workspace where ping depends on foo which depends on bar and contains each of them. Additionally there is a package baz which is in the workspace but has no dependencies.

The topological order might look like:

• baz
• bar
• foo
• ping

Now, if ping has an undeclared dependency on one of baz's headers it will not fail, but it should. This is because the baz environment is sourced when building bar even though bar does not depend on baz.

This only occurs with catkin_make_isolated, and is probably ok, as long as there is a separate option to do the more complicated environment isolation for testing purposes.

[dirk-thomas]

The scenario you describe also applied to the use case when these packages are in workspaces and you build them one after another. And in that case there is logically no way to deal with it better.

Therefore I don't think that it is worth the effort to let CMI figure out a special environment for each package when we can't support the same in other scenarios.

[wjwwood]

I don't think it should be the default, nor do I think it is required to build code, but it would be a useful development tool. I am satisfied to leave as a known issue with the isolation, and make this an enhancement ticket assigned to untargeted.

[dirk-thomas]

+1 for "untargeted" - patches are welcome which implement that logic into CMI.

What about dropping CPATH from the environment? I would be ok with that. Downstream stuff should anyway use find_package() and/or pkg-config and all other env variable are only for runtime environment (not for build env of downstream stuff).

[wjwwood]

No CPATH would break simpler build systems like Make based systems. This is unlikely to be a problem, but if you are extending the PYTHONPATH then in my opinion you should extend the CPATH. From my understanding the CPATH won't be a problem if devel spaces are used and the improved environment isolation feature is implemented.

[dirk-thomas]

To summarize the thread: catkin_make_isolated uses the dependencies to build a topologically ordered list and than processes the packages in order where the previous package provides the environment for the next.
The issue suggests that each package should only get the combined environment from the packages it depends on. That way packages stricter dependencies can be checked.

Since this requires a very different approach for merging environments which is not easily possible it will be marked as untargeted. Pull requests to implement this feature are highly welcome.

[dirk-thomas]

Closing since the requested feature is a duplicate of #330.