Skip to content

Commit dc9efe9

Browse files
Build a commit validation layer into CI []
1 parent 35b7c47 commit dc9efe9

8 files changed

Lines changed: 479 additions & 17 deletions

File tree

‎.git-hooks/pre-push.py‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import json
3737
import os
3838
import platform
39-
import re
4039
import shutil
4140
import subprocess
4241
import sys
@@ -97,10 +96,6 @@ def _is_excluded_file_path(path_str: str) -> bool:
9796
path_str.startswith(prefix) for prefix in EXCLUDES_PATH_PREFIXES
9897
)
9998

100-
COMMIT_SUBJECT_POLICY_REGEX = re.compile(
101-
r'^.*\[(GH #[0-9]+|(EDG[cfjp]+fe/([0-9]+))|,)*\].*$'
102-
)
103-
10499
TOOL_PATH_GIT = shutil.which('git')
105100

106101
def run_git(args, *, check = True, text = True):
@@ -201,11 +196,10 @@ def check_commits(self):
201196
print_indented(show_output, ' ')
202197
print('')
203198

204-
subject = git_output(['show', '--no-patch', '--format=%s', commit])
205-
if COMMIT_SUBJECT_POLICY_REGEX.match(subject) is None:
206-
print(
207-
" [POLICY] Commit message doesn't contain valid PR number (or [])."
208-
)
199+
message = git_output(['show', '--no-patch', '--format=%B', commit])
200+
problem = edgchanges.check_commit_subject(message)
201+
if problem is not None:
202+
print(f" [POLICY] Commit subject: {problem}.")
209203
sys.exit(1)
210204

211205
first_new_commit = commit

‎.github/workflows/changes-fragments.yml‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
#
66
# Check the pending Changes entries (*/pending_changes/*.txt) with
77
# `edg-collect-changes --check` (format, spelling, typos, line length), so a
8-
# bad entry is caught before the nightly collect-changes job meets it, and run
9-
# the unit tests of the tools involved.
8+
# bad entry is caught before the nightly collect-changes job meets it. This
9+
# checks the entries, not the tools; their unit tests are dev-tools-tests.yml.
10+
# The tool paths below are triggers only so that a change to the checks is
11+
# tried against the entries already pending.
1012

1113
name: Pending Changes Entries
1214

@@ -21,7 +23,6 @@ on:
2123
- dev_tools/bin/edg-collect-changes
2224
- dev_tools/pylibs/edgchanges/**
2325
- dev_tools/pylibs/edgutil/**
24-
- dev_tools/tests/**
2526
pull_request:
2627
paths:
2728
- '**/pending_changes/**'
@@ -30,7 +31,6 @@ on:
3031
- dev_tools/bin/edg-collect-changes
3132
- dev_tools/pylibs/edgchanges/**
3233
- dev_tools/pylibs/edgutil/**
33-
- dev_tools/tests/**
3434

3535
permissions:
3636
contents: read
@@ -56,9 +56,6 @@ jobs:
5656
sudo apt-get update
5757
sudo apt-get install -y aspell aspell-en
5858
59-
- name: Run unit tests
60-
run: python3 -m unittest discover -s dev_tools/tests -v
61-
6259
- name: Check pending Changes entries
6360
run: |
6461
status=0
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# Part of the EDG Compiler Project, under the Apache License v2.0 with LLVM
2+
# Exceptions.
3+
# See https://edgcpp.org/LICENSE.txt for license information.
4+
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
5+
#
6+
# Check the subjects of the new commits pushed to main and of every pull
7+
# request's commits with edg-check-commit-subjects (the same policy the
8+
# pre-push hook applies, from edgchanges). Only commits with a violation are
9+
# reported.
10+
#
11+
# A pull request is checked from its base commit to its head commit. A push
12+
# is checked from the previous tip of main. If that tip is unavailable (e.g.
13+
# after a force-push), the range is empty, so nothing is checked.
14+
#
15+
# Merge commits are rejected too. A fork that wants them sets the repository
16+
# variable ALLOW_MERGE_COMMITS to 'true' (Settings > Secrets and variables >
17+
# Actions > Variables).
18+
19+
name: Commit Subjects
20+
21+
on:
22+
push:
23+
branches:
24+
- main
25+
pull_request:
26+
27+
permissions:
28+
contents: read
29+
30+
jobs:
31+
check:
32+
name: check-commit-subjects
33+
runs-on: ubuntu-latest
34+
timeout-minutes: 10
35+
env:
36+
PYTHONPATH: ${{ github.workspace }}/dev_tools/pylibs
37+
steps:
38+
- name: Check out repository
39+
uses: actions/checkout@v7
40+
with:
41+
fetch-depth: 0
42+
ref: ${{ github.event.pull_request.head.sha || github.sha }}
43+
44+
- name: Check commit subjects
45+
env:
46+
EVENT_NAME: ${{ github.event_name }}
47+
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
48+
PUSH_BEFORE_SHA: ${{ github.event.before }}
49+
DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
50+
ALLOW_MERGE_COMMITS: ${{ vars.ALLOW_MERGE_COMMITS }}
51+
run: |
52+
if [[ "${EVENT_NAME}" == pull_request ]]; then
53+
range=("${PR_BASE_SHA}..HEAD")
54+
elif [[ "${PUSH_BEFORE_SHA}" != 0000000000000000000000000000000000000000 ]] &&
55+
git cat-file -e "${PUSH_BEFORE_SHA}^{commit}" 2> /dev/null; then
56+
range=("${PUSH_BEFORE_SHA}..HEAD")
57+
else
58+
range=(HEAD "^origin/${DEFAULT_BRANCH}")
59+
fi
60+
options=()
61+
if [[ "${ALLOW_MERGE_COMMITS}" == true ]]; then
62+
options+=(--allow-merges)
63+
fi
64+
dev_tools/bin/edg-check-commit-subjects "${options[@]}" "${range[@]}"
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Part of the EDG Compiler Project, under the Apache License v2.0 with LLVM
2+
# Exceptions.
3+
# See https://edgcpp.org/LICENSE.txt for license information.
4+
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
5+
#
6+
# Run the unit tests of the dev_tools (dev_tools/tests). Checking the pending
7+
# Changes entries themselves is changes-fragments.yml.
8+
9+
name: Dev Tools Tests
10+
11+
on:
12+
push:
13+
branches:
14+
- main
15+
paths:
16+
- .github/workflows/dev-tools-tests.yml
17+
- dev_tools/bin/edg-check-commit-subjects
18+
- dev_tools/bin/edg-check-file-changes
19+
- dev_tools/bin/edg-collect-changes
20+
- dev_tools/pylibs/edgchanges/**
21+
- dev_tools/pylibs/edgutil/**
22+
- dev_tools/tests/**
23+
pull_request:
24+
paths:
25+
- .github/workflows/dev-tools-tests.yml
26+
- dev_tools/bin/edg-check-commit-subjects
27+
- dev_tools/bin/edg-check-file-changes
28+
- dev_tools/bin/edg-collect-changes
29+
- dev_tools/pylibs/edgchanges/**
30+
- dev_tools/pylibs/edgutil/**
31+
- dev_tools/tests/**
32+
33+
permissions:
34+
contents: read
35+
36+
jobs:
37+
unit-tests:
38+
name: dev-tools-unit-tests
39+
runs-on: ubuntu-latest
40+
timeout-minutes: 10
41+
env:
42+
PYTHONPATH: ${{ github.workspace }}/dev_tools/pylibs
43+
steps:
44+
- name: Check out repository
45+
uses: actions/checkout@v7
46+
47+
# The same dictionary as the dev-env image
48+
# (dev_tools/docker/dev-env/setup-scripts/install-software.sh).
49+
- name: Install aspell
50+
run: |
51+
sudo apt-get update
52+
sudo apt-get install -y aspell aspell-en
53+
54+
- name: Run unit tests
55+
run: python3 -m unittest discover -s dev_tools/tests -v
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
#!/usr/bin/env python3
2+
3+
# Part of the EDG Compiler Project, under the Apache License v2.0 with LLVM
4+
# Exceptions.
5+
# See https://edgcpp.org/LICENSE.txt for license information.
6+
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
7+
8+
'''Check a range of commits against the commit policy: the subject (see
9+
edgchanges.check_commit_subject), and no merge commits.
10+
11+
The arguments are handed to git log, so any revision range it accepts works:
12+
13+
edg-check-commit-subjects origin/main..HEAD
14+
edg-check-commit-subjects main HEAD ^origin/main
15+
16+
A merge commit is a violation by itself; --allow-merges (for forks that use
17+
merge commits) skips merge commits instead, subject and all.
18+
19+
Only commits with a violation are reported. The exit status is 1 if any
20+
commit was reported, and 0 otherwise.
21+
'''
22+
23+
import argparse
24+
import shutil
25+
import subprocess
26+
import sys
27+
28+
from typing import List, NamedTuple, Optional
29+
30+
import edgchanges
31+
import edgutil
32+
33+
TOOL_PATH_GIT = shutil.which('git')
34+
35+
Commit = NamedTuple(
36+
'Commit',
37+
[('hash', str), ('is_merge', bool), ('message', str)]
38+
)
39+
40+
def _list_commits(log_args: List[str]) -> List[Commit]:
41+
'''Return the commits git rev-list selects.
42+
43+
Commit messages are arbitrary bytes, so nothing here depends on their
44+
content: rev-list (hashes only) feeds cat-file --batch, which gives the size
45+
of each object it prints.'''
46+
assert TOOL_PATH_GIT is not None
47+
rev_list_proc = subprocess.Popen(
48+
[TOOL_PATH_GIT, 'rev-list', *log_args],
49+
stdout = subprocess.PIPE,
50+
stderr = subprocess.PIPE
51+
)
52+
cat_file_proc = subprocess.Popen(
53+
[TOOL_PATH_GIT, 'cat-file', '--batch'],
54+
stdin = rev_list_proc.stdout,
55+
stdout = subprocess.PIPE,
56+
stderr = subprocess.PIPE
57+
)
58+
# Leave the pipe to cat-file, so rev-list sees SIGPIPE if it exits early.
59+
assert rev_list_proc.stdout is not None
60+
rev_list_proc.stdout.close()
61+
data, cat_file_stderr = cat_file_proc.communicate()
62+
assert rev_list_proc.stderr is not None
63+
rev_list_stderr = rev_list_proc.stderr.read()
64+
rev_list_proc.stderr.close()
65+
66+
for returncode, stderr in (
67+
(rev_list_proc.wait(), rev_list_stderr),
68+
(cat_file_proc.returncode, cat_file_stderr)
69+
):
70+
if returncode != 0:
71+
raise RuntimeError(stderr.decode('utf-8', errors = 'replace').strip())
72+
73+
commits = []
74+
position = 0
75+
while position < len(data):
76+
line_end = data.index(b'\n', position)
77+
header = data[position:line_end].split()
78+
if len(header) != 3 or header[1] != b'commit':
79+
raise RuntimeError(f"unexpected object: {data[position:line_end]!r}")
80+
content_start = line_end + 1
81+
content_end = content_start + int(header[2])
82+
# The message follows the first blank line, after the headers.
83+
headers, _, message = (
84+
data[content_start:content_end].partition(b'\n\n')
85+
)
86+
parent_count = sum(
87+
1 for line in headers.split(b'\n') if line.startswith(b'parent ')
88+
)
89+
commits.append(
90+
Commit(
91+
header[0].decode(),
92+
parent_count > 1,
93+
message.decode('utf-8', errors = 'replace')
94+
)
95+
)
96+
position = content_end + 1
97+
return commits
98+
99+
def check_commits(log_args: List[str], *, allow_merges: bool) -> int:
100+
'''Report each commit selected by log_args that violates the policy; return
101+
the number of such commits.'''
102+
violations = 0
103+
for commit in _list_commits(log_args):
104+
if commit.is_merge and allow_merges:
105+
continue
106+
107+
problem: Optional[str]
108+
if commit.is_merge:
109+
problem = 'merge commits are not allowed'
110+
else:
111+
problem = edgchanges.check_commit_subject(commit.message)
112+
if problem is not None:
113+
print(f"{commit.hash[:12]} {commit.message.partition(chr(10))[0]}")
114+
print(f" {problem}")
115+
violations += 1
116+
return violations
117+
118+
def _parse_args() -> argparse.Namespace:
119+
parser = argparse.ArgumentParser(
120+
description = (
121+
'Report the commits in a range that violate the commit policy '
122+
'(subject format, no merge commits).'
123+
)
124+
)
125+
parser.add_argument(
126+
'--allow-merges',
127+
action = 'store_true',
128+
help = 'permit merge commits (and skip them) instead of reporting them'
129+
)
130+
parser.add_argument(
131+
'log_args',
132+
nargs = '+',
133+
metavar = 'GIT_LOG_ARG',
134+
help = 'arguments for git log, e.g. a range such as origin/main..HEAD'
135+
)
136+
return parser.parse_args()
137+
138+
def main() -> int:
139+
args = _parse_args()
140+
if TOOL_PATH_GIT is None:
141+
edgutil.eprint('error: git not found')
142+
return 1
143+
144+
try:
145+
violations = check_commits(
146+
args.log_args,
147+
allow_merges = args.allow_merges
148+
)
149+
except RuntimeError as ex:
150+
edgutil.eprint(f"error: {ex}")
151+
return 1
152+
return 1 if violations != 0 else 0
153+
154+
if __name__ == '__main__':
155+
sys.exit(main())

0 commit comments

Comments
 (0)