[v5] devtools: add new SPDX license compliance checker

Message ID 20200714232101.32544-1-stephen@networkplumber.org (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v5] devtools: add new SPDX license compliance checker |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot warning Travis build: failed
ci/iol-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS

Commit Message

Stephen Hemminger July 14, 2020, 11:21 p.m. UTC
  Simple script to look for drivers and scripts that
are missing requires SPDX header.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---

v5 - address review comments
     simplify the display logic
     scan doc and buildtools as well

 MAINTAINERS                              |  1 +
 devtools/check-spdx-tag.sh               | 70 ++++++++++++++++++++++++
 doc/guides/contributing/coding_style.rst |  9 ++-
 3 files changed, 78 insertions(+), 2 deletions(-)
 create mode 100755 devtools/check-spdx-tag.sh
  

Comments

Stephen Hemminger July 14, 2020, 11:25 p.m. UTC | #1
On Tue, 14 Jul 2020 16:21:01 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Simple script to look for drivers and scripts that
> are missing requires SPDX header.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
 Preprocessor Directives

Because of the long delay in merging this tool new
files have come in that are not following the required guidelines!


 $ ./devtools/check-spdx-tag.sh -v
Files without SPDX License
--------------------------
doc/guides/custom.css
doc/guides/linux_gsg/nic_perf_intel_platform.rst
doc/guides/prog_guide/build-sdk-meson.rst
drivers/net/bnxt/tf_core/cfa_resource_types.h
drivers/net/qede/base/ecore_hsi_func_common.h
lib/librte_eal/windows/eal_hugepages.c


Files with redundant license text
---------------------------------
app/test/test_timer_racecond.c
lib/librte_ethdev/rte_ethdev_pci.h
lib/librte_ethdev/rte_ethdev_vdev.h

total: 6 errors, 3 warnings
  
Stephen Hemminger July 23, 2020, 4:36 a.m. UTC | #2
On Tue, 14 Jul 2020 16:21:01 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Simple script to look for drivers and scripts that
> are missing requires SPDX header.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

All comments are addressed should be merged by now.
  
Thomas Monjalon July 30, 2020, 10 p.m. UTC | #3
15/07/2020 01:25, Stephen Hemminger:
> On Tue, 14 Jul 2020 16:21:01 -0700
> Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> > Simple script to look for drivers and scripts that
> > are missing requires SPDX header.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>  Preprocessor Directives
> 
> Because of the long delay in merging this tool new
> files have come in that are not following the required guidelines!

Probably it would have helped if you Cc'ed some persons known to be
interested in this topic. Getting reviews is harder recently.
  
Thomas Monjalon July 30, 2020, 10:06 p.m. UTC | #4
15/07/2020 01:21, Stephen Hemminger:
> Simple script to look for drivers and scripts that
> are missing requires SPDX header.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
[...]
> +#! /bin/sh
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2020 Microsoft Corporation
> +#
> +# Produce a list of files with incorrect license tags
> +
> +errors=0
> +warnings=0
> +quiet=false
> +verbose=false
> +
> +print_usage () {
> +    echo "usage: $(basename $0) [-q] [-v]"
> +    exit 1
> +}
> +
> +check_spdx() {
> +    if  $verbose;  then
> +	echo "Files without SPDX License"
> +	echo "--------------------------"
> +    fi
> +    git grep -L SPDX-License-Identifier -- \
> +	':^.git*' ':^.ci/*' ':^.travis.yml' \
> +	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> +	':^*/Kbuild' ':^*/README' \
> +	':^license/' ':^config/' ':^buildtools/' \
> +	':^*.cocci' ':^*.abignore' \
> +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> +	':^*.svg' ':^*.png'\

I don't agree with this list of files.
But I guess we can start with that and be more strict in future.

> +	> $tmpfile
> +
> +    errors=$(wc -l < $tmpfile)
> +    $quiet || cat $tmpfile
> +}
> +
> +check_boilerplate() {
> +    if $verbose ; then
> +	echo
> +	echo "Files with redundant license text"
> +	echo "---------------------------------"
> +    fi
> +
> +    git grep -l Redistribution -- \
> +	':^license/' ':^/devtools/check-spdx-tag.sh' > $tmpfile
> +
> +    warnings=$(wc -l <$tmpfile)
> +    $quiet || cat $tmpfile
> +}
> +
> +while getopts qvh ARG ; do
> +	case $ARG in
> +		q ) quiet=true ;;
> +		v ) verbose=true ;;
> +		h ) print_usage ; exit 0 ;;
> +		? ) print_usage ; exit 1 ;;
> +	esac
> +done
> +shift $(($OPTIND - 1))
> +
> +tmpfile=$(mktemp)

Should be mktemp -t dpdk.checkspdx.XXXXXX
to keep namespace of our temp files. Will fix.

> +trap 'rm -f -- "$tmpfile"' INT TERM HUP EXIT

Why catching HUP signal?


Applied, thanks
  
Stephen Hemminger July 30, 2020, 11:41 p.m. UTC | #5
On Fri, 31 Jul 2020 00:06:23 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 15/07/2020 01:21, Stephen Hemminger:
> > Simple script to look for drivers and scripts that
> > are missing requires SPDX header.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> [...]
> > +#! /bin/sh
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright 2020 Microsoft Corporation
> > +#
> > +# Produce a list of files with incorrect license tags
> > +
> > +errors=0
> > +warnings=0
> > +quiet=false
> > +verbose=false
> > +
> > +print_usage () {
> > +    echo "usage: $(basename $0) [-q] [-v]"
> > +    exit 1
> > +}
> > +
> > +check_spdx() {
> > +    if  $verbose;  then
> > +	echo "Files without SPDX License"
> > +	echo "--------------------------"
> > +    fi
> > +    git grep -L SPDX-License-Identifier -- \
> > +	':^.git*' ':^.ci/*' ':^.travis.yml' \
> > +	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> > +	':^*/Kbuild' ':^*/README' \
> > +	':^license/' ':^config/' ':^buildtools/' \
> > +	':^*.cocci' ':^*.abignore' \
> > +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> > +	':^*.svg' ':^*.png'\  
> 
> I don't agree with this list of files.
> But I guess we can start with that and be more strict in future.
> 
> > +	> $tmpfile
> > +
> > +    errors=$(wc -l < $tmpfile)
> > +    $quiet || cat $tmpfile
> > +}
> > +
> > +check_boilerplate() {
> > +    if $verbose ; then
> > +	echo
> > +	echo "Files with redundant license text"
> > +	echo "---------------------------------"
> > +    fi
> > +
> > +    git grep -l Redistribution -- \
> > +	':^license/' ':^/devtools/check-spdx-tag.sh' > $tmpfile
> > +
> > +    warnings=$(wc -l <$tmpfile)
> > +    $quiet || cat $tmpfile
> > +}
> > +
> > +while getopts qvh ARG ; do
> > +	case $ARG in
> > +		q ) quiet=true ;;
> > +		v ) verbose=true ;;
> > +		h ) print_usage ; exit 0 ;;
> > +		? ) print_usage ; exit 1 ;;
> > +	esac
> > +done
> > +shift $(($OPTIND - 1))
> > +
> > +tmpfile=$(mktemp)  
> 
> Should be mktemp -t dpdk.checkspdx.XXXXXX
> to keep namespace of our temp files. Will fix.
> 
> > +trap 'rm -f -- "$tmpfile"' INT TERM HUP EXIT  
> 
> Why catching HUP signal?

General practice to have a script cleanup if user logs out.
Back in the old days, connections were lost sometimes :-)_
  
Stephen Hemminger Aug. 26, 2020, 3:12 p.m. UTC | #6
On Fri, 31 Jul 2020 00:06:23 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 15/07/2020 01:21, Stephen Hemminger:
> > Simple script to look for drivers and scripts that
> > are missing requires SPDX header.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> [...]
> > +#! /bin/sh
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright 2020 Microsoft Corporation
> > +#
> > +# Produce a list of files with incorrect license tags
> > +
> > +errors=0
> > +warnings=0
> > +quiet=false
> > +verbose=false
> > +
> > +print_usage () {
> > +    echo "usage: $(basename $0) [-q] [-v]"
> > +    exit 1
> > +}
> > +
> > +check_spdx() {
> > +    if  $verbose;  then
> > +	echo "Files without SPDX License"
> > +	echo "--------------------------"
> > +    fi
> > +    git grep -L SPDX-License-Identifier -- \
> > +	':^.git*' ':^.ci/*' ':^.travis.yml' \
> > +	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> > +	':^*/Kbuild' ':^*/README' \
> > +	':^license/' ':^config/' ':^buildtools/' \
> > +	':^*.cocci' ':^*.abignore' \
> > +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> > +	':^*.svg' ':^*.png'\  
> 
> I don't agree with this list of files.
> But I guess we can start with that and be more strict in future.

I suppose README, MAINTAINERS, .ini and .txt files could be removed from the exception list.
  
Bruce Richardson Aug. 26, 2020, 3:43 p.m. UTC | #7
On Wed, Aug 26, 2020 at 08:12:30AM -0700, Stephen Hemminger wrote:
> On Fri, 31 Jul 2020 00:06:23 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 15/07/2020 01:21, Stephen Hemminger:
> > > Simple script to look for drivers and scripts that
> > > are missing requires SPDX header.
> > > 
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> > [...]
> > > +#! /bin/sh
> > > +# SPDX-License-Identifier: BSD-3-Clause
> > > +# Copyright 2020 Microsoft Corporation
> > > +#
> > > +# Produce a list of files with incorrect license tags
> > > +
> > > +errors=0
> > > +warnings=0
> > > +quiet=false
> > > +verbose=false
> > > +
> > > +print_usage () {
> > > +    echo "usage: $(basename $0) [-q] [-v]"
> > > +    exit 1
> > > +}
> > > +
> > > +check_spdx() {
> > > +    if  $verbose;  then
> > > +	echo "Files without SPDX License"
> > > +	echo "--------------------------"
> > > +    fi
> > > +    git grep -L SPDX-License-Identifier -- \
> > > +	':^.git*' ':^.ci/*' ':^.travis.yml' \
> > > +	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> > > +	':^*/Kbuild' ':^*/README' \
> > > +	':^license/' ':^config/' ':^buildtools/' \
> > > +	':^*.cocci' ':^*.abignore' \
> > > +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> > > +	':^*.svg' ':^*.png'\  
> > 
> > I don't agree with this list of files.
> > But I guess we can start with that and be more strict in future.
> 
> I suppose README, MAINTAINERS, .ini and .txt files could be removed from the exception list.

We also want to scan the files in config and buildtools.
  

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 3cd402b34c91..a9e01330a71e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -90,6 +90,7 @@  F: devtools/check-maintainers.sh
 F: devtools/check-forbidden-tokens.awk
 F: devtools/check-git-log.sh
 F: devtools/check-includes.sh
+F: devtools/check-spdx-tag.sh
 F: devtools/check-symbol-maps.sh
 F: devtools/checkpatches.sh
 F: devtools/get-maintainer.sh
diff --git a/devtools/check-spdx-tag.sh b/devtools/check-spdx-tag.sh
new file mode 100755
index 000000000000..ff9e9c61b697
--- /dev/null
+++ b/devtools/check-spdx-tag.sh
@@ -0,0 +1,70 @@ 
+#! /bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2020 Microsoft Corporation
+#
+# Produce a list of files with incorrect license tags
+
+errors=0
+warnings=0
+quiet=false
+verbose=false
+
+print_usage () {
+    echo "usage: $(basename $0) [-q] [-v]"
+    exit 1
+}
+
+check_spdx() {
+    if  $verbose;  then
+	echo "Files without SPDX License"
+	echo "--------------------------"
+    fi
+    git grep -L SPDX-License-Identifier -- \
+	':^.git*' ':^.ci/*' ':^.travis.yml' \
+	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
+	':^*/Kbuild' ':^*/README' \
+	':^license/' ':^config/' ':^buildtools/' \
+	':^*.cocci' ':^*.abignore' \
+	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
+	':^*.svg' ':^*.png'\
+	> $tmpfile
+
+    errors=$(wc -l < $tmpfile)
+    $quiet || cat $tmpfile
+}
+
+check_boilerplate() {
+    if $verbose ; then
+	echo
+	echo "Files with redundant license text"
+	echo "---------------------------------"
+    fi
+
+    git grep -l Redistribution -- \
+	':^license/' ':^/devtools/check-spdx-tag.sh' > $tmpfile
+
+    warnings=$(wc -l <$tmpfile)
+    $quiet || cat $tmpfile
+}
+
+while getopts qvh ARG ; do
+	case $ARG in
+		q ) quiet=true ;;
+		v ) verbose=true ;;
+		h ) print_usage ; exit 0 ;;
+		? ) print_usage ; exit 1 ;;
+	esac
+done
+shift $(($OPTIND - 1))
+
+tmpfile=$(mktemp)
+trap 'rm -f -- "$tmpfile"' INT TERM HUP EXIT
+
+check_spdx
+$quiet || echo
+
+check_boilerplate
+
+$quiet || echo
+echo "total: $errors errors, $warnings warnings"
+exit $errors
diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
index 4efde93f6af0..b55075eaa240 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -54,8 +54,13 @@  To document a public API, a doxygen-like format must be used: refer to :ref:`dox
 License Header
 ~~~~~~~~~~~~~~
 
-Each file should begin with a special comment containing the appropriate copyright and license for the file.
-Generally this is the BSD License, except for code for Linux Kernel modules.
+Each file must begin with a special comment containing the
+`Software Package Data Exchange (SPDX) License Identfier <https://spdx.org/using-spdx-license-identifier>`_.
+
+Generally this is the BSD License, except for code granted special exceptions.
+The SPDX licences identifier is sufficient, a file should not contain
+an additional text version of the license (boilerplate).
+
 After any copyright header, a blank line should be left before any other contents, e.g. include statements in a C file.
 
 C Preprocessor Directives