DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [PATCH v2] devtools: add new SPDX license compliance checker
@ 2020-01-29 15:59 Stephen Hemminger
  2020-02-07 17:39 ` Stephen Hemminger
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-01-29 15:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Simple script to look for drivers and scripts that
are missing requires SPDX header.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 devtools/spdx-check.sh | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100755 devtools/spdx-check.sh

diff --git a/devtools/spdx-check.sh b/devtools/spdx-check.sh
new file mode 100755
index 000000000000..6713d556d224
--- /dev/null
+++ b/devtools/spdx-check.sh
@@ -0,0 +1,23 @@
+#! /bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Microsoft Corporation
+#
+# Produce a list of files without SPDX license identifiers
+
+echo "Files without SPDX License"
+echo "--------------------------"
+
+git grep -L SPDX-License-Identifier -- \
+    ':^.git*' ':^.ci/*' ':^.travis.yml' \
+    ':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
+    ':^*/Kbuild' ':^*/README' \
+    ':^license/' ':^doc/' ':^config/' ':^buildtools/' \
+    ':^devtools/cocci/' \
+    ':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt'
+
+echo
+echo "Files with redundant BSD boilerplate"
+echo "------------------------------------"
+
+git grep -l SPDX-License-Identifier | \
+    xargs grep -l 'Redistribution'
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2] devtools: add new SPDX license compliance checker
  2020-01-29 15:59 [dpdk-dev] [PATCH v2] devtools: add new SPDX license compliance checker Stephen Hemminger
@ 2020-02-07 17:39 ` Stephen Hemminger
  2020-02-07 17:52 ` [dpdk-dev] [PATCH v3] " Stephen Hemminger
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-02-07 17:39 UTC (permalink / raw)
  To: dev

On Wed, 29 Jan 2020 07:59:07 -0800
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>

Ping. This should have been merged by now.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v3] devtools: add new SPDX license compliance checker
  2020-01-29 15:59 [dpdk-dev] [PATCH v2] devtools: add new SPDX license compliance checker Stephen Hemminger
  2020-02-07 17:39 ` Stephen Hemminger
@ 2020-02-07 17:52 ` " Stephen Hemminger
  2020-02-22 15:43   ` Thomas Monjalon
  2020-02-22 15:45   ` Thomas Monjalon
  2020-02-24 21:01 ` [dpdk-dev] [PATCH v4] " Stephen Hemminger
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-02-07 17:52 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Simple script to look for drivers and scripts that
are missing requires SPDX header.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v3 - pickup more places with boilerplate text
     avoid false positive for cocci scripts or abignore

 devtools/spdx-check.sh | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100755 devtools/spdx-check.sh

diff --git a/devtools/spdx-check.sh b/devtools/spdx-check.sh
new file mode 100755
index 000000000000..a5be10b26f44
--- /dev/null
+++ b/devtools/spdx-check.sh
@@ -0,0 +1,24 @@
+#! /bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Microsoft Corporation
+#
+# Produce a list of files with incorrect license
+# information
+
+echo "Files without SPDX License"
+echo "--------------------------"
+
+git grep -L SPDX-License-Identifier -- \
+    ':^.git*' ':^.ci/*' ':^.travis.yml' \
+    ':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
+    ':^*/Kbuild' ':^*/README' \
+    ':^license/' ':^doc/' ':^config/' ':^buildtools/' \
+    ':^*.cocci' ':^*.abignore' \
+    ':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt'
+
+echo
+echo "Files with additional license text"
+echo "----------------------------------"
+
+git grep -l Redistribution -- \
+    ':^license/' ':^/devtools/spdx-check.sh'
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v3] devtools: add new SPDX license compliance checker
  2020-02-07 17:52 ` [dpdk-dev] [PATCH v3] " Stephen Hemminger
@ 2020-02-22 15:43   ` Thomas Monjalon
  2020-02-22 15:45   ` Thomas Monjalon
  1 sibling, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2020-02-22 15:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

07/02/2020 18:52, Stephen Hemminger:
> Simple script to look for drivers and scripts that
> are missing requires SPDX header.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  devtools/spdx-check.sh | 24 ++++++++++++++++++++++++

For consistency with other scripts, it should be named check-spdx.sh

Please add the new file in MAINTAINERS file in the section
"Developers and Maintainers Tools".

I think it should be mentioned also in the contributors guide.


> --- /dev/null
> +++ b/devtools/spdx-check.sh
> @@ -0,0 +1,24 @@
> +#! /bin/sh
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2019 Microsoft Corporation

While talking about license, are you sure the (c) is required?


> +#
> +# Produce a list of files with incorrect license
> +# information

In order to add this script in a CI, it should return an error code.
Based on the error code, the CI can report it as success, warning or failure
to patchwork.


> +
> +echo "Files without SPDX License"
> +echo "--------------------------"
> +
> +git grep -L SPDX-License-Identifier -- \
> +    ':^.git*' ':^.ci/*' ':^.travis.yml' \
> +    ':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> +    ':^*/Kbuild' ':^*/README' \
> +    ':^license/' ':^doc/' ':^config/' ':^buildtools/' \
> +    ':^*.cocci' ':^*.abignore' \
> +    ':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt'

This should be considered a critical error.

> +
> +echo
> +echo "Files with additional license text"
> +echo "----------------------------------"
> +
> +git grep -l Redistribution -- \
> +    ':^license/' ':^/devtools/spdx-check.sh'

This should be considered as a warning.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v3] devtools: add new SPDX license compliance checker
  2020-02-07 17:52 ` [dpdk-dev] [PATCH v3] " Stephen Hemminger
  2020-02-22 15:43   ` Thomas Monjalon
@ 2020-02-22 15:45   ` Thomas Monjalon
  1 sibling, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2020-02-22 15:45 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, techboard

07/02/2020 18:52, Stephen Hemminger:
> Simple script to look for drivers and scripts that
> are missing requires SPDX header.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
[...]
> +git grep -L SPDX-License-Identifier -- \
> +    ':^.git*' ':^.ci/*' ':^.travis.yml' \
> +    ':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> +    ':^*/Kbuild' ':^*/README' \
> +    ':^license/' ':^doc/' ':^config/' ':^buildtools/' \
> +    ':^*.cocci' ':^*.abignore' \
> +    ':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt'

I think we should plan to add SPDX tag to the doc (rst and svg files).



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-01-29 15:59 [dpdk-dev] [PATCH v2] devtools: add new SPDX license compliance checker Stephen Hemminger
  2020-02-07 17:39 ` Stephen Hemminger
  2020-02-07 17:52 ` [dpdk-dev] [PATCH v3] " Stephen Hemminger
@ 2020-02-24 21:01 ` " Stephen Hemminger
  2020-04-28 20:15   ` Stephen Hemminger
                     ` (3 more replies)
  2020-02-26  1:14 ` [dpdk-dev] [PATCH] " Stephen Hemminger
  2020-07-14 23:21 ` [dpdk-dev] [PATCH v5] " Stephen Hemminger
  4 siblings, 4 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-02-24 21:01 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Simple script to look for drivers and scripts that
are missing requires SPDX header.

Update the contribution guidelines to indicate that SPDX license
identfier is required for this project.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v4 - add MAINTAINERS entry
     update coding style document
     change name of script

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 3d5e8d1104b2..6b0e042c5fbb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -96,6 +96,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..b1b8cdba4e4e
--- /dev/null
+++ b/devtools/check-spdx-tag.sh
@@ -0,0 +1,77 @@
+#! /bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Microsoft Corporation
+#
+# Produce a list of files with incorrect license tags
+
+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/' ':^doc/' ':^config/' ':^buildtools/' \
+	':^*.cocci' ':^*.abignore' \
+	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
+	> $tmpfile
+
+    errors=0
+    while read -r line
+    do $quiet || echo $line
+       errors=$((errors + 1))
+    done < $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' |
+	while read line
+	do $quiet || echo $line
+	   warnings=$((warnings + 1))
+	done
+
+    warnings=0
+    while read -r line
+    do $quiet || echo $line
+       warnings=$((errors + 1))
+    done < $tmpfile
+}
+
+quiet=false
+verbose=false
+
+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 841ef6d5c829..04626667dc18 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
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH] devtools: add new SPDX license compliance checker
  2020-01-29 15:59 [dpdk-dev] [PATCH v2] devtools: add new SPDX license compliance checker Stephen Hemminger
                   ` (2 preceding siblings ...)
  2020-02-24 21:01 ` [dpdk-dev] [PATCH v4] " Stephen Hemminger
@ 2020-02-26  1:14 ` " Stephen Hemminger
  2020-07-14 23:21 ` [dpdk-dev] [PATCH v5] " Stephen Hemminger
  4 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-02-26  1:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Simple script to look for drivers and scripts that
are missing requires SPDX header.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v5 - fix issue in counting warnings
     update Copyright date

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

diff --git a/MAINTAINERS b/MAINTAINERS
index f4e0ed8e056c..75e0dc75f887 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -96,6 +96,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..fc3d962ec647
--- /dev/null
+++ b/devtools/check-spdx-tag.sh
@@ -0,0 +1,73 @@
+#! /bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2020 Microsoft Corporation
+#
+# Produce a list of files with incorrect license tags
+
+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/' ':^doc/' ':^config/' ':^buildtools/' \
+	':^*.cocci' ':^*.abignore' \
+	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
+	> $tmpfile
+
+    errors=0
+    while read -r line
+    do $quiet || echo $line
+       errors=$((errors + 1))
+    done < $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=0
+    while read -r line
+    do $quiet || echo $line
+       warnings=$((warnings + 1))
+    done < $tmpfile
+}
+
+quiet=false
+verbose=false
+
+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 841ef6d5c829..04626667dc18 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
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-02-24 21:01 ` [dpdk-dev] [PATCH v4] " Stephen Hemminger
@ 2020-04-28 20:15   ` Stephen Hemminger
  2020-06-11 18:46   ` Stephen Hemminger
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-04-28 20:15 UTC (permalink / raw)
  To: dev

On Mon, 24 Feb 2020 13:01:30 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Simple script to look for drivers and scripts that
> are missing requires SPDX header.
> 
> Update the contribution guidelines to indicate that SPDX license
> identfier is required for this project.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v4 - add MAINTAINERS entry
>      update coding style document
>      change name of script
> 
>  MAINTAINERS                              |  1 +
>  devtools/check-spdx-tag.sh               | 77 ++++++++++++++++++++++++
>  doc/guides/contributing/coding_style.rst |  9 ++-
>  3 files changed, 85 insertions(+), 2 deletions(-)
>  create mode 100755 devtools/check-spdx-tag.sh

Still waiting for this script all requested changes were done months ago.

Current output is:
$ ./devtools/check-spdx-tag.sh -v
Files without SPDX License
--------------------------


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: 0 errors, 3 warnings

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-02-24 21:01 ` [dpdk-dev] [PATCH v4] " Stephen Hemminger
  2020-04-28 20:15   ` Stephen Hemminger
@ 2020-06-11 18:46   ` Stephen Hemminger
  2020-06-11 21:32     ` Thomas Monjalon
  2020-06-11 21:39   ` Thomas Monjalon
  2020-06-12  9:05   ` Gaëtan Rivet
  3 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2020-06-11 18:46 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Mon, 24 Feb 2020 13:01:30 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Simple script to look for drivers and scripts that
> are missing requires SPDX header.
> 
> Update the contribution guidelines to indicate that SPDX license
> identfier is required for this project.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v4 - add MAINTAINERS entry
>      update coding style document
>      change name of script
> 
>  MAINTAINERS                              |  1 +
>  devtools/check-spdx-tag.sh               | 77 ++++++++++++++++++++++++
>  doc/guides/contributing/coding_style.rst |  9 ++-
>  3 files changed, 85 insertions(+), 2 deletions(-)
>  create mode 100755 devtools/check-spdx-tag.sh
>

Going through the patch backlog.

Why is this not merged yet?
Why is not in DPDK 20.05?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-06-11 18:46   ` Stephen Hemminger
@ 2020-06-11 21:32     ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2020-06-11 21:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

11/06/2020 20:46, Stephen Hemminger:
> On Mon, 24 Feb 2020 13:01:30 -0800
> Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> > Simple script to look for drivers and scripts that
> > are missing requires SPDX header.
> > 
> > Update the contribution guidelines to indicate that SPDX license
> > identfier is required for this project.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> Going through the patch backlog.
> 
> Why is this not merged yet?
> Why is not in DPDK 20.05?

I would like to see some reviews on such patch
which looks like a policy.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-02-24 21:01 ` [dpdk-dev] [PATCH v4] " Stephen Hemminger
  2020-04-28 20:15   ` Stephen Hemminger
  2020-06-11 18:46   ` Stephen Hemminger
@ 2020-06-11 21:39   ` Thomas Monjalon
  2020-06-12  8:36     ` Gaëtan Rivet
  2020-06-12 14:53     ` Stephen Hemminger
  2020-06-12  9:05   ` Gaëtan Rivet
  3 siblings, 2 replies; 22+ messages in thread
From: Thomas Monjalon @ 2020-06-11 21:39 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

24/02/2020 22:01, Stephen Hemminger:
> +tmpfile=$(mktemp)

Please check how other temp files are created in other scripts
for consitency.

> +    git grep -L SPDX-License-Identifier -- \
> +	':^.git*' ':^.ci/*' ':^.travis.yml' \
> +	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> +	':^*/Kbuild' ':^*/README' \
> +	':^license/' ':^doc/' ':^config/' ':^buildtools/' \

I think doc/ should be part of the license check,
same for buildtools/.

> +	':^*.cocci' ':^*.abignore' \
> +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> +	> $tmpfile
> +
> +    errors=0
> +    while read -r line
> +    do $quiet || echo $line
> +       errors=$((errors + 1))

I'm surprised this works for you.
In general, "while" creates a subshell which makes impossible
updating a variable.
I recommend using "for" with IFS=$'\n'.

> +    done < $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' |
> +	while read line
> +	do $quiet || echo $line
> +	   warnings=$((warnings + 1))
> +	done

Same comment about "while" subshell.

> +
> +    warnings=0
> +    while read -r line
> +    do $quiet || echo $line
> +       warnings=$((errors + 1))

Here too

> +    done < $tmpfile
> +}

[...]
> +Each file must begin with a special comment containing the
> +`Software Package Data Exchange (SPDX) License Identfier <https://spdx.org/using-spdx-license-identifier>`_.

Typo: Identifier

> +
> +Generally this is the BSD License, except for code granted special exceptions.

Is a verb missing?

> +The SPDX licences identifier is sufficient, a file should not contain
> +an additional text version of the license (boilerplate).




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-06-11 21:39   ` Thomas Monjalon
@ 2020-06-12  8:36     ` Gaëtan Rivet
  2020-06-12 14:53     ` Stephen Hemminger
  1 sibling, 0 replies; 22+ messages in thread
From: Gaëtan Rivet @ 2020-06-12  8:36 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Stephen Hemminger, dev

Hi Thomas, Stephen,

On 11/06/20 23:39 +0200, Thomas Monjalon wrote:
> 24/02/2020 22:01, Stephen Hemminger:
> > +tmpfile=$(mktemp)
> 
> Please check how other temp files are created in other scripts
> for consitency.
> 
> > +    git grep -L SPDX-License-Identifier -- \
> > +	':^.git*' ':^.ci/*' ':^.travis.yml' \
> > +	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> > +	':^*/Kbuild' ':^*/README' \
> > +	':^license/' ':^doc/' ':^config/' ':^buildtools/' \
> 
> I think doc/ should be part of the license check,
> same for buildtools/.
> 
> > +	':^*.cocci' ':^*.abignore' \
> > +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> > +	> $tmpfile
> > +
> > +    errors=0
> > +    while read -r line
> > +    do $quiet || echo $line
> > +       errors=$((errors + 1))
> 
> I'm surprised this works for you.
> In general, "while" creates a subshell which makes impossible
> updating a variable.
> I recommend using "for" with IFS=$'\n'.
> 

No it should work, while will only spawn a subshell if you use a pipe with it.
Ex:

   err=0; while true; do err=$((err + 1)); done            # $err changes

   err=0; yes | while read -r y; do err=$((err + 1)); done # $err is always 0

Using for could be an issue, as you are then limited by the number of
parameters allowed. Additionally, the script is written for POSIX shell,
using $'\n' is forbidden and the POSIX equivalent is very ugly:

   IFS="$(printf '%b_' '\n')"; IFS="${IFS%_}" # protect trailing \n

> > +    done < $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' |
> > +	while read line

Missing a -r to read here: https://www.shellcheck.net/wiki/SC2162
Generally, best to use shellcheck when writing a script, especially if
using /bin/sh.

> > +	do $quiet || echo $line
> > +	   warnings=$((warnings + 1))
> > +	done
> 
> Same comment about "while" subshell.
> 
> > +
> > +    warnings=0
> > +    while read -r line
> > +    do $quiet || echo $line
> > +       warnings=$((errors + 1))
> 
> Here too
> 
> > +    done < $tmpfile
> > +}
> 
> [...]
> > +Each file must begin with a special comment containing the
> > +`Software Package Data Exchange (SPDX) License Identfier <https://spdx.org/using-spdx-license-identifier>`_.
> 
> Typo: Identifier
> 
> > +
> > +Generally this is the BSD License, except for code granted special exceptions.
> 
> Is a verb missing?
> 
> > +The SPDX licences identifier is sufficient, a file should not contain
> > +an additional text version of the license (boilerplate).
> 
> 
> 

-- 
Gaëtan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-02-24 21:01 ` [dpdk-dev] [PATCH v4] " Stephen Hemminger
                     ` (2 preceding siblings ...)
  2020-06-11 21:39   ` Thomas Monjalon
@ 2020-06-12  9:05   ` Gaëtan Rivet
  2020-07-14 23:23     ` Stephen Hemminger
  3 siblings, 1 reply; 22+ messages in thread
From: Gaëtan Rivet @ 2020-06-12  9:05 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On 24/02/20 13:01 -0800, Stephen Hemminger wrote:
> Simple script to look for drivers and scripts that
> are missing requires SPDX header.
> 
> Update the contribution guidelines to indicate that SPDX license
> identfier is required for this project.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v4 - add MAINTAINERS entry
>      update coding style document
>      change name of script
> 
>  MAINTAINERS                              |  1 +
>  devtools/check-spdx-tag.sh               | 77 ++++++++++++++++++++++++
>  doc/guides/contributing/coding_style.rst |  9 ++-
>  3 files changed, 85 insertions(+), 2 deletions(-)
>  create mode 100755 devtools/check-spdx-tag.sh
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d5e8d1104b2..6b0e042c5fbb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -96,6 +96,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..b1b8cdba4e4e
> --- /dev/null
> +++ b/devtools/check-spdx-tag.sh
> @@ -0,0 +1,77 @@
> +#! /bin/sh
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2019 Microsoft Corporation
> +#
> +# Produce a list of files with incorrect license tags
> +
> +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/' ':^doc/' ':^config/' ':^buildtools/' \
> +	':^*.cocci' ':^*.abignore' \
> +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> +	> $tmpfile

I find it easier to maintain an exclude list by setting a variable and
generating the relevant parameters:

    excludes='.git* .ci/* .travis.yml */Kbuild */README'
    exclude_opt=""
    set -f
    for pattern in $excludes; do
        exclude_opt="$exclude_opt ':^${pattern}'"
    done
    set +f
    printf "\"%s\"\n" "$exclude_opt"

However I recognize that means dealing with contrarian globbing issues in shells,
so it comes at a price. But I find changing the exclude list much easier that way.

> +
> +    errors=0
> +    while read -r line
> +    do $quiet || echo $line
> +       errors=$((errors + 1))
> +    done < $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' |
> +	while read line
> +	do $quiet || echo $line
> +	   warnings=$((warnings + 1))
> +	done
> +
> +    warnings=0
> +    while read -r line
> +    do $quiet || echo $line
> +       warnings=$((errors + 1))
> +    done < $tmpfile
> +}
> +
> +quiet=false
> +verbose=false
> +
> +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 841ef6d5c829..04626667dc18 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
> -- 
> 2.20.1
> 

-- 
Gaëtan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-06-11 21:39   ` Thomas Monjalon
  2020-06-12  8:36     ` Gaëtan Rivet
@ 2020-06-12 14:53     ` Stephen Hemminger
  2020-06-12 15:42       ` Thomas Monjalon
  1 sibling, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2020-06-12 14:53 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Thu, 11 Jun 2020 23:39:55 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 24/02/2020 22:01, Stephen Hemminger:
> > +tmpfile=$(mktemp)  
> 
> Please check how other temp files are created in other scripts
> for consitency.
> 
> > +    git grep -L SPDX-License-Identifier -- \
> > +	':^.git*' ':^.ci/*' ':^.travis.yml' \
> > +	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> > +	':^*/Kbuild' ':^*/README' \
> > +	':^license/' ':^doc/' ':^config/' ':^buildtools/' \  
> 
> I think doc/ should be part of the license check,
> same for buildtools/.
> 
> > +	':^*.cocci' ':^*.abignore' \
> > +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> > +	> $tmpfile
> > +
> > +    errors=0
> > +    while read -r line
> > +    do $quiet || echo $line
> > +       errors=$((errors + 1))  
> 
> I'm surprised this works for you.
> In general, "while" creates a subshell which makes impossible
> updating a variable.
> I recommend using "for" with IFS=$'\n'.
> 
> > +    done < $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' |
> > +	while read line
> > +	do $quiet || echo $line
> > +	   warnings=$((warnings + 1))
> > +	done  
> 
> Same comment about "while" subshell.
> 
> > +
> > +    warnings=0
> > +    while read -r line
> > +    do $quiet || echo $line
> > +       warnings=$((errors + 1))  
> 
> Here too
> 
> > +    done < $tmpfile
> > +}  
> 
> [...]
> > +Each file must begin with a special comment containing the
> > +`Software Package Data Exchange (SPDX) License Identfier <https://spdx.org/using-spdx-license-identifier>`_.  
> 
> Typo: Identifier
> 
> > +
> > +Generally this is the BSD License, except for code granted special exceptions.  
> 
> Is a verb missing?
> 
> > +The SPDX licences identifier is sufficient, a file should not contain
> > +an additional text version of the license (boilerplate).  

Thanks for the feedback.
Text processing is simpler in python, will rewrite in next version?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-06-12 14:53     ` Stephen Hemminger
@ 2020-06-12 15:42       ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2020-06-12 15:42 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

12/06/2020 16:53, Stephen Hemminger:
> On Thu, 11 Jun 2020 23:39:55 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 24/02/2020 22:01, Stephen Hemminger:
> > > +tmpfile=$(mktemp)  
> > 
> > Please check how other temp files are created in other scripts
> > for consitency.
> > 
> > > +    git grep -L SPDX-License-Identifier -- \
> > > +	':^.git*' ':^.ci/*' ':^.travis.yml' \
> > > +	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> > > +	':^*/Kbuild' ':^*/README' \
> > > +	':^license/' ':^doc/' ':^config/' ':^buildtools/' \  
> > 
> > I think doc/ should be part of the license check,
> > same for buildtools/.
> > 
> > > +	':^*.cocci' ':^*.abignore' \
> > > +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> > > +	> $tmpfile
> > > +
> > > +    errors=0
> > > +    while read -r line
> > > +    do $quiet || echo $line
> > > +       errors=$((errors + 1))  
> > 
> > I'm surprised this works for you.
> > In general, "while" creates a subshell which makes impossible
> > updating a variable.
> > I recommend using "for" with IFS=$'\n'.
> > 
> > > +    done < $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' |
> > > +	while read line
> > > +	do $quiet || echo $line
> > > +	   warnings=$((warnings + 1))
> > > +	done  
> > 
> > Same comment about "while" subshell.
> > 
> > > +
> > > +    warnings=0
> > > +    while read -r line
> > > +    do $quiet || echo $line
> > > +       warnings=$((errors + 1))  
> > 
> > Here too
> > 
> > > +    done < $tmpfile
> > > +}  
> > 
> > [...]
> > > +Each file must begin with a special comment containing the
> > > +`Software Package Data Exchange (SPDX) License Identfier <https://spdx.org/using-spdx-license-identifier>`_.  
> > 
> > Typo: Identifier
> > 
> > > +
> > > +Generally this is the BSD License, except for code granted special exceptions.  
> > 
> > Is a verb missing?
> > 
> > > +The SPDX licences identifier is sufficient, a file should not contain
> > > +an additional text version of the license (boilerplate).  
> 
> Thanks for the feedback.
> Text processing is simpler in python, will rewrite in next version?

I don't see a need for rewrite.
I think addressing the comments is simpler.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v5] devtools: add new SPDX license compliance checker
  2020-01-29 15:59 [dpdk-dev] [PATCH v2] devtools: add new SPDX license compliance checker Stephen Hemminger
                   ` (3 preceding siblings ...)
  2020-02-26  1:14 ` [dpdk-dev] [PATCH] " Stephen Hemminger
@ 2020-07-14 23:21 ` " Stephen Hemminger
  2020-07-14 23:25   ` Stephen Hemminger
                     ` (2 more replies)
  4 siblings, 3 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-07-14 23:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

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

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
-- 
2.27.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
  2020-06-12  9:05   ` Gaëtan Rivet
@ 2020-07-14 23:23     ` Stephen Hemminger
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-07-14 23:23 UTC (permalink / raw)
  To: Gaëtan Rivet; +Cc: dev

On Fri, 12 Jun 2020 11:05:27 +0200
Gaëtan Rivet <grive@u256.net> wrote:

> On 24/02/20 13:01 -0800, Stephen Hemminger wrote:
> > Simple script to look for drivers and scripts that
> > are missing requires SPDX header.
> > 
> > Update the contribution guidelines to indicate that SPDX license
> > identfier is required for this project.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > v4 - add MAINTAINERS entry
> >      update coding style document
> >      change name of script
> > 
> >  MAINTAINERS                              |  1 +
> >  devtools/check-spdx-tag.sh               | 77 ++++++++++++++++++++++++
> >  doc/guides/contributing/coding_style.rst |  9 ++-
> >  3 files changed, 85 insertions(+), 2 deletions(-)
> >  create mode 100755 devtools/check-spdx-tag.sh
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 3d5e8d1104b2..6b0e042c5fbb 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -96,6 +96,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..b1b8cdba4e4e
> > --- /dev/null
> > +++ b/devtools/check-spdx-tag.sh
> > @@ -0,0 +1,77 @@
> > +#! /bin/sh
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright (c) 2019 Microsoft Corporation
> > +#
> > +# Produce a list of files with incorrect license tags
> > +
> > +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/' ':^doc/' ':^config/' ':^buildtools/' \
> > +	':^*.cocci' ':^*.abignore' \
> > +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> > +	> $tmpfile  
> 
> I find it easier to maintain an exclude list by setting a variable and
> generating the relevant parameters:
> 
>     excludes='.git* .ci/* .travis.yml */Kbuild */README'
>     exclude_opt=""
>     set -f
>     for pattern in $excludes; do
>         exclude_opt="$exclude_opt ':^${pattern}'"
>     done
>     set +f
>     printf "\"%s\"\n" "$exclude_opt"
> 
> However I recognize that means dealing with contrarian globbing issues in shells,
> so it comes at a price. But I find changing the exclude list much easier that way.

This is no easier to maintain. Doing more string stuff in shell
is not worth it. If gets to be too much trouble, will consider moving
to python3.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v5] devtools: add new SPDX license compliance checker
  2020-07-14 23:21 ` [dpdk-dev] [PATCH v5] " Stephen Hemminger
@ 2020-07-14 23:25   ` Stephen Hemminger
  2020-07-30 22:00     ` Thomas Monjalon
  2020-07-23  4:36   ` Stephen Hemminger
  2020-07-30 22:06   ` Thomas Monjalon
  2 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2020-07-14 23:25 UTC (permalink / raw)
  To: dev

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v5] devtools: add new SPDX license compliance checker
  2020-07-14 23:21 ` [dpdk-dev] [PATCH v5] " Stephen Hemminger
  2020-07-14 23:25   ` Stephen Hemminger
@ 2020-07-23  4:36   ` Stephen Hemminger
  2020-07-30 22:06   ` Thomas Monjalon
  2 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-07-23  4:36 UTC (permalink / raw)
  To: dev

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v5] devtools: add new SPDX license compliance checker
  2020-07-14 23:25   ` Stephen Hemminger
@ 2020-07-30 22:00     ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2020-07-30 22:00 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

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.




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v5] devtools: add new SPDX license compliance checker
  2020-07-14 23:21 ` [dpdk-dev] [PATCH v5] " Stephen Hemminger
  2020-07-14 23:25   ` Stephen Hemminger
  2020-07-23  4:36   ` Stephen Hemminger
@ 2020-07-30 22:06   ` Thomas Monjalon
  2020-07-30 23:41     ` Stephen Hemminger
  2 siblings, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2020-07-30 22:06 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

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




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v5] devtools: add new SPDX license compliance checker
  2020-07-30 22:06   ` Thomas Monjalon
@ 2020-07-30 23:41     ` Stephen Hemminger
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2020-07-30 23:41 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

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 :-)_



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, back to index

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-29 15:59 [dpdk-dev] [PATCH v2] devtools: add new SPDX license compliance checker Stephen Hemminger
2020-02-07 17:39 ` Stephen Hemminger
2020-02-07 17:52 ` [dpdk-dev] [PATCH v3] " Stephen Hemminger
2020-02-22 15:43   ` Thomas Monjalon
2020-02-22 15:45   ` Thomas Monjalon
2020-02-24 21:01 ` [dpdk-dev] [PATCH v4] " Stephen Hemminger
2020-04-28 20:15   ` Stephen Hemminger
2020-06-11 18:46   ` Stephen Hemminger
2020-06-11 21:32     ` Thomas Monjalon
2020-06-11 21:39   ` Thomas Monjalon
2020-06-12  8:36     ` Gaëtan Rivet
2020-06-12 14:53     ` Stephen Hemminger
2020-06-12 15:42       ` Thomas Monjalon
2020-06-12  9:05   ` Gaëtan Rivet
2020-07-14 23:23     ` Stephen Hemminger
2020-02-26  1:14 ` [dpdk-dev] [PATCH] " Stephen Hemminger
2020-07-14 23:21 ` [dpdk-dev] [PATCH v5] " Stephen Hemminger
2020-07-14 23:25   ` Stephen Hemminger
2020-07-30 22:00     ` Thomas Monjalon
2020-07-23  4:36   ` Stephen Hemminger
2020-07-30 22:06   ` Thomas Monjalon
2020-07-30 23:41     ` Stephen Hemminger

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox