new-file | 1 + 1 file changed, 1 insertion(+) create mode 100644 new-file
Do not require the presence of `$balanced_parens` to get the commit SHA;
this allows a `Fixes: deadbeef` tag to get a correct suggestion rather
than a suggestion containing a reference to HEAD.
Given this patch:
```
From c10a7d25e68ffd6222db2b63d86efa2fba04008b Mon Sep 17 00:00:00 2001
From: Tamir Duberstein <tamird@gmail.com>
Date: Fri, 25 Oct 2024 19:30:51 -0400
Subject: [PATCH] Test patch
This is a test patch.
Fixes: bd17e036b495
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v2:
- Added sample input and before and after output to patch description.
- Link to v1: https://lore.kernel.org/r/20241019-checkpatch-fixes-commit-v1-1-0d0cde18ce69@gmail.com
---
new-file | 1 +
1 file changed, 1 insertion(+)
create mode 100644 new-file
diff --git a/new-file b/new-file
new file mode 100644
index 000000000000..90b67dda6d59
--- /dev/null
+++ b/new-file
@@ -0,0 +1 @@
+Test.
--
2.47.0
```
Before:
```
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: c10a7d25e68f ("Test patch")'
Fixes: bd17e036b495
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
new file mode 100644
total: 0 errors, 2 warnings, 1 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] Test patch" has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
```
After:
```
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")'
Fixes: bd17e036b495
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
new file mode 100644
total: 0 errors, 2 warnings, 1 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] Test patch" has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
```
The diff between the outputs:
```
diff --git a/before b/after
index d0cfdb243a1f..6f97b4c53080 100644
--- a/before
+++ b/after
@@ -1,4 +1,4 @@
-WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: c10a7d25e68f ("Test patch")'
+WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")'
#8:
Fixes: bd17e036b495
```
The prior behavior incorrectly suggested the patch's own SHA and title
line rather than the referenced commit's. This fixes that.
Ironically this:
Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
scripts/checkpatch.pl | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4427572b24771ddb1f2bf3de3176f9437f643951..b03d526e4c454af7a42624155175e109d499762f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3209,36 +3209,31 @@ sub process {
# Check Fixes: styles is correct
if (!$in_header_lines &&
- $line =~ /^\s*fixes:?\s*(?:commit\s*)?[0-9a-f]{5,}\b/i) {
- my $orig_commit = "";
- my $id = "0123456789ab";
- my $title = "commit title";
- my $tag_case = 1;
- my $tag_space = 1;
- my $id_length = 1;
- my $id_case = 1;
+ $line =~ /^\s*(fixes:?)\s*(?:commit\s*)?([0-9a-f]{5,40})(?:\s*($balanced_parens))?/i) {
+ my $tag = $1;
+ my $orig_commit = $2;
+ my $title;
my $title_has_quotes = 0;
$fixes_tag = 1;
-
- if ($line =~ /(\s*fixes:?)\s+([0-9a-f]{5,})\s+($balanced_parens)/i) {
- my $tag = $1;
- $orig_commit = $2;
- $title = $3;
-
- $tag_case = 0 if $tag eq "Fixes:";
- $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i);
-
- $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i);
- $id_case = 0 if ($orig_commit !~ /[A-F]/);
-
+ if (defined $3) {
# Always strip leading/trailing parens then double quotes if existing
- $title = substr($title, 1, -1);
+ $title = substr($3, 1, -1);
if ($title =~ /^".*"$/) {
$title = substr($title, 1, -1);
$title_has_quotes = 1;
}
+ } else {
+ $title = "commit title"
}
+
+ my $tag_case = not ($tag eq "Fixes:");
+ my $tag_space = not ($line =~ /^fixes:? [0-9a-f]{5,40} ($balanced_parens)/i);
+
+ my $id_length = not ($orig_commit =~ /^[0-9a-f]{12}$/i);
+ my $id_case = not ($orig_commit !~ /[A-F]/);
+
+ my $id = "0123456789ab";
my ($cid, $ctitle) = git_commit_info($orig_commit, $id,
$title);
---
base-commit: c71f8fb4dc911022748a378b16aad1cc9b43aad8
change-id: 20241019-checkpatch-fixes-commit-aa014fe6d0b3
Best regards,
--
Tamir Duberstein <tamird@gmail.com>On Fri, 25 Oct 2024 19:43:19 -0400 Tamir Duberstein <tamird@gmail.com> wrote:
> Do not require the presence of `$balanced_parens` to get the commit SHA;
> this allows a `Fixes: deadbeef` tag to get a correct suggestion rather
> than a suggestion containing a reference to HEAD.
Got it, thanks. Below is what I ended up with:
From: Tamir Duberstein <tamird@gmail.com>
Subject: checkpatch: always parse orig_commit in fixes tag
Date: Fri, 25 Oct 2024 19:43:19 -0400
Do not require the presence of `$balanced_parens` to get the commit SHA;
this allows a `Fixes: deadbeef` tag to get a correct suggestion rather
than a suggestion containing a reference to HEAD.
Given this patch:
: From: Tamir Duberstein <tamird@gmail.com>
: Subject: Test patch
: Date: Fri, 25 Oct 2024 19:30:51 -0400
:
: This is a test patch.
:
: Fixes: bd17e036b495
: Signed-off-by: Tamir Duberstein <tamird@gmail.com>
: --- /dev/null
: +++ b/new-file
: @@ -0,0 +1 @@
: +Test.
Before:
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: c10a7d25e68f ("Test patch")'
After:
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")'
The prior behavior incorrectly suggested the patch's own SHA and title
line rather than the referenced commit's. This fixes that.
Ironically this:
Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Louis Peens <louis.peens@corigine.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Cc: Philippe Schenker <philippe.schenker@toradex.com>
Cc: Simon Horman <horms@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
scripts/checkpatch.pl | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
--- a/scripts/checkpatch.pl~checkpatch-always-parse-orig_commit-in-fixes-tag
+++ a/scripts/checkpatch.pl
@@ -3209,36 +3209,31 @@ sub process {
# Check Fixes: styles is correct
if (!$in_header_lines &&
- $line =~ /^\s*fixes:?\s*(?:commit\s*)?[0-9a-f]{5,}\b/i) {
- my $orig_commit = "";
- my $id = "0123456789ab";
- my $title = "commit title";
- my $tag_case = 1;
- my $tag_space = 1;
- my $id_length = 1;
- my $id_case = 1;
+ $line =~ /^\s*(fixes:?)\s*(?:commit\s*)?([0-9a-f]{5,40})(?:\s*($balanced_parens))?/i) {
+ my $tag = $1;
+ my $orig_commit = $2;
+ my $title;
my $title_has_quotes = 0;
$fixes_tag = 1;
-
- if ($line =~ /(\s*fixes:?)\s+([0-9a-f]{5,})\s+($balanced_parens)/i) {
- my $tag = $1;
- $orig_commit = $2;
- $title = $3;
-
- $tag_case = 0 if $tag eq "Fixes:";
- $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i);
-
- $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i);
- $id_case = 0 if ($orig_commit !~ /[A-F]/);
-
+ if (defined $3) {
# Always strip leading/trailing parens then double quotes if existing
- $title = substr($title, 1, -1);
+ $title = substr($3, 1, -1);
if ($title =~ /^".*"$/) {
$title = substr($title, 1, -1);
$title_has_quotes = 1;
}
+ } else {
+ $title = "commit title"
}
+
+ my $tag_case = not ($tag eq "Fixes:");
+ my $tag_space = not ($line =~ /^fixes:? [0-9a-f]{5,40} ($balanced_parens)/i);
+
+ my $id_length = not ($orig_commit =~ /^[0-9a-f]{12}$/i);
+ my $id_case = not ($orig_commit !~ /[A-F]/);
+
+ my $id = "0123456789ab";
my ($cid, $ctitle) = git_commit_info($orig_commit, $id,
$title);
_
On Fri, Oct 25, 2024 at 8:44 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 25 Oct 2024 19:43:19 -0400 Tamir Duberstein <tamird@gmail.com> wrote:
>
> > Do not require the presence of `$balanced_parens` to get the commit SHA;
> > this allows a `Fixes: deadbeef` tag to get a correct suggestion rather
> > than a suggestion containing a reference to HEAD.
>
> Got it, thanks. Below is what I ended up with:
>
>
> From: Tamir Duberstein <tamird@gmail.com>
> Subject: checkpatch: always parse orig_commit in fixes tag
> Date: Fri, 25 Oct 2024 19:43:19 -0400
>
> Do not require the presence of `$balanced_parens` to get the commit SHA;
> this allows a `Fixes: deadbeef` tag to get a correct suggestion rather
> than a suggestion containing a reference to HEAD.
>
> Given this patch:
>
> : From: Tamir Duberstein <tamird@gmail.com>
> : Subject: Test patch
> : Date: Fri, 25 Oct 2024 19:30:51 -0400
> :
> : This is a test patch.
> :
> : Fixes: bd17e036b495
> : Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> : --- /dev/null
> : +++ b/new-file
> : @@ -0,0 +1 @@
> : +Test.
>
>
> Before:
>
> WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: c10a7d25e68f ("Test patch")'
>
> After:
>
> WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")'
>
>
>
> The prior behavior incorrectly suggested the patch's own SHA and title
> line rather than the referenced commit's. This fixes that.
>
> Ironically this:
>
> Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> Cc: Andy Whitcroft <apw@canonical.com>
> Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Louis Peens <louis.peens@corigine.com>
> Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Cc: Philippe Schenker <philippe.schenker@toradex.com>
> Cc: Simon Horman <horms@kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> scripts/checkpatch.pl | 37 ++++++++++++++++---------------------
> 1 file changed, 16 insertions(+), 21 deletions(-)
>
> --- a/scripts/checkpatch.pl~checkpatch-always-parse-orig_commit-in-fixes-tag
> +++ a/scripts/checkpatch.pl
> @@ -3209,36 +3209,31 @@ sub process {
>
> # Check Fixes: styles is correct
> if (!$in_header_lines &&
> - $line =~ /^\s*fixes:?\s*(?:commit\s*)?[0-9a-f]{5,}\b/i) {
> - my $orig_commit = "";
> - my $id = "0123456789ab";
> - my $title = "commit title";
> - my $tag_case = 1;
> - my $tag_space = 1;
> - my $id_length = 1;
> - my $id_case = 1;
> + $line =~ /^\s*(fixes:?)\s*(?:commit\s*)?([0-9a-f]{5,40})(?:\s*($balanced_parens))?/i) {
> + my $tag = $1;
> + my $orig_commit = $2;
> + my $title;
> my $title_has_quotes = 0;
> $fixes_tag = 1;
> -
> - if ($line =~ /(\s*fixes:?)\s+([0-9a-f]{5,})\s+($balanced_parens)/i) {
> - my $tag = $1;
> - $orig_commit = $2;
> - $title = $3;
> -
> - $tag_case = 0 if $tag eq "Fixes:";
> - $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i);
> -
> - $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i);
> - $id_case = 0 if ($orig_commit !~ /[A-F]/);
> -
> + if (defined $3) {
> # Always strip leading/trailing parens then double quotes if existing
> - $title = substr($title, 1, -1);
> + $title = substr($3, 1, -1);
> if ($title =~ /^".*"$/) {
> $title = substr($title, 1, -1);
> $title_has_quotes = 1;
> }
> + } else {
> + $title = "commit title"
> }
>
> +
> + my $tag_case = not ($tag eq "Fixes:");
> + my $tag_space = not ($line =~ /^fixes:? [0-9a-f]{5,40} ($balanced_parens)/i);
> +
> + my $id_length = not ($orig_commit =~ /^[0-9a-f]{12}$/i);
> + my $id_case = not ($orig_commit !~ /[A-F]/);
> +
> + my $id = "0123456789ab";
> my ($cid, $ctitle) = git_commit_info($orig_commit, $id,
> $title);
>
> _
>
Thanks!
On Fri, Oct 25, 2024 at 7:43 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Do not require the presence of `$balanced_parens` to get the commit SHA;
> this allows a `Fixes: deadbeef` tag to get a correct suggestion rather
> than a suggestion containing a reference to HEAD.
>
> Given this patch:
> ```
> From c10a7d25e68ffd6222db2b63d86efa2fba04008b Mon Sep 17 00:00:00 2001
> From: Tamir Duberstein <tamird@gmail.com>
> Date: Fri, 25 Oct 2024 19:30:51 -0400
> Subject: [PATCH] Test patch
>
> This is a test patch.
>
> Fixes: bd17e036b495
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> Changes in v2:
> - Added sample input and before and after output to patch description.
> - Link to v1: https://lore.kernel.org/r/20241019-checkpatch-fixes-commit-v1-1-0d0cde18ce69@gmail.com
> ---
Apologies, b4 got confused and put this in the wrong place. If there's
a better way to format the patch, please let me know.
> new-file | 1 +
> 1 file changed, 1 insertion(+)
> create mode 100644 new-file
>
> diff --git a/new-file b/new-file
> new file mode 100644
> index 000000000000..90b67dda6d59
> --- /dev/null
> +++ b/new-file
> @@ -0,0 +1 @@
> +Test.
> --
> 2.47.0
> ```
>
> Before:
> ```
> WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: c10a7d25e68f ("Test patch")'
> Fixes: bd17e036b495
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> new file mode 100644
>
> total: 0 errors, 2 warnings, 1 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> "[PATCH] Test patch" has style problems, please review.
>
> NOTE: If any of the errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> ```
>
> After:
> ```
> WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")'
> Fixes: bd17e036b495
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> new file mode 100644
>
> total: 0 errors, 2 warnings, 1 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> "[PATCH] Test patch" has style problems, please review.
>
> NOTE: If any of the errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> ```
>
> The diff between the outputs:
> ```
> diff --git a/before b/after
> index d0cfdb243a1f..6f97b4c53080 100644
> --- a/before
> +++ b/after
> @@ -1,4 +1,4 @@
> -WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: c10a7d25e68f ("Test patch")'
> +WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")'
> #8:
> Fixes: bd17e036b495
> ```
>
> The prior behavior incorrectly suggested the patch's own SHA and title
> line rather than the referenced commit's. This fixes that.
>
> Ironically this:
>
> Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> scripts/checkpatch.pl | 37 ++++++++++++++++---------------------
> 1 file changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 4427572b24771ddb1f2bf3de3176f9437f643951..b03d526e4c454af7a42624155175e109d499762f 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3209,36 +3209,31 @@ sub process {
>
> # Check Fixes: styles is correct
> if (!$in_header_lines &&
> - $line =~ /^\s*fixes:?\s*(?:commit\s*)?[0-9a-f]{5,}\b/i) {
> - my $orig_commit = "";
> - my $id = "0123456789ab";
> - my $title = "commit title";
> - my $tag_case = 1;
> - my $tag_space = 1;
> - my $id_length = 1;
> - my $id_case = 1;
> + $line =~ /^\s*(fixes:?)\s*(?:commit\s*)?([0-9a-f]{5,40})(?:\s*($balanced_parens))?/i) {
> + my $tag = $1;
> + my $orig_commit = $2;
> + my $title;
> my $title_has_quotes = 0;
> $fixes_tag = 1;
> -
> - if ($line =~ /(\s*fixes:?)\s+([0-9a-f]{5,})\s+($balanced_parens)/i) {
> - my $tag = $1;
> - $orig_commit = $2;
> - $title = $3;
> -
> - $tag_case = 0 if $tag eq "Fixes:";
> - $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i);
> -
> - $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i);
> - $id_case = 0 if ($orig_commit !~ /[A-F]/);
> -
> + if (defined $3) {
> # Always strip leading/trailing parens then double quotes if existing
> - $title = substr($title, 1, -1);
> + $title = substr($3, 1, -1);
> if ($title =~ /^".*"$/) {
> $title = substr($title, 1, -1);
> $title_has_quotes = 1;
> }
> + } else {
> + $title = "commit title"
> }
>
> +
> + my $tag_case = not ($tag eq "Fixes:");
> + my $tag_space = not ($line =~ /^fixes:? [0-9a-f]{5,40} ($balanced_parens)/i);
> +
> + my $id_length = not ($orig_commit =~ /^[0-9a-f]{12}$/i);
> + my $id_case = not ($orig_commit !~ /[A-F]/);
> +
> + my $id = "0123456789ab";
> my ($cid, $ctitle) = git_commit_info($orig_commit, $id,
> $title);
>
>
> ---
> base-commit: c71f8fb4dc911022748a378b16aad1cc9b43aad8
> change-id: 20241019-checkpatch-fixes-commit-aa014fe6d0b3
>
> Best regards,
> --
> Tamir Duberstein <tamird@gmail.com>
>
© 2016 - 2024 Red Hat, Inc.