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

inter-package task dependencies still documented recommendation #547

Closed
2 of 3 tasks
tkruse opened this issue Oct 26, 2013 · 8 comments
Closed
2 of 3 tasks

inter-package task dependencies still documented recommendation #547

tkruse opened this issue Oct 26, 2013 · 8 comments

Comments

@tkruse
Copy link
Member

tkruse commented Oct 26, 2013

As discussed here: https://groups.google.com/forum/?fromgroups#!searchin/ros-sig-buildsystem/package$20dependency/ros-sig-buildsystem/dvVO5QCHBLM/QtXUOf7EOhQJ
and solved here:
ros/genmsg#26

it is bad practice to refer to the names of tasks in other packages, like

add_dependencies(talker beginner_tutorials_generate_message_cpp)

instead the recommended practice was to be

add_dependencies(talker beginner_tutorials_EXPORTED_TARGETS)

Yet the former is still the documented approach for writing catkin packages, e.g. here:

Also it still rendered by

  • catkin_create_pkg

When solved, also consider updating responses at:

@tkruse
Copy link
Member Author

tkruse commented Oct 26, 2013

Duh, sorry, indeed the variable name had been changed to the suffix _EXPORTED_TARGETS, not _CODE_GENERATION_TARGETS in ros/genmsg@2d2dce8

Also see

@dirk-thomas
Copy link
Member

The catkin_pkg template contains

add_dependencies(@{name}_node @{name}_generate_messages_cpp)

which is perfectly correct.

@dirk-thomas
Copy link
Member

For the first answers question the correct answer is already stated.

For the second one there is still missing information to be provided by the questioner.

@tkruse
Copy link
Member Author

tkruse commented Oct 29, 2013

Yes, sorry, for the catkin_create_pkg. Created #548 instead. Second answers question was just to show people may not be aware of catkin_EXPORTED_TARGETS, and it might be good to remind them (even if they have other problems).

@gavanderhoorn
Copy link
Contributor

@dirk-thomas wrote:

The catkin_pkg template contains

add_dependencies(@{name}_node @{name}_generate_messages_cpp)

which is perfectly correct.

I hate to drag up an old thread like this, but just to be clear: is referring to _generate_messages_cpp in a dependent CMakeLists.txt ok, or is it not? I'd sort of understood that the recommended practice was to use _EXPORTED_TARGETS.

There seems to be quite some confusion about this, judging from the discussions that can be found on the web (not just ROS Answers).

@dirk-thomas
Copy link
Member

_EXPORTED_TARGETS is the recommended way to do it. The user does not need to think about which targets it might contain and does not need to check if the targets actually exist or if the variable is empty.

_generate_messages_LANG can still be used (those are the actually values of the above list variable). But then the user is responsible to check if the target name actually exists. This approach will provide better compilation performance since if you only rely on the C++ header being generated you won't wait for Python/Lisp/whatever messages have been generated.

@lianghongzhuo
Copy link

follow up at 2021: if the cmake_minimum_required(VERSION 3.0.2) was bumped to 3.0.2, in your CMakeList.txt
Then only _EXPORTED_TARGETS can compile. _generate_messages_LANG will leads to compile error. Correct me if I was wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants