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.

Sign up for GitHub

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

Jump to bottom

Don't find_package(ament_index_cpp ..) if RMW is configured at build-time #210

Merged
merged 1 commit into from
Nov 4, 2022
Merged

Don't find_package(ament_index_cpp ..) if RMW is configured at build-time #210

merged 1 commit into from
Nov 4, 2022

Conversation

gavanderhoorn
Copy link
Contributor

As per subject.

Should fix #209.

@gavanderhoorn
Build-time RMW selection does not need ament_index_cpp
ff30b90 
So don't find_package(..) it if RMW_IMPLEMENTATION_DISABLE_RUNTIME_SELECTION is OFF.

Signed-off-by: gavanderhoorn <g.a.vanderhoorn@tudelft.nl>
@gavanderhoorn
Copy link
Contributor Author

I'm currently on Humble, so if this could be backported to humble, that would be 👍

@gavanderhoorn gavanderhoorn mentioned this pull request Nov 2, 2022
Closed
@gavanderhoorn
Copy link
Contributor Author

Not sure what to make of the buildfarm build failing.

@clalancette
Copy link
Contributor

Not sure what to make of the buildfarm build failing.

We need to do some Rolling releases. I'll do those in a bit.

fujitatomoya
fujitatomoya approved these changes Nov 2, 2022
View reviewed changes
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm

@clalancette
Copy link
Contributor

@ros-pull-request-builder retest this please

1 similar comment
@clalancette
Copy link
Contributor

@ros-pull-request-builder retest this please

@clalancette
Copy link
Contributor

clalancette commented Nov 3, 2022 •
edited
Loading

Well, I don't know why the Rpr job is failing; I can't reproduce that locally. I'm going to run CI on this, and if that is good, merge.

CI:

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

@gavanderhoorn
Copy link
Contributor Author

Looks like the Jenkins agents get shut down before the build completes?

@clalancette
Copy link
Contributor

Looks like the Jenkins agents get shut down before the build completes?

Sigh, yes. The aarch64 nodes have been flapping a bit lately. They automatically restart, but the tracking badge doesn't follow that. Updated now.

@clalancette
Copy link
Contributor

CI looks good here (the yellow jobs just have a CMake warning, and that was fixed elsewhere). So going ahead and merging this one.

@clalancette clalancette merged commit 11652db into ros2:rolling Nov 4, 2022
@gavanderhoorn gavanderhoorn deleted the no_ament_index_cpp_without_runtime_rmw_selection branch November 4, 2022 18:47
@gavanderhoorn
Copy link
Contributor Author

thanks.

Could this one also be backported to Humble?

@clalancette
Copy link
Contributor

@Mergifyio backport humble

@mergify
Copy link

mergify bot commented Nov 4, 2022

backport humble

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Nov 4, 2022
@gavanderhoorn @mergify
Build-time RMW selection does not need ament_index_cpp (#210)
f52adcc 
So don't find_package(..) it if RMW_IMPLEMENTATION_DISABLE_RUNTIME_SELECTION is OFF.

Signed-off-by: gavanderhoorn <g.a.vanderhoorn@tudelft.nl>
(cherry picked from commit 11652db)
@mergify mergify bot mentioned this pull request Nov 4, 2022
Merged
clalancette pushed a commit that referenced this pull request Dec 22, 2022
@mergify @gavanderhoorn
Build-time RMW selection does not need ament_index_cpp (#210) (#211)
c726d7d 
So don't find_package(..) it if RMW_IMPLEMENTATION_DISABLE_RUNTIME_SELECTION is OFF.

Signed-off-by: gavanderhoorn <g.a.vanderhoorn@tudelft.nl>
(cherry picked from commit 11652db)

Co-authored-by: G.A. vd. Hoorn <g.a.vanderhoorn@tudelft.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Should ament_index_cpp only be find_package(..)ed when runtime selection is enabled?
3 participants
@gavanderhoorn @clalancette @fujitatomoya