Add SPDX license tag check based on the rules defined in
Documentation/process/license-rules.rst. To summarize, SPDX license tags
should be on the 1st line (or 2nd line in scripts) using the appropriate
comment style for the file type.
Cc: Andy Whitcroft <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Philippe Ombredanne <[email protected]>
Cc: Andrew Morton <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
I didn't get around to resending once license-rules.rst landed in -next.
Hopefully, this can be picked up for 4.16 so folks can start using it.
SPDX tags have already become a frequent review comment.
Rob
scripts/checkpatch.pl | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ba03f17ff662..cf1b5a90b20a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2225,6 +2225,8 @@ sub process {
my $camelcase_file_seeded = 0;
+ my $checklicenseline = 1;
+
sanitise_line_reset();
my $line;
foreach my $rawline (@rawlines) {
@@ -2416,6 +2418,7 @@ sub process {
} else {
$check = $check_orig;
}
+ $checklicenseline = 1;
next;
}
@@ -2866,6 +2869,30 @@ sub process {
}
}
+# check for using SPDX license tag at beginning of files
+ if ($realline == $checklicenseline) {
+ if ($realfile =~ /\.(?:sh|pl|py)/ && $rawline =~ /\[ \+]\s*\!\#/) {
+ $checklicenseline = 2;
+ } elsif ($rawline =~ /^\+/) {
+ my $comment = "";
+ if ($realfile =~ /\.(h|s|S)$/) {
+ $comment = '/*';
+ } elsif ($realfile =~ /\.(c|dts|dtsi)$/) {
+ $comment = '//';
+ } elsif ($realfile =~ /\.(sh|pl|py)$/) {
+ $comment = '#';
+ } elsif ($realfile =~ /\.rst$/) {
+ $comment = '..';
+ }
+
+ if ($comment !~ /^$/ &&
+ $rawline !~ /^\+\Q$comment\E SPDX-License-Identifier: /) {
+ WARN("SPDX_LICENSE_TAG",
+ "Missing or malformed SPDX-License-Identifier tag in 1st (or 2nd for scripts) line\n" . $herecurr);
+ }
+ }
+ }
+
# check we are in a valid source file if not then ignore this hunk
next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/);
--
2.14.1
On Thu, 2018-02-01 at 15:14 -0600, Rob Herring wrote:
> Add SPDX license tag check based on the rules defined in
> Documentation/process/license-rules.rst. To summarize, SPDX license tags
> should be on the 1st line (or 2nd line in scripts) using the appropriate
> comment style for the file type.
>
> Cc: Andy Whitcroft <[email protected]>
> Cc: Joe Perches <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Philippe Ombredanne <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
> ---
> I didn't get around to resending once license-rules.rst landed in -next.
> Hopefully, this can be picked up for 4.16 so folks can start using it.
> SPDX tags have already become a frequent review comment.
Seems sensible enough now.
Here are some other suggestions.
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2866,6 +2869,30 @@ sub process {
> }
> }
>
> +# check for using SPDX license tag at beginning of files
> + if ($realline == $checklicenseline) {
> + if ($realfile =~ /\.(?:sh|pl|py)/ && $rawline =~ /\[ \+]\s*\!\#/) {
There are many files with a #! shebang that do
not use these filename types.
$ git grep -P --name-only '^\s*\#\!\s*/(?:bin|usr)' | \
grep -vP
"(?:txt|rst|py|sh|pl)$" | wc -l
158
i.e.: .tc and .awk files and ~100 files without extensions
So I would add awk and tc to the $realfile test and
perhaps extend this check to test if the file is not
binary and executable.
> + $checklicenseline = 2;
> + } elsif ($rawline =~ /^\+/) {
> + my $comment = "";
> + if ($realfile =~ /\.(h|s|S)$/) {
> + $comment = '/*';
> + } elsif ($realfile =~ /\.(c|dts|dtsi)$/) {
> + $comment = '//';
> + } elsif ($realfile =~ /\.(sh|pl|py)$/) {
> + $comment = '#';
> + } elsif ($realfile =~ /\.rst$/) {
> + $comment = '..';
> + }
> +
> + if ($comment !~ /^$/ &&
> + $rawline !~ /^\+\Q$comment\E SPDX-License-Identifier: /) {
> + WARN("SPDX_LICENSE_TAG",
> + "Missing or malformed SPDX-License-Identifier tag in 1st (or 2nd for scripts) line\n" . $herecurr);
Perhaps 'Missing ... in line $checklicense\n"
On Thu, Feb 01, 2018 at 03:14:29PM -0600, Rob Herring wrote:
> Add SPDX license tag check based on the rules defined in
> Documentation/process/license-rules.rst. To summarize, SPDX license tags
> should be on the 1st line (or 2nd line in scripts) using the appropriate
> comment style for the file type.
>
> Cc: Andy Whitcroft <[email protected]>
> Cc: Joe Perches <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Philippe Ombredanne <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
On Thu, Feb 1, 2018 at 10:14 PM, Rob Herring <[email protected]> wrote:
> Add SPDX license tag check based on the rules defined in
> Documentation/process/license-rules.rst. To summarize, SPDX license tags
> should be on the 1st line (or 2nd line in scripts) using the appropriate
> comment style for the file type.
>
> Cc: Andy Whitcroft <[email protected]>
> Cc: Joe Perches <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Philippe Ombredanne <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
Acked-by: Philippe Ombredanne <[email protected]>
(Sorry for the late reply but I was busy with FOSDEM)
On Thu, 2017-11-09 at 12:12 -0600, Rob Herring wrote:
> On Thu, Nov 9, 2017 at 9:39 AM, Joe Perches <[email protected]> wrote:
> > On Thu, 2017-11-09 at 07:47 -0600, Rob Herring wrote:
> > > On Wed, Nov 8, 2017 at 8:10 PM, Joe Perches <[email protected]> wrote:
> > > > On Wed, 2017-11-08 at 19:10 -0600, Rob Herring wrote:
> > > > > Add a check warning if SPDX-License-Identifier tags are not used in
> > > > > newly added files.
> > > >
> > > > If this is to be done, and I think it's not a great idea,
> > >
> > > Which part? SPDX tags or checking new files or just using checkpatch for this?
> >
> > SPDX tags in all files.
Is having an SPDX tag in every file really desired?
> >
> > There's no real way to check a patch for this.
> >
> > You have to check the entire file.
>
> Changing existing files is a separate problem. There is a script for
> that (though the data file is not public). I'm only worried with new
> files here because that's what I review and have to tell folks to
> replace their 2 pages of license text with SPDX tags. (It will be much
> easier to just tell them to run checkpatch. ;) ).
>
> > checkpatch could, as you've done, scan for new files
> > against /dev/null, but a single patch can add
> > multiple files and each newly added file should have
> > a missing SPDX indicator check.
>
> I was going with the easy route of just giving one warning per patch.
> I'd hope that's enough info for folks to figure out what's needed from
> there. However, it should be possible to make it per file. The main
> complication is we need to look for either '^+++' or the end of the
> patch which I didn't see an easy/clean way to do.
EOF is easy.
There already is a $realfile test for start of file.
> > My concern is that there are ~50,000 files in the
> > kernel source tree and, after that scripted patch
> > adding the tags, only about a quarter of them have
> > an SPDX tag.
> >
> > So which files actually _need_ a SPDX tag?
> >
> > files in -next with an SPDX tag:
> >
> > $ git grep --name-only -i -P "spdx-licen[cs]e-identifier" | \
> > while read file ; do basename $file ; done | \
> > sed -r -e 's/^.*(\..*)/\1/' | \
> > sort | uniq -c | sort -rn | head -10
> > 7514 .h
> > 3435 .c
> > 1193 Makefile
> > 486 .S
> > 221 .dts
> > 186 Kconfig
> > 185 .dtsi
> > 97 .sh
> > 34 .tc
> > 24 .debug
> >
> > vs all files in -next (not Documentation/)
> >
> > $ git ls-files | grep -v "^Documentation/" | \
> > while read file ; do basename $file ; done | \
> > sed -r -e 's/^.*(\..*)/\1/' | \
> > sort | uniq -c | sort -rn | head -10
> > 25946 .c
> > 20360 .h
> > 2437 Makefile
> > 1454 .S
> > 1442 .dts
> > 1380 Kconfig
> > 1099 .dtsi
> > 207 .json
> > 204 .gitignore
> > 194 .sh
> >
From 1583613182607708001@xxx Thu Nov 09 18:13:39 +0000 2017
X-GM-THRID: 1583548959067315589
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Thu, Nov 9, 2017 at 9:39 AM, Joe Perches <[email protected]> wrote:
> On Thu, 2017-11-09 at 07:47 -0600, Rob Herring wrote:
>> On Wed, Nov 8, 2017 at 8:10 PM, Joe Perches <[email protected]> wrote:
>> > On Wed, 2017-11-08 at 19:10 -0600, Rob Herring wrote:
>> > > Add a check warning if SPDX-License-Identifier tags are not used in
>> > > newly added files.
>> >
>> > If this is to be done, and I think it's not a great idea,
>>
>> Which part? SPDX tags or checking new files or just using checkpatch for this?
>
> SPDX tags in all files.
>
> There's no real way to check a patch for this.
>
> You have to check the entire file.
Changing existing files is a separate problem. There is a script for
that (though the data file is not public). I'm only worried with new
files here because that's what I review and have to tell folks to
replace their 2 pages of license text with SPDX tags. (It will be much
easier to just tell them to run checkpatch. ;) ).
> checkpatch could, as you've done, scan for new files
> against /dev/null, but a single patch can add
> multiple files and each newly added file should have
> a missing SPDX indicator check.
I was going with the easy route of just giving one warning per patch.
I'd hope that's enough info for folks to figure out what's needed from
there. However, it should be possible to make it per file. The main
complication is we need to look for either '^+++' or the end of the
patch which I didn't see an easy/clean way to do.
> My concern is that there are ~50,000 files in the
> kernel source tree and, after that scripted patch
> adding the tags, only about a quarter of them have
> an SPDX tag.
>
> So which files actually _need_ a SPDX tag?
>
> files in -next with an SPDX tag:
>
> $ git grep --name-only -i -P "spdx-licen[cs]e-identifier" | \
> while read file ; do basename $file ; done | \
> sed -r -e 's/^.*(\..*)/\1/' | \
> sort | uniq -c | sort -rn | head -10
> 7514 .h
> 3435 .c
> 1193 Makefile
> 486 .S
> 221 .dts
> 186 Kconfig
> 185 .dtsi
> 97 .sh
> 34 .tc
> 24 .debug
>
> vs all files in -next (not Documentation/)
>
> $ git ls-files | grep -v "^Documentation/" | \
> while read file ; do basename $file ; done | \
> sed -r -e 's/^.*(\..*)/\1/' | \
> sort | uniq -c | sort -rn | head -10
> 25946 .c
> 20360 .h
> 2437 Makefile
> 1454 .S
> 1442 .dts
> 1380 Kconfig
> 1099 .dtsi
> 207 .json
> 204 .gitignore
> 194 .sh
>
From 1583603546401276702@xxx Thu Nov 09 15:40:29 +0000 2017
X-GM-THRID: 1583548959067315589
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Thu, 2017-11-09 at 07:47 -0600, Rob Herring wrote:
> On Wed, Nov 8, 2017 at 8:10 PM, Joe Perches <[email protected]> wrote:
> > On Wed, 2017-11-08 at 19:10 -0600, Rob Herring wrote:
> > > Add a check warning if SPDX-License-Identifier tags are not used in
> > > newly added files.
> >
> > If this is to be done, and I think it's not a great idea,
>
> Which part? SPDX tags or checking new files or just using checkpatch for this?
SPDX tags in all files.
There's no real way to check a patch for this.
You have to check the entire file.
checkpatch could, as you've done, scan for new files
against /dev/null, but a single patch can add
multiple files and each newly added file should have
a missing SPDX indicator check.
My concern is that there are ~50,000 files in the
kernel source tree and, after that scripted patch
adding the tags, only about a quarter of them have
an SPDX tag.
So which files actually _need_ a SPDX tag?
files in -next with an SPDX tag:
$ git grep --name-only -i -P "spdx-licen[cs]e-identifier" | \
while read file ; do basename $file ; done | \
sed -r -e 's/^.*(\..*)/\1/' | \
sort | uniq -c | sort -rn | head -10
7514 .h
3435 .c
1193 Makefile
486 .S
221 .dts
186 Kconfig
185 .dtsi
97 .sh
34 .tc
24 .debug
vs all files in -next (not Documentation/)
$ git ls-files | grep -v "^Documentation/" | \
while read file ; do basename $file ; done | \
sed -r -e 's/^.*(\..*)/\1/' | \
sort | uniq -c | sort -rn | head -10
25946 .c
20360 .h
2437 Makefile
1454 .S
1442 .dts
1380 Kconfig
1099 .dtsi
207 .json
204 .gitignore
194 .sh
From 1583596985820144410@xxx Thu Nov 09 13:56:12 +0000 2017
X-GM-THRID: 1583548959067315589
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Thu, 2017-11-09 at 10:12 +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 08, 2017 at 06:10:19PM -0800, Joe Perches wrote:
> > On Wed, 2017-11-08 at 19:10 -0600, Rob Herring wrote:
> > > Add a check warning if SPDX-License-Identifier tags are not used in
> > > newly added files.
> >
> > If this is to be done, and I think it's not a great idea,
> > there are better ways of doing this that emit this warning
> > on a per-file basis instead of a per-patch.
>
> Any hints as to how to do that? :)
Sure, but only after it's decided if adding spdx license tag
to all compilable source files is appropriate.
And if that's true, then that requirement also has to be
added in the Documentation directory somewhere.
From 1583596555946531167@xxx Thu Nov 09 13:49:22 +0000 2017
X-GM-THRID: 1583548959067315589
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Wed, Nov 8, 2017 at 8:10 PM, Joe Perches <[email protected]> wrote:
> On Wed, 2017-11-08 at 19:10 -0600, Rob Herring wrote:
>> Add a check warning if SPDX-License-Identifier tags are not used in
>> newly added files.
>
> If this is to be done, and I think it's not a great idea,
Which part? SPDX tags or checking new files or just using checkpatch for this?
> there are better ways of doing this that emit this warning
> on a per-file basis instead of a per-patch.
You had mentioned using something like checkincludes.pl before. The
problem I see with that is few people run those tools. Lots of people
run checkpatch.pl. We want this to be correct when added, not after
the fact by someone else who is not the author. It fits in the
workflow, because if checkpatch doesn't catch it, then I have to in
reviews.
I do agree though that the implementation is a bit ugly given the line
by line way checkpatch works.
Rob
From 1583579317864234972@xxx Thu Nov 09 09:15:23 +0000 2017
X-GM-THRID: 1583548959067315589
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Wed, Nov 08, 2017 at 06:10:19PM -0800, Joe Perches wrote:
> On Wed, 2017-11-08 at 19:10 -0600, Rob Herring wrote:
> > Add a check warning if SPDX-License-Identifier tags are not used in
> > newly added files.
>
> If this is to be done, and I think it's not a great idea,
> there are better ways of doing this that emit this warning
> on a per-file basis instead of a per-patch.
Any hints as to how to do that? :)
thanks,
greg k-h
From 1583552632222023499@xxx Thu Nov 09 02:11:14 +0000 2017
X-GM-THRID: 1583548959067315589
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Wed, 2017-11-08 at 19:10 -0600, Rob Herring wrote:
> Add a check warning if SPDX-License-Identifier tags are not used in
> newly added files.
If this is to be done, and I think it's not a great idea,
there are better ways of doing this that emit this warning
on a per-file basis instead of a per-patch.
From 1583548959067315589@xxx Thu Nov 09 01:12:51 +0000 2017
X-GM-THRID: 1583548959067315589
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread