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

Make demo_nodes_cpp_native install stuff only when it builds #590

Merged
merged 2 commits into from
Nov 11, 2022

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Nov 10, 2022

This fixes a problem when using the tarball produced by this job: https://build.ros2.org/view/Hci/job/Hci__nightly-cyclonedds_ubuntu_jammy_amd64/

Rosdep sees demo_nodes_cpp_native requires rmw_fastrtps_cpp, but the tarball doesn't have that package. This causes rosdep to fail to find a system dependency with that name.

The ament_package() call installs the package.xml and an ament_index entry for the demo_nodes_cpp_native package, but I don't think it should because the package didn't build anything. This uses return() to prevent ament_package() from being called when rmw_fastrtps_cpp isn't found.

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz added the bug Something isn't working label Nov 10, 2022
@sloretz sloretz self-assigned this Nov 10, 2022
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I've left one non-blocking comment. I'll let you decide whether to accept it. Otherwise, looks good to me with green CI.

demo_nodes_cpp_native/CMakeLists.txt Outdated Show resolved Hide resolved
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Copy link
Contributor Author

sloretz commented Nov 11, 2022

CI (build: --packages-up-to demo_nodes_cpp_native test: --packages-select demo_nodes_cpp_native)

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

CI (CycloneDDS only build: --packages-ignore fastcdr foonathan_memory_vendor rosbag2_converter_default_plugins --packages-ignore-regex .*connext.* .*fastrtps.* --packages-up-to demo_nodes_cpp_native test: --packages-ignore fastcdr foonathan_memory_vendor rosbag2_converter_default_plugins --packages-ignore-regex .*connext.* .*fastrtps.* --packages-select ament_cmake)

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

@sloretz sloretz merged commit 74db664 into rolling Nov 11, 2022
@delete-merged-branch delete-merged-branch bot deleted the sloretz__demos__install_nothing_when_not_building branch November 11, 2022 22:58
@sloretz
Copy link
Contributor Author

sloretz commented Nov 17, 2022

@Mergifyio backport humble

@mergify
Copy link

mergify bot commented Nov 17, 2022

backport humble

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Nov 17, 2022
* Make demo_nodes_cpp_native install stuff only when it builds

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

* doing nothing -> skipping

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

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
(cherry picked from commit 74db664)
sloretz added a commit that referenced this pull request Dec 5, 2022
…591)

* Make demo_nodes_cpp_native install stuff only when it builds

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

* doing nothing -> skipping

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

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
(cherry picked from commit 74db664)

Co-authored-by: Shane Loretz <sloretz@osrfoundation.org>
cardboardcode pushed a commit to cardboardcode/demos that referenced this pull request Jan 23, 2023
* Make demo_nodes_cpp_native install stuff only when it builds

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

* doing nothing -> skipping

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

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
cardboardcode pushed a commit to cardboardcode/demos that referenced this pull request Jan 23, 2023
* Make demo_nodes_cpp_native install stuff only when it builds

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

* doing nothing -> skipping

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

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Bey Hao Yun <beyhy94@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants