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

Install includes to include/${PROJECT_NAME} and more modern CMake #419

Merged
merged 1 commit into from
Jan 14, 2022

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Jan 5, 2022

This PR has quite a bit. It could probably be split into smaller PRs.

Part of ros2/ros2#1150

This installs includes to include/${PROJECT_NAME} to mitigate include directory search order issues when overriding packages in desktop.

Part of ament/ament_cmake#365

This removes ament_export_libraries and ament_export_include_directories as they're redundant with the exported CMake targets.

Part of ament/ament_cmake#292

This replaces ament_target_dependencies() calls with target_link_libraries().

Part of ros2/python_cmake_module#6

This uses FindPython3 instead of the deprecated FindPythonInterp and FindPythonLibs.

I think it also removes support for OpenCV 2 and earlier

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Copy link

@audrow audrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me with green CI.

@sloretz
Copy link
Contributor Author

sloretz commented Jan 10, 2022

Green CI is a little different since it's not in the ros2.repos file - I'm not sure if this supports Windows for example. Here's CI with a supplemental file.

CI (supplemental repos file build: --packages-up-to image_geometry cv_bridge opencv_tests test: --packages-select image_geometry cv_bridge opencv_tests)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@sloretz
Copy link
Contributor Author

sloretz commented Jan 10, 2022

Well, all CI jobs failed because ci.ros2.org ignores these package :-/ . CI beyond the PR jobs appears to not be possible on this repo.

https://github.com/ros2/ci/blob/f97ca359f7961afe27ec6ba5bbf087624de34161/ros2_batch_job/__main__.py#L128-L137

Which results in this output

# BEGIN SUBSECTION: ignored packages
12:09:54 Trying to ignore the following packages:
12:09:54 - actionlib_msgs
12:09:54 - common_interfaces
12:09:54 - cv_bridge
12:09:54 - opencv_tests
12:09:54 - ros1_bridge
12:09:54 - shape_msgs
12:09:54 - stereo_msgs
12:09:54 - vision_opencv
12:09:54 - rmw_connext_cpp
12:09:54 - rosidl_typesupport_connext_c
12:09:54 - rosidl_typesupport_connext_cpp
12:09:54 - rmw_connext_dynamic_cpp
12:09:54 - connext_cmake_module
12:09:54 - rmw_connext_shared_cpp
12:09:54 - rmw_fastrtps_dynamic_cpp
12:09:57 Create marker file: src\ros2\common_interfaces\actionlib_msgs\COLCON_IGNORE
12:09:57 Create marker file: src\ros2\common_interfaces\common_interfaces\COLCON_IGNORE
12:09:57 Create marker file: src\ros-perception\vision_opencv\cv_bridge\COLCON_IGNORE
12:09:57 Create marker file: src\ros-perception\vision_opencv\opencv_tests\COLCON_IGNORE
12:09:57 Create marker file: src\ros2\rmw_fastrtps\rmw_fastrtps_dynamic_cpp\COLCON_IGNORE
12:09:57 Create marker file: src\ros2\ros1_bridge\COLCON_IGNORE
12:09:57 Create marker file: src\ros2\common_interfaces\shape_msgs\COLCON_IGNORE
12:09:57 Create marker file: src\ros2\common_interfaces\stereo_msgs\COLCON_IGNORE
12:09:57 Create marker file: src\ros-perception\vision_opencv\vision_opencv\COLCON_IGNORE
12:09:57 # END SUBSECTION

@mjcarroll
Copy link
Contributor

CI beyond the PR jobs appears to not be possible on this repo.

I believe that you can unblacklist and it should run. They are blacklisted due to blowing up build times, iirc.

@mjcarroll
Copy link
Contributor

But also these changes look good and the PR jobs passing is a pretty good sign.

@sloretz
Copy link
Contributor Author

sloretz commented Jan 14, 2022

CI attempt (CI branch: sloretz__DONOTMERGE_unblacklist_vision_opencv, supplemental repos, build: --packages-up-to image_geometry cv_bridge opencv_tests test: --packages-select image_geometry cv_bridge opencv_tests)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@sloretz
Copy link
Contributor Author

sloretz commented Jan 14, 2022

CI attempt is failing because boost isn't installed on the CI machines.

10:32:58 CMake Error at C:/Program Files/CMake/share/cmake-3.22/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
10:32:58   Could NOT find Boost (missing: Boost_INCLUDE_DIR python3)

It's a bit more work than I anticipated to make ci.ros2.org support building these packages again, and there's not much value since the PR jobs are passing. Merging.

@sloretz sloretz merged commit 9ea8908 into ros2 Jan 14, 2022
@sloretz sloretz deleted the sloretz__easy_idso_part1 branch January 14, 2022 19:00
@mjcarroll
Copy link
Contributor

CI attempt is failing because boost isn't installed on the CI machines.

Ah, I remember that now. My longish term plan was to strip boost and put in pybind, but I haven't found cycles.

ijnek added a commit that referenced this pull request Sep 17, 2022
This reverts commit 9ea8908.

Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
mergify bot pushed a commit that referenced this pull request Sep 17, 2022
This reverts commit 9ea8908.

Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
(cherry picked from commit 1a78fcb)
Ace314159 added a commit to Ace314159/vision_opencv that referenced this pull request Sep 18, 2022
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.

3 participants