Fix CMake install so overriding works (backport #926)

[mergify]

• Fix overriding of install: hardware_interface
• Fix overriding of install: controller_interface
• Fix overriding of install: ros2_control_test_assets
• Fix overriding of install: controller_manager
• Fix overriding of install: controller_manager_msgs
• Fix overriding of install: joint_limits
• Fix overriding of install: transmission_interface
• Upgrade CMake version

Description

This is related to:

• Fix overriding of install kinematics_interface#13
• Fix overriding of install ros2_controllers#510
• Fix overriding of install realtime_tools#105

Here are the colcon docs on overriding: https://colcon.readthedocs.io/en/released/user/overriding-packages.html#how-to-make-it-easier-for-your-users-to-override

Here is the ament_cmake user guide: https://docs.ros.org/en/rolling/How-To-Guides/Ament-CMake-Documentation.html

These changes have been tested locally and are in line with the changes we have been making to CMake for MoveIt. There are a few goals here:

• Standardize the way we use CMake to be more declarative and modern
• Correctly use the ament tooling so building a source copy of this repo on a machine with a binary install works (also called overriding)

One thing I did not do is turn on -Werror or increase the warning set when building this library. Given the time I'll follow up with PRs that fix the warnings and turn on -Werror.

---

This is an automatic backport of pull request #926 done by Mergify.

[mergify]

Cherry-pick of 727297e has failed:

On branch mergify/bp/humble/pr-926
Your branch is up to date with 'origin/humble'.

You are currently cherry-picking commit 727297e.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   controller_manager_msgs/CMakeLists.txt
	modified:   ros2_control/CMakeLists.txt

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   controller_interface/CMakeLists.txt
	both modified:   controller_interface/package.xml
	both modified:   controller_manager/CMakeLists.txt
	both modified:   controller_manager/package.xml
	both modified:   hardware_interface/CMakeLists.txt
	both modified:   joint_limits/CMakeLists.txt
	both modified:   ros2_control_test_assets/CMakeLists.txt
	both modified:   transmission_interface/CMakeLists.txt

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

[christian-rauch]

@christophfroehlich I fixed some of the CI issues with #2482, but I cannot run the full CI pipeline there, and I also cannot propose changes to your PR branch this way. Can you cherry-pick my commit to your PR and trigger the CI again?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.69%. Comparing base (e6ff120) to head (ae6a589).
⚠️ Report is 6 commits behind head on humble.

Additional details and impacted files

@@            Coverage Diff             @@
##           humble    #2480      +/-   ##
==========================================
+ Coverage   63.67%   63.69%   +0.02%     
==========================================
  Files         112      112              
  Lines       12933    12933              
  Branches     8735     8738       +3     
==========================================
+ Hits         8235     8238       +3     
+ Misses        806      804       -2     
+ Partials     3892     3891       -1     

Flag	Coverage Δ
unittests	63.69% <ø> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[christian-rauch]

I can confirm that the ros2_control in the state of this PR solves the issue I reported in #2479.

There is only on test failure left. This is on Red Hat, or AlmaLinux respectively, which I cannot reproduce locally or in CI. Given that this PR is 5 commits behind the current humble branch and the history on the humble branch also shows previous CI failures, I would simply assume that this is unrelated.

@christophfroehlich Can we go ahead with this PR?

[christophfroehlich]

The failing RHEL job is known and not related.

@bmagyar @saikishor do you see problems in backporting this?

[saikishor]

The failing RHEL job is known and not related.

@bmagyar @saikishor do you see problems in backporting this?

I think it should be fine

[bmagyar]

Let me check this in the morning on a bigger screen :)

[bmagyar]

@christophfroehlich how close is this to the original changeset? I think we should update the title as this isn't primarily a fix for overrides anymore but migrating to modern cmake targets.

[christophfroehlich]

It is as close as we were able to fix the merge conflicts and apply the changes in a meaningful way ;)

[christian-rauch]

It is as close as we were able to fix the merge conflicts and apply the changes in a meaningful way ;)

I think @bmagyar meant, how related are the new commits to the original intent of the PR. Originally, this PR was just a backport of #926. But we added a couple of new commits. The commits I added were necessary fixes to have the CI pass.

@christophfroehlich are the commits you added required for this backport PR? Or could they be moved to another dedicated PR?

[christophfroehlich]

some of them where unintentional removals while trying to fix the merge conflicts, only the last one is "new"

[christian-rauch]

some of them where unintentional removals while trying to fix the merge conflicts, only the last one is "new"

Do we need all your commits on this PR branch? As far as I remember, up to and including my last commit 979d069, the CI was passing all pipelines, except the "RHEL" workflow, and this one still fails.

@christophfroehlich Can you move your commits after mine to a separate PR? This way, we can continue with this pure "backport + test fixes" PR here.

[christophfroehlich]

if tests were deactivated, how should the Ci complain..

[christian-rauch]

If the tests were deactivated before this PR, doesn't it mean they should stay this way? Or do you mean that the backport deactivated the tests, and you activated them again? I.e. were you reverting some of the changes done in the first commit 1c9ffcf? Sorry, if I did not see that.

[christophfroehlich]

Exactly, the backport had lots of merge conflicts because the master branch from the time of the PR to be backported already diverted a lot to the current humble branch; And I accidentally removed them with the first merge-conflict-fix-commit. Summarized: The same tests from the current humble head are activate in this PR now.

[christian-rauch]

I see. Thanks for clarifying. In this case, I think we can stick with the PR title as the functional changes are the ones that are back ported and every thing else is just fixes for the backport itself.

@bmagyar Are you ok with this? Can we go ahead with merging the PR?

[christophfroehlich]

We need to make sure to not break any downstream packages. We are currently trying to get the prerelease checks from ROS 1 buildfarm running

[christophfroehlich]

#2502 nmea_hardware_interface failed. I can check later if this is related to this pr.

[christophfroehlich]

It fails because the switches depending on environment variables fails, it tries to find the rolling version.
https://github.com/OUXT-Polaris/nmea_hardware_interface/blob/1d49893cc5e21ad8feadcfe2dc161236413ea883/include/nmea_hardware_interface/gps_hardware_interface.hpp#L21-L25

All other level 1 dependencies build successfully.

[bmagyar]

If we break anything, they will roll our release back. We need to have a fix for the referenced package above released first.

[christophfroehlich]

it is red on the buildfarm anyways, maybe never was green

[bmagyar]

Yeah it seems it's been a while since it was maintained. Let's try then