Skip to content

Commit 1dc0673

Browse files
committed
ci: copy over check for commit metadata
Copy the commit metadata CI check from the opentitan repository. Signed-off-by: James McCorrie <[email protected]>
1 parent 1f05ec0 commit 1dc0673

File tree

3 files changed

+216
-3
lines changed

3 files changed

+216
-3
lines changed

.github/workflows/ci.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ jobs:
1919
steps:
2020

2121
- uses: actions/checkout@v4
22+
with:
23+
fetch-depth: 0 # Required so we can lint commit messages.
2224

2325
- uses: DeterminateSystems/nix-installer-action@main
2426
- uses: DeterminateSystems/magic-nix-cache-action@main
@@ -39,6 +41,21 @@ jobs:
3941
source .venv/bin/activate
4042
echo PATH=$PATH >> $GITHUB_ENV
4143
44+
- name: Commit metadata
45+
run: |
46+
merge_base="$(git merge-base "origin/$GITHUB_BASE_REF" HEAD)" || {
47+
echo >&2 "Failed to find fork point for origin/$GITHUB_BASE_REF."
48+
exit 1
49+
}
50+
# Notes:
51+
# * Merge commits are not checked. We always use rebases instead of
52+
# merges to keep a linear history, which makes merge commits disappear
53+
# ultimately, making them only a CI artifact which should not be
54+
# checked.
55+
scripts/lint_commits.py "$merge_base"
56+
57+
if: ${{ github.event_name == 'pull_request' }}
58+
4259
- name: License header check
4360
run: scripts/license_check.py
4461

pyproject.toml

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,16 @@ test = [
3636
"pytest-cov>=5.0.0",
3737
]
3838
dev = ["dvsim[linting,typing,test]"]
39-
ci = ["dvsim[linting,typing,test]"]
39+
ci = [
40+
"dvsim[linting,typing,test]",
41+
"gitpython",
42+
]
4043

4144
# drop out ruff as it contains a rust binary that needs to be installed by the flake
42-
nix = ["dvsim[typing,test]"]
43-
45+
nix = [
46+
"dvsim[typing,test]",
47+
"gitpython",
48+
]
4449

4550
[project.scripts]
4651
dvsim = "dvsim.cli:main"

scripts/lint_commits.py

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
#!/usr/bin/env python3
2+
# Copyright lowRISC contributors (OpenTitan project).
3+
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
4+
# SPDX-License-Identifier: Apache-2.0
5+
6+
import argparse
7+
import logging
8+
import re
9+
import sys
10+
11+
from git import Repo
12+
13+
logger = logging.getLogger(__name__)
14+
15+
# Maximum length of the summary line in the commit message (the first line)
16+
# There is no hard limit, but a typical convention is to keep this line at or
17+
# below 50 characters, with occasional outliers.
18+
COMMIT_MSG_MAX_SUMMARY_LEN = 100
19+
20+
21+
def error(msg: str, commit: str | None = None) -> None:
22+
"""Log error."""
23+
full_msg = f"Commit {commit.hexsha}: {msg}" if commit else msg
24+
logger.error(full_msg)
25+
26+
27+
def warning(msg: str, commit: str | None = None) -> None:
28+
"""Log warning."""
29+
full_msg = f"Commit {commit.hexsha}: {msg}" if commit else msg
30+
logger.warning(full_msg)
31+
32+
33+
def lint_commit_author(commit: str) -> bool:
34+
"""Check commit author."""
35+
success = True
36+
if commit.author.email.endswith("users.noreply.github.com"):
37+
error(
38+
f"Commit author has no valid email address set: " # noqa: F541
39+
"{commit.author.email!r}. "
40+
'Use "git config user.email [email protected]" to '
41+
"set a valid email address, then update the commit "
42+
'with "git rebase -i" and/or '
43+
'"git commit --amend --signoff --reset-author". '
44+
"Also check your GitHub settings at "
45+
"https://github.com/settings/emails: your email address "
46+
'must be verified, and the option "Keep my email address '
47+
'private" must be disabled. '
48+
"This command will also sign off your commit indicating agreement "
49+
"to the Contributor License Agreement. See CONTRIBUTING.md for "
50+
"more details.",
51+
commit,
52+
)
53+
success = False
54+
55+
if " " not in commit.author.name:
56+
warning(
57+
f"The commit author name {commit.author.name!r} contains no space. "
58+
"Use \"git config user.name 'Johnny English'\" to "
59+
'set your real name, and update the commit with "git rebase -i " '
60+
'and/or "git commit --amend --signoff --reset-author". '
61+
"This command will also sign off your commit indicating agreement "
62+
"to the Contributor License Agreement. See CONTRIBUTING.md for "
63+
"more details.",
64+
commit,
65+
)
66+
# A warning doesn't fail lint.
67+
68+
return success
69+
70+
71+
def lint_commit_message(commit: str) -> bool:
72+
"""Check commit message."""
73+
success = True
74+
lines = commit.message.splitlines()
75+
76+
# Check length of summary line.
77+
summary_line_len = len(lines[0])
78+
if summary_line_len > COMMIT_MSG_MAX_SUMMARY_LEN:
79+
error(
80+
f"The summary line in the commit message is {summary_line_len} "
81+
f"characters long; only {COMMIT_MSG_MAX_SUMMARY_LEN} characters "
82+
"are allowed.",
83+
commit,
84+
)
85+
success = False
86+
87+
# Check for an empty line separating the summary line from the long
88+
# description.
89+
if len(lines) > 1 and lines[1] != "":
90+
error(
91+
"The second line of a commit message must be empty, as it "
92+
"separates the summary from the long description.",
93+
commit,
94+
)
95+
success = False
96+
97+
# Check that the commit message contains at least one Signed-off-by line
98+
# that matches the author name and email. There might be other signoffs (if
99+
# there are multiple authors). We don't have any requirements about those
100+
# at the moment and just pass them through.
101+
signoff_lines = []
102+
signoff_pfx = "Signed-off-by: "
103+
for line in lines:
104+
if not line.startswith(signoff_pfx):
105+
continue
106+
107+
signoff_body = line[len(signoff_pfx) :]
108+
match = re.match(r"[^<]+ <[^>]*>$", signoff_body)
109+
if match is None:
110+
error(
111+
f"Commit has Signed-off-by line {line!r}, but the second part "
112+
"is not of the required form. It should be of the form "
113+
'"Signed-off-by: NAME <EMAIL>".'
114+
)
115+
success = False
116+
117+
signoff_lines.append(line)
118+
119+
expected_signoff_line = f"Signed-off-by: {commit.author.name} <{commit.author.email}>"
120+
signoff_req_msg = (
121+
"The commit message must contain a Signed-off-by line "
122+
"that matches the commit author name and email, "
123+
"indicating agreement to the Contributor License "
124+
"Agreement. See CONTRIBUTING.md for more details. "
125+
'You can use "git commit --signoff" to ask git to add '
126+
"this line for you."
127+
)
128+
129+
if not signoff_lines:
130+
error(f"Commit has no Signed-off-by line. {signoff_req_msg}")
131+
success = False
132+
133+
elif expected_signoff_line not in signoff_lines:
134+
error(
135+
(
136+
"Commit has one or more Signed-off-by lines, but not the one "
137+
f'we expect. We expected to find "{expected_signoff_line}". '
138+
)
139+
+ signoff_req_msg
140+
)
141+
success = False
142+
143+
return success
144+
145+
146+
def lint_commit(commit: str) -> bool:
147+
"""Check a commit."""
148+
return all(
149+
[
150+
lint_commit_author(commit),
151+
lint_commit_message(commit),
152+
]
153+
)
154+
155+
156+
def main() -> None:
157+
"""Commit message lint check."""
158+
logging.basicConfig(level=logging.INFO)
159+
160+
parser = argparse.ArgumentParser(
161+
description="Check commit metadata for common mistakes",
162+
)
163+
parser.add_argument(
164+
"commit_range",
165+
metavar="commit-range",
166+
help="commit range to check (must be understood by git log)",
167+
)
168+
args = parser.parse_args()
169+
170+
commit_range = f"{args.commit_range}..HEAD"
171+
logger.info("Checking commit range %s", commit_range)
172+
173+
lint_successful = True
174+
175+
for commit in Repo().iter_commits(commit_range):
176+
logger.info("Checking commit %s", commit.hexsha)
177+
178+
if len(commit.parents) > 1:
179+
logger.info("Skipping merge commit.")
180+
continue
181+
182+
if not lint_commit(commit):
183+
lint_successful = False
184+
185+
if not lint_successful:
186+
error("Commit lint failed.")
187+
sys.exit(1)
188+
189+
190+
if __name__ == "__main__":
191+
main()

0 commit comments

Comments
 (0)