checkpatch: handle line break between commit and hash value
PATCH 1/1 checkpatch: handle line break between commit and hash value
While checking for improperly formed commit desciptions, those cases where there is a line break between commit and hash value, are not handled properly and an error is given.
This adds new conditions to parse and identify such commits and to not give error in such cases.
following type of cases are handled:
- Without quotes in commits message.
commitf4d51dffc6c01 ("Linux 5.9-rc4")
- With quotes in commit message.
commit77f4689de17c ("fix regression in "epoll: Keep a reference on files added to the check list"")
(commits taken as example only)
--- scripts/checkpatch.pl | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
scripts/checkpatch.pl +17 -2
@@ -2831,8 +2831,9 @@ sub process { if ($in_commit_log && !$commit_log_possible_stack_dump && $line !~ /^\s*(?:Link|Patchwork|http|https|BugLink|base-commit):/i && $line !~ /^This reverts commit [0-9a-f]{7,40}/ && - ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || - ($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'\(\[]|$)/i && + ($line =~ /\bcommit$/i || + $line =~ /\bcommit\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";
@@ -2852,6 +2853,8 @@ sub process { $orig_commit = lc($2); } elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) { $orig_commit = lc($1); + } elsif ($rawlines[$linenr] =~ /^([0-9a-f]{12,40})\b/i) { + $orig_commit = lc($1); } $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i);
@@ -2874,6 +2877,18 @@ sub process { $rawlines[$linenr] =~ /^\s*([^"]+)"\)/; $orig_desc .= " " . $1; $hasparens = 1; + } elsif ($line =~ /\bcommit$/i && + defined $rawlines[$linenr]) { + if($rawlines[$linenr] =~ /\b[0-9a-f]{5,}\s+\("([^"]+)"\)/i) { + $orig_desc = $1; + } elsif($rawlines[$linenr] =~ /\b[0-9a-f]{5,}\s+\("([^"]+"[^"]+[^"]")"\)/i) { + $orig_desc = $1; + } + $hasparens = 1; + $space = 0; + $short = 0 if ($rawlines[$linenr] =~ /^[0-9a-f]{12,40}/i); + $long = 1 if ($rawlines[$linenr] =~ /^[0-9a-f]{41,}/i); + $case = 0 if ($rawlines[$linenr] =~ /^[0-9a-f]{5,40}[^A-F]/i); } ($id, $description) = git_commit_info($orig_commit,
Signature · 6 lines
2.28.0 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 4 lines · Ayush
While checking for improperly formed commit desciptions, those cases where there is a line break between commit and hash value, are not handled properly and an error is given.
This sentence above is not comprehensible.
How about
checkpatch.pl checks for proper references to other commits with
the intended format, commit <12 characters of SHA-1 ID> ("<commit message") and warns about typical mistakes. Currently, it does not handle the case where there is a line break between commit and the hash value.
It falsely warns that the hash is not prefixed by the word commit.
At the very end, you can also write:
This issue was discovered through a thorough investigation of checkpatch.pl warnings of type COMMIT_ID on commit range ??.
Maybe you can then also report a before and after statistics to see the impact of your change?
Quoted 3 lines
This adds new conditions to parse and identify such commits and to not give error in such cases.
"to not give error" is not proper English, how about "to not report an error"?
Quoted 6 lines
following type of cases are handled:
- Without quotes in commits message.
commitf4d51dffc6c01("Linux 5.9-rc4")
Quoted 6 lines
- With quotes in commit message.
commit77f4689de17c("fix regression in "epoll: Keep a reference on files added to the check list"")(commits taken as example only)
THIS IS IMPORTANT:
You are doing two things at once, and hence the review is more difficult.
How about one commit for each new case you are handling?
One commit for handling the line break, then another commit for handling quotes in commit messages?
Looking forward to the next version.
Lukas
Quoted 53 lines
Signed-off-by Ayush <[email protected]>--- scripts/checkpatch.pl | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)scripts/checkpatch.pl +17
-2@@ -2831,8 +2831,9 @@ sub process { if ($in_commit_log && !$commit_log_possible_stack_dump && $line !~ /^\s*(?:Link|Patchwork|http|https|BugLink|base-commit):/i && $line !~ /^This reverts commit [0-9a-f]{7,40}/ && - ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || - ($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'\(\[]|$)/i && + ($line =~ /\bcommit$/i || + $line =~ /\bcommit\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";@@ -2852,6 +2853,8 @@ sub process { $orig_commit = lc($2); } elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) { $orig_commit = lc($1); + } elsif ($rawlines[$linenr] =~ /^([0-9a-f]{12,40})\b/i) { + $orig_commit = lc($1); } $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i);@@ -2874,6 +2877,18 @@ sub process { $rawlines[$linenr] =~ /^\s*([^"]+)"\)/; $orig_desc .= " " . $1; $hasparens = 1; + } elsif ($line =~ /\bcommit$/i && + defined $rawlines[$linenr]) { + if($rawlines[$linenr] =~ /\b[0-9a-f]{5,}\s+\("([^"]+)"\)/i) { + $orig_desc = $1; + } elsif($rawlines[$linenr] =~ /\b[0-9a-f]{5,}\s+\("([^"]+"[^"]+[^"]")"\)/i) { + $orig_desc = $1; + } + $hasparens = 1; + $space = 0; + $short = 0 if ($rawlines[$linenr] =~ /^[0-9a-f]{12,40}/i); + $long = 1 if ($rawlines[$linenr] =~ /^[0-9a-f]{41,}/i); + $case = 0 if ($rawlines[$linenr] =~ /^[0-9a-f]{5,40}[^A-F]/i); } ($id, $description) = git_commit_info($orig_commit,Signature · 1 line
2.28.0
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees