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

Don't redefine add_dependencies #408

Merged
merged 1 commit into from
Oct 24, 2019
Merged

Conversation

rotu
Copy link
Contributor

@rotu rotu commented Oct 24, 2019

rosidl defines add_dependencies, and cmake will gladly redefine functions without a warning. If you have a multi-project CMake tree, this can lead to weirdness

CMake Error at src/ros2/demos/demo_nodes_cpp/CMakeLists.txt:42 (target_compile_definitions):
target_compile_definitions called with non-compilable target type
Call Stack (most recent call first):
install/share/rosidl_typesupport_cpp/cmake/rosidl_typesupport_cpp_generate_interfaces.cmake:129 (add_dependencies)
install/share/ament_cmake_core/cmake/core/ament_execute_extensions.cmake:48 (include)
install/share/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake:286 (ament_execute_extensions)
src/ros2/demos/logging_demo/CMakeLists.txt:21 (rosidl_generate_interfaces)

Signed-off-by: Dan Rose dan@digilabs.io

rosidl defines `add_dependencies`, and cmake will gladly redefine functions without a warning. If you have a multi-project CMake tree, this can lead to weirdness

CMake Error at src/ros2/demos/demo_nodes_cpp/CMakeLists.txt:42 (target_compile_definitions):
  target_compile_definitions called with non-compilable target type
Call Stack (most recent call first):
  install/share/rosidl_typesupport_cpp/cmake/rosidl_typesupport_cpp_generate_interfaces.cmake:129 (add_dependencies)
  install/share/ament_cmake_core/cmake/core/ament_execute_extensions.cmake:48 (include)
  install/share/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake:286 (ament_execute_extensions)
  src/ros2/demos/logging_demo/CMakeLists.txt:21 (rosidl_generate_interfaces)

Signed-off-by: Dan Rose <dan@digilabs.io>
@rotu
Copy link
Contributor Author

rotu commented Oct 24, 2019

Btw, @dirk-thomas, this might be interesting to you. I didn't realize until this point that functions and macros are defined globally. So while you can collect multiple projects with add_subdirectory (e.g. https://gist.github.com/rotu/1eac858b808b82bbf1b475f515e91636), I think it's too much to ask to expect the entire ROS2 tree to build in this way.

@dirk-thomas
Copy link
Member

I think it's too much to ask to expect the entire ROS2 tree to build in this way.

That is the reason we have dropped this compared to ROS 1 / catkin (see http://design.ros2.org/articles/ament.html#building-within-a-single-cmake-context).

@dirk-thomas
Copy link
Member

Thanks for the patch.

@dirk-thomas dirk-thomas merged commit 8b6ee34 into ros2:master Oct 24, 2019
@rotu
Copy link
Contributor Author

rotu commented Oct 24, 2019

Thanks for the background! This has turned out to be a very educational misadventure

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

Successfully merging this pull request may close these issues.

None yet

2 participants