2015-08-11 14:20:37

by Javi Merino

[permalink] [raw]
Subject: [PATCH] checkpatch: check Signed-off-by: lines for patches coming from stdin

Commit 34d8815f9512 ("checkpatch: add --showfile to allow input via pipe
to show filenames") disabled the ability to check for Signed-off-by
lines in patches that are fed to scripts/checkpatch.pl from stdin. This
makes things like:

git rebase --interactive --exec 'git format-patch --stdout -1 | scripts/checkpatch.pl --strict -'

not complain about patches without a Signed-off-by.

While commit 34d8815f9512 ("checkpatch: add --showfile to allow input
via pipe to show filenames") is meant to work with the output of git
diff, this should be achieved by explicitly passing --no-signoff
to checkpatch.pl:

git diff | ./scripts/checkpatch --no-signoff -

Fixes: 34d8815f9512 ("checkpatch: add --showfile to allow input via pipe to show filenames")
Cc: Joe Perches <[email protected]>
Cc: Petr Mladek <[email protected]>
Cc: Andy Whitcroft <[email protected]>
Cc: Andrew Morton <[email protected]>
Signed-off-by: Javi Merino <[email protected]>
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d5c8e9a3a73c..984e7ddfd7e3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5684,7 +5684,7 @@ sub process {
ERROR("NOT_UNIFIED_DIFF",
"Does not appear to be a unified-diff format patch\n");
}
- if ($is_patch && $filename ne '-' && $chk_signoff && $signoff == 0) {
+ if ($is_patch && $chk_signoff && $signoff == 0) {
ERROR("MISSING_SIGN_OFF",
"Missing Signed-off-by: line(s)\n");
}
--
1.9.1


2015-08-11 16:47:54

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: check Signed-off-by: lines for patches coming from stdin

On Tue, 2015-08-11 at 15:20 +0100, Javi Merino wrote:
> Commit 34d8815f9512 ("checkpatch: add --showfile to allow input via pipe
> to show filenames") disabled the ability to check for Signed-off-by
> lines in patches that are fed to scripts/checkpatch.pl from stdin. This
> makes things like:
>
> git rebase --interactive --exec 'git format-patch --stdout -1 | scripts/checkpatch.pl --strict -'

This is akin to running checkpatch on patches to the stable tree.
Generally unnecessary.

I think patches should never be committed without a sign-off so
the concept of using checkpatch when rebasing is fundamentally
odd, but <shrug>, different workflows for different folks.

You should add a "Reference: http://xkcd.com/1172/" line to the
commit message.

2015-08-11 17:04:24

by Javi Merino

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: check Signed-off-by: lines for patches coming from stdin

On Tue, Aug 11, 2015 at 05:47:49PM +0100, Joe Perches wrote:
> On Tue, 2015-08-11 at 15:20 +0100, Javi Merino wrote:
> > Commit 34d8815f9512 ("checkpatch: add --showfile to allow input via pipe
> > to show filenames") disabled the ability to check for Signed-off-by
> > lines in patches that are fed to scripts/checkpatch.pl from stdin. This
> > makes things like:
> >
> > git rebase --interactive --exec 'git format-patch --stdout -1 | scripts/checkpatch.pl --strict -'
>
> This is akin to running checkpatch on patches to the stable tree.
> Generally unnecessary.

No, the rebase is not moving the base of the patches. The rebase is to run
checkpatch on every patch before sending it to the list. It's for
patches aiming mainline.

> I think patches should never be committed without a sign-off so
> the concept of using checkpatch when rebasing is fundamentally
> odd, but <shrug>, different workflows for different folks.

I guess other people run "git format-patch -o blah" and then run
scripts/checkpatch.pl on blah. As you said, different workflows for
different folks.

Cheers,
Javi