mirror of
https://git.launchpad.net/~ubuntu-release/britney/+git/britney2-ubuntu
synced 2025-06-30 19:11:28 +00:00
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.
This commit is contained in:
parent
47f5f0da72
commit
11705f4a7b
@ -70,16 +70,14 @@ class SourcePPAPolicy(BasePolicy):
|
|||||||
return ''
|
return ''
|
||||||
|
|
||||||
def initialise(self, britney):
|
def initialise(self, britney):
|
||||||
"""Load and discover all source ppa data"""
|
"""Load cached source ppa data"""
|
||||||
super().initialise(britney)
|
super().initialise(britney)
|
||||||
self.britney = britney
|
self.britney = britney
|
||||||
|
|
||||||
try:
|
if os.path.exists(self.filename):
|
||||||
with open(self.filename, encoding='utf-8') as data:
|
with open(self.filename, encoding='utf-8') as data:
|
||||||
self.cache = json.load(data)
|
self.cache = json.load(data)
|
||||||
self.log("Loaded cached source ppa data from %s" % self.filename)
|
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):
|
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"""
|
"""Reject package if any other package copied from same PPA is invalid"""
|
||||||
@ -87,22 +85,30 @@ class SourcePPAPolicy(BasePolicy):
|
|||||||
britney_excuses = self.britney.excuses
|
britney_excuses = self.britney.excuses
|
||||||
version = source_data_srcdist.version
|
version = source_data_srcdist.version
|
||||||
sourceppa = self.lp_get_source_ppa(source_name, 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
|
self.source_ppas_by_pkg[source_name][version] = sourceppa
|
||||||
if sourceppa:
|
sourceppa_info[source_name] = shortppa
|
||||||
# Check for other packages that might invalidate this one
|
# Check for other packages that might invalidate this one
|
||||||
for friend in self.pkgs_by_source_ppa[sourceppa]:
|
for friend in self.pkgs_by_source_ppa[sourceppa]:
|
||||||
if not britney_excuses[friend].is_valid:
|
sourceppa_info[friend] = shortppa
|
||||||
accept = False
|
if not britney_excuses[friend].is_valid:
|
||||||
self.pkgs_by_source_ppa[sourceppa].add(source_name)
|
accept = False
|
||||||
|
self.pkgs_by_source_ppa[sourceppa].add(source_name)
|
||||||
|
|
||||||
if not accept:
|
if not accept:
|
||||||
# Invalidate all packages in this source ppa
|
# Invalidate all packages in this source ppa
|
||||||
shortppa = sourceppa.replace(LAUNCHPAD_URL, '')
|
|
||||||
for friend in self.pkgs_by_source_ppa[sourceppa]:
|
for friend in self.pkgs_by_source_ppa[sourceppa]:
|
||||||
friend_exc = britney_excuses.get(friend, excuse)
|
friend_exc = britney_excuses.get(friend, excuse)
|
||||||
friend_exc.is_valid = False
|
if friend_exc.is_valid:
|
||||||
friend_exc.addreason('source-ppa')
|
friend_exc.is_valid = False
|
||||||
self.log("Blocking %s because %s from %s" % (friend, source_name, shortppa))
|
friend_exc.addreason('source-ppa')
|
||||||
sourceppa_info[friend] = shortppa
|
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.REJECTED_PERMANENTLY
|
||||||
return PolicyVerdict.PASS
|
return PolicyVerdict.PASS
|
||||||
|
|
||||||
|
@ -2042,8 +2042,9 @@ class T(TestBase):
|
|||||||
# Tests for source ppa grouping
|
# Tests for source ppa grouping
|
||||||
################################################################
|
################################################################
|
||||||
|
|
||||||
def test_sourceppa(self):
|
def test_sourceppa_policy(self):
|
||||||
'''Packages from same source PPA should be grouped.'''
|
'''Packages from same source PPA get rejected for failed peer policy'''
|
||||||
|
|
||||||
self.sourceppa_cache['green'] = {'2': 'team/ubuntu/ppa'}
|
self.sourceppa_cache['green'] = {'2': 'team/ubuntu/ppa'}
|
||||||
self.sourceppa_cache['red'] = {'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:
|
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(
|
exc = self.do_test(
|
||||||
[('green', {'Version': '2'}, 'autopkgtest'),
|
[('green', {'Version': '2'}, 'autopkgtest'),
|
||||||
('red', {'Version': '2'}, 'autopkgtest')],
|
('red', {'Version': '2'}, 'autopkgtest')],
|
||||||
{'green': (False, {'green': {'i386': 'RUNNING-ALWAYSFAIL', 'amd64': 'RUNNING-ALWAYSFAIL'}})},
|
{'green': (False, {'green': {'i386': 'RUNNING-ALWAYSFAIL', 'amd64': 'RUNNING-ALWAYSFAIL'}}),
|
||||||
{'green': [('is-candidate', False)],
|
'red': (False, {'red': {'i386': 'RUNNING-ALWAYSFAIL', 'amd64': 'RUNNING-ALWAYSFAIL'}})},
|
||||||
'red': [('is-candidate', False)]}
|
{'green': [('reason', 'block')],
|
||||||
|
'red': [('reason', 'source-ppa')]}
|
||||||
)[1]
|
)[1]
|
||||||
self.assertEqual(exc['red']['policy_info']['source-ppa'], {'red': 'team/ubuntu/ppa', 'green': 'team/ubuntu/ppa'})
|
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__':
|
if __name__ == '__main__':
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
@ -31,10 +31,14 @@ class FakeOptions:
|
|||||||
class FakeExcuse:
|
class FakeExcuse:
|
||||||
ver = ('1.0', '2.0')
|
ver = ('1.0', '2.0')
|
||||||
is_valid = True
|
is_valid = True
|
||||||
|
policy_info = {}
|
||||||
|
|
||||||
def addreason(self, reason):
|
def addreason(self, reason):
|
||||||
"""Ignore reasons."""
|
"""Ignore reasons."""
|
||||||
|
|
||||||
|
def addhtml(self, reason):
|
||||||
|
"""Ignore reasons."""
|
||||||
|
|
||||||
|
|
||||||
class FakeBritney:
|
class FakeBritney:
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
@ -96,13 +100,14 @@ class T(unittest.TestCase):
|
|||||||
|
|
||||||
def test_approve_ppa(self):
|
def test_approve_ppa(self):
|
||||||
"""Approve packages by their PPA."""
|
"""Approve packages by their PPA."""
|
||||||
|
shortppa = 'team/ubuntu/ppa'
|
||||||
pol = SourcePPAPolicy(FakeOptions)
|
pol = SourcePPAPolicy(FakeOptions)
|
||||||
pol.filename = CACHE_FILE
|
pol.filename = CACHE_FILE
|
||||||
pol.initialise(FakeBritney())
|
pol.initialise(FakeBritney())
|
||||||
output = {}
|
output = {}
|
||||||
for pkg in ('pal', 'buddy', 'friend', 'noppa'):
|
for pkg in ('pal', 'buddy', 'friend', 'noppa'):
|
||||||
self.assertEqual(pol.apply_policy_impl(output, None, pkg, None, FakeData, FakeExcuse), PolicyVerdict.PASS)
|
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):
|
def test_reject_ppa(self):
|
||||||
"""Reject packages by their PPA."""
|
"""Reject packages by their PPA."""
|
||||||
|
Loading…
x
Reference in New Issue
Block a user