Catkin finds system packages instead of Catkin packages of exported dependencies

[calderpg]

If a CMake-findable package exists on the system (for example, there is a FindPACKAGENAME.cmake in /usr/share/cmake-2.8/Modules), and a Catkin package with the same name exists, find_package(catkin REQUIRED COMPONENTS PACKAGENAME) will find the system package, not the Catkin package.

An example of when the problem occurs:

1. You have a package installed to the system that provides a CMake find package file (for example, Findosg.cmake for OpenSceneGraph)
2. You have a Catkin package named osg
3. You have a Catkin package that depends on osg via find_package(catkin REQUIRED COMPONENTS osg)

In this case, the system package OpenSceneGraph will be found, rather than your Catkin package osg.

In contrast, the problem does not appear to occur in the comparable case of local Catkin package that shadows a system Catkin package. In this case, Catkin reorders the packages to ensure that the shadow package is processed before the dependant package. This would be much more reasonable behavior in the case of this bug.

In most cases, having a Catkin package with the same name as a system package should be avoided, but given the long list of system CMake files, some of which (like Findosg.cmake, which is deprecated in favor of FindOpenSceneGraph.cmake, but is still shipped for compatibility) have different names than their respective packages, it is fairly easy to accidentally shadow a system package.

[wjwwood]

I do not believe that catkin does anything to the ordering when a catkin package overlays a another system version. I think the issue here is that Findosg.cmake is a CMake Module and your catkin package called osg produces a CMake Config file called osgConfig.cmake and I think, based on your error report, that the CMake's find_package() macro prefers CMake Modules to CMake Config files. I'm not 100% sure that's the case, but that's what I would guess.

One thing we could do is check that anything passed to find_package(catkin REQUIRED COMPONENTS ...) is both found and defines the <package_name>_FOUND_CATKIN_PROJECT CMake variable (which is set by all CMake config files generated by catkin).

[dirk-thomas]

What @wjwwood described is the intended CMake behavior (https://cmake.org/cmake/help/v2.8.12/cmake.html#command:find_package):

If no module is found ... the command proceeds to Config mode.

I don't think that catkin behave any different then the CMake default behavior. Therefore I would suggest if you only want to find CMake config's but no modules find the specific package separately from catkin using the NO_MODULE option.

[calderpg]

I don't disagree with this being the standard behavior of CMake. I think we can agree that this is not intuitive behavior unless you understand both how CMake finds packages, and how Catkin generates config files for packages.

I'm not that concerned about the case of deliberately shadowing a system package - if people really want to do this, a note on the Catkin CMakeLists page should be enough to figure out how to do what they want. What concerns me more is that if you do it accidentally, it is very hard to figure out what is going wrong (for example, in the example I've provided, you get a CMake error

variable OSG_INCLUDE_DIRS used before declaration

when CMake processes the dependant package and that's it), and it's unclear if it's a Catkin issue or a CMake issue.

In terms of avoiding this in the future, better documentation is probably a sufficient solution in the short term.

In terms of long-term options:

• Do what @wjwwood suggested and check to make sure that every find_package'd Catkin package is actually found as a Catkin package, and throw an error if a system package has been found instead?
• Given that Catkin already provides warnings and errors about incorrect package names/version numbers and inconsistent package/CMakeLists files, would it be possible to provide a warning that a package shadows a system package?
• On a design basis, is there a particular reason for generating CMake configs instead of modules? Given the structure of developing Catkin packages,which are (almost) always going to be developed in a workspace overlaying something (a ROS distro, other workspace, etc), from a developer perspective you always want your local packages to be used instead of the overlaid versions - and that's what happends except for this particular example. From an outside perspective, it seems that using CMake modules would ensure this behavior matches developer expectations. Obviously, changing to using modules would break existing systems, but such a switch could be made in a future version of ROS.

[dirk-thomas]

Looking into the current implementation it seems that find_package(catkin COMPONENTS ...) already uses the NO_MODULE option:

catkin/cmake/catkinConfig.cmake.in

Lines 74 to 83 in 3b8a3b9

	if(catkin_FIND_REQUIRED)
	find_package(${component} REQUIRED NO_MODULE PATHS ${paths}
	NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
	elseif(catkin_FIND_QUIETLY)
	find_package(${component} QUIET NO_MODULE PATHS ${paths}
	NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
	else()
	find_package(${component} NO_MODULE PATHS ${paths}
	NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
	endif()

So it is unclear how the CMake module was found in your example.

Regarding the three questions:

• I don't think that catkin should check for the <package_name>_FOUND_CATKIN_PROJECT variable. There are plain CMake packages which are being found using that mechanism and this would break that use case.
• Afaik if a CMake package was found it is not possible to figure out if it shadowed another package since CMake only reports the overlayed one. Could you please sketch how this is supposed to be done?
• CMake currently generates config files and also only find config files (and no modules). Also this is afaik the recommended way to export information for CMake projects. So I don't think this should change in the future.

[calderpg]

So it is unclear how the CMake module was found in your example

Is there a good way to get more debugging information so we could track this down? I'm happy to reproduce the issue and try to figure out why NO_MODULE isn't doing what we expect.

Could you please sketch how this is supposed to be done?

I think the following process is a possible approach:

1. When processing the new Catkin package, call find_package(<package_name>)
2. Depending on the results of find_package, provide a warning/error to the user:

• If find_package fails, OK
• If find_package succeeds, and <package_name>_FOUND_CATKIN_PROJECT is set, OK
• If find_package succeeds, but <package_name>_FOUND_CATKIN_PROJECT is not set, warn

[dirk-thomas]

The first thing would be to have a detailed reproducible example. Based on that you can look at the <pkgname>_DIR variable as well as the CMakeCache.txt file. After that the only further "debugging" technique will be to annotate the CMake code to follow the exact execution path and what happens where and when.

If find_package succeeds, but <package_name>_FOUND_CATKIN_PROJECT is not set, warn

Since I would consider that a valid use case I don't think it is feasible to warn about it.

[calderpg]

I'll reproduce the issue this week and try to figure out where the issue is actually happening.

Since I would consider that a valid use case I don't think it is feasible to warn about it

Can you clarify when this is actually a valid use case? As far as I can tell based on the current behavior, if you have a Catkin package with the same name as another non-Catkin package, what actually happens depends entirely on whether the other package is found via CMake module or CMake config, which is not necessarily something we should expect every developer know, nor something that should be assumed to be the same across multiple systems in the context of system packages. It seems perfectly reasonable to warn the user that they have created a potentially ambiguous situation.

As an example, it's completely possible (albeit unlikely) that on one system a package is found via module, and on another it's found via config - from a developer perspective it normally doesn't matter when calling find_package - but if they're using an ambiguous package it will work on some systems and fail on others.

[dirk-thomas]

find_package(catkin REQUIRED COMPONENTS foo ...) is being used where foo has a CMake config file but which is not a catkin package. Therefore warning about foo not having the variable _FOUND_CATKIN_PROJECT would be wrong in that case.

[calderpg]

To clarify, I'm not suggesting that the check happen during the find_package(catkin REQUIRE COMPONENTS ...) call, instead, that the check should take place in the catkin_package() call.

[dirk-thomas]

I don't understand. Maybe you can elaborate what you have in mind?

[calderpg]

What I have in mind is that you create a Catkin package foo, and in the catkin_package(...) call in foo's CMakeLists.txt we check if the package name (via ${PROJECT_NAME}) is potentially ambiguous:

Essentially the following process would be added to catkin_package(...) :

find_package(${PROJECT_NAME})
if (${PROJECT_NAME}_FOUND)
    if (${PROJECT_NAME}_FOUND_CATKIN_PROJECT)
        # Do nothing, we're shadowing another Catkin package
    else ()
        message(WARN "Package named ${PROJECT_NAME} appears to shadow a system package. Depending on system configuration, this may prevent Catkin from finding package ${PROJECT_NAME}")
else()
    # No existing package found, we're OK

[dirk-thomas]

First of all it is still unclear why the "system package" was found in your case. By only searching for configs and in the order of the CMAKE_PREFIX_PATH this shouldn't be the case in the first place. Let's consider the options once we know what actually happened.

Second the find_package() call will have arbitrary side effects (e.g. setting CMake variables, find other resources) which might have an unpredictable effect on this projects build. Therefore I don't think this is feasible as a general purpose approach for all catkin packages.

[dirk-thomas]

Without further detailed information I can't act on this. Therefore I will close the ticket for now. Please feel free to comment with more information and the ticket can be reopened.

[dmcconachie]

So after playing around a bit I found a very simple minimum working example.

1. Create a package with a name (OpenSceneGraph) that conflicts with a system package
2. Create a package (depends_level_1) that depends on the package with a system conflict.
   • Edit the generated CMakeLists.txt to export the dependency (OpenSceneGraph) as a CATKIN_DEPENDS
3. Create a package (depends_level_2) that depends on the previous package (depends_level_1).
4. Run catkin_make

I've attached a workspace that demonstrates the problem.

catkin_package_name_test_min_working_example.zip

If I remove/rename the cmake module for OpenSceneGraph then this workspace is able to build without any issues.

Edit: it's worth noting here that OpenSceneGraph is not installed in any form on this system, this is just one library that cmake has a Find<...>.cmake file for already.

[dirk-thomas]

Thanks for providing the example workspace.

The original title suggested the problem would be related to find_package(catkin COMPONENTS ...) but that code (

catkin/cmake/catkinConfig.cmake.in

Lines 73 to 83 in eaba48d

	# find package component
	if(catkin_FIND_REQUIRED)
	find_package(${component} REQUIRED NO_MODULE PATHS ${paths}
	NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
	elseif(catkin_FIND_QUIETLY)
	find_package(${component} QUIET NO_MODULE PATHS ${paths}
	NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
	else()
	find_package(${component} NO_MODULE PATHS ${paths}
	NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
	endif()

) explicitly uses NO_MODULE and therefore doesn't find "system packages".

Based on the example I traced it to this find_package() call for exported dependencies of a catkin package:

catkin/cmake/templates/pkgConfig.cmake.in

Line 165 in eaba48d

find_package(${@PROJECT_NAME@_dep} REQUIRED)

Therefore I reopened this ticket and created PR #772 with a potential fix. Could you please give it a try and report back if it works for you.

[dmcconachie]

This change did work for both the minimum working example, as well as my actual project with no problems.

[NikolausDemmel]

@dirk-thomas while I think this issue is closed, while reading this I was wondering about your statement:

find_package(catkin REQUIRED COMPONENTS foo ...) is being used where foo has a CMake config file but which is not a catkin package. Therefore warning about foo not having the variable _FOUND_CATKIN_PROJECT would be wrong in that case.

This use case seems to not be in any of the docs (or I haven't found it). Comparing http://docs.ros.org/hydro/api/catkin/html/howto/format2/catkin_library_dependencies.html#finding-the-library and http://docs.ros.org/hydro/api/catkin/html/howto/format2/system_library_dependencies.html#finding-the-library reads to me like it is suggested to always call find_package manually on system dependencies. At no point is it suggested to add it as a component in find_package(catkin ...). http://docs.ros.org/hydro/api/catkin/html/user_guide/find_package.html also doesn't mention it.

Do packages in practice actually use that? Then I think the docs should mention the possibility of using this to get things find-packaged automatically and appended to catkin_LIBRARIES and catkin_INCLUDE_DIRS.

[dirk-thomas]

It is not recommended to let catkin COMPONENTS search for system packages. But if packages are being migrated from being a catkin package to being a plain CMake package that can happen. I don't think the suggested check helps especially since catkin can't tell what the intention of the potential overlay is.

[dirk-thomas]

The find logic has been updated in #782 for Kinetic. Can this be closed?

[calderpg]

Considering that Indigo is a long-term support release, with 14.04+Indigo a standard combination for people with PR2 or Baxter robots, how difficult would it be to (either) port the fix to Indigo and Jade, (or) add additional documentation to help anyone who encounters the problem?

As far as I understand, any existing packages for Indigo or Jade that break with this change were already depending on incorrect behavior, so backporting the fix shouldn't cause problems.

[dirk-thomas]

The problem is that you don't know how many (potentially unreleased and non-public) packages use the currently possible behavior. You might be able to audit / fix the publically visible ones but there are many more.

And since the current behavior has been present since almost two years I am not inclined to backport it since the regression potential is unforeseeable. Additional document pointing out that potentially problematic behavior is of course always welcome.

[roehling]

Just adding my two cents: catkin_lint treats searching for system packages with catkin COMPONENTS as error, and has been doing so since day one. Hopefully, that means very few packages will be bitten by this fix.

Still, I do agree with @dirk-thomas that backporting the fix would break the "no-surprise" rule of long-term stable releases.