search

ros2 / rmw_implementation

CMake infrastructure and dependencies for rmw implementations
Apache License 2.0
21 stars 48 forks source link

Fix build when only one rmw implementation is available #202

Closed ivanpauno closed 2 years ago

ivanpauno commented 2 years ago

After https://github.com/ros2/rmw_cyclonedds/pull/357, fix with only rmw_cyclonedds available were broken (I usually run CI with only one rmw impl to save time, except when it makes sense to test them all). Example: https://github.com/ros2/rcl/pull/956#issuecomment-997050045.

These removes the old trick done in the configs extra file, and it instead reexports and interface library. I could have kept the config extras file and add an IMPORTED target when appropriate there, but the end result I think is the same and the version is simpler.

Note:

I will run "one rmw" jobs for each rmw implementation to make sure this works in all cases, and also a multiple rmw job.

ivanpauno commented 2 years ago

I will run "one rmw" jobs for each rmw implementation to make sure this works in all cases, and also a multiple rmw job.

That seem to have worked locally, so running all the jobs.

Fastrtps only:

cyclonedds only:

connextdds only:

fastrtps/cyclonedds/connextdds combined:

ivanpauno commented 2 years ago

I don't understand how this was passing locally, I guess I didn't clean up my build and install folders well enough ... It seems that fastrtps and connexts are not exporting cmake targes, so I will first update that.

sloretz commented 2 years ago

I think this and #201 are solving the same issue. Unfortunately, I'm having strange import issues in launch_xml with the other PR that I haven't understood yet :-/

ivanpauno commented 2 years ago

I think this and #201 are solving the same issue. Unfortunately, I'm having strange import issues in launch_xml with the other PR that I haven't understood yet :-/

Yes, it's the same issue. I will close this one, as your PR avoids the issue I was running into altogether (https://github.com/ros2/rmw_implementation/pull/202#issuecomment-998153884).

If we modify rmw implementations to export targets correctly, the approach of this PR requires a bit less boilerplate, but that doesn't matter much.