All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Alcock <nick.alcock@oracle.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: mcgrof@kernel.org, linux-modules@vger.kernel.org,
	linux-kernel@vger.kernel.org, Rob Herring <robh@kernel.org>,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/8] kbuild, PCI: generic,versatile: comment out MODULE_LICENSE in non-modules
Date: Tue, 14 Feb 2023 15:41:32 +0000	[thread overview]
Message-ID: <873578xnir.fsf@esperi.org.uk> (raw)
In-Reply-To: <20230213225723.GA2941414@bhelgaas> (Bjorn Helgaas's message of "Mon, 13 Feb 2023 16:57:23 -0600")

On 13 Feb 2023, Bjorn Helgaas spake thusly:

> On Fri, Feb 10, 2023 at 04:47:42PM +0000, Nick Alcock wrote:
>> Since commit 8b41fc4454e ("kbuild: create modules.builtin without
>> Makefile.modbuiltin or tristate.conf"), MODULE_LICENSE declarations
>> are used to identify modules. As a consequence, uses of the macro
>> in non-modules will cause modprobe to misidentify their containing
>> object file as a module when it is not (false positives), and modprobe
>> might succeed rather than failing with a suitable error message.
>> 
>> So comment out all uses of MODULE_LICENSE that are not in real modules
>> (the license declaration is left in as documentation).
>
> Weird that all the patches are for drivers/pci/, but the cover letter
> didn't go to linux-pci@vger.kernel.org.
>
>   - Please drop "kbuild," from the subject; I don't think it's really
>     relevant.

Every other treewide kbuild modification that I can see has a kbuild
prefix. Many of them have *nothing* else, just kbuild:, no per-subsystem
descriptor at all. If I remove the kbuild: prefix from all these
commits, will other people promptly complain? Luis?

>   - Please follow the subject line convention for each file.  They're
>     mostly there after dropping "kbuild", but do capitalize the
>     sentence that follows the prefix.  The prefix should always be
>     "PCI/<driver-tag>: "

These prefixes were automatically generated by a splitting script which
computes a prefix to apply to each commit's log by reusing the most
commonly used prefix which is present at least once in all affected
files in the subsystem (in this case, that's simple because there's just
one file).

(However, there appears to be a bug in the script here: "PCI:
generic,versatile" appears only once. I would expect "PCI: versatile",
which is used repeatedly in the history of that file. Will fix.)

I guess the convention you refer to is new, because there are a total of
zero uses of "PCI/versatile" in the entirety of git history. Are you
*sure* you want me to use that?


(More generally, I have no idea where these "driver tags" come from,
hence the weird ad-hoc approach I had to use to generate commit log
first-line prefixes when splitting this commit by subsystem. There seems
to be very little consistency here. They certainly don't come from
MAINTAINERS or from the files themselves, nor are they the filename, at
least not always. Could you give me some sort of procedure for
generating them, if picking popular prefixes from git history won't cut
it? It's essential that the process be automatable: I've had to resplit
this commit more than half a dozen times already and if I have to label
each commit by hand every time it will become a nightmare of human
error. If the rules for generating prefixes vary by subsystem this means
I'll have to fight through God knows how many annoyed maintainers to get
this incredibly trivial change in.)

>   - AFAICT, SPDX is the dispositive license and MODULE_LICENSE just
>     determines which interfaces are available to the module, so
>     dropping MODULE_LICENSE shouldn't be a problem as far as legal
>     issues.

Yeah, looks like. I'll just drop them, everyone seems to want that. If
someone dislikes my doing that they can always put a commented-out
MODULE_LICENSE back.

-- 
NULL && (void)

WARNING: multiple messages have this Message-ID (diff)
From: Nick Alcock <nick.alcock@oracle.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: mcgrof@kernel.org, linux-modules@vger.kernel.org,
	linux-kernel@vger.kernel.org, Rob Herring <robh@kernel.org>,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/8] kbuild, PCI: generic,versatile: comment out MODULE_LICENSE in non-modules
Date: Tue, 14 Feb 2023 15:41:32 +0000	[thread overview]
Message-ID: <873578xnir.fsf@esperi.org.uk> (raw)
In-Reply-To: <20230213225723.GA2941414@bhelgaas> (Bjorn Helgaas's message of "Mon, 13 Feb 2023 16:57:23 -0600")

On 13 Feb 2023, Bjorn Helgaas spake thusly:

> On Fri, Feb 10, 2023 at 04:47:42PM +0000, Nick Alcock wrote:
>> Since commit 8b41fc4454e ("kbuild: create modules.builtin without
>> Makefile.modbuiltin or tristate.conf"), MODULE_LICENSE declarations
>> are used to identify modules. As a consequence, uses of the macro
>> in non-modules will cause modprobe to misidentify their containing
>> object file as a module when it is not (false positives), and modprobe
>> might succeed rather than failing with a suitable error message.
>> 
>> So comment out all uses of MODULE_LICENSE that are not in real modules
>> (the license declaration is left in as documentation).
>
> Weird that all the patches are for drivers/pci/, but the cover letter
> didn't go to linux-pci@vger.kernel.org.
>
>   - Please drop "kbuild," from the subject; I don't think it's really
>     relevant.

Every other treewide kbuild modification that I can see has a kbuild
prefix. Many of them have *nothing* else, just kbuild:, no per-subsystem
descriptor at all. If I remove the kbuild: prefix from all these
commits, will other people promptly complain? Luis?

>   - Please follow the subject line convention for each file.  They're
>     mostly there after dropping "kbuild", but do capitalize the
>     sentence that follows the prefix.  The prefix should always be
>     "PCI/<driver-tag>: "

These prefixes were automatically generated by a splitting script which
computes a prefix to apply to each commit's log by reusing the most
commonly used prefix which is present at least once in all affected
files in the subsystem (in this case, that's simple because there's just
one file).

(However, there appears to be a bug in the script here: "PCI:
generic,versatile" appears only once. I would expect "PCI: versatile",
which is used repeatedly in the history of that file. Will fix.)

I guess the convention you refer to is new, because there are a total of
zero uses of "PCI/versatile" in the entirety of git history. Are you
*sure* you want me to use that?


(More generally, I have no idea where these "driver tags" come from,
hence the weird ad-hoc approach I had to use to generate commit log
first-line prefixes when splitting this commit by subsystem. There seems
to be very little consistency here. They certainly don't come from
MAINTAINERS or from the files themselves, nor are they the filename, at
least not always. Could you give me some sort of procedure for
generating them, if picking popular prefixes from git history won't cut
it? It's essential that the process be automatable: I've had to resplit
this commit more than half a dozen times already and if I have to label
each commit by hand every time it will become a nightmare of human
error. If the rules for generating prefixes vary by subsystem this means
I'll have to fight through God knows how many annoyed maintainers to get
this incredibly trivial change in.)

>   - AFAICT, SPDX is the dispositive license and MODULE_LICENSE just
>     determines which interfaces are available to the module, so
>     dropping MODULE_LICENSE shouldn't be a problem as far as legal
>     issues.

Yeah, looks like. I'll just drop them, everyone seems to want that. If
someone dislikes my doing that they can always put a commented-out
MODULE_LICENSE back.

-- 
NULL && (void)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-02-14 15:41 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-10 16:47 [PATCH 0/8] MODULE_LICENSE removals, first tranche Nick Alcock
2023-02-10 16:47 ` [PATCH 1/8] kbuild, PCI: generic,versatile: comment out MODULE_LICENSE in non-modules Nick Alcock
2023-02-10 16:47   ` Nick Alcock
2023-02-10 17:36   ` Rob Herring
2023-02-10 17:36     ` Rob Herring
2023-02-10 18:43     ` Nick Alcock
2023-02-10 18:43       ` Nick Alcock
2023-02-13 22:57   ` Bjorn Helgaas
2023-02-13 22:57     ` Bjorn Helgaas
2023-02-14 15:41     ` Nick Alcock [this message]
2023-02-14 15:41       ` Nick Alcock
2023-02-14 17:20       ` Bjorn Helgaas
2023-02-14 17:20         ` Bjorn Helgaas
2023-02-16 13:34         ` Nick Alcock
2023-02-16 13:34           ` Nick Alcock
2023-02-10 16:47 ` [PATCH 2/8] kbuild, PCI: mobiveil: " Nick Alcock
2023-02-10 16:47 ` [PATCH 3/8] kbuild, ARM: tegra: " Nick Alcock
2023-02-10 16:47 ` [PATCH 4/8] kbuild, PCI: endpoint: " Nick Alcock
2023-02-10 16:47 ` [PATCH 5/8] kbuild, PCI: hip: " Nick Alcock
2023-02-10 16:47 ` [PATCH 6/8] kbuild, shpchp: " Nick Alcock
2023-02-10 16:47 ` [PATCH 7/8] kbuild, PCI: dwc: histb: " Nick Alcock
2023-02-10 16:47 ` [PATCH 8/8] kbuild, PCI: microchip: " Nick Alcock
2023-02-10 16:47   ` Nick Alcock
2023-02-10 18:27   ` Conor Dooley
2023-02-10 18:27     ` Conor Dooley
2023-02-10 19:26     ` Nick Alcock
2023-02-10 19:26       ` Nick Alcock
2023-02-10 20:10       ` Conor Dooley
2023-02-10 20:10         ` Conor Dooley
2023-02-12 18:37         ` Leon Romanovsky
2023-02-12 18:37           ` Leon Romanovsky
2023-02-12 19:52           ` Nick Alcock
2023-02-12 19:52             ` Nick Alcock
2023-02-13 15:53           ` Nick Alcock
2023-02-13 15:53             ` Nick Alcock
2023-02-13 16:13           ` Nick Alcock
2023-02-13 16:13             ` Nick Alcock
2023-02-13 16:51             ` Conor Dooley
2023-02-13 16:51               ` Conor Dooley
2023-02-13 17:06             ` Leon Romanovsky
2023-02-13 17:06               ` Leon Romanovsky
2023-02-15 19:06               ` Nick Alcock
2023-02-15 19:06                 ` Nick Alcock
2023-02-13 17:30           ` Jonathan Corbet
2023-02-13 17:30             ` Jonathan Corbet
2023-02-13 19:23             ` Leon Romanovsky
2023-02-13 19:23               ` Leon Romanovsky
2023-02-16 12:05               ` Nick Alcock
2023-02-16 12:05                 ` Nick Alcock

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=873578xnir.fsf@esperi.org.uk \
    --to=nick.alcock@oracle.com \
    --cc=helgaas@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.