catkin implicit workspace chaining dangerous

[tkruse]

Unless I am mistaken, currently catkin workspace chaining works by using the current CMAKE_PREFIX_PATH of the shell.
This enables chaining workspaces voluntarily very easily.

Also, it enables users to involuntatrily chain workspaces very easily. I think this is a very bad situation.
I suggest that instead users have to specify during configure using a var what workspace(s) they want to chain against. This should ideally also allow unchaining workspaces by changing that var.

I suggest parsing the CMAKE_PREFIX_PATH, and removing all catkin workspaces, and then adding those the user provided with the extra var.

This can be made somewhat less painful with a python wrapper command for setting up a full workspace against another one.

[tkruse]

To be more precise: the setup.sh /setup_utils.py should not be generated to remember the CMAKE_PREFIX_PATH the user had when calling cmake, but instead only paths the user explicitly wanted to use for overlaying in the future.

However the current CMAKE_PREFIX_PATH should still be used every time as given by the user, so i do not suggest changing the CMAKE_PREFIX_PATH, I only suggest not using it during generation of setup.sh

I believe cmake also does not store the CMAKE_PREFIX_PATH between invocations of cmake, so catkin should keep that behavior.

I noticed this btw while writing tutorials, which also becomes a real pain if one has to consider all the things that could break if the user is not very careful with his CMAKE_PREFIX_PATH.

[dirk-thomas]

Yes, chaining works using the CMAKE_PREFIX_PATH (CPP) variable. I think this makes it very natural for CMake users. Basically a catkin workspace is nothing special: it provides the CMake config file for find_package().

No, vanilla CMake does store the CPP in the cache. Else it would not be able to rerun the configuration when a CMake file is touched. Therefore catkin should and does keep exactly that behavior.

Furthermore I think that setting the CPP in setup.sh is mandatory. If a user depends on a workspace foo it is guaranteed that he automatically gets all workspaces that foo depends on. This is the same as dependency management in general - the user should not have to care what foo depends on.

The proposed unchaining of workspaces will in the majority of the cases break the build since dependencies are not found any more. Therefore this should not be the default behavior. For the rare case that this is desired a user can do so manually by modifying the CPP variable or passing CPP as an explicit argument to the CMake invocation.

[tkruse]

Maybe I was still not precise enough.

I think yes, the setup.sh should set the CPP so that other workspaces that we overlay are put in the CPP. I do not want to change that, I do not suggest that this is bad. What I am saying is that the configure step should not use the current CPP as the CPP that is stored in setup.sh and restored later. Later, when the setup.sh restores a CPP, it should ONLY put into the CPP the workspaces that the user expressively wanted to depend on, and not some arbitrary value in the CPP the user had forgotten all about. Since we cannot in general know what values in the CPP the user put there explicitly and what are just leftovers, we should treat all values of the CPP as if they were no intended for chaining, and do chaining only for things the user explicitly told us to store.

E.g.

in .bashrc:

source ~/groovy/devel/setup.sh # sets CPP to fuerte

in terminal

cd ~
mkdir hydro
cd hydro
mkdir src
mkdir build
cd build
cmake ../src # this automatically chains the new hydro workspace to the groovy workspace, something the user never specified, just because the user did not know.

instead, the cmake ../src invocation should run cmake with the current (wrong) CPP path, but the next time the user opens a shell and sources ~/groovy/devel/setup.sh, the CPP should not contain references to ~/groovy.

The underlying problem is that users source setup.sh's in their .bashrc, and with new terminals created that way will create new catkin workspaces.

[dirk-thomas]

There are only to options:

1. implicitly use the current CPP when calling cmake
2. explicitly passing a CPP when calling cmake

The first option is only a problem when creating an underlay. Therefore the user needs a clean environment - either using a new shell without stuff in the bashrc - or by calling a command to clean the environment. Currently such a command does not exist but setup.sh does the same internally so it could be exposed. Keep in mind that underlays are the much less common use cases

The second option is obviously the desired way for the common case of creating an overlay which will be used by the majority. They should not be required to specify the CPP which might need to contain multiple workspace. That would be extremely error prone.