colcon / colcon-cmake

Extension for colcon to support CMake packages
http://colcon.readthedocs.io
Apache License 2.0
16 stars 25 forks source link

fix CMAKE_MODULE_PATH env var #19

Closed dirk-thomas closed 6 years ago

dirk-thomas commented 6 years ago

Currently the environment hooks are being generated in (depending in where the CMake module was found):

<prefix>/share/<pkgname>/cmake/Modules/share/<pkgname>/hook/cmake_module_path.bat

and the appended path points to:

<prefix>

With this patch the location of the environment hook is not nested anymore:

<prefix>/share/<pkgname>/hook/cmake_module_path.bat

and the appended path is the directory containing the CMake module:

<prefix>/share/<pkgname>/cmake/Modules

The first fix is just cosmetic. The second one avoid cross-talk between packages (one extending the module path and another one benefiting from it).

Additionally the patch fixes the case where a package installs multiple CMake module into different location. Before the last hook was overwriting all previous ones.

dirk-thomas commented 6 years ago

In a non-isolated install this bug also results in numerous extra environment hooks to be created.

dirk-thomas commented 6 years ago

So the idea was good and it works well in isolated installations. But...

In the --merge-install case this is still a problem. After a package A installed its CMake module (and got the intended hook for it) a later processed package B will find the same CMake module and also generate a hook for it. While the presence of the extra hook doesn't do any harm to the environment (since the entry is already there it will be skipped) it does add a lot of unnecessary hooks which takes extra time for sourcing the setup files.

I think the only "reliable" option is to limit the search to location which are considered by CMake as well as contain the package name in the path. I will create a follow up PR for this after confirming this works for our cases...