Handle platform and normal profiles

Currently, a rule like "pkg [platform:rpm test]" will install the
package if one of the conditions is true - so it will install pkg even
on non-RPM systems.

Change the logic so that packages are only installed if one platform
rule is true.

So, for platform:dpkg and the following file using test profile:

install
install2 [test]
non-install3 [platform:rpm]
install4 [platform:dpkg]
non-install5 [quark]
install6 [platform:dpkg test]

Only the "install*" packages should be installed.

Change-Id: I8f71f2b53e786d8aca6d02f973a00fecc2bca4f3
Co-Authored-By: Robert Collins <robertc@robertcollins.net>

README.rst

@@ -82,6 +82,13 @@ profile (or selected ``default``). ``[default postgresql test]`` would match
 those three profiles but not ``mysql``. ``[platform:rhel]`` will match only
 when running in a RHEL linux environment.
 
+Note that platform selectors are treated as kind of filter: If a line
+contains a platform selector, then the package only gets installed if
+at least one of the platform selectors matches in addition to the
+match on the other selectors. As an example, ``[platform:rpm test]``
+would only install a package on a RPM platform if the test selector is
+used.
+
 Version constraints are a comma separated list of constraints where each
 constraint is  (== | < | <= | >= | > | !=) VERSION, and the constraints are ANDed
 together (the same as pip requirements version constraints).

bindep/depends.py

@@ -61,6 +61,23 @@ blank = ws? '\n' -> None
 class Depends(object):
     """Project dependencies."""
 
+    # Truth table for combining platform and user profiles:
+    # (platform, user) where False means that component
+    # voted definitely no, True means that that component
+    # voted definitely yes and None means that that component
+    # hasn't voted.
+    _include = {
+        (False, False): False,
+        (False, None): False,
+        (False, True): False,
+        (None, False): False,
+        (None, None): True,
+        (None, True): True,
+        (True, False): False,
+        (True, None): True,
+        (True, True): True,
+    }
+
     def __init__(self, depends_string):
         """Construct a Depends instance.
 
@@ -71,6 +88,49 @@ class Depends(object):
         parser = makeGrammar(grammar, {})(depends_string)
         self._rules = parser.rules()
 
+    def _partition(self, rule):
+        """Separate conditions into platform and user profiles.
+
+        :return Two lists, the platform and user profiles.
+        """
+        platform = []
+        user = []
+        for sense, profile in rule[1]:
+            if profile.startswith("platform:"):
+                platform.append((sense, profile))
+            else:
+                user.append((sense, profile))
+        return platform, user
+
+    def _evaluate(self, partition_rule, profiles):
+        """Evaluate rule. Does it match the profiles?
+
+        :return Result is trinary: False for definitely no, True for
+        definitely yes, None for no rules present.
+        """
+
+        if partition_rule == []:
+            return None
+
+        # Have we seen any positive selectors - if not, the absence of
+        # negatives means we include the rule, but if we any positive
+        # selectors we need a match.
+        positive = False
+        match_found = False
+        negative = False
+        for sense, profile in partition_rule:
+            if sense:
+                positive = True
+                if profile in profiles:
+                    match_found = True
+            else:
+                if profile in profiles:
+                    negative = True
+                    break
+        if not negative and (match_found or not positive):
+                return True
+        return False
+
     def active_rules(self, profiles):
         """Return the rules active given profiles.
 
@@ -80,22 +140,15 @@ class Depends(object):
         profiles = set(profiles)
         result = []
         for rule in self._rules:
-            # Have we seen any positive selectors - if not, the absence of
+            # Partition rules
-            # negatives means we include the rule, but if we any positive
+            platform_profiles, user_profiles = self._partition(rule)
-            # selectors we need a match.
+            # Evaluate each partition separately
-            positive = False
+            platform_status = self._evaluate(platform_profiles, profiles)
-            match_found = False
+            user_status = self._evaluate(user_profiles, profiles)
-            negative = False
+            # Combine results
-            for sense, profile in rule[1]:
+            # These are trinary: False for definitely no, True for
-                if sense:
+            # definitely yes, None for no rules present.
-                    positive = True
+            if self._include[platform_status, user_status]:
-                    if profile in profiles:
-                        match_found = True
-                else:
-                    if profile in profiles:
-                        negative = True
-                        break
-            if not negative and (match_found or not positive):
                 result.append(rule)
         return result
 

bindep/tests/test_depends.py

@@ -230,6 +230,87 @@ class TestDepends(TestCase):
         self.assertRaises(ometa.runtime.ParseError,
                           lambda: Depends("foo [platform:bar@baz]\n"))
 
+    def test_platforms_include(self):
+        # 9 tests for the nine cases of _include in Depends
+        depends = Depends(dedent("""\
+            # False, False -> False
+            install1 [platform:dpkg quark]
+            # False, None -> False
+            install2 [platform:dpkg]
+            # False, True -> False
+            install3 [platform:dpkg test]
+            # None, False -> False
+            install4 [quark]
+            # None, None -> True
+            install5
+            # None, True -> True
+            install6 [test]
+            # True, False -> False
+            install7 [platform:rpm quark]
+            # True, None -> True
+            install8 [platform:rpm]
+            # True, True -> True
+            install9 [platform:rpm test]
+            """))
+
+        # With platform:dpkg and quark False and platform:rpm and test
+        # True, the above mimics the conditions from _include.
+        self.expectThat(
+            set(r[0] for r in depends.active_rules(['platform:rpm', 'test'])),
+            Equals({"install5", "install6", "install8", "install9"}))
+
+    def test_platforms(self):
+        depends = Depends(dedent("""\
+            install1
+            install2 [test]
+            install3 [platform:rpm]
+            install4 [platform:dpkg]
+            install5 [quark]
+            install6 [platform:dpkg test]
+            install7 [quark test]
+            install8 [platform:dpkg platform:rpm]
+            install9 [platform:dpkg platform:rpm test]
+            installA [!platform:dpkg]
+            installB [!platform:dpkg test]
+            installC [!platform:dpkg !test]
+            installD [platform:dpkg !test]
+            installE [platform:dpkg !platform:rpm]
+            installF [platform:dpkg !platform:rpm test]
+            installG [!platform:dpkg !platform:rpm]
+            installH [!platform:dpkg !platform:rpm test]
+            installI [!platform:dpkg !platform:rpm !test]
+            installJ [platform:dpkg !platform:rpm !test]
+            """))
+
+        # Platform-only rules and rules with no platform are activated
+        # by a matching platform.
+        self.expectThat(
+            set(r[0] for r in depends.active_rules(['platform:dpkg'])),
+            Equals({"install1", "install4", "install8", "installD",
+                    "installE", "installJ"}))
+
+        # Non-platform rules matching one-or-more profiles plus any
+        # matching platform guarded rules.
+        self.expectThat(
+            set(r[0] for r in depends.active_rules(['platform:dpkg', 'test'])),
+            Equals({"install1", "install2", "install4", "install6", "install7",
+                    "install8", "install9", "installE", "installF"}))
+
+        # When multiple platforms are present, none-or-any-platform is
+        # enough to match.
+        self.expectThat(
+            set(r[0] for r in depends.active_rules(['platform:rpm'])),
+            Equals({"install1", "install3", "install8", "installA",
+                    "installC"}))
+
+        # If there are any platform profiles on a rule one of them
+        # must match an active platform even when other profiles match
+        # for the rule to be active.
+        self.expectThat(
+            set(r[0] for r in depends.active_rules(['platform:rpm', 'test'])),
+            Equals({"install1", "install2", "install3", "install7", "install8",
+                    "install9", "installA", "installB"}))
+
 
 class TestDpkg(TestCase):