Aggregate all migration decisions and present it in excuses

With this change, Britney can now provide a very brief summary of the
migration via one single value (YAML) or line (HTML).  This solves two
issues:

 * It provides an aggregated version of the policy decision without
   having to loop over all policies (and even those would not give
   a full verdict on their own as not all rejections come from
   policies)

 * It enables a simple way to inform readers of the HTML excuses of
   whether a rejection is permanent or not.  This should hopefully
   make it easier for contributors to understand Britney and react
   more pro-actively.

Signed-off-by: Niels Thykier <niels@thykier.net>

britney.py

@@ -1085,7 +1085,7 @@ class Britney(object):
             self.excuses[excuse.name] = excuse
             return False
 
-        excuse.is_valid = True
+        excuse.policy_verdict = PolicyVerdict.PASS
         self.excuses[excuse.name] = excuse
         return True
 
@@ -1237,7 +1237,7 @@ class Britney(object):
 
         # if there is nothing wrong and there is something worth doing, this is a valid candidate
         if not anywrongver and anyworthdoing:
-            excuse.is_valid = True
+            excuse.policy_verdict = PolicyVerdict.PASS
             self.excuses[excuse.name] = excuse
             return True
         # else if there is something worth doing (but something wrong, too) this package won't be considered
@@ -1293,7 +1293,7 @@ class Britney(object):
             return False
 
         # the starting point is that we will update the candidate
-        excuse.is_valid = True
+        excuse.policy_verdict = PolicyVerdict.PASS
 
         # if there is a `remove' hint and the requested version is the same as the
         # version in testing, then stop here and return False
@@ -1303,7 +1303,7 @@ class Britney(object):
                 excuse.add_hint(hint)
                 excuse.addhtml("Removal request by %s" % (hint.user))
                 excuse.addhtml("Trying to remove package, not update it")
-                excuse.is_valid = False
+                excuse.policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY
                 break
 
         # check if there is a `block' or `block-udeb' hint for this package, or a `block-all source' hint
@@ -1358,7 +1358,7 @@ class Britney(object):
                 else:
                     excuse.addhtml("NEEDS APPROVAL BY RM")
                     excuse.addreason("block")
-                excuse.is_valid = False
+                excuse.policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY
 
         # at this point, we check the status of the builds on all the supported architectures
         # to catch the out-of-date ones
@@ -1404,7 +1404,7 @@ class Britney(object):
                         # we would not need to be forgiving at
                         # all. However, due to how arch:all packages
                         # are handled, we do run into occasionally.
-                        excuse.is_valid = False
+                        excuse.policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY
 
             # if there are out-of-date packages, warn about them in the excuse and set excuse.is_valid
             # to False to block the update; if the architecture where the package is out-of-date is
@@ -1434,9 +1434,9 @@ class Britney(object):
                         if self.options.ignore_cruft:
                             text = text + " (but ignoring cruft, so nevermind)"
                         else:
-                            excuse.is_valid = False
+                            excuse.policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY
                     else:
-                        excuse.is_valid = False
+                        excuse.policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY
                         excuse.missing_build_on_arch(arch)
 
                 excuse.addhtml(text)
@@ -1445,21 +1445,20 @@ class Britney(object):
         if not source_u.binaries:
             excuse.addhtml("%s has no binaries on any arch" % src)
             excuse.addreason("no-binaries")
-            excuse.is_valid = False
+            excuse.policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY
 
         # if the suite is unstable, then we have to check the urgency and the minimum days of
         # permanence in unstable before updating testing; if the source package is too young,
         # the check fails and we set is_valid to False to block the update; consider
         # the age-days hint, if specified for the package
+        policy_verdict = excuse.policy_verdict
         policy_info = excuse.policy_info
-        policy_verdict = PolicyVerdict.PASS
         for policy in self.policies:
             if suite in policy.applicable_suites:
                 v = policy.apply_policy(policy_info, suite, src, source_t, source_u, excuse)
                 if v.value > policy_verdict.value:
                     policy_verdict = v
-                    if policy_verdict.is_rejected:
+        excuse.policy_verdict = policy_verdict
-                        excuse.is_valid = False
 
         if suite in ('pu', 'tpu') and source_t:
             # o-o-d(ish) checks for (t-)p-u
@@ -1490,7 +1489,7 @@ class Britney(object):
                     text = text + " (but %s isn't keeping up, so never mind)" % (arch)
                     excuse.missing_build_on_ood_arch(arch)
                 else:
-                    excuse.is_valid = False
+                    excuse.policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY
                     excuse.missing_build_on_arch(arch)
 
                 excuse.addhtml(text)
@@ -1585,7 +1584,10 @@ class Britney(object):
             excuse.addhtml("Removal request by %s" % (hint.user))
             excuse.addhtml("Package is broken, will try to remove")
             excuse.add_hint(hint)
-            excuse.is_valid = True
+            # Using "PASS" here as "Created by a hint" != "accepted due to hint".  In a future
+            # where there might be policy checks on removals, it would make sense to distinguish
+            # those two states.  Not sure that future will ever be.
+            excuse.policy_verdict = PolicyVerdict.PASS
             excuses[excuse.name] = excuse
 
         # extract the not considered packages, which are in the excuses but not in upgrade_me

britney2/excuse.py

@@ -17,6 +17,7 @@
 from collections import defaultdict
 import re
 
+from britney2.policies.policy import PolicyVerdict
 
 class Excuse(object):
     """Excuse class
@@ -51,6 +52,7 @@ class Excuse(object):
         self.needs_approval = False
         self.hints = []
         self.forced = False
+        self._policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY
 
         self.invalid_deps = []
         self.deps = {}
@@ -73,13 +75,19 @@ class Excuse(object):
 
     @property
     def is_valid(self):
-        return self._is_valid
+        return False if self._policy_verdict.is_rejected else True
-
-    @is_valid.setter
-    def is_valid(self, value):
-        self._is_valid = value
 
+    @property
+    def policy_verdict(self):
+        return self._policy_verdict
 
+    @policy_verdict.setter
+    def policy_verdict(self, value):
+        if value.is_rejected and self.forced:
+            # By virtue of being forced, the item was hinted to
+            # undo the rejection
+            value = PolicyVerdict.PASS_HINTED
+        self._policy_verdict = value
 
     def set_vers(self, tver, uver):
         """Set the testing and unstable versions"""
@@ -120,8 +128,8 @@ class Excuse(object):
     def force(self):
         """Add force hint"""
         self.forced = True
-        if not self._is_valid:
+        if self._policy_verdict.is_rejected:
-            self._is_valid = True
+            self._policy_verdict = PolicyVerdict.PASS_HINTED
             return True
         return False
 
@@ -144,10 +152,24 @@ class Excuse(object):
     def add_hint(self, hint):
         self.hints.append(hint)
 
+    def _format_verdict_summary(self):
+        verdict = self._policy_verdict
+        if not verdict.is_rejected:
+            msg = 'OK: Will attempt migration'
+            if verdict == PolicyVerdict.PASS_HINTED:
+                msg = 'OK: Will attempt migration due to a hint'
+            msg += " (Any information below is purely informational)"
+            return msg
+        if verdict == PolicyVerdict.REJECTED_PERMANENTLY:
+            msg = "BLOCKED: Will not migrate (Please review if it introduces a regression or needs approval/unblock)"
+            return msg
+        return "TEMP-BLOCKED: Waiting for test results, another package or too young (no action required at this time)"
+
     def html(self):
         """Render the excuse in HTML"""
         res = "<a id=\"%s\" name=\"%s\">%s</a> (%s to %s)\n<ul>\n" % \
             (self.name, self.name, self.name, self.ver[0], self.ver[1])
+        res += "<li>Migration status: %s" % self._format_verdict_summary()
         if self.maint:
             res = res + "<li>Maintainer: %s\n" % (self.maint)
         if self.section and self.section.find("/") > -1:
@@ -210,6 +232,7 @@ class Excuse(object):
         excusedata["excuses"] = self._text()
         excusedata["item-name"] = self.name
         excusedata["source"] = source
+        excusedata["migration-policy-verdict"] = self._policy_verdict
         excusedata["old-version"] = self.ver[0]
         excusedata["new-version"] = self.ver[1]
         if self.maint:

britney2/utils.py

@@ -39,6 +39,7 @@ from britney2.consts import (VERSION, PROVIDES, DEPENDS, CONFLICTS,
                              SOURCE, MAINTAINER, MULTIARCH,
                              ESSENTIAL)
 from britney2.migrationitem import MigrationItem, UnversionnedMigrationItem
+from britney2.policies.policy import PolicyVerdict
 
 
 def ifilter_except(container, iterable=None):
@@ -814,7 +815,8 @@ def invalidate_excuses(excuses, valid, invalid):
                 invalid.append(valid.pop(p))
                 excuses[x].addhtml("Invalidated by dependency")
                 excuses[x].addreason("depends")
-                excuses[x].is_valid = False
+                if excuses[x].policy_verdict.value < PolicyVerdict.REJECTED_TEMPORARILY.value:
+                    excuses[x].policy_verdict = PolicyVerdict.REJECTED_TEMPORARILY
 
 
 def compile_nuninst(binaries_t, inst_tester, architectures, nobreakall_arches):