checkpatch: add option to change warning return value.

There are a lot of warnings in checkpatch.pl that are rather subjective. For instance, when a line is 81 characters long, and the patch submitter is just cleaning up existing code, a maintainer may not care that checkpatch.pl warns about the over 80-character line. It should be up to the maintainer what warnings they want to pay attention to.

Unfortunately, if you try to run checkpatch.pl as part of a git pre-commit hook and a patch in a series being applied by git-am fails checkpatch.pl, the whole process stops. At that point I usually end up just disabling that git hook, which means I don't see real errors in the patches.

Add an option to checkpatch.pl to print the warnings, but not return an error code when the --lazy flag is present. (Anyone with a better name can pipe up, lazy just seemed to be the opposite to strict.) This allows the maintainer to view the warnings, but still allow the patch application process to continue.

Signed-off-by Sarah Sharp <[email protected]>

---
 scripts/checkpatch.pl |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

scripts/checkpatch.pl +6 -0

index 4c0383d..4519b5b 100755
@@ -15,6 +15,7 @@ my $V = '0.31';
 use Getopt::Long qw(:config no_auto_abbrev);
 
 my $quiet = 0;
+my $lazy = 0;
 my $tree = 1;
 my $chk_signoff = 1;
 my $chk_patch = 1;

@@ -46,6 +47,7 @@ Options:
   --terse                    one line per report
   -f, --file                 treat FILE as regular source file
   --subjective, --strict     enable more subjective tests
+  --lazy                     print warnings, but don't return an error condition
   --root=PATH                PATH to the kernel tree root
   --no-summary               suppress the per-file summary
   --mailback                 only produce a report in case of warnings/errors

@@ -80,6 +82,7 @@ GetOptions(
 
 	'debug=s'	=> \%debug,
 	'test-only=s'	=> \$tst_only,
+	'lazy+'		=> \$lazy,
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);

@@ -1096,6 +1099,9 @@ sub report {
 
 	push(our @report, $line);
 
+	if ($lazy == 1) {
+		return 0;
+	}
 	return 1;
 }
 sub report_dump {

Signature · 7 lines

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

BTW, I'm not a perl hacker, and I don't really understand the reporting code, so someone who knows it (Andy?) should make the correct patch. I didn't notice before sending, but right now the warning count reporting is bogus, although the warnings don't make the script return an error code:

Note that second-to-last patch should have said 3 warnings. So my patch needs to be changed, but I really do want this functionality.

Quoted 2 lines

(quoted diff) +0 -0

@@ -1096,6 +1099,9 @@ sub report {
 

Review sub report { · line 1107

Quoted 8 lines

(quoted diff) +3 -0

 	push(our @report, $line);
 
+	if ($lazy == 1) {
+		return 0;
+	}
 	return 1;
 }
 sub report_dump {

Quoted 10 lines · Sarah Sharp

Review sub report { · line 1107

Quoted 10 lines

(quoted diff) +3 -0

@@ -1096,6 +1099,9 @@ sub report {
 
 	push(our @report, $line);
 
+	if ($lazy == 1) {
+		return 0;
+	}
 	return 1;
 }
 sub report_dump {

Instead of this hunk, have:

scripts/checkpatch.pl +0 -0

index 58848e3..54ec3d9 100755

Quoted 24 lines · Steven Rostedt

Quoted 10 lines · Sarah Sharp

Review sub report { · line 1107

Quoted 10 lines

(quoted diff) +3 -0

@@ -1096,6 +1099,9 @@ sub report {
 
 	push(our @report, $line);
 
+	if ($lazy == 1) {
+		return 0;
+	}
 	return 1;
 }
 sub report_dump {

Instead of this hunk, have:

scripts/checkpatch.pl +0 -1

index 58848e3..54ec3d9 100755
@@ -316,7 +316,7 @@ for my $filename (@ARGV) {
 	}
 	close($FILE);
 	if (!process($filename)) {
-		$exit = 1;

scripts/checkpatch.pl for my $filename (@ARGV) { · line 322

Quoted 4 lines

scripts/checkpatch.pl +1 -0

+		$exit = 1 if (!$lazy);
 	}
 	@rawlines = ();
 	@lines = ();

Hmm, but isn't that going to skip returning an exit value if there's an error as well? I really do want it to fail if there's an error, just not if there's a warning.

Quoted 25 lines · Sarah Sharp

Quoted 24 lines · Steven Rostedt

Quoted 10 lines · Sarah Sharp

Review sub report { · line 1107

Quoted 10 lines

(quoted diff) +3 -0

@@ -1096,6 +1099,9 @@ sub report {
 
 	push(our @report, $line);
 
+	if ($lazy == 1) {
+		return 0;
+	}
 	return 1;
 }
 sub report_dump {

Instead of this hunk, have:

scripts/checkpatch.pl +0 -1

index 58848e3..54ec3d9 100755
@@ -316,7 +316,7 @@ for my $filename (@ARGV) {
 	}
 	close($FILE);
 	if (!process($filename)) {
-		$exit = 1;

Quoted 8 lines

scripts/checkpatch.pl for my $filename (@ARGV) { · line 322

Quoted 4 lines

scripts/checkpatch.pl +1 -0

+		$exit = 1 if (!$lazy);
 	}
 	@rawlines = ();
 	@lines = ();

Hmm, but isn't that going to skip returning an exit value if there's an error as well? I really do want it to fail if there's an error, just not if there's a warning.

Ah, I was a bit confused as you had report return 0, which is used by both errors and warnings.

Would you want something like this:

scripts/checkpatch.pl +0 -0

index 58848e3..40cbc2f 100755

There are a lot of warnings in checkpatch.pl that are rather subjective. For instance, when a line is 81 characters long, and the patch submitter is just cleaning up existing code, a maintainer may not care that checkpatch.pl warns about the over 80-character line. It should be up to the maintainer what warnings they want to pay attention to.

Unfortunately, if you try to run checkpatch.pl as part of a git pre-commit hook and a patch in a series being applied by git-am fails checkpatch.pl, the whole process stops. At that point I usually end up just disabling that git hook, which means I don't see real errors in the patches.

Add an option to checkpatch.pl to print the warnings, but not return an error code when the --lazy flag is present. (Anyone with a better name can pipe up, lazy just seemed to be the opposite to strict.) This allows the maintainer to view the warnings, but still allow the patch application process to continue.

Quoted 11 lines · Sarah Sharp

There are a lot of warnings in checkpatch.pl that are rather subjective. For instance, when a line is 81 characters long, and the patch submitter is just cleaning up existing code, a maintainer may not care that checkpatch.pl warns about the over 80-character line. It should be up to the maintainer what warnings they want to pay attention to.

Unfortunately, if you try to run checkpatch.pl as part of a git pre-commit hook and a patch in a series being applied by git-am fails checkpatch.pl, the whole process stops. At that point I usually end up just disabling that git hook, which means I don't see real errors in the patches.

Quoted 5 lines

Add an option to checkpatch.pl to print the warnings, but not return an error code when the --lazy flag is present. (Anyone with a better name can pipe up, lazy just seemed to be the opposite to strict.) This allows the maintainer to view the warnings, but still allow the patch application process to continue.

I first thought you would add code that allowed turning only select errors into warnings.

If you just want to have git continue in any case, why not append '|| true' to the line that invokes checkpatch in your hook script?