Modified: trunk/Tools/CISupport/ews-build/steps.py (294581 => 294582)
--- trunk/Tools/CISupport/ews-build/steps.py 2022-05-20 21:41:14 UTC (rev 294581)
+++ trunk/Tools/CISupport/ews-build/steps.py 2022-05-20 21:42:03 UTC (rev 294582)
@@ -479,6 +479,7 @@
if name and emails:
bugzilla_email = emails[0].lower() # We're requiring that the first email is the primary bugzilla email
cls.contributors[bugzilla_email] = {'name': name, 'status': value.get('status')}
+ cls.contributors[name] = {'status': value.get('status')}
if github_username and name and emails:
cls.contributors[github_username.lower()] = dict(
name=name,
@@ -4871,7 +4872,7 @@
return not self.doStepIf(step)
-class ValidateCommitMessage(steps.ShellSequence, ShellMixin):
+class ValidateCommitMessage(steps.ShellSequence, ShellMixin, AddToLogMixin):
name = 'validate-commit-message'
haltOnFailure = False
flunkOnFailure = True
@@ -4878,15 +4879,36 @@
OOPS_RE = 'OO*PP*S!'
REVIEWED_STRINGS = (
'Reviewed by',
- 'Unreviewed',
'Rubber-stamped by',
'Rubber stamped by',
+ 'Unreviewed',
)
RE_CHANGELOG = br'^(\+\+\+)\s+(.*ChangeLog.*)'
+ BY_RE = re.compile(r'.+\s+by\s+(.+)$')
+ SPLIT_RE = re.compile(r'(,\s*)|( and )|\.')
def __init__(self, **kwargs):
super(ValidateCommitMessage, self).__init__(logEnviron=False, timeout=60, **kwargs)
+ self.contributors = {}
+ @classmethod
+ def extract_reviewers(cls, text):
+ reviewers = set()
+ stripped_text = ''
+ for line in text.splitlines():
+ match = cls.BY_RE.match(line)
+ if not match:
+ stripped_text += line + '\n'
+ continue
+ for person in cls.SPLIT_RE.split(match.group(1)):
+ if person and not cls.SPLIT_RE.match(person):
+ reviewers.add(person)
+ return reviewers, stripped_text
+
+ def is_reviewer(self, name):
+ contributor = self.contributors.get(name)
+ return contributor and contributor['status'] == 'reviewer'
+
def _files(self):
sourcestamp = self.build.getSourceStamp(self.getProperty('codebase', ''))
if sourcestamp and sourcestamp.changes:
@@ -4911,6 +4933,9 @@
"git log {} ^{} | grep -q '\\({}\\)' || echo 'No reviewer information in commit message'".format(
head_ref, base_ref,
'\\|'.join(self.REVIEWED_STRINGS),
+ ), "git log {} ^{} | grep '\\({}\\)'".format(
+ head_ref, base_ref,
+ '\\|'.join(self.REVIEWED_STRINGS[:-1]),
),
]
for command in commands:
@@ -4925,7 +4950,15 @@
self.summary = 'Patches have no commit message'
return rc
- log_text = self.log_observer.getStdout().rstrip()
+ self.contributors, errors = Contributors.load(use_network=True)
+ for error in errors:
+ print(error)
+ self._addToLog('stdio', error)
+
+ reviewers, log_text = self.extract_reviewers(self.log_observer.getStdout())
+ log_text = log_text.rstrip()
+ author = self.getProperty('author', '')
+
if any(['ChangeLog' in file for file in self._files()]):
self.summary = 'ChangeLog modified, WebKit only allows commit messages'
rc = FAILURE
@@ -4933,7 +4966,21 @@
self.summary = log_text
rc = FAILURE
elif rc == SUCCESS:
- self.summary = 'Validated commit message'
+ if reviewers and not self.contributors:
+ self.summary = "Failed to load contributors.json, can't validate reviewers"
+ rc = FAILURE
+ elif reviewers and any([not self.is_reviewer(reviewer) for reviewer in reviewers]):
+ self.summary = "'{}' is not a reviewer"
+ for reviewer in reviewers:
+ if not self.is_reviewer(reviewer):
+ self.summary = self.summary.format(reviewer)
+ break
+ rc = FAILURE
+ elif reviewers and author and any([author.startswith(reviewer) for reviewer in reviewers]):
+ self.summary = f"'{author}' cannot review their own change"
+ rc = FAILURE
+ else:
+ self.summary = 'Validated commit message'
else:
self.summary = 'Error parsing commit message'
rc = FAILURE
Modified: trunk/Tools/CISupport/ews-build/steps_unittest.py (294581 => 294582)
--- trunk/Tools/CISupport/ews-build/steps_unittest.py 2022-05-20 21:41:14 UTC (rev 294581)
+++ trunk/Tools/CISupport/ews-build/steps_unittest.py 2022-05-20 21:42:03 UTC (rev 294582)
@@ -77,8 +77,10 @@
return {
'revie...@apple.com': {'name': 'WebKit Reviewer', 'status': 'reviewer', 'email': 'revie...@apple.com'},
'webkit-reviewer': {'name': 'WebKit Reviewer', 'status': 'reviewer', 'email': 'revie...@apple.com'},
+ 'WebKit Reviewer': {'status': 'reviewer'},
'commit...@webkit.org': {'name': 'WebKit Committer', 'status': 'committer', 'email': 'commit...@webkit.org'},
'webkit-commit-queue': {'name': 'WebKit Committer', 'status': 'committer', 'email': 'commit...@webkit.org'},
+ 'WebKit Committer': {'status': 'committer'},
}, []
@@ -5944,6 +5946,7 @@
class TestValidateCommitMessage(BuildStepMixinAdditions, unittest.TestCase):
def setUp(self):
self.longMessage = True
+ Contributors.load = mock_load_contributors
return self.setUpBuildStep()
def tearDown(self):
@@ -5960,9 +5963,13 @@
+ 0, ExpectShell(workdir='wkdir',
logEnviron=False,
timeout=60,
- command=['/bin/sh', '-c', "git log HEAD ^origin/main | grep -q '\\(Reviewed by\\|Unreviewed\\|Rubber-stamped by\\|Rubber stamped by\\)' || echo 'No reviewer information in commit message'"])
+ command=['/bin/sh', '-c', "git log HEAD ^origin/main | grep -q '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\|Unreviewed\\)' || echo 'No reviewer information in commit message'"])
+ + 0, ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log HEAD ^origin/main | grep '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\)'"])
+ 0
- + ExpectShell.log('stdio', stdout=''),
+ + ExpectShell.log('stdio', stdout=' Reviewed by WebKit Reviewer.\n'),
)
self.expectOutcome(result=SUCCESS, state_string='Validated commit message')
return self.runStep()
@@ -5981,9 +5988,13 @@
+ 0, ExpectShell(workdir='wkdir',
logEnviron=False,
timeout=60,
- command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q '\\(Reviewed by\\|Unreviewed\\|Rubber-stamped by\\|Rubber stamped by\\)' || echo 'No reviewer information in commit message'"])
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\|Unreviewed\\)' || echo 'No reviewer information in commit message'"])
+ + 0, ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\)'"])
+ 0
- + ExpectShell.log('stdio', stdout=''),
+ + ExpectShell.log('stdio', stdout=' Reviewed by WebKit Reviewer.\n'),
)
self.expectOutcome(result=SUCCESS, state_string='Validated commit message')
return self.runStep()
@@ -6021,7 +6032,11 @@
+ 0, ExpectShell(workdir='wkdir',
logEnviron=False,
timeout=60,
- command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q '\\(Reviewed by\\|Unreviewed\\|Rubber-stamped by\\|Rubber stamped by\\)' || echo 'No reviewer information in commit message'"])
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\|Unreviewed\\)' || echo 'No reviewer information in commit message'"])
+ + 0, ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\)'"])
+ 0
+ ExpectShell.log('stdio', stdout='No reviewer information in commit message\n'),
)
@@ -6044,14 +6059,69 @@
+ 0, ExpectShell(workdir='wkdir',
logEnviron=False,
timeout=60,
- command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q '\\(Reviewed by\\|Unreviewed\\|Rubber-stamped by\\|Rubber stamped by\\)' || echo 'No reviewer information in commit message'"])
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\|Unreviewed\\)' || echo 'No reviewer information in commit message'"])
+ + 0, ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\)'"])
+ 0
- + ExpectShell.log('stdio', stdout=''),
+ + ExpectShell.log('stdio', stdout=' Reviewed by WebKit Reviewer.\n'),
)
self.expectOutcome(result=FAILURE, state_string='ChangeLog modified, WebKit only allows commit messages')
return self.runStep()
+ def test_invalid_reviewer(self):
+ self.setupStep(ValidateCommitMessage())
+ ValidateCommitMessage._files = lambda x: ['+++ Tools/CISupport/ews-build/steps.py']
+ self.setProperty('github.number', '1234')
+ self.setProperty('github.base.ref', 'main')
+ self.setProperty('github.head.ref', 'eng/pull-request-branch')
+ self.expectRemoteCommands(
+ ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q 'OO*PP*S!' && echo 'Commit message contains (OOPS!)' || test $? -eq 1"])
+ + 0, ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\|Unreviewed\\)' || echo 'No reviewer information in commit message'"])
+ + 0, ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\)'"])
+ + 0
+ + ExpectShell.log('stdio', stdout=' Reviewed by WebKit Contributor.\n'),
+ )
+ self.expectOutcome(result=FAILURE, state_string="'WebKit Contributor' is not a reviewer")
+ return self.runStep()
+ def test_self_reviewer(self):
+ self.setupStep(ValidateCommitMessage())
+ ValidateCommitMessage._files = lambda x: ['+++ Tools/CISupport/ews-build/steps.py']
+ self.setProperty('github.number', '1234')
+ self.setProperty('github.base.ref', 'main')
+ self.setProperty('github.head.ref', 'eng/pull-request-branch')
+ self.setProperty('author', 'WebKit Reviewer <revie...@apple.com>')
+ self.expectRemoteCommands(
+ ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q 'OO*PP*S!' && echo 'Commit message contains (OOPS!)' || test $? -eq 1"])
+ + 0, ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep -q '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\|Unreviewed\\)' || echo 'No reviewer information in commit message'"])
+ + 0, ExpectShell(workdir='wkdir',
+ logEnviron=False,
+ timeout=60,
+ command=['/bin/sh', '-c', "git log eng/pull-request-branch ^main | grep '\\(Reviewed by\\|Rubber-stamped by\\|Rubber stamped by\\)'"])
+ + 0
+ + ExpectShell.log('stdio', stdout=' Reviewed by WebKit Reviewer.\n'),
+ )
+ self.expectOutcome(result=FAILURE, state_string="'WebKit Reviewer <revie...@apple.com>' cannot review their own change")
+ return self.runStep()
+
+
class TestCanonicalize(BuildStepMixinAdditions, unittest.TestCase):
def setUp(self):
self.longMessage = True
