Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Philippe Ombredanne]

On Thu, Feb 1, 2018 at 10:14 PM, 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 
> Cc: Joe Perches 
> Cc: Greg Kroah-Hartman 
> Cc: Thomas Gleixner 
> Cc: Philippe Ombredanne 
> Cc: Andrew Morton 
> Signed-off-by: Rob Herring 

Acked-by:  Philippe Ombredanne 

(Sorry for the late reply but I was busy with FOSDEM)

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Philippe Ombredanne]

On Thu, Feb 1, 2018 at 10:14 PM, 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 
> Cc: Joe Perches 
> Cc: Greg Kroah-Hartman 
> Cc: Thomas Gleixner 
> Cc: Philippe Ombredanne 
> Cc: Andrew Morton 
> Signed-off-by: Rob Herring 

Acked-by:  Philippe Ombredanne 

(Sorry for the late reply but I was busy with FOSDEM)

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Greg Kroah-Hartman]

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 
> Cc: Joe Perches 
> Cc: Greg Kroah-Hartman 
> Cc: Thomas Gleixner 
> Cc: Philippe Ombredanne 
> Cc: Andrew Morton 
> Signed-off-by: Rob Herring 

Acked-by: Greg Kroah-Hartman 

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Greg Kroah-Hartman]

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 
> Cc: Joe Perches 
> Cc: Greg Kroah-Hartman 
> Cc: Thomas Gleixner 
> Cc: Philippe Ombredanne 
> Cc: Andrew Morton 
> Signed-off-by: Rob Herring 

Acked-by: Greg Kroah-Hartman 

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

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 
> Cc: Joe Perches 
> Cc: Greg Kroah-Hartman 
> Cc: Thomas Gleixner 
> Cc: Philippe Ombredanne 
> Cc: Andrew Morton 
> Signed-off-by: Rob Herring 
> ---
> 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"

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

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 
> Cc: Joe Perches 
> Cc: Greg Kroah-Hartman 
> Cc: Thomas Gleixner 
> Cc: Philippe Ombredanne 
> Cc: Andrew Morton 
> Signed-off-by: Rob Herring 
> ---
> 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"

[PATCH] checkpatch.pl: Add SPDX license tag check

[Rob Herring]

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 
Cc: Joe Perches 
Cc: Greg Kroah-Hartman 
Cc: Thomas Gleixner 
Cc: Philippe Ombredanne 
Cc: Andrew Morton 
Signed-off-by: Rob Herring 
---
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

[PATCH] checkpatch.pl: Add SPDX license tag check

[Rob Herring]

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 
Cc: Joe Perches 
Cc: Greg Kroah-Hartman 
Cc: Thomas Gleixner 
Cc: Philippe Ombredanne 
Cc: Andrew Morton 
Signed-off-by: Rob Herring 
---
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

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

On Thu, 2017-11-09 at 12:12 -0600, Rob Herring wrote:
> On Thu, Nov 9, 2017 at 9:39 AM, Joe Perches  wrote:
> > On Thu, 2017-11-09 at 07:47 -0600, Rob Herring wrote:
> > > On Wed, Nov 8, 2017 at 8:10 PM, 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,
> > > 
> > > 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
> > 

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

On Thu, 2017-11-09 at 12:12 -0600, Rob Herring wrote:
> On Thu, Nov 9, 2017 at 9:39 AM, Joe Perches  wrote:
> > On Thu, 2017-11-09 at 07:47 -0600, Rob Herring wrote:
> > > On Wed, Nov 8, 2017 at 8:10 PM, 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,
> > > 
> > > 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
> > 

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Rob Herring]

On Thu, Nov 9, 2017 at 9:39 AM, Joe Perches  wrote:
> On Thu, 2017-11-09 at 07:47 -0600, Rob Herring wrote:
>> On Wed, Nov 8, 2017 at 8:10 PM, 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,
>>
>> 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
>

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Rob Herring]

On Thu, Nov 9, 2017 at 9:39 AM, Joe Perches  wrote:
> On Thu, 2017-11-09 at 07:47 -0600, Rob Herring wrote:
>> On Wed, Nov 8, 2017 at 8:10 PM, 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,
>>
>> 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
>

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

On Thu, 2017-11-09 at 07:47 -0600, Rob Herring wrote:
> On Wed, Nov 8, 2017 at 8:10 PM, 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,
> 
> 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

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

On Thu, 2017-11-09 at 07:47 -0600, Rob Herring wrote:
> On Wed, Nov 8, 2017 at 8:10 PM, 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,
> 
> 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

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

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.

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

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.

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Rob Herring]

On Wed, Nov 8, 2017 at 8:10 PM, 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,

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

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Rob Herring]

On Wed, Nov 8, 2017 at 8:10 PM, 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,

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

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Greg Kroah-Hartman]

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

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Greg Kroah-Hartman]

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

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

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.

Re: [PATCH] checkpatch.pl: Add SPDX license tag check

[Joe Perches]

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.

[PATCH] checkpatch.pl: Add SPDX license tag check

[Rob Herring]

Add a check warning if SPDX-License-Identifier tags are not used in
newly added files.

Cc: Andy Whitcroft 
Cc: Joe Perches 
Cc: Greg Kroah-Hartman 
Signed-off-by: Rob Herring 
---
I rewrote my previous version to check more than just dts files. It also 
now looks for a tag in added files rather than trying a fuzzy match on 
freeform license text.

 scripts/checkpatch.pl | 25 +
 1 file changed, 25 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8b80bac055e4..6665735123e5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2185,6 +2185,8 @@ sub process {
my $commit_log_has_diff = 0;
my $reported_maintainer_file = 0;
my $non_utf8_charset = 0;
+   my $added_file = 0;
+   my $missing_spdx_license = 0;
 
my $last_blank_line = 0;
my $last_coalesced_string_linenr = -1;
@@ -2368,6 +2370,16 @@ sub process {
$here = "#$linenr: " if (!$file);
$here = "#$realline: " if ($file);
 
+   # determine if this is a new file
+   if ($line =~ m@^\-\-\-\s/@) {
+   if ($line =~ m@/dev/null@) {
+   $added_file = 1;
+   $missing_spdx_license++;
+   } else {
+   $added_file = 0;
+   }
+   }
+
my $found_file = 0;
# extract the filename as it passes
if ($line =~ /^diff --git.*?(\S+)$/) {
@@ -2865,6 +2877,14 @@ sub process {
}
}
 
+# check for using SPDX tag instead of free form license text
+   if ($added_file &&
+   ($rawline =~ /\bSPDX-License-Identifier/ ||
+$realfile =~ /Documentation/)) {
+   $missing_spdx_license--;
+   $added_file = 0;
+   }
+
 # check we are in a valid source file if not then ignore this hunk
next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/);
 
@@ -6399,6 +6419,11 @@ sub process {
  "Missing Signed-off-by: line(s)\n");
}
 
+   if ($missing_spdx_license) {
+   WARN("SPDX_LICENSE_TAG",
+"Missing SPDX-License-Identifier tags in added files. Use 
tags instead of full license text.\n");
+   }
+
print report_dump();
if ($summary && !($clean == 1 && $quiet == 1)) {
print "$filename " if ($summary_file);
-- 
2.14.1

[PATCH] checkpatch.pl: Add SPDX license tag check

[Rob Herring]

Add a check warning if SPDX-License-Identifier tags are not used in
newly added files.

Cc: Andy Whitcroft 
Cc: Joe Perches 
Cc: Greg Kroah-Hartman 
Signed-off-by: Rob Herring 
---
I rewrote my previous version to check more than just dts files. It also 
now looks for a tag in added files rather than trying a fuzzy match on 
freeform license text.

 scripts/checkpatch.pl | 25 +
 1 file changed, 25 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8b80bac055e4..6665735123e5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2185,6 +2185,8 @@ sub process {
my $commit_log_has_diff = 0;
my $reported_maintainer_file = 0;
my $non_utf8_charset = 0;
+   my $added_file = 0;
+   my $missing_spdx_license = 0;
 
my $last_blank_line = 0;
my $last_coalesced_string_linenr = -1;
@@ -2368,6 +2370,16 @@ sub process {
$here = "#$linenr: " if (!$file);
$here = "#$realline: " if ($file);
 
+   # determine if this is a new file
+   if ($line =~ m@^\-\-\-\s/@) {
+   if ($line =~ m@/dev/null@) {
+   $added_file = 1;
+   $missing_spdx_license++;
+   } else {
+   $added_file = 0;
+   }
+   }
+
my $found_file = 0;
# extract the filename as it passes
if ($line =~ /^diff --git.*?(\S+)$/) {
@@ -2865,6 +2877,14 @@ sub process {
}
}
 
+# check for using SPDX tag instead of free form license text
+   if ($added_file &&
+   ($rawline =~ /\bSPDX-License-Identifier/ ||
+$realfile =~ /Documentation/)) {
+   $missing_spdx_license--;
+   $added_file = 0;
+   }
+
 # check we are in a valid source file if not then ignore this hunk
next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/);
 
@@ -6399,6 +6419,11 @@ sub process {
  "Missing Signed-off-by: line(s)\n");
}
 
+   if ($missing_spdx_license) {
+   WARN("SPDX_LICENSE_TAG",
+"Missing SPDX-License-Identifier tags in added files. Use 
tags instead of full license text.\n");
+   }
+
print report_dump();
if ($summary && !($clean == 1 && $quiet == 1)) {
print "$filename " if ($summary_file);
-- 
2.14.1

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Rob Herring]

On Wed, Jun 07, 2017 at 05:45:03PM +0900, Masahiro Yamada wrote:
> Hi Rob,
> 
> 2017-02-24 0:56 GMT+09:00 Rob Herring :
> > Add a check for using SPDX-License-Identifier tags to define the license of
> > .dts{i} and DT header files rather than using free form license text. This
> > check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
> > license text which are the commonly used DT licenses.
> >
> > Signed-off-by: Rob Herring 
> > Cc: Andy Whitcroft 
> > Cc: Joe Perches 
> 
> I have a question.
> 
> Is SPDX encouraged only for DT?
> 
> What should we do with C source files?

Depends on the maintainer and your lawyer I guess. gregkh is 
supportive[1]. I wouldn't recommend changing existing license text 
though.

Rob

[1] https://lkml.org/lkml/2017/2/15/968

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Rob Herring]

On Wed, Jun 07, 2017 at 05:45:03PM +0900, Masahiro Yamada wrote:
> Hi Rob,
> 
> 2017-02-24 0:56 GMT+09:00 Rob Herring :
> > Add a check for using SPDX-License-Identifier tags to define the license of
> > .dts{i} and DT header files rather than using free form license text. This
> > check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
> > license text which are the commonly used DT licenses.
> >
> > Signed-off-by: Rob Herring 
> > Cc: Andy Whitcroft 
> > Cc: Joe Perches 
> 
> I have a question.
> 
> Is SPDX encouraged only for DT?
> 
> What should we do with C source files?

Depends on the maintainer and your lawyer I guess. gregkh is 
supportive[1]. I wouldn't recommend changing existing license text 
though.

Rob

[1] https://lkml.org/lkml/2017/2/15/968

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Masahiro Yamada]

Hi Rob,

2017-02-24 0:56 GMT+09:00 Rob Herring :
> Add a check for using SPDX-License-Identifier tags to define the license of
> .dts{i} and DT header files rather than using free form license text. This
> check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
> license text which are the commonly used DT licenses.
>
> Signed-off-by: Rob Herring 
> Cc: Andy Whitcroft 
> Cc: Joe Perches 

I have a question.

Is SPDX encouraged only for DT?

What should we do with C source files?


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Masahiro Yamada]

Hi Rob,

2017-02-24 0:56 GMT+09:00 Rob Herring :
> Add a check for using SPDX-License-Identifier tags to define the license of
> .dts{i} and DT header files rather than using free form license text. This
> check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
> license text which are the commonly used DT licenses.
>
> Signed-off-by: Rob Herring 
> Cc: Andy Whitcroft 
> Cc: Joe Perches 

I have a question.

Is SPDX encouraged only for DT?

What should we do with C source files?


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Joe Perches]

On Thu, 2017-02-23 at 12:45 -0600, Rob Herring wrote:
> On Thu, Feb 23, 2017 at 11:51 AM, Joe Perches  wrote:
> > On Thu, 2017-02-23 at 09:56 -0600, Rob Herring wrote:
> > > Add a check for using SPDX-License-Identifier tags to define the license 
> > > of
> > > .dts{i} and DT header files rather than using free form license text. This
> > > check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
> > > license text which are the commonly used DT licenses.
> > 
> > Hi Rob.
> > 
> > > Signed-off-by: Rob Herring 
> > > Cc: Andy Whitcroft 
> > > Cc: Joe Perches 
> > > ---
> > >  scripts/checkpatch.pl | 13 +
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 982c52ca6473..ce802b3146e3 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -2139,6 +2139,7 @@ sub process {
> > >   my $commit_log_has_diff = 0;
> > >   my $reported_maintainer_file = 0;
> > >   my $non_utf8_charset = 0;
> > > + my $licensefile = '';
> > 
> > Maybe this should be $spdx_license_file
> > but what's the actual reason to check if
> > multiple license bits are in a single file?
> 
> Yes, just to get a single warning per file since the license matching
> can get multiple hits.
> 
> Really what I'd like to do is warn if an SPDX tag is not present in
> any added file. Having a check for something missing doesn't really
> work well with checkpatch at least in a scalable way that I saw.

That isn't really possible with checkpatch as
I'm sure you know that it looks at patch
contexts and it isn't really meant to scan
entire files.

You might be better off with a separate script
like the checkincludes.pl one.

You might possibly integrate that script into
checkpatch by looking at the "new file mode"
block, remember those and see if in the patch
each appropriate file has one of the spdx tags.

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Joe Perches]

On Thu, 2017-02-23 at 12:45 -0600, Rob Herring wrote:
> On Thu, Feb 23, 2017 at 11:51 AM, Joe Perches  wrote:
> > On Thu, 2017-02-23 at 09:56 -0600, Rob Herring wrote:
> > > Add a check for using SPDX-License-Identifier tags to define the license 
> > > of
> > > .dts{i} and DT header files rather than using free form license text. This
> > > check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
> > > license text which are the commonly used DT licenses.
> > 
> > Hi Rob.
> > 
> > > Signed-off-by: Rob Herring 
> > > Cc: Andy Whitcroft 
> > > Cc: Joe Perches 
> > > ---
> > >  scripts/checkpatch.pl | 13 +
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 982c52ca6473..ce802b3146e3 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -2139,6 +2139,7 @@ sub process {
> > >   my $commit_log_has_diff = 0;
> > >   my $reported_maintainer_file = 0;
> > >   my $non_utf8_charset = 0;
> > > + my $licensefile = '';
> > 
> > Maybe this should be $spdx_license_file
> > but what's the actual reason to check if
> > multiple license bits are in a single file?
> 
> Yes, just to get a single warning per file since the license matching
> can get multiple hits.
> 
> Really what I'd like to do is warn if an SPDX tag is not present in
> any added file. Having a check for something missing doesn't really
> work well with checkpatch at least in a scalable way that I saw.

That isn't really possible with checkpatch as
I'm sure you know that it looks at patch
contexts and it isn't really meant to scan
entire files.

You might be better off with a separate script
like the checkincludes.pl one.

You might possibly integrate that script into
checkpatch by looking at the "new file mode"
block, remember those and see if in the patch
each appropriate file has one of the spdx tags.

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Rob Herring]

On Thu, Feb 23, 2017 at 11:51 AM, Joe Perches  wrote:
> On Thu, 2017-02-23 at 09:56 -0600, Rob Herring wrote:
>> Add a check for using SPDX-License-Identifier tags to define the license of
>> .dts{i} and DT header files rather than using free form license text. This
>> check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
>> license text which are the commonly used DT licenses.
>
> Hi Rob.
>
>> Signed-off-by: Rob Herring 
>> Cc: Andy Whitcroft 
>> Cc: Joe Perches 
>> ---
>>  scripts/checkpatch.pl | 13 +
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 982c52ca6473..ce802b3146e3 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -2139,6 +2139,7 @@ sub process {
>>   my $commit_log_has_diff = 0;
>>   my $reported_maintainer_file = 0;
>>   my $non_utf8_charset = 0;
>> + my $licensefile = '';
>
> Maybe this should be $spdx_license_file
> but what's the actual reason to check if
> multiple license bits are in a single file?

Yes, just to get a single warning per file since the license matching
can get multiple hits.

Really what I'd like to do is warn if an SPDX tag is not present in
any added file. Having a check for something missing doesn't really
work well with checkpatch at least in a scalable way that I saw.

Rob

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Rob Herring]

On Thu, Feb 23, 2017 at 11:51 AM, Joe Perches  wrote:
> On Thu, 2017-02-23 at 09:56 -0600, Rob Herring wrote:
>> Add a check for using SPDX-License-Identifier tags to define the license of
>> .dts{i} and DT header files rather than using free form license text. This
>> check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
>> license text which are the commonly used DT licenses.
>
> Hi Rob.
>
>> Signed-off-by: Rob Herring 
>> Cc: Andy Whitcroft 
>> Cc: Joe Perches 
>> ---
>>  scripts/checkpatch.pl | 13 +
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 982c52ca6473..ce802b3146e3 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -2139,6 +2139,7 @@ sub process {
>>   my $commit_log_has_diff = 0;
>>   my $reported_maintainer_file = 0;
>>   my $non_utf8_charset = 0;
>> + my $licensefile = '';
>
> Maybe this should be $spdx_license_file
> but what's the actual reason to check if
> multiple license bits are in a single file?

Yes, just to get a single warning per file since the license matching
can get multiple hits.

Really what I'd like to do is warn if an SPDX tag is not present in
any added file. Having a check for something missing doesn't really
work well with checkpatch at least in a scalable way that I saw.

Rob

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Joe Perches]

On Thu, 2017-02-23 at 09:56 -0600, Rob Herring wrote:
> Add a check for using SPDX-License-Identifier tags to define the license of
> .dts{i} and DT header files rather than using free form license text. This
> check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
> license text which are the commonly used DT licenses.

Hi Rob.

> Signed-off-by: Rob Herring 
> Cc: Andy Whitcroft 
> Cc: Joe Perches 
> ---
>  scripts/checkpatch.pl | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 982c52ca6473..ce802b3146e3 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2139,6 +2139,7 @@ sub process {
>   my $commit_log_has_diff = 0;
>   my $reported_maintainer_file = 0;
>   my $non_utf8_charset = 0;
> + my $licensefile = '';

Maybe this should be $spdx_license_file
but what's the actual reason to check if
multiple license bits are in a single file?

> +# check for using SPDX tag instead of free form license text in dts and 
> binding header files
> + if ($licensefile ne $realfile &&
> + ($realfile =~ /\.dtsi?$/ || $realfile =~ 
> /dt-bindings\/.*\.h$/) &&

It's nicer to use a non / leading char like @
so you don't have to escape the /.

maybe
$realfile =~ m@/dt-bindings/.*\.h$@

> + $rawline !~ /\bSPDX-License-Identifier/ &&
> + ($rawline =~ /^\+.*\bGeneral\s+Public\s+License/i ||
> + $rawline =~ 
> /^\+.*\bTHE\s+SOFTWARE\s+IS\s+PROVIDED\s+\"AS\s+IS\"/i ||
> + $rawline =~ /^\+.*\b(GPL|BSD|X11)/)) {

nicer to indent these last 2 lines one more space
to keep alignment to open parenthesis.

> + $licensefile = $realfile;
> + WARN("SPDX_LICENSE_TAG",
> +  "Use SPDX-License-Identifier tags instead of full 
> license text\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)$/);

Re: [PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Joe Perches]

On Thu, 2017-02-23 at 09:56 -0600, Rob Herring wrote:
> Add a check for using SPDX-License-Identifier tags to define the license of
> .dts{i} and DT header files rather than using free form license text. This
> check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
> license text which are the commonly used DT licenses.

Hi Rob.

> Signed-off-by: Rob Herring 
> Cc: Andy Whitcroft 
> Cc: Joe Perches 
> ---
>  scripts/checkpatch.pl | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 982c52ca6473..ce802b3146e3 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2139,6 +2139,7 @@ sub process {
>   my $commit_log_has_diff = 0;
>   my $reported_maintainer_file = 0;
>   my $non_utf8_charset = 0;
> + my $licensefile = '';

Maybe this should be $spdx_license_file
but what's the actual reason to check if
multiple license bits are in a single file?

> +# check for using SPDX tag instead of free form license text in dts and 
> binding header files
> + if ($licensefile ne $realfile &&
> + ($realfile =~ /\.dtsi?$/ || $realfile =~ 
> /dt-bindings\/.*\.h$/) &&

It's nicer to use a non / leading char like @
so you don't have to escape the /.

maybe
$realfile =~ m@/dt-bindings/.*\.h$@

> + $rawline !~ /\bSPDX-License-Identifier/ &&
> + ($rawline =~ /^\+.*\bGeneral\s+Public\s+License/i ||
> + $rawline =~ 
> /^\+.*\bTHE\s+SOFTWARE\s+IS\s+PROVIDED\s+\"AS\s+IS\"/i ||
> + $rawline =~ /^\+.*\b(GPL|BSD|X11)/)) {

nicer to indent these last 2 lines one more space
to keep alignment to open parenthesis.

> + $licensefile = $realfile;
> + WARN("SPDX_LICENSE_TAG",
> +  "Use SPDX-License-Identifier tags instead of full 
> license text\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)$/);

[PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Rob Herring]

Add a check for using SPDX-License-Identifier tags to define the license of
.dts{i} and DT header files rather than using free form license text. This
check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
license text which are the commonly used DT licenses.

Signed-off-by: Rob Herring 
Cc: Andy Whitcroft 
Cc: Joe Perches 
---
 scripts/checkpatch.pl | 13 +
 1 file changed, 13 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 982c52ca6473..ce802b3146e3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2139,6 +2139,7 @@ sub process {
my $commit_log_has_diff = 0;
my $reported_maintainer_file = 0;
my $non_utf8_charset = 0;
+   my $licensefile = '';
 
my $last_blank_line = 0;
my $last_coalesced_string_linenr = -1;
@@ -2805,6 +2806,18 @@ sub process {
}
}
 
+# check for using SPDX tag instead of free form license text in dts and 
binding header files
+   if ($licensefile ne $realfile &&
+   ($realfile =~ /\.dtsi?$/ || $realfile =~ 
/dt-bindings\/.*\.h$/) &&
+   $rawline !~ /\bSPDX-License-Identifier/ &&
+   ($rawline =~ /^\+.*\bGeneral\s+Public\s+License/i ||
+   $rawline =~ 
/^\+.*\bTHE\s+SOFTWARE\s+IS\s+PROVIDED\s+\"AS\s+IS\"/i ||
+   $rawline =~ /^\+.*\b(GPL|BSD|X11)/)) {
+   $licensefile = $realfile;
+   WARN("SPDX_LICENSE_TAG",
+"Use SPDX-License-Identifier tags instead of full 
license text\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.10.1

[PATCH] checkpatch.pl: Add SPDX license tag check for dts files and headers

[Rob Herring]

Add a check for using SPDX-License-Identifier tags to define the license of
.dts{i} and DT header files rather than using free form license text. This
check looks for GPL, BSD, or X11(really incorrectly labeled MIT license)
license text which are the commonly used DT licenses.

Signed-off-by: Rob Herring 
Cc: Andy Whitcroft 
Cc: Joe Perches 
---
 scripts/checkpatch.pl | 13 +
 1 file changed, 13 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 982c52ca6473..ce802b3146e3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2139,6 +2139,7 @@ sub process {
my $commit_log_has_diff = 0;
my $reported_maintainer_file = 0;
my $non_utf8_charset = 0;
+   my $licensefile = '';
 
my $last_blank_line = 0;
my $last_coalesced_string_linenr = -1;
@@ -2805,6 +2806,18 @@ sub process {
}
}
 
+# check for using SPDX tag instead of free form license text in dts and 
binding header files
+   if ($licensefile ne $realfile &&
+   ($realfile =~ /\.dtsi?$/ || $realfile =~ 
/dt-bindings\/.*\.h$/) &&
+   $rawline !~ /\bSPDX-License-Identifier/ &&
+   ($rawline =~ /^\+.*\bGeneral\s+Public\s+License/i ||
+   $rawline =~ 
/^\+.*\bTHE\s+SOFTWARE\s+IS\s+PROVIDED\s+\"AS\s+IS\"/i ||
+   $rawline =~ /^\+.*\b(GPL|BSD|X11)/)) {
+   $licensefile = $realfile;
+   WARN("SPDX_LICENSE_TAG",
+"Use SPDX-License-Identifier tags instead of full 
license text\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.10.1