[dashing] Backports #1023 #1167

[ivanpauno]

This PR will be merged with --ff-only, to preserve commit hashes.
Backports #1023 #1167.

#1167 was modified, as #844 wasn't backported to eloquent.
I don't think it's a good idea to backport #844, as it introduces a behavior change that can affect some use cases (see discussion in #1156).

[ivanpauno]

@nuclearsandwich friednly ping

[nuclearsandwich]

I will wait until tomorrow to merge, so all nightly jobs were ran once with these changes in.

This line doesn't make sense to me. Unmerged changes won't be part of nightlies and ci.ros2.org doesn't run Dashing nightlies.

[ivanpauno]

This line doesn't make sense to me. Unmerged changes won't be part of nightlies and ci.ros2.org doesn't run Dashing nightlies.

Sorry, that is old.

I merged the original PR the same day I opened the backport.
CI jobs in the original PR were more limited than full CI.
I wanted to double check that there were no regressions in full CI.

That has been checked, so the comment doesn't apply anymore.

[ivanpauno]

The test failing in the PR checker is a known flaky test.

[ivanpauno]

@nuclearsandwich friendly ping

[hidmic]

LGTM but for one question

[ivanpauno]

@hidmic based on the failing PR check, spin_until_future_complete will be solved at compile time in another package and it needs to link against spin_once_impl.

I will undo the last change and use RCLCPP_PUBLIC again.

[hidmic]

@hidmic based on the failing PR check, spin_until_future_complete will be solved at compile time in another package and it needs to link against spin_once_impl.

Oh, right, this is a template class. Hmm, that means that all downstream packages have to be recompiled to make use of this :/

[ivanpauno]

Oh, right, this is a template class.

The method is a template, not the class.

Hmm, that means that all downstream packages have to be recompiled to make use of this :/

Users will need to recompile to get the fix, but users won't have to recompile if they are not interested in this fix (i.e. this is ABI compatible).
For fixes in templates (and in inline functions), there's no other way to get a fix than recompiling (in the extreme case of a header only library, you always have to recompile to get the latest fixes).

[hidmic]

LGTM

[hidmic]

Users will need to recompile to get the fix, but users won't have to recompile if they are not interested in this fix (i.e. this is ABI compatible).

Yeah, I know. It striked me as a bit odd, but it won't do any harm. The added throw could mean trouble but I'm sure spin_once() was already throwing.