[PATCH] scripts: checkpatch: ignore stable commit annotation

[PATCH] scripts: checkpatch: ignore stable commit annotation

[Felipe Balbi]

When running scripts/checkpatch.pl on stable commits,
we always get the error for not using 12-character
commit hash abbreviation with shortlog within ("").

Because that's actually a false positive, let's ignore
stable commit annotation.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 scripts/checkpatch.pl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 90e1edc8dd42..2ad0ef46ad08 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2320,6 +2320,7 @@ sub process {
 		if ($in_commit_log && $line =~ /\b(c)ommit\s+([0-9a-f]{5,})/i) {
 			my $init_char = $1;
 			my $orig_commit = lc($2);
+			my $stable = 0;
 			my $short = 1;
 			my $long = 0;
 			my $case = 1;
@@ -2330,6 +2331,7 @@ sub process {
 			my $orig_desc = "commit description";
 			my $description = "";
 
+			$stable = 1 if ($line =~ /\bcommit\s+[0-9a-f]{40}\s+upstream\./i);
 			$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);
@@ -2355,7 +2357,7 @@ sub process {
 			($id, $description) = git_commit_info($orig_commit,
 							      $id, $orig_desc);
 
-			if ($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens) {
+			if (!$stable && ($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);
 			}
-- 
2.4.4

Re: [PATCH] scripts: checkpatch: ignore stable commit annotation

[Joe Perches]

On Tue, 2015-06-30 at 17:07 -0500, Felipe Balbi wrote:
> When running scripts/checkpatch.pl on stable commits,
> we always get the error for not using 12-character
> commit hash abbreviation with shortlog within ("").

Looking at linux-stable v3.14.44 (randomly chosen), there
are also commit entries like:

   This is commit 0f540c3a7cfb91c9d7a19eb0c95c24 upstream.
 commit:  cb64edb6b89491edfdbae52ba7db9a8b8391d339 upstream

is -stable now using some script to tag these or is it
freehanded and happens to be reasonably consistent?

> Because that's actually a false positive, let's ignore
> stable commit annotation.

This will have to be respun against another patch:
https://lkml.org/lkml/2015/6/19/491

Please cc Andrew Morton if you resubmit.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2330,6 +2331,7 @@ sub process {
>  			my $orig_desc = "commit description";
>  			my $description = "";
>  
> +			$stable = 1 if ($line =~ /\bcommit\s+[0-9a-f]{40}\s+upstream\./i);

If these are really scripted, the case-insensitive /i
shouldn't be necessary.

Re: [PATCH] scripts: checkpatch: ignore stable commit annotation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]

On Tue, Jun 30, 2015 at 03:30:35PM -0700, Joe Perches wrote:
> On Tue, 2015-06-30 at 17:07 -0500, Felipe Balbi wrote:
> > When running scripts/checkpatch.pl on stable commits,
> > we always get the error for not using 12-character
> > commit hash abbreviation with shortlog within ("").
> 
> Looking at linux-stable v3.14.44 (randomly chosen), there
> are also commit entries like:
> 
>    This is commit 0f540c3a7cfb91c9d7a19eb0c95c24 upstream.
>  commit:  cb64edb6b89491edfdbae52ba7db9a8b8391d339 upstream
> 
> is -stable now using some script to tag these or is it
> freehanded and happens to be reasonably consistent?

afaict, Greg uses a script. Greg ?

> > Because that's actually a false positive, let's ignore
> > stable commit annotation.
> 
> This will have to be respun against another patch:
> https://lkml.org/lkml/2015/6/19/491
> 
> Please cc Andrew Morton if you resubmit.

hmmm, he doesn't get listed on get_maintainers.pl...

> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -2330,6 +2331,7 @@ sub process {
> >  			my $orig_desc = "commit description";
> >  			my $description = "";
> >  
> > +			$stable = 1 if ($line =~ /\bcommit\s+[0-9a-f]{40}\s+upstream\./i);
> 
> If these are really scripted, the case-insensitive /i
> shouldn't be necessary.

yeah, it was really just to be safer. What if Greg decides to change his
script in the future ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] scripts: checkpatch: ignore stable commit annotation

[Greg KH]

On Tue, Jun 30, 2015 at 05:42:31PM -0500, Felipe Balbi wrote:
> On Tue, Jun 30, 2015 at 03:30:35PM -0700, Joe Perches wrote:
> > On Tue, 2015-06-30 at 17:07 -0500, Felipe Balbi wrote:
> > > When running scripts/checkpatch.pl on stable commits,
> > > we always get the error for not using 12-character
> > > commit hash abbreviation with shortlog within ("").
> > 
> > Looking at linux-stable v3.14.44 (randomly chosen), there
> > are also commit entries like:
> > 
> >    This is commit 0f540c3a7cfb91c9d7a19eb0c95c24 upstream.
> >  commit:  cb64edb6b89491edfdbae52ba7db9a8b8391d339 upstream
> > 
> > is -stable now using some script to tag these or is it
> > freehanded and happens to be reasonably consistent?
> 
> afaict, Greg uses a script. Greg ?

I use a script, but my script didn't create both of those lines, only
one of them, the bottom one.  The top one comes from David Miller's
scripts which he uses when he sends me networking stable patches.

Never run checkpatch on stable commits, that makes no sense at all as I
am pulling them directly from Linus's tree and there is nothing I can do
to change anything in them, nor would I ever want to.

> > If these are really scripted, the case-insensitive /i
> > shouldn't be necessary.
> 
> yeah, it was really just to be safer. What if Greg decides to change his
> script in the future ?

You shouldn't care, as you shouldn't be running the script and thinking
it matters, on stable tree commits.

greg k-h

Re: [PATCH] scripts: checkpatch: ignore stable commit annotation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]

On Tue, Jun 30, 2015 at 04:45:54PM -0700, Greg KH wrote:
> On Tue, Jun 30, 2015 at 05:42:31PM -0500, Felipe Balbi wrote:
> > On Tue, Jun 30, 2015 at 03:30:35PM -0700, Joe Perches wrote:
> > > On Tue, 2015-06-30 at 17:07 -0500, Felipe Balbi wrote:
> > > > When running scripts/checkpatch.pl on stable commits,
> > > > we always get the error for not using 12-character
> > > > commit hash abbreviation with shortlog within ("").
> > > 
> > > Looking at linux-stable v3.14.44 (randomly chosen), there
> > > are also commit entries like:
> > > 
> > >    This is commit 0f540c3a7cfb91c9d7a19eb0c95c24 upstream.
> > >  commit:  cb64edb6b89491edfdbae52ba7db9a8b8391d339 upstream
> > > 
> > > is -stable now using some script to tag these or is it
> > > freehanded and happens to be reasonably consistent?
> > 
> > afaict, Greg uses a script. Greg ?
> 
> I use a script, but my script didn't create both of those lines, only
> one of them, the bottom one.  The top one comes from David Miller's
> scripts which he uses when he sends me networking stable patches.
> 
> Never run checkpatch on stable commits, that makes no sense at all as I
> am pulling them directly from Linus's tree and there is nothing I can do
> to change anything in them, nor would I ever want to.
> 
> > > If these are really scripted, the case-insensitive /i
> > > shouldn't be necessary.
> > 
> > yeah, it was really just to be safer. What if Greg decides to change his
> > script in the future ?
> 
> You shouldn't care, as you shouldn't be running the script and thinking
> it matters, on stable tree commits.

fair enough, branch deleted. I had that hunch, but since there were some
complaining of checkpatch errors on stable commits, I thought about just
patching it up. Thanks for confirming.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]