Skip to content

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

Merged
sloretz merged 1 commit intoros2from
sloretz__easy_idso_part1
Jan 14, 2022
Merged

Install includes to include/${PROJECT_NAME} and more modern CMake#419
sloretz merged 1 commit intoros2from
sloretz__easy_idso_part1

Conversation

@sloretz
Copy link
Copy Markdown
Contributor

@sloretz sloretz commented Jan 5, 2022 •
edited
Loading

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

@sloretz
More alot
c5508c7 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz self-assigned this Jan 5, 2022
@sloretz sloretz mentioned this pull request Jan 5, 2022
Merged
audrow
audrow approved these changes Jan 10, 2022
View reviewed changes
Copy link
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
Contributor Author

sloretz commented Jan 10, 2022 •
edited
Loading

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
Copy Markdown
Contributor

mjcarroll commented Jan 13, 2022

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
Copy Markdown
Contributor

mjcarroll commented Jan 13, 2022

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

@sloretz
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

mjcarroll commented Jan 14, 2022

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 ijnek mentioned this pull request Sep 17, 2022
Closed
ijnek added a commit that referenced this pull request Sep 17, 2022
@ijnek
Revert "More alot (#419)"
1a78fcb 
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
@ijnek @mergify
Revert "More alot (#419)"
a09a5e5 
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
@Ace314159
Revert "Revert "More alot (ros-perception#419)""
deb39d4 
This reverts commit 1a78fcb.
@Ryanf55 Ryanf55 mentioned this pull request Jun 24, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mjcarroll mjcarroll mjcarroll approved these changes

@audrow audrow audrow approved these changes

Assignees

@sloretz sloretz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sloretz @mjcarroll @audrow