diff mbox series

checkpatch: add warning for non-lore mailing list URLs

Message ID	20221019202843.40810-1-helgaas@kernel.org
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Bjorn Helgaas <helgaas@kernel.org> To: Andy Whitcroft <apw@canonical.com>, Joe Perches <joe@perches.com> Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>, Lukas Bulwahn <lukas.bulwahn@gmail.com>, Kees Cook <keescook@chromium.org>, Randy Dunlap <rdunlap@infradead.org>, linux-kernel@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com> Subject: [PATCH] checkpatch: add warning for non-lore mailing list URLs Date: Wed, 19 Oct 2022 15:28:43 -0500 Message-Id: <20221019202843.40810-1-helgaas@kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	checkpatch: add warning for non-lore mailing list URLs \| checkpatch: add warning for non-lore mailing list URLs

Commit Message

Bjorn Helgaas Oct. 19, 2022, 8:28 p.m. UTC

  From: Bjorn Helgaas <bhelgaas@google.com>

The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
as lore.kernel.org because they use different styles, add advertising, and
may disappear in the future.  The lore archives are more consistent and
more likely to stick around, so prefer https://lore.kernel.org URLs when
they exist.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---

Sample commits for testing with "checkpatch -g":

  bd82d4bd2188 www.spinics.net/lists/arm-kernel/msg716956.html
  fdec2a9ef853 www.spinics.net/lists/kvm-arm
  1cdca16c043a www.spinics.net/lists/linux-mmc
  48ea02184a9d www.spinics.net/lists/linux-pci
  f32ae8a5f131 www.spinics.net/lists/netdev
  b7dca6dd1e59 lkml.org
  265df32eae58 lkml.org/lkml/
  4a9ceb7dbadf marc.info/?l=linux-kernel&m=155656897409107&w=2.
  c03914b7aa31 marc.info/?l=linux-mm
  f108c887d089 marc.info/?l=linux-netdev
  7424edbb5590 marc.info/?t=156200975600004&r=1&w=2
  dabac6e460ce https://marc.info/?l=linux-rdma&m=152296522708522&w=2
  b02f6a2ef0a1 www.mail-archive.com/linux-kernel@vger.kernel.org
  5e91bf5ce9b8 lists.infradead.org/pipermail/linux-snps-arc/2019-May
  3cde818cd02b mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
  a5448fdc469d http://lists.infradead.org/pipermail/linux-nvme/2019-June/024721.html

Previously posted:
  https://lore.kernel.org/all/20201217235615.43328-1-helgaas@kernel.org/
  https://lore.kernel.org/all/20220401201417.126664-1-helgaas@kernel.org/
---
 scripts/checkpatch.pl | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Joe Perches Oct. 19, 2022, 9:12 p.m. UTC | #1

On Wed, 2022-10-19 at 15:28 -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> as lore.kernel.org because they use different styles, add advertising, and
> may disappear in the future.  The lore archives are more consistent and
> more likely to stick around, so prefer https://lore.kernel.org URLs when
> they exist.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Seems sensible, thanks.

> ---
> 
> Sample commits for testing with "checkpatch -g":
> 
>   bd82d4bd2188 www.spinics.net/lists/arm-kernel/msg716956.html
>   fdec2a9ef853 www.spinics.net/lists/kvm-arm
>   1cdca16c043a www.spinics.net/lists/linux-mmc
>   48ea02184a9d www.spinics.net/lists/linux-pci
>   f32ae8a5f131 www.spinics.net/lists/netdev
>   b7dca6dd1e59 lkml.org
>   265df32eae58 lkml.org/lkml/
>   4a9ceb7dbadf marc.info/?l=linux-kernel&m=155656897409107&w=2.
>   c03914b7aa31 marc.info/?l=linux-mm
>   f108c887d089 marc.info/?l=linux-netdev
>   7424edbb5590 marc.info/?t=156200975600004&r=1&w=2
>   dabac6e460ce https://marc.info/?l=linux-rdma&m=152296522708522&w=2
>   b02f6a2ef0a1 www.mail-archive.com/linux-kernel@vger.kernel.org
>   5e91bf5ce9b8 lists.infradead.org/pipermail/linux-snps-arc/2019-May
>   3cde818cd02b mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
>   a5448fdc469d http://lists.infradead.org/pipermail/linux-nvme/2019-June/024721.html
> 
> Previously posted:
>   https://lore.kernel.org/all/20201217235615.43328-1-helgaas@kernel.org/
>   https://lore.kernel.org/all/20220401201417.126664-1-helgaas@kernel.org/
> ---
>  scripts/checkpatch.pl | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 1e5e66ae5a52..4e187202e77a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -702,6 +702,17 @@ sub find_standard_signature {
>  	return "";
>  }
>  
> +our $obsolete_archives = qr{(?xi:
> +	\Qfreedesktop.org/archives/dri-devel\E |
> +	\Qlists.infradead.org\E |
> +	\Qlkml.org\E |
> +	\Qmail-archive.com\E |
> +	\Qmailman.alsa-project.org/pipermail\E |
> +	\Qmarc.info\E |
> +	\Qozlabs.org/pipermail\E |
> +	\Qspinics.net\E
> +)};
> +
>  our @typeListMisordered = (
>  	qr{char\s+(?:un)?signed},
>  	qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -3324,6 +3335,12 @@ sub process {
>  			$last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);
>  		}
>  
> +# Check for mailing list archives other than lore.kernel.org
> +		if ($rawline =~ m{\b$obsolete_archives}) {
> +			WARN("PREFER_LORE_ARCHIVE",
> +			     "Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr);
> +		}
> +
>  # Check for added, moved or deleted files
>  		if (!$reported_maintainer_file && !$in_commit_log &&
>  		    ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ ||

Peter Collingbourne Nov. 4, 2022, 1:07 a.m. UTC | #2

On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> as lore.kernel.org because they use different styles, add advertising, and
> may disappear in the future.  The lore archives are more consistent and
> more likely to stick around, so prefer https://lore.kernel.org URLs when
> they exist.

If the commit message contains a line like:

Cc: linux-arm-kernel@lists.infradead.org

this patch causes checkpatch.pl to complain. Would it be possible to
restrict this to URLs?

Peter

Joe Perches Nov. 4, 2022, 1:27 a.m. UTC | #3

On Thu, 2022-11-03 at 18:07 -0700, Peter Collingbourne wrote:
> On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > as lore.kernel.org because they use different styles, add advertising, and
> > may disappear in the future.  The lore archives are more consistent and
> > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > they exist.
> 
> If the commit message contains a line like:
> 
> Cc: linux-arm-kernel@lists.infradead.org
> 
> this patch causes checkpatch.pl to complain. Would it be possible to
> restrict this to URLs?

Yes, I believe this would probably work well enough:
---
 scripts/checkpatch.pl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7be93c3df2bcb..fe25642d8bacc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3336,7 +3336,8 @@ sub process {
 		}
 
 # Check for mailing list archives other than lore.kernel.org
-		if ($rawline =~ m{\b$obsolete_archives}) {
+		if ($rawline =~ m{\b$obsolete_archives} &&
+		    $rawline !~ /^\s*cc:/i) {
 			WARN("PREFER_LORE_ARCHIVE",
 			     "Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr);
 		}

Peter Collingbourne Nov. 4, 2022, 1:34 a.m. UTC | #4

On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <joe@perches.com> wrote:
>
> On Thu, 2022-11-03 at 18:07 -0700, Peter Collingbourne wrote:
> > On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> > > From: Bjorn Helgaas <bhelgaas@google.com>
> > >
> > > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > > as lore.kernel.org because they use different styles, add advertising, and
> > > may disappear in the future.  The lore archives are more consistent and
> > > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > > they exist.
> >
> > If the commit message contains a line like:
> >
> > Cc: linux-arm-kernel@lists.infradead.org
> >
> > this patch causes checkpatch.pl to complain. Would it be possible to
> > restrict this to URLs?
>
> Yes, I believe this would probably work well enough:
> ---
>  scripts/checkpatch.pl | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7be93c3df2bcb..fe25642d8bacc 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3336,7 +3336,8 @@ sub process {
>                 }
>
>  # Check for mailing list archives other than lore.kernel.org
> -               if ($rawline =~ m{\b$obsolete_archives}) {
> +               if ($rawline =~ m{\b$obsolete_archives} &&
> +                   $rawline !~ /^\s*cc:/i) {

Can we make this (to|cc): instead? Otherwise developers (like me) who
use custom scripts to add To: headers to their patches before passing
them to checkpatch.pl will also hit this warning if their patch is
being sent To: one of these mailing lists.

Peter

Joe Perches Nov. 4, 2022, 1:40 a.m. UTC | #5

On Thu, 2022-11-03 at 18:34 -0700, Peter Collingbourne wrote:
> On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <joe@perches.com> wrote:
> > 
> > On Thu, 2022-11-03 at 18:07 -0700, Peter Collingbourne wrote:
> > > On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > > 
> > > > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > > > as lore.kernel.org because they use different styles, add advertising, and
> > > > may disappear in the future.  The lore archives are more consistent and
> > > > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > > > they exist.
> > > 
> > > If the commit message contains a line like:
> > > 
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > 
> > > this patch causes checkpatch.pl to complain. Would it be possible to
> > > restrict this to URLs?
> > 
> > Yes, I believe this would probably work well enough:
> > ---
> >  scripts/checkpatch.pl | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 7be93c3df2bcb..fe25642d8bacc 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -3336,7 +3336,8 @@ sub process {
> >                 }
> > 
> >  # Check for mailing list archives other than lore.kernel.org
> > -               if ($rawline =~ m{\b$obsolete_archives}) {
> > +               if ($rawline =~ m{\b$obsolete_archives} &&
> > +                   $rawline !~ /^\s*cc:/i) {
> 
> Can we make this (to|cc): instead? Otherwise developers (like me) who
> use custom scripts to add To: headers to their patches before passing
> them to checkpatch.pl will also hit this warning if their patch is
> being sent To: one of these mailing lists.

I think adding "To:" would be odd and unnecessary as it's not
something that would actually be in a patch.

You could use another front-end script to strip those "To:" from
checkpatch inputs.

Peter Collingbourne Nov. 4, 2022, 4:49 p.m. UTC | #6

On Thu, Nov 3, 2022 at 6:41 PM Joe Perches <joe@perches.com> wrote:
>
> On Thu, 2022-11-03 at 18:34 -0700, Peter Collingbourne wrote:
> > On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <joe@perches.com> wrote:
> > >
> > > On Thu, 2022-11-03 at 18:07 -0700, Peter Collingbourne wrote:
> > > > On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> > > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > > >
> > > > > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > > > > as lore.kernel.org because they use different styles, add advertising, and
> > > > > may disappear in the future.  The lore archives are more consistent and
> > > > > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > > > > they exist.
> > > >
> > > > If the commit message contains a line like:
> > > >
> > > > Cc: linux-arm-kernel@lists.infradead.org
> > > >
> > > > this patch causes checkpatch.pl to complain. Would it be possible to
> > > > restrict this to URLs?
> > >
> > > Yes, I believe this would probably work well enough:
> > > ---
> > >  scripts/checkpatch.pl | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 7be93c3df2bcb..fe25642d8bacc 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -3336,7 +3336,8 @@ sub process {
> > >                 }
> > >
> > >  # Check for mailing list archives other than lore.kernel.org
> > > -               if ($rawline =~ m{\b$obsolete_archives}) {
> > > +               if ($rawline =~ m{\b$obsolete_archives} &&
> > > +                   $rawline !~ /^\s*cc:/i) {
> >
> > Can we make this (to|cc): instead? Otherwise developers (like me) who
> > use custom scripts to add To: headers to their patches before passing
> > them to checkpatch.pl will also hit this warning if their patch is
> > being sent To: one of these mailing lists.
>
> I think adding "To:" would be odd and unnecessary as it's not
> something that would actually be in a patch.
>
> You could use another front-end script to strip those "To:" from
> checkpatch inputs.

OK, I made that work, so I guess I don't mind much what we do here.

Peter

Bjorn Helgaas Nov. 7, 2022, 8:54 p.m. UTC | #7

On Thu, Nov 03, 2022 at 06:34:31PM -0700, Peter Collingbourne wrote:
> On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <joe@perches.com> wrote:
> > On Thu, 2022-11-03 at 18:07 -0700, Peter Collingbourne wrote:
> > > On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > >
> > > > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > > > as lore.kernel.org because they use different styles, add advertising, and
> > > > may disappear in the future.  The lore archives are more consistent and
> > > > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > > > they exist.
> > >
> > > If the commit message contains a line like:
> > >
> > > Cc: linux-arm-kernel@lists.infradead.org
> > >
> > > this patch causes checkpatch.pl to complain. Would it be possible to
> > > restrict this to URLs?
> >
> > Yes, I believe this would probably work well enough:
> > ---
> >  scripts/checkpatch.pl | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 7be93c3df2bcb..fe25642d8bacc 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -3336,7 +3336,8 @@ sub process {
> >                 }
> >
> >  # Check for mailing list archives other than lore.kernel.org
> > -               if ($rawline =~ m{\b$obsolete_archives}) {
> > +               if ($rawline =~ m{\b$obsolete_archives} &&
> > +                   $rawline !~ /^\s*cc:/i) {
> 
> Can we make this (to|cc): instead? Otherwise developers (like me) who
> use custom scripts to add To: headers to their patches before passing
> them to checkpatch.pl will also hit this warning if their patch is
> being sent To: one of these mailing lists.

Why not make it look for "http" instead of the absence of "cc"?

Peter Collingbourne Nov. 7, 2022, 9 p.m. UTC | #8

On Mon, Nov 7, 2022 at 12:54 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Thu, Nov 03, 2022 at 06:34:31PM -0700, Peter Collingbourne wrote:
> > On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <joe@perches.com> wrote:
> > > On Thu, 2022-11-03 at 18:07 -0700, Peter Collingbourne wrote:
> > > > On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> > > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > > >
> > > > > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > > > > as lore.kernel.org because they use different styles, add advertising, and
> > > > > may disappear in the future.  The lore archives are more consistent and
> > > > > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > > > > they exist.
> > > >
> > > > If the commit message contains a line like:
> > > >
> > > > Cc: linux-arm-kernel@lists.infradead.org
> > > >
> > > > this patch causes checkpatch.pl to complain. Would it be possible to
> > > > restrict this to URLs?
> > >
> > > Yes, I believe this would probably work well enough:
> > > ---
> > >  scripts/checkpatch.pl | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 7be93c3df2bcb..fe25642d8bacc 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -3336,7 +3336,8 @@ sub process {
> > >                 }
> > >
> > >  # Check for mailing list archives other than lore.kernel.org
> > > -               if ($rawline =~ m{\b$obsolete_archives}) {
> > > +               if ($rawline =~ m{\b$obsolete_archives} &&
> > > +                   $rawline !~ /^\s*cc:/i) {
> >
> > Can we make this (to|cc): instead? Otherwise developers (like me) who
> > use custom scripts to add To: headers to their patches before passing
> > them to checkpatch.pl will also hit this warning if their patch is
> > being sent To: one of these mailing lists.
>
> Why not make it look for "http" instead of the absence of "cc"?

"https" as well, but yes, that would make more sense to me, and would
be less likely to require user workarounds.

Peter

Bjorn Helgaas Nov. 14, 2022, 10:43 p.m. UTC | #9

On Mon, Nov 07, 2022 at 01:00:59PM -0800, Peter Collingbourne wrote:
> On Mon, Nov 7, 2022 at 12:54 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Thu, Nov 03, 2022 at 06:34:31PM -0700, Peter Collingbourne wrote:
> > > On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <joe@perches.com> wrote:
> > > > On Thu, 2022-11-03 at 18:07 -0700, Peter Collingbourne wrote:
> > > > > On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> > > > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > > > >
> > > > > > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > > > > > as lore.kernel.org because they use different styles, add advertising, and
> > > > > > may disappear in the future.  The lore archives are more consistent and
> > > > > > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > > > > > they exist.
> > > > >
> > > > > If the commit message contains a line like:
> > > > >
> > > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > >
> > > > > this patch causes checkpatch.pl to complain. Would it be possible to
> > > > > restrict this to URLs?
> > > >
> > > > Yes, I believe this would probably work well enough:
> > > > ---
> > > >  scripts/checkpatch.pl | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > index 7be93c3df2bcb..fe25642d8bacc 100755
> > > > --- a/scripts/checkpatch.pl
> > > > +++ b/scripts/checkpatch.pl
> > > > @@ -3336,7 +3336,8 @@ sub process {
> > > >                 }
> > > >
> > > >  # Check for mailing list archives other than lore.kernel.org
> > > > -               if ($rawline =~ m{\b$obsolete_archives}) {
> > > > +               if ($rawline =~ m{\b$obsolete_archives} &&
> > > > +                   $rawline !~ /^\s*cc:/i) {
> > >
> > > Can we make this (to|cc): instead? Otherwise developers (like me) who
> > > use custom scripts to add To: headers to their patches before passing
> > > them to checkpatch.pl will also hit this warning if their patch is
> > > being sent To: one of these mailing lists.
> >
> > Why not make it look for "http" instead of the absence of "cc"?
> 
> "https" as well, but yes, that would make more sense to me, and would
> be less likely to require user workarounds.

Maybe like this?  (On top of my previous attempt, which is in -next)


commit d15f85247948 ("checkpatch: warn only for URLs to non-lore archives")
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Mon Nov 14 16:33:12 2022 -0600

    checkpatch: warn only for URLs to non-lore archives
    
    Previously we warned for anything that contained the archive hostname, but
    some email addresses also contain those hostnames, and we'd rather not warn
    about those.  Only warn if we see "http" before the archive hostname.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    ---
    
    Sample commit for testing with "checkpatch -g":
    
      5e91e57e6809 Cc: linux-arm-kernel@lists.infradead.org

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1c3d13e65c2d..78cc595b98ce 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3336,7 +3336,7 @@ sub process {
 		}
 
 # Check for mailing list archives other than lore.kernel.org
-		if ($rawline =~ m{\b$obsolete_archives}) {
+		if ($rawline =~ m{http.*\b$obsolete_archives}) {
 			WARN("PREFER_LORE_ARCHIVE",
 			     "Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr);
 		}

Peter Collingbourne Nov. 14, 2022, 10:50 p.m. UTC | #10

On Mon, Nov 14, 2022 at 2:43 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Nov 07, 2022 at 01:00:59PM -0800, Peter Collingbourne wrote:
> > On Mon, Nov 7, 2022 at 12:54 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > >
> > > On Thu, Nov 03, 2022 at 06:34:31PM -0700, Peter Collingbourne wrote:
> > > > On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <joe@perches.com> wrote:
> > > > > On Thu, 2022-11-03 at 18:07 -0700, Peter Collingbourne wrote:
> > > > > > On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> > > > > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > > > > >
> > > > > > > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > > > > > > as lore.kernel.org because they use different styles, add advertising, and
> > > > > > > may disappear in the future.  The lore archives are more consistent and
> > > > > > > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > > > > > > they exist.
> > > > > >
> > > > > > If the commit message contains a line like:
> > > > > >
> > > > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > > >
> > > > > > this patch causes checkpatch.pl to complain. Would it be possible to
> > > > > > restrict this to URLs?
> > > > >
> > > > > Yes, I believe this would probably work well enough:
> > > > > ---
> > > > >  scripts/checkpatch.pl | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > > index 7be93c3df2bcb..fe25642d8bacc 100755
> > > > > --- a/scripts/checkpatch.pl
> > > > > +++ b/scripts/checkpatch.pl
> > > > > @@ -3336,7 +3336,8 @@ sub process {
> > > > >                 }
> > > > >
> > > > >  # Check for mailing list archives other than lore.kernel.org
> > > > > -               if ($rawline =~ m{\b$obsolete_archives}) {
> > > > > +               if ($rawline =~ m{\b$obsolete_archives} &&
> > > > > +                   $rawline !~ /^\s*cc:/i) {
> > > >
> > > > Can we make this (to|cc): instead? Otherwise developers (like me) who
> > > > use custom scripts to add To: headers to their patches before passing
> > > > them to checkpatch.pl will also hit this warning if their patch is
> > > > being sent To: one of these mailing lists.
> > >
> > > Why not make it look for "http" instead of the absence of "cc"?
> >
> > "https" as well, but yes, that would make more sense to me, and would
> > be less likely to require user workarounds.
>
> Maybe like this?  (On top of my previous attempt, which is in -next)
>
>
> commit d15f85247948 ("checkpatch: warn only for URLs to non-lore archives")
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Mon Nov 14 16:33:12 2022 -0600
>
>     checkpatch: warn only for URLs to non-lore archives
>
>     Previously we warned for anything that contained the archive hostname, but
>     some email addresses also contain those hostnames, and we'd rather not warn
>     about those.  Only warn if we see "http" before the archive hostname.
>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Reviewed-by: Peter Collingbourne <pcc@google.com>

Peter

diff mbox series

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1e5e66ae5a52..4e187202e77a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -702,6 +702,17 @@  sub find_standard_signature {
 	return "";
 }
 
+our $obsolete_archives = qr{(?xi:
+	\Qfreedesktop.org/archives/dri-devel\E |
+	\Qlists.infradead.org\E |
+	\Qlkml.org\E |
+	\Qmail-archive.com\E |
+	\Qmailman.alsa-project.org/pipermail\E |
+	\Qmarc.info\E |
+	\Qozlabs.org/pipermail\E |
+	\Qspinics.net\E
+)};
+
 our @typeListMisordered = (
 	qr{char\s+(?:un)?signed},
 	qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -3324,6 +3335,12 @@  sub process {
 			$last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);
 		}
 
+# Check for mailing list archives other than lore.kernel.org
+		if ($rawline =~ m{\b$obsolete_archives}) {
+			WARN("PREFER_LORE_ARCHIVE",
+			     "Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr);
+		}
+
 # Check for added, moved or deleted files
 		if (!$reported_maintainer_file && !$in_commit_log &&
 		    ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ ||