Skip to content

Commit af7e843

Browse files
committed
Make is_revert stricter, to match GCCs changelog scripts
Reverts like https://gcc.gnu.org/cgit/gcc/commit/?id=eec8da328cf1f91db302ab4cee803e269e68ad33 are let through by the hooks, but then lead to failures in the changelog generating script: https://inbox.sourceware.org/gccadmin/20260415001632.71DF64BA23C4@sourceware.org/ This check should prevent that.
1 parent d1c4b37 commit af7e843

6 files changed

Lines changed: 148 additions & 5 deletions

File tree

hooks/config.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
"hooks.no-precommit-check": {"default": (), "type": tuple},
9494
"hooks.no-rh-character-range-check": {"default": False, "type": bool},
9595
"hooks.no-rh-style-checks": {"default": (), "type": tuple},
96+
"hooks.no-rh-near-revert-check": {"default": False, "type": bool},
9697
"hooks.no-style-checks": {"default": (), "type": tuple},
9798
"hooks.pre-receive-hook": {"default": None},
9899
"hooks.post-receive-hook": {"default": None},

hooks/pre_commit_checks.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,29 @@ def check_missing_ticket_number(commit):
309309
)
310310

311311

312+
def reject_almost_reversions(commit):
313+
"""Raise InvalidUpdate if the commit's revlog contains "This reverts
314+
commit" in it.
315+
316+
The GCC ChangeLog scripts expect a line that contains that sentence to also
317+
contain a reference to the commit being reverted. If this line is altered,
318+
therefore, they will fail.
319+
320+
PARAMETERS
321+
commit: A CommitInfo object corresponding to the commit being checked.
322+
"""
323+
if git_config("hooks.no-rh-near-revert-check"):
324+
return
325+
326+
if "This reverts commit" in commit.raw_revlog:
327+
raise InvalidUpdate(
328+
"Commit %s looks like it was intended as a revert." % commit.rev,
329+
"",
330+
"When reverting, you should leave the 'This reverts commit'",
331+
"line unaltered.",
332+
)
333+
334+
312335
def check_revision_history(commit):
313336
"""Apply pre-commit checks to the commit's revision history.
314337
@@ -321,6 +344,7 @@ def check_revision_history(commit):
321344
return
322345

323346
# Various checks on the revision history...
347+
reject_almost_reversions(commit)
324348
ensure_iso_8859_15_only(commit)
325349
ensure_empty_line_after_subject(commit)
326350
reject_lines_too_long(commit)

hooks/updates/commits.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,19 @@
44
from io_utils import safe_decode
55
from updates.mailinglists import expanded_mailing_list
66
from utils import debug
7+
import re
8+
9+
10+
_REVERT_COMMIT_RE = re.compile(
11+
r"^This reverts commit (?P<hash>[0-9a-f]+)\.$",
12+
re.M
13+
)
14+
"""A regex that matches the same revert lines as ``revert_regex`` does in
15+
``contrib/gcc-changelog/git_commit.py``.
16+
17+
Note that this regex is matched against an entire body of a commit rather than
18+
each line in it, though.
19+
"""
720

821

922
class CommitInfo(object):
@@ -235,11 +248,7 @@ def is_revert(self):
235248
revision log of such commits, hoping that a user is not deleting
236249
them afterwards.
237250
"""
238-
if "This reverts commit" in self.raw_revlog:
239-
return True
240-
241-
# No recognizable pattern. Probably not a revert commit.
242-
return False
251+
return bool(_REVERT_COMMIT_RE.search(self.raw_revlog));
243252

244253
@classmethod
245254
def __all_files_from_commit_rev(cls, rev):

testsuite/tests/GCC__stricter_revert_pattern/bare_repo_config

Whitespace-only changes.
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
[hooks]
2+
from-domain = gcc.gnu.org
3+
mailinglist = /git/gcc.git/hooks-bin/email_to.py
4+
5+
# We do not want to force a maximum line length in commit
6+
# revision logs, as they get in the way of copy-pasting
7+
# debugging session, error messages, logs, etc.
8+
max-rh-line-length = 0
9+
10+
# We allow a 0.5MiB email diff maximum.
11+
max-email-diff-size = 524288
12+
13+
# Reject merge commits on a certain number of branches:
14+
# - on master: We request that people rebase their changes
15+
# before pushing instead (merge commits tend to confuse
16+
# git newcomers).
17+
# - on release: We apply the same policy to release branches
18+
# as we have on master.
19+
reject-merge-commits = refs/heads/master,refs/heads/trunk,refs/heads/releases/.*
20+
21+
# The URL where we can inspect the commit, inserted in the commit
22+
# notification email, and also copy sent to the file-commit-cmd.
23+
commit-url = "https://gcc.gnu.org/g:%(rev)s"
24+
25+
# This style checker does nothing at present.
26+
style-checker = /git/gcc.git/hooks-bin/style_checker
27+
28+
# Send a copy to bugzilla if a commit has a PR number in it.
29+
# The script is a wrapper around
30+
# /sourceware/infra/bin/email-to-bugzilla.
31+
file-commit-cmd = "/git/gcc.git/hooks-bin/email-to-bugzilla-filtered"
32+
33+
# Work around <https://github.com/AdaCore/git-hooks/issues/9>
34+
# to allow larger merges.
35+
max-commit-emails = 5000
36+
37+
# Allow deliberate merges to use the default commit message.
38+
# Branches that do not allow merge commits are listed in
39+
# reject-merge-commits.
40+
disable-merge-commit-checks = true
41+
42+
# Do not send emails for commits that are already in the
43+
# repository being added to development branches or user or
44+
# vendor branches (through merges or rebases).
45+
email-new-commits-only = refs/heads/devel/.*
46+
email-new-commits-only = refs/users/.*
47+
email-new-commits-only = refs/vendors/.*
48+
49+
# GCC-specific ref naming conventions for user and vendor
50+
# branches.
51+
branch-ref-namespace = refs/users/[^/]*/heads/.*
52+
branch-ref-namespace = refs/vendors/[^/]*/heads/.*
53+
54+
# GCC-specific ref naming conventions for user and vendor
55+
# tags.
56+
tag-ref-namespace = refs/users/[^/]*/tags/.*
57+
tag-ref-namespace = refs/vendors/[^/]*/tags/.*
58+
59+
# Branch deletion is disabled by default.
60+
restrict-branch-deletion = true
61+
62+
# Branch deletion is allowed for user and vendor branches.
63+
allow-delete-branch = refs/users/[^/]*/heads/.*
64+
allow-delete-branch = refs/vendors/[^/]*/heads/.*
65+
66+
# Non-fast-forward updates are allowed in the user and vendor
67+
# namespaces.
68+
allow-non-fast-forward = refs/users/.*
69+
allow-non-fast-forward = refs/vendors/.*
70+
71+
# Message to give for rejected branch deletion.
72+
rejected-branch-deletion-tip = Branch deletion is only allowed for user and vendor branches. If another branch was created by mistake, contact an administrator to delete it on the server with git update-ref. If a development branch is dead, also contact an administrator to move it under refs/dead/heads/ rather than deleting it.
73+
74+
# Commit messages should not be restricted to ISO-8859-15.
75+
no-rh-character-range-check = true
76+
77+
# Custom checker script for each new commit of each ref being
78+
# updated. This makes several checks on the commit message,
79+
# including for ChangeLog formatting and contents.
80+
commit-extra-checker = /git/gcc.git/hooks-bin/commit_checker
81+
82+
# Custom checker script for ref updates. This checks for
83+
# branch naming conventions and not introducing new references
84+
# to the git-svn history.
85+
update-hook = /git/gcc.git/hooks-bin/update_hook
86+
87+
# Custom email formatter. This inserts GCC monotonically
88+
# increasing commit ids in the commit emails.
89+
commit-email-formatter = /git/gcc.git/hooks-bin/commit_email_formatter
90+
91+
# For GCC/Rust development that happens outside of GCC proper,
92+
# <https://rust-gcc.github.io/>, the Git commit messages
93+
# don't always adhere to standard GCC style; see
94+
# <https://github.com/Rust-GCC/gccrs/issues/143>.
95+
no-precommit-check = refs/heads/devel/rust/.*
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
def test_push_bad_revert_commit(testcase):
2+
"""Try pushing master..."""
3+
p = testcase.run("git push origin trunk".split())
4+
testcase.assertNotEqual(p.status, 0, p.image)
5+
testcase.assertRunOutputEqual(p, """\
6+
remote: *** Commit df3a09266b6685060fb1e11268922b491e3e5cd8 looks like it was intended as a revert.
7+
remote: ***
8+
remote: *** When reverting, you should leave the 'This reverts commit'
9+
remote: *** line unaltered.
10+
remote: error: hook declined to update refs/heads/trunk
11+
To ../bare/repo.git/
12+
! [remote rejected] trunk -> trunk (hook declined)
13+
error: failed to push some refs to '../bare/repo.git/'
14+
""")

0 commit comments

Comments
 (0)