Optionally break version-incompatible dependencies (#45)

This flag is intended to support scenarios where a developer
intentionally includes packages in a workspace where the underlying
build system will not use dependencies which have incompatible versions.

Because this scenario is somewhat of a deviation from the established
expectations of colcon behavior, we'll print a warning to the developer
(at least for now).

At this stage in package enumeration, we may encounter multiple packages
with a given name. This change also re-structures the enumeration so
that all packages with a given name can be checked, where previously
only a single instance of the package was checked.

Author: cottsay

colcon_package_information/package_augmentation/check_dependency_constraint.py

@@ -29,18 +29,37 @@ def __init__(self):  # noqa: D107
     def augment_packages(  # noqa: D102
         self, descs, *, additional_argument_names=None
     ):
-        descs_dict = {desc.name: desc for desc in descs}
-        for name, desc in descs_dict.items():
+        descs_dict = {}
+        for desc in descs:
+            descs_dict.setdefault(desc.name, []).append(desc)
+        for desc in descs:
             deps = desc.get_dependencies()
             for dep in deps:
                 # skip dependencies which don't have a descriptor
                 if dep.name not in descs_dict:
                     continue
 
-                self._check_version_constraints(
-                    desc, dep, descs_dict[dep.name])
+                miss = all(
+                    self._check_version_constraints(desc, dep, dep_desc)
+                    for dep_desc in descs_dict.get(dep.name, ()))
+                if miss and dep.metadata.get('skip_incompatible'):
+                    logger.warning(
+                        'Dropping version-incompatible dependency on ' +
+                        dep.name + ' from ' + desc.name)
+                    for category in dep.metadata.get('categories') or ():
+                        desc.dependencies[category].discard(dep)
 
     def _check_version_constraints(self, desc, dep, dep_desc):
+        """
+        Check dependency version constraints against another package.
+
+        :param desc: The package declaring the dependency being checked
+        :param dep: The dependency being checked
+        :param dep_desc: The package to which the dependency refers
+
+        :returns: True if one or more constraints fail, False if all
+          constraints pass, and None if no constraints were checked.
+        """
         # if the dependency descriptor doesn't have a version there is nothing
         # to compare to
         if 'version' not in dep_desc.metadata:
@@ -65,6 +84,7 @@ def gte(a, b):
             'version_gte': (gte, 'greater than or equal to'),
             'version_gt': (operator.gt, 'greater than'),
         }
+        miss = None
         for key, value in dep.metadata.items():
             # only consider version operator metadata
             if key not in operators:
@@ -81,12 +101,15 @@ def gte(a, b):
                 continue
 
             if not op(dep_version, version_constraint):
+                miss = True
                 logger.warning(
                     desc.name + ' depends on ' + dep.name +
                     ' which has version ' + dep_desc.metadata['version'] +
                     ' but expects it to be ' + msg + ' ' + value)
             else:
+                miss = miss or False
                 logger.debug(
                     desc.name + ' depends on ' + dep.name +
                     ' which has version ' + dep_desc.metadata['version'] +
                     ' which satisfies to be ' + msg + ' ' + value)
+        return miss

test/test_check_dependency_constraint.py

@@ -116,3 +116,47 @@ def test_dependency_invalid_version_operator():
 
     extension = CheckDependencyConstraintPackageAugmentation()
     extension.augment_packages({pkg_a, pkg_b})
+
+
+@pytest.mark.parametrize(
+    'a_versions,skip_incompatible,dep_categories,expect_present', [
+        (['1.0'],        True,  {'build'},        False),
+        (['1.0', '3.0'], True,  {'build'},        False),
+        (['1.0', '2.0'], True,  {'build'},        True),
+        (['1.0'],        False, {'build'},        True),
+        (['1.0'],        True,  {'build', 'run'}, False),
+        ([None],         True,  {'build'},        True),
+    ])
+def test_skip_incompatible(
+    a_versions, skip_incompatible, dep_categories, expect_present
+):
+    descs = set()
+    for i, v in enumerate(a_versions):
+        pkg_a = PackageDescriptor(f'/tmp/pkg_a_{i}')
+        pkg_a.name = 'pkg_a'
+        if v is not None:
+            pkg_a.metadata['version'] = v
+        descs.add(pkg_a)
+
+    metadata = {
+        'version_eq': '2.0',
+        'skip_incompatible': skip_incompatible,
+        'categories': dep_categories,
+    }
+    pkg_a_dep = DependencyDescriptor('pkg_a', metadata=metadata)
+
+    pkg_b = PackageDescriptor('/tmp/pkg_b')
+    pkg_b.name = 'pkg_b'
+    for cat in dep_categories:
+        pkg_b.dependencies[cat] = {pkg_a_dep}
+
+    descs.add(pkg_b)
+
+    extension = CheckDependencyConstraintPackageAugmentation()
+    extension.augment_packages(descs)
+
+    for cat in dep_categories:
+        if expect_present:
+            assert pkg_a_dep in pkg_b.dependencies[cat]
+        else:
+            assert pkg_a_dep not in pkg_b.dependencies.get(cat, set())