Some PPA landing fixes

The original commit had an unreliable policy info (the peer PPA group), as
correct updating of friends' excuses depends on the processing order.

 - Also add the currently investigated package to the policy info.
 - Add a HTML excuse too, so that excuses.html actually shows the reason for
   blocking.
 - Don't invalidate other excuses multiple times.
 - Change the policy_info to always carry the friend information with it, as
   that's useful to know for accepted packages too. Adjust test_approve_ppa()
   accordingly.
 - Verify "reason" field in test_sourceppa_policy()
 - Add another integration test to ensure that this also applies to unbuilt
   packages, not just to rejection from other Policies.
 - Avoid FileNotFoundError, this does not yet exist in Python 3.2.

policies/sourceppa.py

@@ -70,16 +70,14 @@ class SourcePPAPolicy(BasePolicy):
             return ''
 
     def initialise(self, britney):
-        """Load and discover all source ppa data"""
+        """Load cached source ppa data"""
         super().initialise(britney)
         self.britney = britney
 
-        try:
+        if os.path.exists(self.filename):
             with open(self.filename, encoding='utf-8') as data:
                 self.cache = json.load(data)
             self.log("Loaded cached source ppa data from %s" % self.filename)
-        except FileNotFoundError:
-            pass
 
     def apply_policy_impl(self, sourceppa_info, suite, source_name, source_data_tdist, source_data_srcdist, excuse):
         """Reject package if any other package copied from same PPA is invalid"""
@@ -87,22 +85,30 @@ class SourcePPAPolicy(BasePolicy):
         britney_excuses = self.britney.excuses
         version = source_data_srcdist.version
         sourceppa = self.lp_get_source_ppa(source_name, version)
+        if not sourceppa:
+            return PolicyVerdict.PASS
+
+        shortppa = sourceppa.replace(LAUNCHPAD_URL, '')
         self.source_ppas_by_pkg[source_name][version] = sourceppa
-        if sourceppa:
-            # Check for other packages that might invalidate this one
-            for friend in self.pkgs_by_source_ppa[sourceppa]:
-                if not britney_excuses[friend].is_valid:
-                    accept = False
-            self.pkgs_by_source_ppa[sourceppa].add(source_name)
+        sourceppa_info[source_name] = shortppa
+        # Check for other packages that might invalidate this one
+        for friend in self.pkgs_by_source_ppa[sourceppa]:
+            sourceppa_info[friend] = shortppa
+            if not britney_excuses[friend].is_valid:
+                accept = False
+        self.pkgs_by_source_ppa[sourceppa].add(source_name)
+
         if not accept:
             # Invalidate all packages in this source ppa
-            shortppa = sourceppa.replace(LAUNCHPAD_URL, '')
             for friend in self.pkgs_by_source_ppa[sourceppa]:
                 friend_exc = britney_excuses.get(friend, excuse)
-                friend_exc.is_valid = False
-                friend_exc.addreason('source-ppa')
-                self.log("Blocking %s because %s from %s" % (friend, source_name, shortppa))
-                sourceppa_info[friend] = shortppa
+                if friend_exc.is_valid:
+                    friend_exc.is_valid = False
+                    friend_exc.addreason('source-ppa')
+                    friend_exc.policy_info['source-ppa'] = sourceppa_info
+                    self.log("Blocking %s because %s from %s" % (friend, source_name, shortppa))
+                    friend_exc.addhtml("Blocking because %s from the same PPA %s is invalid" %
+                                       (friend, shortppa))
             return PolicyVerdict.REJECTED_PERMANENTLY
         return PolicyVerdict.PASS
 

tests/test_autopkgtest.py

@@ -2042,8 +2042,9 @@ class T(TestBase):
     # Tests for source ppa grouping
     ################################################################
 
-    def test_sourceppa(self):
-        '''Packages from same source PPA should be grouped.'''
+    def test_sourceppa_policy(self):
+        '''Packages from same source PPA get rejected for failed peer policy'''
+
         self.sourceppa_cache['green'] = {'2': 'team/ubuntu/ppa'}
         self.sourceppa_cache['red'] = {'2': 'team/ubuntu/ppa'}
         with open(os.path.join(self.data.path, 'data/series-proposed/Blocks'), 'w') as f:
@@ -2052,12 +2053,30 @@ class T(TestBase):
         exc = self.do_test(
             [('green', {'Version': '2'}, 'autopkgtest'),
              ('red', {'Version': '2'}, 'autopkgtest')],
-            {'green': (False, {'green': {'i386': 'RUNNING-ALWAYSFAIL', 'amd64': 'RUNNING-ALWAYSFAIL'}})},
-            {'green': [('is-candidate', False)],
-             'red': [('is-candidate', False)]}
+            {'green': (False, {'green': {'i386': 'RUNNING-ALWAYSFAIL', 'amd64': 'RUNNING-ALWAYSFAIL'}}),
+             'red': (False, {'red': {'i386': 'RUNNING-ALWAYSFAIL', 'amd64': 'RUNNING-ALWAYSFAIL'}})},
+            {'green': [('reason', 'block')],
+             'red': [('reason', 'source-ppa')]}
         )[1]
         self.assertEqual(exc['red']['policy_info']['source-ppa'], {'red': 'team/ubuntu/ppa', 'green': 'team/ubuntu/ppa'})
 
+    def test_sourceppa_missingbuild(self):
+        '''Packages from same source PPA get rejected for failed peer FTBFS'''
+
+        self.sourceppa_cache['green'] = {'2': 'team/ubuntu/ppa'}
+        self.sourceppa_cache['red'] = {'2': 'team/ubuntu/ppa'}
+
+        self.data.add_src('green', True, {'Version': '2', 'Testsuite': 'autopkgtest'})
+        self.data.add('libgreen1', True, {'Version': '2', 'Source': 'green', 'Architecture': 'i386'}, add_src=False)
+
+        exc = self.do_test(
+            [('red', {'Version': '2'}, 'autopkgtest')],
+            {'green': (False, {}), 'red': (False, {})},
+            {'green': [('missing-builds', {'on-architectures': ['amd64', 'arm64', 'armhf', 'powerpc', 'ppc64el'],
+                                           'on-unimportant-architectures': []})],
+             'red': [('reason', 'source-ppa')]}
+        )[1]
+        self.assertEqual(exc['red']['policy_info']['source-ppa'], {'red': 'team/ubuntu/ppa', 'green': 'team/ubuntu/ppa'})
 
 if __name__ == '__main__':
     unittest.main()

tests/test_sourceppa.py

@@ -31,10 +31,14 @@ class FakeOptions:
 class FakeExcuse:
     ver = ('1.0', '2.0')
     is_valid = True
+    policy_info = {}
 
     def addreason(self, reason):
         """Ignore reasons."""
 
+    def addhtml(self, reason):
+        """Ignore reasons."""
+
 
 class FakeBritney:
     def __init__(self):
@@ -96,13 +100,14 @@ class T(unittest.TestCase):
 
     def test_approve_ppa(self):
         """Approve packages by their PPA."""
+        shortppa = 'team/ubuntu/ppa'
         pol = SourcePPAPolicy(FakeOptions)
         pol.filename = CACHE_FILE
         pol.initialise(FakeBritney())
         output = {}
         for pkg in ('pal', 'buddy', 'friend', 'noppa'):
             self.assertEqual(pol.apply_policy_impl(output, None, pkg, None, FakeData, FakeExcuse), PolicyVerdict.PASS)
-        self.assertEqual(output, {})
+        self.assertEqual(output, dict(pal=shortppa, buddy=shortppa, friend=shortppa))
 
     def test_reject_ppa(self):
         """Reject packages by their PPA."""