linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] checkpatch: add support to check 'Fixes:' tag format
@ 2020-05-01 15:40 Wang YanQing
  2020-05-01 15:57 ` Joe Perches
  2020-05-01 16:08 ` Markus Elfring
  0 siblings, 2 replies; 6+ messages in thread
From: Wang YanQing @ 2020-05-01 15:40 UTC (permalink / raw)
  To: joe
  Cc: Andy Whitcroft, linux-kernel, Alexei Starovoitov, Matteo Croce,
	Markus.Elfring, kernel-janitors

According to submitting-patches.rst, 'Fixes:' tag has a little
stricter condition about the one line summary than normal git
commit description:
“...
Do not split the tag across multiple lines, tags are exempt from
the "wrap at 75 columns" rule in order to simplify parsing scripts
...”

And there is no sanity check for 'Fixes:' tag format in checkpatch
the same as GIT_COMMIT_ID for git commit description, so let's expand
the GIT_COMMIT_ID to add 'Fixes:' tag format check support.

The check supports below formats:
Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")
Fixes: 85f7cd3a2aad ("Revert "media: Kconfig: better support hybrid TV devices"")
Fixes: 878520ac45f9 ("ext4: save the error code which triggered...")
Fixes: 878520ac45f9 ("ext4: save the error code which triggered")
Fixes: 277f27e2f277 ("SUNRPC/cache: Allow garbage collection ... ")

The check doesn't support below formats:
Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface"
Fixes: 6c73698904aa pinctrl: qcom: Introduce readl/writel accessors
Fixes: 3fd6e7d9a146 (ASoC: tas571x: New driver for TI TAS571x power amplifiers)
Fixes: 55697cbb44e4 ("arm64: dts: renesas: r8a779{65,80,90}: Add IPMMU devices nodes)
Fixes: ba35f8588f47 (“ipvlan: Defer multicast / broadcast processing to a work-queue”)
Fixes: cd758a9b57ee "KVM: PPC: Book3S HV: Use __gfn_to_pfn_memslot in HPT page fault handler"
Fixes:      9b1640686470 ("scsi: lpfc: Fix use-after-free mailbox cmd completion")
Fixes: 03f6fc6de919 ('ASoC: rt5682: Add the soundwire support')

Note: this patch also fixes double quotation mark issue for normal git
      commit description, and now it supports double quotation mark in
      title line, for example:
      Commit e33e2241e272 ("Revert "cfg80211: Use 5MHz bandwidth by default
      when checking usable channels"")

Based on original patch by Joe Perches <joe@perches.com>

Link: https://lore.kernel.org/lkml/40bfc40958fca6e2cc9b86101153aa0715fac4f7.camel@perches.com/
Signed-off-by: Wang YanQing <udknight@gmail.com>
---
 v3:
 1: Fix a bug in short title line support.

 v2:
 1: Add support for double quotation mark in title line, suggested by Markus Elfring.
 2: Add support for short title line with/without ellipsis.
 3: Add supported format examples and unsupported format examples in changelog.
 4: Fix a little wording issue in changelog , suggested by Markus Elfring.

 scripts/checkpatch.pl | 68 +++++++++++++++++++++++++++++++------------
 1 file changed, 49 insertions(+), 19 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 23a001a66006..a649b9f711b6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2818,57 +2818,87 @@ sub process {
 		    $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 =~ /\bfixes:\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))) {
+		      $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i))) {
 			my $init_char = "c";
 			my $orig_commit = "";
+			my $prefix = "commit";
+			my $prefix_case = "[Cc]ommit";
 			my $short = 1;
 			my $long = 0;
 			my $case = 1;
 			my $space = 1;
 			my $hasdesc = 0;
-			my $hasparens = 0;
+			my $has_parens_and_dqm = 0; # Double quotation mark
 			my $id = '0123456789ab';
 			my $orig_desc = "commit description";
 			my $description = "";
+			my $acrosslines = 0;
+			my $title = "title line";
+			my $desc_mismatch = 0;
 
-			if ($line =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) {
+			if ($line =~ /\b(f)ixes:\s+([0-9a-f]{5,})\b/i) {
+				$init_char = $1;
+				$orig_commit = lc($2);
+				$prefix = "Fixes:";
+				$prefix_case = "Fixes:";
+				$init_char = "F";
+				$title = "a single line title (without line breaks but ellipsis is fine!)";
+			} elsif ($line =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) {
 				$init_char = $1;
 				$orig_commit = lc($2);
 			} elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) {
 				$orig_commit = lc($1);
 			}
 
-			$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);
-			$case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
-			if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
+			$short = 0 if ($line =~ /\b$prefix\s+[0-9a-f]{12,40}/i);
+			$long = 1 if ($line =~ /\b$prefix\s+[0-9a-f]{41,}/i);
+			$space = 0 if ($line =~ /\b$prefix [0-9a-f]/i);
+			$case = 0 if ($line =~ /\b$prefix_case\s+[0-9a-f]{5,40}[^A-F]/);
+			if ($line =~ /\b$prefix\s+[0-9a-f]{5,}\s+\("(.+)"\)/i) {
 				$orig_desc = $1;
-				$hasparens = 1;
+				$has_parens_and_dqm = 1;
+				# Drop the ellipsis
+				if ($prefix eq "Fixes:" && $orig_desc =~ /(\s*\.{3}\s*$)/) {
+				    $orig_desc = substr($orig_desc, 0, length($orig_desc) - length($1));
+				}
 			} elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
 				 defined $rawlines[$linenr] &&
-				 $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
+				 $rawlines[$linenr] =~ /^\s*\("(.+)"\)/) {
 				$orig_desc = $1;
-				$hasparens = 1;
-			} elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
+				$has_parens_and_dqm = 1;
+			} elsif ($line =~ /\b$prefix\s+[0-9a-f]{5,}\s+\(".+$/i &&
 				 defined $rawlines[$linenr] &&
-				 $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
-				$line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
+				 $rawlines[$linenr] =~ /^\s*.+"\)/) {
+				$line =~ /\b$prefix\s+[0-9a-f]{5,}\s+\("(.+)$/i;
 				$orig_desc = $1;
-				$rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
+				$rawlines[$linenr] =~ /^\s*(.+)"\)/;
 				$orig_desc .= " " . $1;
-				$hasparens = 1;
+				$has_parens_and_dqm = 1;
+				$acrosslines = 1 if ($prefix eq "Fixes:");
 			}
 
 			($id, $description) = git_commit_info($orig_commit,
 							      $id, $orig_desc);
 
+			if (defined($id) && ($orig_desc ne $description)) {
+			    # Allow short description without too short!
+			    if ($prefix eq "Fixes:" && $has_parens_and_dqm  && length($orig_desc) >= length($description)/2) {
+				my $desc = substr($description, 0, length($orig_desc));
+
+				if ($orig_desc ne $desc) {
+				    $desc_mismatch = 1;
+				}
+			    } else {
+				$desc_mismatch = 1;
+			    }
+			}
+
 			if (defined($id) &&
-			   ($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens)) {
+			   ($short || $long || $space || $case || $desc_mismatch || !$has_parens_and_dqm || $acrosslines)) {
 				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);
+				      "Please use git commit description style '$prefix <12+ chars of sha1> (\"<$title>\")' - ie: '${init_char}" . substr($prefix, 1) . " $id (\"$description\")'\n" . $herecurr);
 			}
 		}
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] checkpatch: add support to check 'Fixes:' tag format
  2020-05-01 15:40 [PATCH v3] checkpatch: add support to check 'Fixes:' tag format Wang YanQing
@ 2020-05-01 15:57 ` Joe Perches
  2020-05-01 16:34   ` Wang YanQing
  2020-05-01 16:08 ` Markus Elfring
  1 sibling, 1 reply; 6+ messages in thread
From: Joe Perches @ 2020-05-01 15:57 UTC (permalink / raw)
  To: Wang YanQing
  Cc: Andy Whitcroft, linux-kernel, Alexei Starovoitov, Matteo Croce,
	Markus.Elfring, kernel-janitors

On Fri, 2020-05-01 at 23:40 +0800, Wang YanQing wrote:
> According to submitting-patches.rst, 'Fixes:' tag has a little
> stricter condition about the one line summary than normal git
> commit description:
> “...
> Do not split the tag across multiple lines, tags are exempt from
> the "wrap at 75 columns" rule in order to simplify parsing scripts
> ...”
> 
> And there is no sanity check for 'Fixes:' tag format in checkpatch
> the same as GIT_COMMIT_ID for git commit description, so let's expand
> the GIT_COMMIT_ID to add 'Fixes:' tag format check support.
> 
> The check supports below formats:
> Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")
> Fixes: 85f7cd3a2aad ("Revert "media: Kconfig: better support hybrid TV devices"")
> Fixes: 878520ac45f9 ("ext4: save the error code which triggered...")
> Fixes: 878520ac45f9 ("ext4: save the error code which triggered")
> Fixes: 277f27e2f277 ("SUNRPC/cache: Allow garbage collection ... ")

Hi again YanQing.

I think all the non-standard and incomplete forms
should have a warning emitted.

> The check doesn't support below formats:
> Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface"
> Fixes: 6c73698904aa pinctrl: qcom: Introduce readl/writel accessors
> Fixes: 3fd6e7d9a146 (ASoC: tas571x: New driver for TI TAS571x power amplifiers)
> Fixes: 55697cbb44e4 ("arm64: dts: renesas: r8a779{65,80,90}: Add IPMMU devices nodes)
> Fixes: ba35f8588f47 (“ipvlan: Defer multicast / broadcast processing to a work-queue”)
> Fixes: cd758a9b57ee "KVM: PPC: Book3S HV: Use __gfn_to_pfn_memslot in HPT page fault handler"
> Fixes:      9b1640686470 ("scsi: lpfc: Fix use-after-free mailbox cmd completion")
> Fixes: 03f6fc6de919 ('ASoC: rt5682: Add the soundwire support')


> Note: this patch also fixes double quotation mark issue for normal git
>       commit description, and now it supports double quotation mark in
>       title line, for example:
>       Commit e33e2241e272 ("Revert "cfg80211: Use 5MHz bandwidth by default
>       when checking usable channels"")

Nice.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] checkpatch: add support to check 'Fixes:' tag format
  2020-05-01 15:40 [PATCH v3] checkpatch: add support to check 'Fixes:' tag format Wang YanQing
  2020-05-01 15:57 ` Joe Perches
@ 2020-05-01 16:08 ` Markus Elfring
  1 sibling, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-05-01 16:08 UTC (permalink / raw)
  To: Wang YanQing, Joe Perches, Andy Whitcroft, kernel-janitors, linux-doc
  Cc: linux-kernel, Alexei Starovoitov, Matteo Croce

> The check supports below formats:

I suggest to omit the concrete examples.
I would prefer the explicit wording for the support of (Unicode) ellipses
also in the shown commit titles.

Will the document “submitting-patches.rst” need a corresponding adjustment?


> The check doesn't support below formats:

How do you think about to extend error diagnostics for patch checking?


> Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface"

Missing closing parenthesis


> Fixes: 6c73698904aa pinctrl: qcom: Introduce readl/writel accessors

Missing enclosing delimiters


> Fixes: 3fd6e7d9a146 (ASoC: tas571x: New driver for TI TAS571x power amplifiers)
> Fixes: 55697cbb44e4 ("arm64: dts: renesas: r8a779{65,80,90}: Add IPMMU devices nodes)

Missing quotation characters


> Fixes: ba35f8588f47 (“ipvlan: Defer multicast / broadcast processing to a work-queue”)
> Fixes: 03f6fc6de919 ('ASoC: rt5682: Add the soundwire support')

Would we like to tolerate such quotation character alternatives?


> Fixes:      9b1640686470 ("scsi: lpfc: Fix use-after-free mailbox cmd completion")

How do you think about to tolerate extra white-space characters around
the commit identifier?


> Note: this patch also fixes double quotation mark issue for normal git
>       commit description, and now it supports double quotation mark in
>       title line, for example:
>       Commit e33e2241e272 ("Revert "cfg80211: Use 5MHz bandwidth by default
>       when checking usable channels"")

I find that such an example should trigger further software development considerations
for a safe data format description.


>  v2:
>  1: Add support for double quotation mark in title line, suggested by Markus Elfring.

I guess that the clarification is still evolving also around this aspect.

Regards,
Markus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] checkpatch: add support to check 'Fixes:' tag format
  2020-05-01 15:57 ` Joe Perches
@ 2020-05-01 16:34   ` Wang YanQing
  2020-05-01 16:44     ` Joe Perches
  2020-05-01 16:52     ` Markus Elfring
  0 siblings, 2 replies; 6+ messages in thread
From: Wang YanQing @ 2020-05-01 16:34 UTC (permalink / raw)
  To: Joe Perches
  Cc: Andy Whitcroft, linux-kernel, Alexei Starovoitov, Matteo Croce,
	Markus.Elfring, kernel-janitors

On Fri, May 01, 2020 at 08:57:42AM -0700, Joe Perches wrote:
> On Fri, 2020-05-01 at 23:40 +0800, Wang YanQing wrote:
> > According to submitting-patches.rst, 'Fixes:' tag has a little
> > stricter condition about the one line summary than normal git
> > commit description:
> > “...
> > Do not split the tag across multiple lines, tags are exempt from
> > the "wrap at 75 columns" rule in order to simplify parsing scripts
> > ...”
> > 
> > And there is no sanity check for 'Fixes:' tag format in checkpatch
> > the same as GIT_COMMIT_ID for git commit description, so let's expand
> > the GIT_COMMIT_ID to add 'Fixes:' tag format check support.
> > 
> > The check supports below formats:
> > Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")
> > Fixes: 85f7cd3a2aad ("Revert "media: Kconfig: better support hybrid TV devices"")
> > Fixes: 878520ac45f9 ("ext4: save the error code which triggered...")
> > Fixes: 878520ac45f9 ("ext4: save the error code which triggered")
> > Fixes: 277f27e2f277 ("SUNRPC/cache: Allow garbage collection ... ")
> 
> Hi again YanQing.
> 
> I think all the non-standard and incomplete forms
> should have a warning emitted.

Hi Joe Perches
I am not sure whether I get your words, you mean we need to emit warning
for incomplete title line format? For example:
Fixes: 277f27e2f277 ("SUNRPC/cache: Allow garbage collection ... ")


Thanks.
> 
> > The check doesn't support below formats:
> > Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface"
> > Fixes: 6c73698904aa pinctrl: qcom: Introduce readl/writel accessors
> > Fixes: 3fd6e7d9a146 (ASoC: tas571x: New driver for TI TAS571x power amplifiers)
> > Fixes: 55697cbb44e4 ("arm64: dts: renesas: r8a779{65,80,90}: Add IPMMU devices nodes)
> > Fixes: ba35f8588f47 (“ipvlan: Defer multicast / broadcast processing to a work-queue”)
> > Fixes: cd758a9b57ee "KVM: PPC: Book3S HV: Use __gfn_to_pfn_memslot in HPT page fault handler"
> > Fixes:      9b1640686470 ("scsi: lpfc: Fix use-after-free mailbox cmd completion")
> > Fixes: 03f6fc6de919 ('ASoC: rt5682: Add the soundwire support')
> 
> 
> > Note: this patch also fixes double quotation mark issue for normal git
> >       commit description, and now it supports double quotation mark in
> >       title line, for example:
> >       Commit e33e2241e272 ("Revert "cfg80211: Use 5MHz bandwidth by default
> >       when checking usable channels"")
> 
> Nice.
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] checkpatch: add support to check 'Fixes:' tag format
  2020-05-01 16:34   ` Wang YanQing
@ 2020-05-01 16:44     ` Joe Perches
  2020-05-01 16:52     ` Markus Elfring
  1 sibling, 0 replies; 6+ messages in thread
From: Joe Perches @ 2020-05-01 16:44 UTC (permalink / raw)
  To: Wang YanQing
  Cc: Andy Whitcroft, linux-kernel, Alexei Starovoitov, Matteo Croce,
	Markus.Elfring, kernel-janitors

On Sat, 2020-05-02 at 00:34 +0800, Wang YanQing wrote:
> On Fri, May 01, 2020 at 08:57:42AM -0700, Joe Perches wrote:
> > On Fri, 2020-05-01 at 23:40 +0800, Wang YanQing wrote:
> > > According to submitting-patches.rst, 'Fixes:' tag has a little
> > > stricter condition about the one line summary than normal git
> > > commit description:
> > > “...
> > > Do not split the tag across multiple lines, tags are exempt from
> > > the "wrap at 75 columns" rule in order to simplify parsing scripts
> > > ...”
> > > 
> > > And there is no sanity check for 'Fixes:' tag format in checkpatch
> > > the same as GIT_COMMIT_ID for git commit description, so let's expand
> > > the GIT_COMMIT_ID to add 'Fixes:' tag format check support.
> > > 
> > > The check supports below formats:
> > > Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")
> > > Fixes: 85f7cd3a2aad ("Revert "media: Kconfig: better support hybrid TV devices"")
> > > Fixes: 878520ac45f9 ("ext4: save the error code which triggered...")
> > > Fixes: 878520ac45f9 ("ext4: save the error code which triggered")
> > > Fixes: 277f27e2f277 ("SUNRPC/cache: Allow garbage collection ... ")
> > 
> > Hi again YanQing.
> > 
> > I think all the non-standard and incomplete forms
> > should have a warning emitted.
> 
> Hi Joe Perches

Hi, again. It's just Joe to my friends...

> I am not sure whether I get your words, you mean we need to emit warning
> for incomplete title line format? For example:
> Fixes: 277f27e2f277 ("SUNRPC/cache: Allow garbage collection ... ")

I think so yes.

It _might_ be useful to show "why" the message is being emitted.
(sha1 too short, no quotes around description, etc...)



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] checkpatch: add support to check 'Fixes:' tag format
  2020-05-01 16:34   ` Wang YanQing
  2020-05-01 16:44     ` Joe Perches
@ 2020-05-01 16:52     ` Markus Elfring
  1 sibling, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-05-01 16:52 UTC (permalink / raw)
  To: Wang YanQing, Joe Perches, Andy Whitcroft, kernel-janitors, linux-doc
  Cc: linux-kernel, Alexei Starovoitov, Matteo Croce

> I am not sure whether I get your words, you mean we need to emit warning
> for incomplete title line format? For example:
> Fixes: 277f27e2f277 ("SUNRPC/cache: Allow garbage collection ... ")

I suggest to increase the precision for the usage the ellipsis at the end.

* Triple ASCII dots

* U+2026

* The commit title should probably not be abbreviated too much.

Regards,
Markus

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-01 16:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-01 15:40 [PATCH v3] checkpatch: add support to check 'Fixes:' tag format Wang YanQing
2020-05-01 15:57 ` Joe Perches
2020-05-01 16:34   ` Wang YanQing
2020-05-01 16:44     ` Joe Perches
2020-05-01 16:52     ` Markus Elfring
2020-05-01 16:08 ` Markus Elfring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).