Since v6.3, checkpatch.pl now complains about the use of "Closes:" tags
followed by a link [1]. It also complains if a "Reported-by:" tag is
followed by a "Closes:" one [2].
As detailed in the first patch, this "Closes:" tag is used for a bit of
time, mainly by DRM and MPTCP subsystems. It is used by some bug
trackers to automate the closure of issues when a patch is accepted.
It is even planned to use this tag with bugzilla.kernel.org [3].
The first patch updates the documentation to explain what is this
"Closes:" tag and how/when to use it. The second patch modifies
checkpatch.pl to stop complaining about it.
The DRM maintainers and their mailing list have been added in Cc as they
are probably interested by these two patches as well.
[1] https://lore.kernel.org/all/3b036087d80b8c0e07a46a1dbaaf4ad0d018f8d5.1674217480.git.linux@leemhuis.info/
[2] https://lore.kernel.org/all/bb5dfd55ea2026303ab2296f4a6df3da7dd64006.1674217480.git.linux@leemhuis.info/
[3] https://lore.kernel.org/linux-doc/[email protected]/
Signed-off-by: Matthieu Baerts <[email protected]>
---
Note: After having re-read the comments from the v1, it is still unclear
to me if this "Closes:" can be accepted or not. But because it seems
that the future Bugzilla bot for kernel.org and regzbot would like to
use it as well, I'm sending here new versions. I'm sorry if I
misunderstood the comments from v1. Please tell me if I did.
Changes in v4:
- Patches 1/5, 3/5 and 4/5 have been added to ask using the "Closes" tag
instead of the "Link" one for any bug reports. (Thorsten)
- The Fixes tags have been removed from patch 4/5. (Joe)
- The "Reported-by being followed by a link tag" check is now only
looking for the tag, not the URL which is done elsewhere in patch 5/5.
(Thorsten)
- A new patch has been added to fix a small issues in checkpatch.pl when
checking if "Reported-by:" tag is on the last line.
- Link to v3: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v3-0-d1bdcf31c71c@tessares.net
Changes in v3:
- Patch 1/4 now allow using the "Closes" tag with any kind of bug
reports, as long as the link is public. (Thorsten)
- The former patch 2/2 has been split in two: first to use a list for
the different "link" tags (Joe). Then to allow the 'Closes' tag.
- A new patch has been added to let checkpatch.pl checking if "Closes"
and "Links" are used with a URL.
- Link to v2: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v2-0-f4a417861f6d@tessares.net
Changes in v2:
- The text on patch 1/2 has been reworked thanks to Jon, Bagas and
Thorsten. See the individual changelog on the patch for more details.
- Private bug trackers and invalid URLs are clearly marked as forbidden
to avoid being misused. (Linus)
- Rebased on top of Linus' repo.
- Link to v1: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v1-0-1b83072e9a9a@tessares.net
---
Matthieu Baerts (5):
docs: process: allow Closes tags with links
checkpatch: don't print the next line if not defined
checkpatch: use a list of "link" tags
checkpatch: allow Closes tags with links
checkpatch: check for misuse of the link tags
Documentation/process/5.Posting.rst | 22 ++++++++++----
Documentation/process/submitting-patches.rst | 26 +++++++++++------
scripts/checkpatch.pl | 43 ++++++++++++++++++++++------
3 files changed, 70 insertions(+), 21 deletions(-)
---
base-commit: 7e364e56293bb98cae1b55fd835f5991c4e96e7d
change-id: 20230314-doc-checkpatch-closes-tag-1731b57556b1
Best regards,
--
Matthieu Baerts <[email protected]>
"Link:" and "Closes:" tags have to be used with public URLs.
It is difficult to make sure the link is public but at least we can
verify the tag is followed by 'http(s)://'.
With that, we avoid such a tag that is not allowed [1]:
Closes: <number>
Now that we check the "link" tags are followed by a URL, we can relax
the check linked to "Reported-by being followed by a link tag" to only
verify if a "link" tag is present after the "Reported-by" one.
Link: https://lore.kernel.org/linux-doc/CAHk-=wh0v1EeDV3v8TzK81nDC40=XuTdY2MCr0xy3m3FiBV3+Q@mail.gmail.com/ [1]
Signed-off-by: Matthieu Baerts <[email protected]>
---
v4:
- Relax "Reported-by being followed by a link tag" check
- Check for "http(s)://" not just "http(s):"
v3:
- new patch addressing Linus' concerns.
---
scripts/checkpatch.pl | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f8a57f400570..6d602c61d72a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3179,7 +3179,7 @@ sub process {
if (!defined $lines[$linenr]) {
WARN("BAD_REPORTED_BY_LINK",
"Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . "\n");
- } elsif ($rawlines[$linenr] !~ m{^closes:\s*https?://}i) {
+ } elsif ($rawlines[$linenr] !~ /^closes:\s*/i) {
WARN("BAD_REPORTED_BY_LINK",
"Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
}
@@ -3292,6 +3292,17 @@ sub process {
}
}
+# Check for misuse of the link tags
+ if ($in_commit_log &&
+ $line =~ /^\s*(\w+:)\s*(\S+)/) {
+ my $tag = $1;
+ my $value = $2;
+ if ($tag =~ /^$link_tags_search$/ && $value !~ m{^https?://}) {
+ WARN("COMMIT_LOG_WRONG_LINK",
+ "'$tag' should be followed by a public http(s) link\n" . $herecurr);
+ }
+ }
+
# Check for lines starting with a #
if ($in_commit_log && $line =~ /^#/) {
if (WARN("COMMIT_COMMENT_SYMBOL",
--
2.39.2
As a follow-up of a previous patch modifying the documentation to
allow using the "Closes:" tag, checkpatch.pl is updated accordingly.
checkpatch.pl now no longer complain when the "Closes:" tag is used by
itself:
commit 76f381bb77a0 ("checkpatch: warn when unknown tags are used for links")
... or after the "Reported-by:" tag:
commit d7f1d71e5ef6 ("checkpatch: warn when Reported-by: is not followed by Link:")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/373
Signed-off-by: Matthieu Baerts <[email protected]>
---
v4:
- remove the "Fixes" tags. (Joe Perches)
- "Reported-by:" should be followed by a "Closes:" tag. (Thorsten
Leemhuis)
v3:
- split into 2 patches: the previous one adds a list with all the
"link" tags. This one only allows the "Closes" tag. (Joe Perches)
- "Closes" is no longer printed between parenthesis. (Thorsten
Leemhuis)
---
scripts/checkpatch.pl | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1647ef72480e..f8a57f400570 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -620,7 +620,7 @@ our $signature_tags = qr{(?xi:
Cc:
)};
-our @link_tags = qw(Link);
+our @link_tags = qw(Link Closes);
#Create a search and print patterns for all these strings to be used directly below
our $link_tags_search = "";
@@ -3174,14 +3174,14 @@ sub process {
}
}
-# check if Reported-by: is followed by a Link:
+# check if Reported-by: is followed by a Closes: tag
if ($sign_off =~ /^reported(?:|-and-tested)-by:$/i) {
if (!defined $lines[$linenr]) {
WARN("BAD_REPORTED_BY_LINK",
- "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . "\n");
- } elsif ($rawlines[$linenr] !~ m{^link:\s*https?://}i) {
+ "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . "\n");
+ } elsif ($rawlines[$linenr] !~ m{^closes:\s*https?://}i) {
WARN("BAD_REPORTED_BY_LINK",
- "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
+ "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
}
}
}
--
2.39.2
The following commit will allow the use of a similar "link" tag.
Because there is a possibility that other similar tags will be added in
the future and to reduce the number of places where the code will be
modified to allow this new tag, a list with all these "link" tags is now
used.
Two variables are created from it: one to search for such tags and one
to print all tags in a warning message.
Suggested-by: Joe Perches <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
v4:
- "Reported-by:" should be followed by a "Closes:" tag. (Thorsten
Leemhuis)
v3:
- new patch. (Joe Perches)
---
scripts/checkpatch.pl | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b170fc7ef258..1647ef72480e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -620,6 +620,22 @@ our $signature_tags = qr{(?xi:
Cc:
)};
+our @link_tags = qw(Link);
+
+#Create a search and print patterns for all these strings to be used directly below
+our $link_tags_search = "";
+our $link_tags_print = "";
+foreach my $entry (@link_tags) {
+ if ($link_tags_search ne "") {
+ $link_tags_search .= '|';
+ $link_tags_print .= ' or ';
+ }
+ $entry .= ':';
+ $link_tags_search .= $entry;
+ $link_tags_print .= "'$entry'";
+}
+$link_tags_search = "(?:${link_tags_search})";
+
our $tracing_logging_tags = qr{(?xi:
[=-]*> |
<[=-]* |
@@ -3250,8 +3266,8 @@ sub process {
# file delta changes
$line =~ /^\s*(?:[\w\.\-\+]*\/)++[\w\.\-\+]+:/ ||
# filename then :
- $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
- # A Fixes: or Link: line or signature tag line
+ $line =~ /^\s*(?:Fixes:|$link_tags_search|$signature_tags)/i ||
+ # A Fixes:, link or signature tag line
$commit_log_possible_stack_dump)) {
WARN("COMMIT_LOG_LONG_LINE",
"Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
@@ -3266,13 +3282,13 @@ sub process {
# Check for odd tags before a URI/URL
if ($in_commit_log &&
- $line =~ /^\s*(\w+):\s*http/ && $1 ne 'Link') {
+ $line =~ /^\s*(\w+:)\s*http/ && $1 !~ /^$link_tags_search$/) {
if ($1 =~ /^v(?:ersion)?\d+/i) {
WARN("COMMIT_LOG_VERSIONING",
"Patch version information should be after the --- line\n" . $herecurr);
} else {
WARN("COMMIT_LOG_USE_LINK",
- "Unknown link reference '$1:', use 'Link:' instead\n" . $herecurr);
+ "Unknown link reference '$1', use $link_tags_print instead\n" . $herecurr);
}
}
--
2.39.2
On Mon, 2023-04-03 at 18:23 +0200, Matthieu Baerts wrote:
> Since v6.3, checkpatch.pl now complains about the use of "Closes:" tags
> followed by a link [1]. It also complains if a "Reported-by:" tag is
> followed by a "Closes:" one [2].
All these patches seems sensible, thanks.
Assuming Linus approves the use of "Closes:"
Acked-by: Joe Perches <[email protected]>
> As detailed in the first patch, this "Closes:" tag is used for a bit of
> time, mainly by DRM and MPTCP subsystems. It is used by some bug
> trackers to automate the closure of issues when a patch is accepted.
> It is even planned to use this tag with bugzilla.kernel.org [3].
>
> The first patch updates the documentation to explain what is this
> "Closes:" tag and how/when to use it. The second patch modifies
> checkpatch.pl to stop complaining about it.
>
> The DRM maintainers and their mailing list have been added in Cc as they
> are probably interested by these two patches as well.
>
> [1] https://lore.kernel.org/all/3b036087d80b8c0e07a46a1dbaaf4ad0d018f8d5.1674217480.git.linux@leemhuis.info/
> [2] https://lore.kernel.org/all/bb5dfd55ea2026303ab2296f4a6df3da7dd64006.1674217480.git.linux@leemhuis.info/
> [3] https://lore.kernel.org/linux-doc/[email protected]/
>
> Signed-off-by: Matthieu Baerts <[email protected]>
> ---
> Note: After having re-read the comments from the v1, it is still unclear
> to me if this "Closes:" can be accepted or not. But because it seems
> that the future Bugzilla bot for kernel.org and regzbot would like to
> use it as well, I'm sending here new versions. I'm sorry if I
> misunderstood the comments from v1. Please tell me if I did.
>
> Changes in v4:
> - Patches 1/5, 3/5 and 4/5 have been added to ask using the "Closes" tag
> instead of the "Link" one for any bug reports. (Thorsten)
> - The Fixes tags have been removed from patch 4/5. (Joe)
> - The "Reported-by being followed by a link tag" check is now only
> looking for the tag, not the URL which is done elsewhere in patch 5/5.
> (Thorsten)
> - A new patch has been added to fix a small issues in checkpatch.pl when
> checking if "Reported-by:" tag is on the last line.
> - Link to v3: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v3-0-d1bdcf31c71c@tessares.net
>
> Changes in v3:
> - Patch 1/4 now allow using the "Closes" tag with any kind of bug
> reports, as long as the link is public. (Thorsten)
> - The former patch 2/2 has been split in two: first to use a list for
> the different "link" tags (Joe). Then to allow the 'Closes' tag.
> - A new patch has been added to let checkpatch.pl checking if "Closes"
> and "Links" are used with a URL.
> - Link to v2: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v2-0-f4a417861f6d@tessares.net
>
> Changes in v2:
> - The text on patch 1/2 has been reworked thanks to Jon, Bagas and
> Thorsten. See the individual changelog on the patch for more details.
> - Private bug trackers and invalid URLs are clearly marked as forbidden
> to avoid being misused. (Linus)
> - Rebased on top of Linus' repo.
> - Link to v1: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v1-0-1b83072e9a9a@tessares.net
>
> ---
> Matthieu Baerts (5):
> docs: process: allow Closes tags with links
> checkpatch: don't print the next line if not defined
> checkpatch: use a list of "link" tags
> checkpatch: allow Closes tags with links
> checkpatch: check for misuse of the link tags
>
> Documentation/process/5.Posting.rst | 22 ++++++++++----
> Documentation/process/submitting-patches.rst | 26 +++++++++++------
> scripts/checkpatch.pl | 43 ++++++++++++++++++++++------
> 3 files changed, 70 insertions(+), 21 deletions(-)
> ---
> base-commit: 7e364e56293bb98cae1b55fd835f5991c4e96e7d
> change-id: 20230314-doc-checkpatch-closes-tag-1731b57556b1
>
> Best regards,