| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2194d19d-f195-1a1e-41fc-7827ae569351@leemhuis.info>
Date: Fri, 31 Mar 2023 11:39:22 +0200
From: Thorsten Leemhuis <linux@...mhuis.info>
To: Matthieu Baerts <matthieu.baerts@...sares.net>,
Jonathan Corbet <corbet@....net>,
Andy Whitcroft <apw@...onical.com>,
Joe Perches <joe@...ches.com>,
Dwaipayan Ray <dwaipayanray1@...il.com>,
Lukas Bulwahn <lukas.bulwahn@...il.com>,
Kai Wasserbäch <kai@....carbon-project.org>,
Andrew Morton <akpm@...ux-foundation.org>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Konstantin Ryabitsev <konstantin@...uxfoundation.org>,
Bagas Sanjaya <bagasdotme@...il.com>,
Linus Torvalds <torvalds@...ux-foundation.org>
Cc: linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org, mptcp@...ts.linux.dev
Subject: Re: [PATCH v3 0/4] docs & checkpatch: allow Closes tags with links
On 30.03.23 20:13, 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].
>
> 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/20230315181205.f3av7h6owqzzw64p@meerkat.local/
>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@...sares.net>
Maybe it's just me, but I think those changes do not make it clear
enough when to use Link: and when to use Closes. Find below an
alternative proposal how I'd do it for consideration that goes
'all-in' for the sake of simplicity.
[untested -- and I hope thunderbird won't mangle the patch]
Ciao, Thorsten
diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst
index 7a670a075ab6..fc194b4d1674 100644
--- a/Documentation/process/5.Posting.rst
+++ b/Documentation/process/5.Posting.rst
@@ -207,11 +207,17 @@ the patch::
Fixes: 1f2e3d4c5b6a ("The first line of the commit specified by the first 12 characters of its SHA-1 ID")
Another tag is used for linking web pages with additional backgrounds or
-details, for example a report about a bug fixed by the patch or a document
+details, for example earlier discussion which lead to the patch or a document
with a specification implemented by the patch::
Link: https://example.com/somewhere.html optional-other-stuff
+If the URL points to a report about a bug fixed by the patch, use this instead::
+
+ Closes: https://example.com/somewhere.html optional-other-stuff
+
+Ensure any such links are publicly accessible.
+
Many maintainers when applying a patch also add this tag to link to the
latest public review posting of the patch; often this is automatically done
by tools like b4 or a git hook like the one described in
@@ -251,7 +257,7 @@ The tags in common use are:
- Reported-by: names a user who reported a problem which is fixed by this
patch; this tag is used to give credit to the (often underappreciated)
people who test our code and let us know when things do not work
- correctly. Note, this tag should be followed by a Link: tag pointing to the
+ correctly. Note, this tag should be followed by a Closes: tag pointing to the
report, unless the report is not available on the web.
- Cc: the named person received a copy of the patch and had the
diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 69ce64e03c70..73611cf1c372 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -126,8 +126,10 @@ For example::
Link: https://lore.kernel.org/r/30th.anniversary.repost@klaava.Helsinki.FI/
-Please check the link to make sure that it is actually working and points
-to the relevant message.
+If the URL points to a bug report that is fixed by the patch, use 'Closes:'
+instead.
+
+Ensure any such links are publicly accessible.
However, try to make your explanation understandable without external
resources. In addition to giving a URL to a mailing list archive or bug,
@@ -498,7 +500,7 @@ Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
The Reported-by tag gives credit to people who find bugs and report them and it
hopefully inspires them to help us again in the future. The tag is intended for
bugs; please do not use it to credit feature requests. The tag should be
-followed by a Link: tag pointing to the report, unless the report is not
+followed by a Closes: tag pointing to the report, unless the report is not
available on the web. Please note that if the bug was reported in private, then
ask for permission first before using the Reported-by tag.
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd44d12965c9..f9a7c2b856ae 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3158,14 +3158,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: to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
- } elsif ($rawlines[$linenr] !~ m{^link:\s*https?://}i) {
+ "Reported-by: should be immediately followed by Closes: to the report\n" . $herecurr . $rawlines[$linenr] . "\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");
}
}
}
@@ -3266,13 +3266,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 ne 'Link' && $1 ne 'Closes') {
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:' or 'Closes:' instead\n" . $herecurr);
}
}
Powered by blists - more mailing lists