Skip to content

Commit 33a7b65

Browse files
mathieuchopstmcfriedt
authored andcommitted
scripts: ci: check_compliance: use correct error when author didn't sign
Commit e4650bc modified check_compliance to avoid false negatives when a commit has multiple signoff lines and the author's signoff line isn't the first one. To avoid this, a signoff line which matches exactly the author's name and email is searched instead of matching any line with the proper syntax and checking the fields. While this solved the original issue, this also introduced a new issue: whereas the old script would distinguish a properly formed signoff line with the wrong author from a malformed signoff line, the new method now checks both the syntax and the author name/email at the same time. As such, the script will report "signoff line does not follow the syntax" for both situations, but the signoff line DOES follow the syntax in the latter case: the error message is then only a misleading red herring. Fix this issue - and improve the check - by verifying every signoff line in a commit's message body. Invalid syntax on any line is reported as a "does not follow syntax" failure, whereas inability to find the commit author is reported as a separate "no signoff entry matches commit author" failure as it should always have... Signed-off-by: Mathieu Choplain <[email protected]>
1 parent 82fd615 commit 33a7b65

File tree

1 file changed

+19
-15
lines changed

1 file changed

+19
-15
lines changed

scripts/ci/check_compliance.py

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,27 +1746,31 @@ def run(self):
17461746
self.failure(f'Unable to parse commit message for {shaidx}')
17471747
continue
17481748

1749-
match_signoff = re.search(r"signed-off-by:\s(.*)", body,
1750-
re.IGNORECASE)
1751-
detailed_match = re.search(rf"signed-off-by:\s({re.escape(auth_name)}) <({re.escape(auth_email)})>",
1752-
body,
1753-
re.IGNORECASE)
1754-
17551749
if auth_email.endswith("@users.noreply.github.com"):
17561750
failures.append(f"{shaidx}: author email ({auth_email}) must "
17571751
"be a real email and cannot end in "
17581752
"@users.noreply.github.com")
17591753

1760-
if not match_signoff:
1754+
# Returns an array of everything to the right of ':' on each signoff line
1755+
signoff_lines = re.findall(r"signed-off-by:\s(.*)", body, re.IGNORECASE)
1756+
if len(signoff_lines) == 0:
17611757
failures.append(f'{shaidx}: Missing signed-off-by line')
1762-
elif not detailed_match:
1763-
signoff = match_signoff.group(0)
1764-
failures.append(f"{shaidx}: Signed-off-by line ({signoff}) "
1765-
"does not follow the syntax: First "
1766-
"Last <email>.")
1767-
elif (auth_name, auth_email) != detailed_match.groups():
1768-
failures.append(f"{shaidx}: author email ({auth_email}) needs "
1769-
"to match one of the signed-off-by entries.")
1758+
else:
1759+
# Validate all signoff lines' syntax while also searching for commit author
1760+
found_author_signoff = False
1761+
for signoff in signoff_lines:
1762+
match = re.search(r"(.+) <(.+)>", signoff)
1763+
1764+
if not match:
1765+
failures.append(f"{shaidx}: Signed-off-by line ({signoff}) "
1766+
"does not follow the syntax: First "
1767+
"Last <email>.")
1768+
elif (auth_name, auth_email) == match.groups():
1769+
found_author_signoff = True
1770+
1771+
if not found_author_signoff:
1772+
failures.append(f"{shaidx}: author name ({auth_name}) and email ({auth_email}) "
1773+
"needs to match one of the signed-off-by entries.")
17701774

17711775
if failures:
17721776
self.failure('\n'.join(failures))

0 commit comments

Comments
 (0)