From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: apw@canonical.com, joe@perches.com,
Willem de Bruijn <willemb@google.com>
Subject: [PATCH] checkpatch: validate Fixes tags
Date: Wed, 30 May 2018 12:33:31 -0400 [thread overview]
Message-ID: <20180530163331.169328-1-willemdebruijn.kernel@gmail.com> (raw)
From: Willem de Bruijn <willemb@google.com>
Apply commit syntax validation to Fixes tags:
Remove the existing exclusion match on " !~ /\bfixes ..." and
generalize keyword match to "[Cc]ommit|Fixes:".
Unlike commit tags, fixes tags must take up an entire line and
may not have line breaks.
When applied to a set of 2000+ recent patches, this added patches
with warnings:
- 8x SHA1 too short
- 4x line break in description
- 3x missing parentheses
- 3x extraneous double-quote in description (false positive)
- 2x missing quotes
- 1x using single instead of double quotes
It changed existing warnings only
- 3x output Commit instead of commit
Also add a test for lines that start with the fixes keyword but do not
match the commit syntax condition. These are likely intended as tags,
but not based on SHA1. This added patches with warnings:
- 3x keyword in message body on line by itself (false positive)
- 1x points to url instead of SHA1
- 1x has keyword commit before SHA1
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
scripts/checkpatch.pl | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2d42eb9cd1a5..70bc023cae42 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2513,6 +2513,14 @@ sub process {
$in_commit_log = 0;
}
+# Check format of Fixes line
+ if ($in_commit_log && $line =~ /^\s*fixes:/i) {
+ if ($line !~ /^\s*fixes:\s+[0-9a-f]{5,}/i) {
+ WARN("FIXES_LINE_SYNTAX",
+ "Fixes line is not of form \"Fixes: <12+ chars of sha1> [...]");
+ }
+ }
+
# Check if MAINTAINERS is being updated. If so, there's probably no need to
# emit the "does MAINTAINERS need updating?" message on file add/move/delete
if ($line =~ /^\s*MAINTAINERS\s*\|/) {
@@ -2643,11 +2651,9 @@ sub process {
if ($in_commit_log && !$commit_log_possible_stack_dump &&
$line !~ /^\s*(?:Link|Patchwork|http|https|BugLink):/i &&
$line !~ /^This reverts commit [0-9a-f]{7,40}/ &&
- ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i ||
+ ($line =~ /\b(?:commit|fixes:)\s+[0-9a-f]{5,}\b/i ||
($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'\(\[]|$)/i &&
- $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i &&
- $line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) {
- my $init_char = "c";
+ $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i))) {
my $orig_commit = "";
my $short = 1;
my $long = 0;
@@ -2658,19 +2664,29 @@ sub process {
my $id = '0123456789ab';
my $orig_desc = "commit description";
my $description = "";
+ my $keyword = "commit";
+ my $keywordcase = "[Cc]ommit";
+ my $pre = '';
+ my $post = '';
+
+ if ($line =~ /\bfixes:\s+[0-9a-f]{5,}\b/i) {
+ $keywordcase = "Fixes:";
+ $pre = '^';
+ $post = '$';
+ }
- if ($line =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) {
- $init_char = $1;
+ if ($line =~ /\b($keywordcase)\s+([0-9a-f]{5,})\b/i) {
+ $keyword = $1;
$orig_commit = lc($2);
} elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) {
$orig_commit = lc($1);
}
- $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i);
- $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
- $space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
- $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
- if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
+ $short = 0 if ($line =~ /\b$keyword\s+[0-9a-f]{12,40}/i);
+ $long = 1 if ($line =~ /\b$keyword\s+[0-9a-f]{41,}/i);
+ $space = 0 if ($line =~ /\b$keyword [0-9a-f]/i);
+ $case = 0 if ($line =~ /\b$keywordcase\s+[0-9a-f]{5,40}[^A-F]/);
+ if ($line =~ /$pre\b$keyword\s+[0-9a-f]{5,}\s+\("([^"]+)"\)$post/i) {
$orig_desc = $1;
$hasparens = 1;
} elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
@@ -2694,7 +2710,7 @@ sub process {
if (defined($id) &&
($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens)) {
ERROR("GIT_COMMIT_ID",
- "Please use git commit description style 'commit <12+ chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herecurr);
+ "Please use git commit description style '$keyword <12+ chars of sha1> (\"<title line>\")' - ie: '$keyword $id (\"$description\")'\n" . $herecurr);
}
}
--
2.17.0.921.gf22659ad46-goog
reply other threads:[~2018-05-30 16:33 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180530163331.169328-1-willemdebruijn.kernel@gmail.com \
--to=willemdebruijn.kernel@gmail.com \
--cc=apw@canonical.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=willemb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).