* [PATCH RESEND v2 1/2] checkpatch: Add support for checkpatch-ignore notes
2025-02-17 17:32 [PATCH RESEND v2 0/2] checkpatch: Add support for checkpatch-ignore note Brendan Jackman
@ 2025-02-17 17:32 ` Brendan Jackman
2025-02-17 17:32 ` [PATCH RESEND v2 2/2] docs: checkpatch: Document checkpatch-ignore feature Brendan Jackman
1 sibling, 0 replies; 3+ messages in thread
From: Brendan Jackman @ 2025-02-17 17:32 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches, Dwaipayan Ray, Lukas Bulwahn,
Jonathan Corbet, Konstantin Ryabitsev
Cc: linux-kernel, workflows, linux-doc, Brendan Jackman
Checkpatch sometimes has false positives. This makes it less useful for
automatic usage: tools like b4 [0] can run checkpatch on all of your
patches and give you a quick overview. When iterating on a branch, it's
tiresome to manually re-check that any errors are known false positives.
This patch adds a mechanism to record alongside the patch that it might
produce certain checkpatch errors, and that these are expected false
positives. There are two aspects to this mechanism:
1. If a block like:
Notes (checkpatch-ignore):
FOO,BAR
BAZ
Is found before the diff in the patch content, FOO, BAR and BAZ error
types are ignored while processing this file.
Its expected that users put this in the "graveyard" i.e. the region
between the --- and the beginning of the diff.
2. --notes=checkpatch-ignore is added to the `git format-patch`
command that checkpatch.pl uses in --git mode, so that if the commit
being inspected has a note [1] under the checkpatch-ignore ref, it
will be formatted into a block like the one above.
To avoid significant reworks to the Perl code, this is implemented by
mutating a global variable while processing each patch. (The variable
name refers to a patch as a "file" for consistency with other code).
Because the main loop in process() begins to emit errors before it has
necessarily processed the checkpatch-ignore block, this parsing is done
separately in its own loop.
[0] b4 - see "--check" arg
https://b4.docs.kernel.org/en/latest/contributor/prep.html
[1] https://git-scm.com/docs/git-notes
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
scripts/checkpatch.pl | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9eed3683ad76caffbbb2418e5dbea7551d374406..ce6914a845ec3f936ad656fa123f58aa85ce4b2f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -53,7 +53,10 @@ my %debug;
my %camelcase = ();
my %use_type = ();
my @use = ();
+# Error types to ignore during the whole invocation.
my %ignore_type = ();
+# Error types to be ignored in the present "file" (i.e. patch).
+my %file_ignore_type = ();
my @ignore = ();
my $help = 0;
my $configuration_file = ".checkpatch.conf";
@@ -1306,7 +1309,7 @@ for my $filename (@ARGV) {
my $oldfile = $file;
$file = 1 if ($is_git_file);
if ($git) {
- open($FILE, '-|', "git format-patch -M --stdout -1 $filename") ||
+ open($FILE, '-|', "git format-patch --notes=checkpatch-ignore -M --stdout -1 $filename") ||
die "$P: $filename: git format-patch failed - $!\n";
} elsif ($file) {
open($FILE, '-|', "diff -u /dev/null $filename") ||
@@ -2329,7 +2332,7 @@ sub show_type {
return defined $use_type{$type} if (scalar keys %use_type > 0);
- return !defined $ignore_type{$type};
+ return !defined $ignore_type{$type} && !defined $file_ignore_type{$type};
}
sub report {
@@ -2624,6 +2627,29 @@ sub exclude_global_initialisers {
$realfile =~ m@/bpf/.*\.bpf\.c$@;
}
+# Parse the "Notes (checkpatch-ignore):" block in the region before the diff,
+# and set file_ignore_type accordingly.
+sub parse_checkpatch_ignore {
+ my $linesRef = shift;
+ my $in_checkpatch_ignore = 0;
+
+ foreach my $line (@$linesRef) {
+ # have we reached the actual diff?
+ if ($line =~ /^diff --git.*?(\s+)$/ || $line =~ /^\+\+\+\s+(\s+)/) {
+ last;
+ }
+
+ if ($in_checkpatch_ignore) {
+ if ($line =~ /^\s*$/) {
+ last;
+ }
+ hash_save_array_words(\%file_ignore_type, [$line]);
+ } elsif ($line =~ /^Notes \(checkpatch-ignore\):\s*/) {
+ $in_checkpatch_ignore = 1;
+ }
+ }
+}
+
sub process {
my $filename = shift;
@@ -2701,6 +2727,8 @@ sub process {
my $checklicenseline = 1;
+ %file_ignore_type = ();
+
sanitise_line_reset();
my $line;
foreach my $rawline (@rawlines) {
@@ -2780,6 +2808,8 @@ sub process {
}
}
+ parse_checkpatch_ignore(\@lines);
+
$prefix = '';
$realcnt = 0;
--
2.48.1.601.g30ceb7b040-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH RESEND v2 2/2] docs: checkpatch: Document checkpatch-ignore feature
2025-02-17 17:32 [PATCH RESEND v2 0/2] checkpatch: Add support for checkpatch-ignore note Brendan Jackman
2025-02-17 17:32 ` [PATCH RESEND v2 1/2] checkpatch: Add support for checkpatch-ignore notes Brendan Jackman
@ 2025-02-17 17:32 ` Brendan Jackman
1 sibling, 0 replies; 3+ messages in thread
From: Brendan Jackman @ 2025-02-17 17:32 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches, Dwaipayan Ray, Lukas Bulwahn,
Jonathan Corbet, Konstantin Ryabitsev
Cc: linux-kernel, workflows, linux-doc, Brendan Jackman
If included in patch descriptions, this will function much like the
--ignore flag.
It requires some rather obscure Git features to take advantage of
this, so provide some examples of how to do that.
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
Documentation/dev-tools/checkpatch.rst | 46 ++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index abb3ff6820766ee0c29112b256bcc44ce41fffba..6215b24b25b36709c815cf08de33f1609c80c0c7 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -12,6 +12,49 @@ Checkpatch is not always right. Your judgement takes precedence over checkpatch
messages. If your code looks better with the violations, then its probably
best left alone.
+Inoring violations
+==================
+
+As well as the --ignore flag documented below, violation types can be ignored
+for a specific patch by including a block after the "---" in the following
+format::
+
+ Notes(checkpatch-ignore):
+ TYPE_1,TYPE_2
+ TYPE_3
+
+If using Git, you can store that information alongside your commit using
+`notes <https://git-scm.com/docs/git-notes>`_. To set this up:
+
+1. Configure git to include the `checkpatch-ignore` notes ref in formatted
+ patches::
+
+ git config set format.notes checkpatch-ignore
+
+ If you use checkpatch in `--git` mode, this isn't necessary, it will include
+ the `checkpatch-ignore` note regardless.
+
+2. Configure git to persist notes across amends and rebases::
+
+ git config set notes.rewriteRef "refs/notes/checkpatch-ignore"
+
+ (To enable this behaviour for _all_ notes, set `refs/notes/**` instead).
+
+ Also ensure that `notes.rewrite.rebase` and `notes.rewrite.amend` have not
+ been set to `false`.
+
+3. Now, to set the note on the HEAD commit, use a command like::
+
+ git notes --ref checkpatch-ignore add -m "TYPE_1,TYPE_2"
+
+ Beware that blank lines terminate the `checkpatch-ignore` block, so if you
+ use `git notes append` to ignore additional types, you'll need to also set
+ `--no-separator`::
+
+ git notes --ref checkpatch-ignore append -m "TYPE_3" --no-separator
+
+To see the names of the error type in checkpatch output, set the `--show-types`
+option.
Options
=======
@@ -114,6 +157,9 @@ Available options:
Checkpatch will not emit messages for the specified types.
+ Note that violations can also be permanently disabled using the
+ Checkpatch-ignore patch footer.
+
Example::
./scripts/checkpatch.pl mypatch.patch --ignore EMAIL_SUBJECT,BRACES
--
2.48.1.601.g30ceb7b040-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread