Warn when overriding packages at build time

[sloretz]

Part of ros2/ros2#1150

This PR adds a new warning to colcon build when a package to be built would override one in an underlay workspace. It also adds an option --alow-overriding to disable the a warning and allow building it anyways.

Trying this PR out

1. Bootstrap colcon from source with this PR and the companion PR in Add AmentInstalledPackageFinder for override warning colcon-ros#129
2. Install ros-rolling-desktop
3. Create a colcon workspace and one or more ROS 2 repos in it (below I did [email protected]:ros2/rcutils.git and [email protected]:ros2/rcl.git)
4. source /opt/ros/rolling/setup.bash
5. Run colcon build

$ colcon build
[0.387s] ERROR:colcon:colcon build: Some selected packages are already built in one or more underlay workspaces:
	'rcutils' is in: /opt/ros/rolling
	'rcl_yaml_param_parser' is in: /opt/ros/rolling
	'rcl' is in: /opt/ros/rolling
	'rcl_action' is in: /opt/ros/rolling
	'rcl_lifecycle' is in: /opt/ros/rolling
If a package in a merged underlay workspace is overridden and it installs headers, then all packages in the overlay must sort their include directories by workspace order. Failure to do so may result in build failures or undefined behavior at run time.
If the overridden package is used by another package in any underlay, then the overriding package in the overlay must be API and ABI compatible or undefined behavior at run time may occur.

If you understand the risks and want to override a package anyways, add the following to the command line:
	--allow-overriding rcl rcl_action rcl_lifecycle rcl_yaml_param_parser rcutils

How it works

This adds an extension point FindInstalledPackagesExtensionPoint which allows extensions to add to the packages found when find_installed_packages(...) is called. This hook allows colcon-ros to implement an extension point that reads the ament index to find packages. In the main() of the Build verb it finds all installed packages in chained prefixes, and checks if any package being built already exists in one of them. If so, the warning is emitted.

Limitations

Warning at build time is not perfect, because it's possible a user will install a package into the underlay /opt/ros/... later that was built by this overlay beforehand. I tried looking at how to make the setup scripts warn when they're sourced, but it seems difficult to do cleanly in the case of /opt/ros/....

[sloretz]

Made draft while pushing debug commits to see what's happening in Window CI

[sloretz]

Added unit tests and fixed CI! Ready for review :)

[codecov]

Codecov Report

Merging #449 (560656a) into master (1f8d954) will increase coverage by 0.53%.
The diff coverage is 70.40%.

@@            Coverage Diff             @@
##           master     #449      +/-   ##
==========================================
+ Coverage   80.27%   80.80%   +0.53%     
==========================================
  Files          58       59       +1     
  Lines        3442     3517      +75     
  Branches      640      668      +28     
==========================================
+ Hits         2763     2842      +79     
- Misses        623      630       +7     
+ Partials       56       45      -11     

Impacted Files	Coverage Δ
colcon_core/verb/build.py	0.00% <0.00%> (ø)
colcon_core/shell/__init__.py	98.54% <100.00%> (+1.04%)	⬆️
colcon_core/shell/installed_packages.py	100.00% <100.00%> (ø)
colcon_core/dependency_descriptor.py	100.00% <0.00%> (ø)
colcon_core/executor/sequential.py	95.65% <0.00%> (+1.44%)	⬆️
colcon_core/logging.py	91.37% <0.00%> (+3.44%)	⬆️
colcon_core/subprocess.py	65.62% <0.00%> (+5.46%)	⬆️
colcon_core/event/command.py	100.00% <0.00%> (+5.66%)	⬆️
colcon_core/shell/sh.py	91.30% <0.00%> (+5.79%)	⬆️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f8d954...560656a. Read the comment docs.

[jacobperron]

LGTM

[sloretz]

@dirk-thomas would you like to review this and colcon/colcon-ros#129, or may I merge them?

[dirk-thomas]

@sloretz since I don't have the bandwidth to review colcon changes in my spare time, please feel free to move forward at your own discretion.

On a very brief look the reduction in coverage might be something to consider addressing. Also the core changes will likely need to be released first, with a proper version dependency in the non-core package.

[sloretz]

On a very brief look the reduction in coverage might be something to consider addressing

I added a couple more tests for edge cases, which I think is the maximum amount of testable code in this PR.

Also the core changes will likely need to be released first, with a proper version dependency in the non-core package.

Noted 👍

[jacobperron]

Latest changes also LGTM