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

Only set catkin_* variables when being invoked with find_package COMPONENTS #629

Merged

Conversation

dirk-thomas
Copy link
Member

This is a necessary change to support ros/rosdistro#3010.

@tfoote @wjwwood Please review and consider potential side effects. Is it possible / reasonable to backport this? Otherwise it will force a lot of packages to have a separate indigo branch (which they might not yet have).

@tfoote
Copy link
Member

tfoote commented May 3, 2014

+1 tested locally on geometry.

re backporting: This only fixes c++ unit tests which want the ifdefed tests. @bulwahn has made a good number of pull requests into hydro which have been merged. Geometry appears to be the first one noticed to regress without this patch. The case that will break is if there are people misusing find package and find_package(foo) which is a catkin package and then using the unmatched variables like ${catkin_LIBRARIES}. As this is more of a bug taking advantage of side effects from under the hood it's probably worth backporting. This will allow users to have conditional testing on hydro and not require forking to have it in indigo.

@wjwwood
Copy link
Member

wjwwood commented May 3, 2014

+1

dirk-thomas added a commit that referenced this pull request May 5, 2014
…omponents

Only set catkin_* variables when being invoked with find_package COMPONENTS
@dirk-thomas dirk-thomas merged commit 6677020 into indigo-devel May 5, 2014
@dirk-thomas dirk-thomas deleted the cmake_config_set_variable_only_for_components branch May 5, 2014 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants