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

Use target output name for exporting typesupport library #625

Merged
merged 1 commit into from
Dec 6, 2021
Merged

Use target output name for exporting typesupport library #625

merged 1 commit into from
Dec 6, 2021

Conversation

jonselling
Copy link
Contributor

Fixes #623

@jonselling
Use target output name for exporting typesupport library
c442ef6 
Signed-off-by: Jonathan Selling <jselling@neyarobotics.com>
@aprotyas
Copy link
Member

Looks like the Rpr failure is unrelated. It happened in the last PR too.

@jonselling
Copy link
Contributor Author

jonselling commented Oct 30, 2021 •
edited
Loading

I had the issue when testing locally, I think it was due to ament_cmake_core not having a file in master that is present in the foxy branch that was expected to find the cmake function that it fails on.

@clalancette
Copy link
Contributor

The Rpr job will be fixed once we merge ros/rosdistro#31089

@jonselling
Copy link
Contributor Author

Is there a way for me to kick off another build since that PR was merged? I looked on Jenkins with no "Build Now" button or anything.

@clalancette
Copy link
Contributor

@ros-pull-request-builder retest this please

@jonselling
Copy link
Contributor Author

Seems like I jumped the gun on the PR merging in vs. getting into testing (I was unfamiliar with the release process and looked into it a little). I can kick off another build when the packages are in testing.

@clalancette
Copy link
Contributor

Seems like I jumped the gun on the PR merging in vs. getting into testing (I was unfamiliar with the release process and looked into it a little). I can kick off another build when the packages are in testing.

Ah yeah, I didn't check. Because desktop didn't get built (see https://github.com/ros2/ros_buildfarm_config/blob/ros2/rolling/release-build.yaml#L20), sync-to-testing wasn't done. I believe this will be fixed by ros/rosdistro#31106

@clalancette
Copy link
Contributor

@ros-pull-request-builder retest this please

@jonselling
Copy link
Contributor Author

@aprotyas Is this waiting on anything else?

Also, I assumed it was fine if I signed off on the commit or does it need to be a maintainer? The contributing document said all commits needed to be signed off

@aprotyas
Copy link
Member

aprotyas commented Nov 3, 2021 •
edited by clalancette
Loading

@aprotyas Is this waiting on anything else?

Just need to run CI, which I can do shortly.

Also, I assumed it was fine if I signed off on the commit or does it need to be a maintainer?

Yeah, just the former is okay. It looks like you've signed the commit already.

Running CI.
Repos file: https://gist.githubusercontent.com/aprotyas/207c62d108174aa8e303cea2ad41c794/raw/53f225a72f0c60a937c6a451d83d9ab8af2d1c2f/ros2.repos
Build args: --packages-above-and-dependencies rosidl_cmake
Test args: --packages-above rosidl_cmake
Job: https://ci.ros2.org/job/ci_launcher/9294/

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

@aprotyas
Copy link
Member

aprotyas commented Nov 4, 2021

The windows CI failure is annoying, but I reckon it's unrelated to this PR - I've seen the same test fails in a separate CI run yesterday too.

@clalancette
Copy link
Contributor

Nightly Windows seems to be green, so I've kicked off another Windows build to make sure it goes green here.

@aprotyas
Copy link
Member

aprotyas commented Nov 5, 2021 •
edited
Loading

Hmm, even more failures this time around... again, every failed test seems to be of the "exit gracefully"/"finite time-limit" type. No further insights.

Edit (11/05): Running Windows CI one last time before investigating further.

  • Windows Build Status

@jonselling
Copy link
Contributor Author

It looks like CI passed that time

@jonselling
Copy link
Contributor Author

@aprotyas anything else?

@aprotyas aprotyas requested a review from hidmic November 11, 2021 07:51
@aprotyas
Copy link
Member

CC @hidmic requesting your review since you were involved in the initial discussion over at #623.

hidmic
hidmic approved these changes Dec 6, 2021
View reviewed changes
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM @jonselling!

@hidmic hidmic merged commit e76ed13 into ros2:master Dec 6, 2021
@hidmic
Copy link
Contributor

hidmic commented Dec 6, 2021

@jonselling we can now open a backport PR to Foxy or Galactic if you need so.

@aprotyas
Copy link
Member

aprotyas commented Dec 6, 2021

@mergify backport galactic foxy

@aprotyas
Copy link
Member

aprotyas commented Dec 6, 2021

Opened backports for consideration in both distros since this was a bug.

mergify bot pushed a commit that referenced this pull request Dec 6, 2021
@jonselling
Use target output name for exporting typesupport library (#625)
69d28d7 
Signed-off-by: Jonathan Selling <jselling@neyarobotics.com>
(cherry picked from commit e76ed13)
@mergify mergify bot mentioned this pull request Dec 6, 2021
Merged
mergify bot pushed a commit that referenced this pull request Dec 6, 2021
@jonselling
Use target output name for exporting typesupport library (#625)
c948bc5 
Signed-off-by: Jonathan Selling <jselling@neyarobotics.com>
(cherry picked from commit e76ed13)
@mergify mergify bot mentioned this pull request Dec 6, 2021
Merged
@mergify
Copy link

mergify bot commented Dec 6, 2021

backport galactic foxy

✅ Backports have been created

Hey, I reacted but my real name is @Mergifyio

hidmic pushed a commit that referenced this pull request Feb 8, 2022
@jonselling @hidmic
Use target output name for exporting typesupport library (#625)
3053f9a 
Signed-off-by: Jonathan Selling <jselling@neyarobotics.com>
(cherry picked from commit e76ed13)
hidmic pushed a commit that referenced this pull request Feb 8, 2022
@jonselling @hidmic
Use target output name for exporting typesupport library (#625)
adda021 
Signed-off-by: Jonathan Selling <jselling@neyarobotics.com>
(cherry picked from commit e76ed13)
sloretz pushed a commit that referenced this pull request Jul 12, 2022
@jonselling @sloretz
Use target output name for exporting typesupport library (#625)
ad14886 
Signed-off-by: Jonathan Selling <jselling@neyarobotics.com>
(cherry picked from commit e76ed13)
sloretz pushed a commit that referenced this pull request Jul 19, 2022
@mergify @jonselling
Use target output name for exporting typesupport library (#625) (#638)
a0752a9 
Signed-off-by: Jonathan Selling <jselling@neyarobotics.com>
(cherry picked from commit e76ed13)

Co-authored-by: Jonathan Selling <jselling@neyarobotics.com>
quarkytale pushed a commit that referenced this pull request May 17, 2023
@mergify @jonselling @sloretz
Use target output name for exporting typesupport library (#625) (#639)
b62e8c4 
Signed-off-by: Jonathan Selling <jselling@neyarobotics.com>
(cherry picked from commit e76ed13)

Co-authored-by: Jonathan Selling <jselling@neyarobotics.com>
Co-authored-by: Shane Loretz <sloretz@osrfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hidmic hidmic hidmic approved these changes

@aprotyas aprotyas aprotyas approved these changes

Assignees

@hidmic hidmic

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Exporting typesupport libraries does not respect LIBRARY_NAME
4 participants
@jonselling @aprotyas @clalancette @hidmic