[PATCH 0/6] Address issues with SPDX requirements and PEP-263

[PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Mauro Carvalho Chehab]

The  description at Documentation/process/license-rules.rst is very strict
with regards to the position where the SPDX tags should be.

In the past several developers and maintainers interpreted it on a
more permissive way, placing the SPDX header between lines 1 to 15, 
with are the ones which the  scripts/spdxcheck.py script verifies.

However, recently, devs are becoming more strict about such
requirement and want it to strictly follow the rule, with states that
the SPDX rule should be at the first line ever on most files, and
at the second line for scripts.

Well, for Python script, such requirement causes violation to PEP-263, 
making regressions on scripts that contain encoding lines, as PEP-263
also states about the same.

This series addresses it.

Patches 1 to 3 fix some Python scripts that violates PEP-263;

Patch 4 mentions PEP-263 for Python scripts, allowing to go up to
line 3, when both "#!" and the encoding line is found;

Patch 5 changes the scripts/spdxcheck.py for it to identify on what
line each SPDX header is found, optinally allowing to print an histogram
about that;

Patch 6 adds a pedantic^Wstrict mode to scripts/spdxcheck.py,
making it to also check for violations at the line with contains the
SPDX header.

PS.: I sent already a RFC version for those patches along with this
thread:

    https://lore.kernel.org/lkml/b32c2e46b91e7bcda2a9bd140673f06d71b2487a.camel@perches.com/

Mauro Carvalho Chehab (6):
  docs: sphinx: add SPDX header for some sphinx extensions
  tools: perf: fix SPDX header in the light of PEP-263
  tools: intel_pstate_tracer.py: fix SPDX header in the light of PEP-263
  docs: license-rules.txt: cover SPDX headers on Python scripts
  scripts/spdxcheck.py: keep track on what line SPDX header was found
  scripts/spdxcheck.py: check if the line number follows the strict rule

 Documentation/process/license-rules.rst       |  7 ++-
 Documentation/sphinx/kernel_include.py        |  1 +
 Documentation/sphinx/rstFlatTable.py          |  1 +
 scripts/spdxcheck.py                          | 55 +++++++++++++++----
 tools/perf/python/tracepoint.py               |  3 +-
 tools/perf/python/twatch.py                   |  3 +-
 .../intel_pstate_tracer.py                    |  2 +-
 7 files changed, 55 insertions(+), 17 deletions(-)

-- 
2.21.0

[PATCH 1/6] docs: sphinx: add SPDX header for some sphinx extensions

[Mauro Carvalho Chehab]

Those extensions are released under GPLv2, as stated at the
:license: markup tag.

Add the corresponding SPDX tags for such license.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 Documentation/sphinx/kernel_include.py | 1 +
 Documentation/sphinx/rstFlatTable.py   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/sphinx/kernel_include.py b/Documentation/sphinx/kernel_include.py
index f523aa68a36b..7aaea4e31f78 100755
--- a/Documentation/sphinx/kernel_include.py
+++ b/Documentation/sphinx/kernel_include.py
@@ -1,5 +1,6 @@
 #!/usr/bin/env python3
 # -*- coding: utf-8; mode: python -*-
+# SPDX-License-Identifier: GPL-2.0
 # pylint: disable=R0903, C0330, R0914, R0912, E0401
 
 u"""
diff --git a/Documentation/sphinx/rstFlatTable.py b/Documentation/sphinx/rstFlatTable.py
index 2019a55f6b18..15769d01831b 100755
--- a/Documentation/sphinx/rstFlatTable.py
+++ b/Documentation/sphinx/rstFlatTable.py
@@ -1,5 +1,6 @@
 #!/usr/bin/env python3
 # -*- coding: utf-8; mode: python -*-
+# SPDX-License-Identifier: GPL-2.0
 # pylint: disable=C0330, R0903, R0912
 
 u"""
-- 
2.21.0

[PATCH 2/6] tools: perf: fix SPDX header in the light of PEP-263

[Mauro Carvalho Chehab]

As stated at PEP-263, the coding tag should be at the first or
second line. On those two scripts, the tag is at the wrong line.

It also contains a separate e-macs line to tell it to consider
the file as a python one. Merge this with the encoding tag,
using the same coding line that we're using on other python files.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 tools/perf/python/tracepoint.py | 3 +--
 tools/perf/python/twatch.py     | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/perf/python/tracepoint.py b/tools/perf/python/tracepoint.py
index eb76f6516247..b7717b501fd8 100755
--- a/tools/perf/python/tracepoint.py
+++ b/tools/perf/python/tracepoint.py
@@ -1,7 +1,6 @@
 #! /usr/bin/python
+# -*- coding: utf-8; mode: python -*-
 # SPDX-License-Identifier: GPL-2.0
-# -*- python -*-
-# -*- coding: utf-8 -*-
 
 import perf
 
diff --git a/tools/perf/python/twatch.py b/tools/perf/python/twatch.py
index ff87ccf5b708..a95e59373ebd 100755
--- a/tools/perf/python/twatch.py
+++ b/tools/perf/python/twatch.py
@@ -1,7 +1,6 @@
 #! /usr/bin/python
+# -*- coding: utf-8; mode: python -*-
 # SPDX-License-Identifier: GPL-2.0-only
-# -*- python -*-
-# -*- coding: utf-8 -*-
 #   twatch - Experimental use of the perf python interface
 #   Copyright (C) 2011 Arnaldo Carvalho de Melo <acme@redhat.com>
 #
-- 
2.21.0

[PATCH 3/6] tools: intel_pstate_tracer.py: fix SPDX header in the light of PEP-263

[Mauro Carvalho Chehab]

As stated at PEP-263, the coding tag should be at the first or
second line. On those two scripts, the tag is at the wrong line.

Place it at the right place and use the same kind of line that we're
using on other python scripts that also work fine with emacs.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
index 2d6d342b148f..1009be489f9a 100755
--- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
+++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
@@ -1,6 +1,6 @@
 #!/usr/bin/python
+# -*- coding: utf-8; mode: python -*-
 # SPDX-License-Identifier: GPL-2.0-only
-# -*- coding: utf-8 -*-
 #
 """ This utility can be used to debug and tune the performance of the
 intel_pstate driver. This utility can be used in two ways:
-- 
2.21.0

[PATCH 4/6] docs: license-rules.txt: cover SPDX headers on Python scripts

[Mauro Carvalho Chehab]

The author of the license-rules.rst file wanted to be very restrict
with regards to the location of the SPDX header. It says that
the SPDX header "shall be added at the first  possible line in
a file which can contain a comment". Not happy with this already
restrictive requiement, it goes further:

"For the majority  of files this is the first line, except for
scripts", opening an exception to have the SPDX header at the
second line, if the first line starts with "#!".

Well, it turns that this is too restrictive for Python scripts,
and may cause regressions if this would be enforced.

As mentioned on:
	https://stackoverflow.com/questions/728891/correct-way-to-define-python-source-code-encoding

Python's PEP-263 [1] dictates that an script that needs to default to
UTF-8 encoding has to follow this rule:

	'Python will default to ASCII as standard encoding if no other
	 encoding hints are given.

	 To define a source code encoding, a magic comment must be placed
	 into the source files either as first or second line in the file'

And:
	'More precisely, the first or second line must match the following
	 regular expression:

	 ^[ \t\f]*#.*?coding[:=][ \t]*([-_.a-zA-Z0-9]+)'

[1] https://www.python.org/dev/peps/pep-0263/

If a script has both "#!" and the charset encoding line, we can't place
a SPDX tag without either violating license-rules.rst or breaking the
script by making it crash with non-ASCII characters.

So, add a sort notice saying that, for Python scripts, the SPDX
header may be up to the third line, in order to cover the case
where both "#!" and "# .*coding.*UTF-8" lines are found.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 Documentation/process/license-rules.rst | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/process/license-rules.rst b/Documentation/process/license-rules.rst
index 2ef44ada3f11..5d23e3498b1c 100644
--- a/Documentation/process/license-rules.rst
+++ b/Documentation/process/license-rules.rst
@@ -64,9 +64,12 @@ License identifier syntax
    possible line in a file which can contain a comment.  For the majority
    of files this is the first line, except for scripts which require the
    '#!PATH_TO_INTERPRETER' in the first line.  For those scripts the SPDX
-   identifier goes into the second line.
+   identifier goes into the second line\ [1]_.
 
-|
+.. [1] Please notice that Python scripts may also need an encoding rule
+   as defined on PEP-263, which should be defined either at the first
+   or the second line. So, for such scripts, the SPDX identifier may
+   go up to the third line.
 
 2. Style:
 
-- 
2.21.0

[PATCH 5/6] scripts/spdxcheck.py: keep track on what line SPDX header was found

[Mauro Carvalho Chehab]

According with Documentation/process/license-rules.rst, SPDX headers
can be found only at the first lines.

However, this script doesn't enforce it, and several files violate
that. It could be useful to be able to show a histogram with the
number of files that have the SPDX header on each line number.

This feature is optional, enabled with -H or --histogram.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 scripts/spdxcheck.py | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py
index 6374e078a5f2..c969b050366f 100755
--- a/scripts/spdxcheck.py
+++ b/scripts/spdxcheck.py
@@ -189,7 +189,9 @@ class id_parser(object):
                 # Should we check for more SPDX ids in the same file and
                 # complain if there are any?
                 #
-                break
+                return self.curline - 1
+
+            return -1
 
         except ParserException as pe:
             if pe.tok:
@@ -200,7 +202,7 @@ class id_parser(object):
                 sys.stdout.write('%s: %d:0 %s\n' %(fname, self.curline, col, pe.txt))
             self.spdx_errors += 1
 
-def scan_git_tree(tree):
+def scan_git_tree(ln_count, tree):
     for el in tree.traverse():
         # Exclude stuff which would make pointless noise
         # FIXME: Put this somewhere more sensible
@@ -211,12 +213,15 @@ def scan_git_tree(tree):
         if not os.path.isfile(el.path):
             continue
         with open(el.path, 'rb') as fd:
-            parser.parse_lines(fd, args.maxlines, el.path)
+            ln = parser.parse_lines(fd, args.maxlines, el.path)
+            if ln >= 0:
+                ln_count[ln] += 1;
+    return ln_count
 
-def scan_git_subtree(tree, path):
+def scan_git_subtree(ln_count, tree, path):
     for p in path.strip('/').split('/'):
         tree = tree[p]
-    scan_git_tree(tree)
+    scan_git_tree(ln_count, tree)
 
 if __name__ == '__main__':
 
@@ -225,6 +230,7 @@ if __name__ == '__main__':
     ap.add_argument('-m', '--maxlines', type=int, default=15,
                     help='Maximum number of lines to scan in a file. Default 15')
     ap.add_argument('-v', '--verbose', action='store_true', help='Verbose statistics output')
+    ap.add_argument('-H', '--histogram', action='store_true', help='Verbose histogram about SPDX header position')
     args = ap.parse_args()
 
     # Sanity check path arguments
@@ -255,23 +261,31 @@ if __name__ == '__main__':
         sys.stderr.write('%s\n' %traceback.format_exc())
         sys.exit(1)
 
+    ln_count= [0] * args.maxlines
+
     try:
         if len(args.path) and args.path[0] == '-':
             stdin = os.fdopen(sys.stdin.fileno(), 'rb')
-            parser.parse_lines(stdin, args.maxlines, '-')
+            ln = parser.parse_lines(stdin, args.maxlines, '-')
+            if ln >= 0:
+                ln_count[ln] += 1;
+
         else:
             if args.path:
                 for p in args.path:
                     if os.path.isfile(p):
-                        parser.parse_lines(open(p, 'rb'), args.maxlines, p)
+                        ln = parser.parse_lines(open(p, 'rb'), args.maxlines, p)
+                        if ln >= 0:
+                            ln_count[ln] += 1;
+
                     elif os.path.isdir(p):
-                        scan_git_subtree(repo.head.reference.commit.tree, p)
+                        scan_git_subtree(ln_count, repo.head.reference.commit.tree, p)
                     else:
                         sys.stderr.write('path %s does not exist\n' %p)
                         sys.exit(1)
             else:
                 # Full git tree scan
-                scan_git_tree(repo.head.commit.tree)
+                scan_git_tree(ln_count, repo.head.commit.tree)
 
             if args.verbose:
                 sys.stderr.write('\n')
@@ -284,6 +298,11 @@ if __name__ == '__main__':
                 sys.stderr.write('Lines checked:     %12d\n' %parser.lines_checked)
                 sys.stderr.write('Files with SPDX:   %12d\n' %parser.spdx_valid)
                 sys.stderr.write('Files with errors: %12d\n' %parser.spdx_errors)
+                sys.stderr.write('\n')
+            if args.histogram:
+                for i in range(0, len(ln_count)):
+                    if ln_count[i] > 0:
+                        sys.stderr.write('Files with SPDX at line #%-5d:   %12d\n' % (i + 1, ln_count[i]))
 
             sys.exit(0)
 
-- 
2.21.0

[PATCH 6/6] scripts/spdxcheck.py: check if the line number follows the strict rule

[Mauro Carvalho Chehab]

There is a very strict rule saying on what line a SPDX header
should be. Add an optional pedantic check for it.

When the check is enabled, it will verify if the file has the
SPDX header "at the first possible line in a file which can contain
a comment", as stated at:

	Documentation/process/license-rules.rst

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 scripts/spdxcheck.py | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py
index c969b050366f..f3260391f091 100755
--- a/scripts/spdxcheck.py
+++ b/scripts/spdxcheck.py
@@ -164,9 +164,15 @@ class id_parser(object):
         self.lastid = None
         self.parser.parse(expr, lexer = self.lexer)
 
-    def parse_lines(self, fd, maxlines, fname):
+    def parse_lines(self, fd, maxlines, fname, strict):
         self.checked += 1
         self.curline = 0
+        self.max_line = 1
+        self.is_python = False
+
+        if fname.find("COPYING") >= 0:
+            self.max_line = maxlines
+
         try:
             for line in fd:
                 line = line.decode(locale.getpreferredencoding(False), errors='ignore')
@@ -174,6 +180,13 @@ class id_parser(object):
                 if self.curline > maxlines:
                     break
                 self.lines_checked += 1
+                if self.curline == 1:
+		    if re.match("\#\!", line):
+                        self.max_line = 2
+			if re.match("\#\!.*python", line):
+			    is_python = True
+                if self.curline == 2 and self.is_python and re.match("^[ \t\f]*#.*?coding[:=][ \t]*([-_.a-zA-Z0-9]+)", line):
+                        self.max_line = 3
                 if line.find("SPDX-License-Identifier:") < 0:
                     continue
                 expr = line.split(':')[1].strip()
@@ -189,6 +202,8 @@ class id_parser(object):
                 # Should we check for more SPDX ids in the same file and
                 # complain if there are any?
                 #
+                if strict and self.curline > self.max_line:
+                    sys.stderr.write('Warning: SPDX header for file %s is at line %d\n' % (fname,self.curline))
                 return self.curline - 1
 
             return -1
@@ -202,7 +217,7 @@ class id_parser(object):
                 sys.stdout.write('%s: %d:0 %s\n' %(fname, self.curline, col, pe.txt))
             self.spdx_errors += 1
 
-def scan_git_tree(ln_count, tree):
+def scan_git_tree(ln_count, tree, strict):
     for el in tree.traverse():
         # Exclude stuff which would make pointless noise
         # FIXME: Put this somewhere more sensible
@@ -213,15 +228,15 @@ def scan_git_tree(ln_count, tree):
         if not os.path.isfile(el.path):
             continue
         with open(el.path, 'rb') as fd:
-            ln = parser.parse_lines(fd, args.maxlines, el.path)
+            ln = parser.parse_lines(fd, args.maxlines, el.path, strict)
             if ln >= 0:
                 ln_count[ln] += 1;
     return ln_count
 
-def scan_git_subtree(ln_count, tree, path):
+def scan_git_subtree(ln_count, tree, path, strict):
     for p in path.strip('/').split('/'):
         tree = tree[p]
-    scan_git_tree(ln_count, tree)
+    scan_git_tree(ln_count, tree, strict)
 
 if __name__ == '__main__':
 
@@ -231,6 +246,7 @@ if __name__ == '__main__':
                     help='Maximum number of lines to scan in a file. Default 15')
     ap.add_argument('-v', '--verbose', action='store_true', help='Verbose statistics output')
     ap.add_argument('-H', '--histogram', action='store_true', help='Verbose histogram about SPDX header position')
+    ap.add_argument('-s', '--strict', action='store_true', help='Enable strict mode, making it complain about SPDX line position')
     args = ap.parse_args()
 
     # Sanity check path arguments
@@ -266,7 +282,7 @@ if __name__ == '__main__':
     try:
         if len(args.path) and args.path[0] == '-':
             stdin = os.fdopen(sys.stdin.fileno(), 'rb')
-            ln = parser.parse_lines(stdin, args.maxlines, '-')
+            ln = parser.parse_lines(stdin, args.maxlines, '-', args.strict)
             if ln >= 0:
                 ln_count[ln] += 1;
 
@@ -274,18 +290,18 @@ if __name__ == '__main__':
             if args.path:
                 for p in args.path:
                     if os.path.isfile(p):
-                        ln = parser.parse_lines(open(p, 'rb'), args.maxlines, p)
+                        ln = parser.parse_lines(open(p, 'rb'), args.maxlines, p, args.strict)
                         if ln >= 0:
                             ln_count[ln] += 1;
 
                     elif os.path.isdir(p):
-                        scan_git_subtree(ln_count, repo.head.reference.commit.tree, p)
+                        scan_git_subtree(ln_count, repo.head.reference.commit.tree, p, args.strict)
                     else:
                         sys.stderr.write('path %s does not exist\n' %p)
                         sys.exit(1)
             else:
                 # Full git tree scan
-                scan_git_tree(ln_count, repo.head.commit.tree)
+                scan_git_tree(ln_count, repo.head.commit.tree, args.strict)
 
             if args.verbose:
                 sys.stderr.write('\n')
-- 
2.21.0

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Joe Perches]

On Thu, 2019-09-05 at 16:57 -0300, Mauro Carvalho Chehab wrote:
> The  description at Documentation/process/license-rules.rst is very strict
> with regards to the position where the SPDX tags should be.
[]
> PS.: I sent already a RFC version for those patches along with this
> thread:
> 
>     https://lore.kernel.org/lkml/b32c2e46b91e7bcda2a9bd140673f06d71b2487a.camel@perches.com/

Nice, thanks.

Now I guess I have to update checkpatch too.
(unless you want to... ;)

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Mauro Carvalho Chehab]

Em Thu, 05 Sep 2019 13:05:08 -0700
Joe Perches <joe@perches.com> escreveu:

> On Thu, 2019-09-05 at 16:57 -0300, Mauro Carvalho Chehab wrote:
> > The  description at Documentation/process/license-rules.rst is very strict
> > with regards to the position where the SPDX tags should be.  
> []
> > PS.: I sent already a RFC version for those patches along with this
> > thread:
> > 
> >     https://lore.kernel.org/lkml/b32c2e46b91e7bcda2a9bd140673f06d71b2487a.camel@perches.com/  
> 
> Nice, thanks.
> 
> Now I guess I have to update checkpatch too.
> (unless you want to... ;)

Yeah, it makes sense to update checkpatch too. Perhaps it could be
changed to use the "-s" flag on the check, instead of implementing
its own logic to handle the position.

Thanks,
Mauro

[PATCH v2 6/6] scripts/spdxcheck.py: check if the line number follows the strict rule

[Mauro Carvalho Chehab]

There is a very strict rule saying on what line a SPDX header
should be. Add an optional pedantic check for it.

When the check is enabled, it will verify if the file has the
SPDX header "at the first possible line in a file which can contain
a comment", as stated at:

	Documentation/process/license-rules.rst

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py
index c969b050366f..920cceb0d036 100755
--- a/scripts/spdxcheck.py
+++ b/scripts/spdxcheck.py
@@ -164,9 +164,18 @@ class id_parser(object):
         self.lastid = None
         self.parser.parse(expr, lexer = self.lexer)
 
-    def parse_lines(self, fd, maxlines, fname):
+    def parse_lines(self, fd, maxlines, fname, strict):
         self.checked += 1
         self.curline = 0
+        self.max_line = 1
+        self.is_python = False
+
+        if fname.endswith(".py"):
+            self.is_python = True
+
+        if fname.find("COPYING") >= 0:
+            self.max_line = maxlines
+
         try:
             for line in fd:
                 line = line.decode(locale.getpreferredencoding(False), errors='ignore')
@@ -174,6 +183,13 @@ class id_parser(object):
                 if self.curline > maxlines:
                     break
                 self.lines_checked += 1
+                if self.curline == 1:
+		    if re.match("\#\!", line):
+                        self.max_line = 2
+			if re.match("\#\!.*python", line):
+			    is_python = True
+                if self.curline <= 2 and self.is_python and re.match("^[ \t\f]*#.*?coding[:=][ \t]*([-_.a-zA-Z0-9]+)", line):
+                        self.max_line += 1
                 if line.find("SPDX-License-Identifier:") < 0:
                     continue
                 expr = line.split(':')[1].strip()
@@ -189,6 +205,8 @@ class id_parser(object):
                 # Should we check for more SPDX ids in the same file and
                 # complain if there are any?
                 #
+                if strict and self.curline > self.max_line:
+                    sys.stderr.write('Warning: SPDX header for file %s is at line %d\n' % (fname,self.curline))
                 return self.curline - 1
 
             return -1
@@ -202,7 +220,7 @@ class id_parser(object):
                 sys.stdout.write('%s: %d:0 %s\n' %(fname, self.curline, col, pe.txt))
             self.spdx_errors += 1
 
-def scan_git_tree(ln_count, tree):
+def scan_git_tree(ln_count, tree, strict):
     for el in tree.traverse():
         # Exclude stuff which would make pointless noise
         # FIXME: Put this somewhere more sensible
@@ -213,15 +231,15 @@ def scan_git_tree(ln_count, tree):
         if not os.path.isfile(el.path):
             continue
         with open(el.path, 'rb') as fd:
-            ln = parser.parse_lines(fd, args.maxlines, el.path)
+            ln = parser.parse_lines(fd, args.maxlines, el.path, strict)
             if ln >= 0:
                 ln_count[ln] += 1;
     return ln_count
 
-def scan_git_subtree(ln_count, tree, path):
+def scan_git_subtree(ln_count, tree, path, strict):
     for p in path.strip('/').split('/'):
         tree = tree[p]
-    scan_git_tree(ln_count, tree)
+    scan_git_tree(ln_count, tree, strict)
 
 if __name__ == '__main__':
 
@@ -231,6 +249,7 @@ if __name__ == '__main__':
                     help='Maximum number of lines to scan in a file. Default 15')
     ap.add_argument('-v', '--verbose', action='store_true', help='Verbose statistics output')
     ap.add_argument('-H', '--histogram', action='store_true', help='Verbose histogram about SPDX header position')
+    ap.add_argument('-s', '--strict', action='store_true', help='Enable strict mode, making it complain about SPDX line position')
     args = ap.parse_args()
 
     # Sanity check path arguments
@@ -266,7 +285,7 @@ if __name__ == '__main__':
     try:
         if len(args.path) and args.path[0] == '-':
             stdin = os.fdopen(sys.stdin.fileno(), 'rb')
-            ln = parser.parse_lines(stdin, args.maxlines, '-')
+            ln = parser.parse_lines(stdin, args.maxlines, '-', args.strict)
             if ln >= 0:
                 ln_count[ln] += 1;
 
@@ -274,18 +293,18 @@ if __name__ == '__main__':
             if args.path:
                 for p in args.path:
                     if os.path.isfile(p):
-                        ln = parser.parse_lines(open(p, 'rb'), args.maxlines, p)
+                        ln = parser.parse_lines(open(p, 'rb'), args.maxlines, p, args.strict)
                         if ln >= 0:
                             ln_count[ln] += 1;
 
                     elif os.path.isdir(p):
-                        scan_git_subtree(ln_count, repo.head.reference.commit.tree, p)
+                        scan_git_subtree(ln_count, repo.head.reference.commit.tree, p, args.strict)
                     else:
                         sys.stderr.write('path %s does not exist\n' %p)
                         sys.exit(1)
             else:
                 # Full git tree scan
-                scan_git_tree(ln_count, repo.head.commit.tree)
+                scan_git_tree(ln_count, repo.head.commit.tree, args.strict)
 
             if args.verbose:
                 sys.stderr.write('\n')

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Joe Perches]

On Fri, 2019-09-06 at 09:02 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 05 Sep 2019 13:05:08 -0700
> Joe Perches <joe@perches.com> escreveu:
> 
> > On Thu, 2019-09-05 at 16:57 -0300, Mauro Carvalho Chehab wrote:
> > > The  description at Documentation/process/license-rules.rst is very strict
> > > with regards to the position where the SPDX tags should be.  
> > []
> > > PS.: I sent already a RFC version for those patches along with this
> > > thread:
> > > 
> > >     https://lore.kernel.org/lkml/b32c2e46b91e7bcda2a9bd140673f06d71b2487a.camel@perches.com/  
> > 
> > Nice, thanks.
> > 
> > Now I guess I have to update checkpatch too.
> > (unless you want to... ;)
> 
> Yeah, it makes sense to update checkpatch too. Perhaps it could be
> changed to use the "-s" flag on the check, instead of implementing
> its own logic to handle the position.

I think checkpatch needs its own logic as its
source input is a patch file.

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Jonathan Corbet]

On Thu,  5 Sep 2019 16:57:47 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:

> The  description at Documentation/process/license-rules.rst is very strict
> with regards to the position where the SPDX tags should be.
> 
> In the past several developers and maintainers interpreted it on a
> more permissive way, placing the SPDX header between lines 1 to 15, 
> with are the ones which the  scripts/spdxcheck.py script verifies.
> 
> However, recently, devs are becoming more strict about such
> requirement and want it to strictly follow the rule, with states that
> the SPDX rule should be at the first line ever on most files, and
> at the second line for scripts.
> 
> Well, for Python script, such requirement causes violation to PEP-263, 
> making regressions on scripts that contain encoding lines, as PEP-263
> also states about the same.
> 
> This series addresses it.

So I really don't want to be overly difficult here, but I would like to
approach this from yet another angle...

> Patches 1 to 3 fix some Python scripts that violates PEP-263;

I just checked all of those scripts, and they are all just plain ASCII.
So it really doesn't matter whether the environment defaults to UTF-8 or
ASCII here.  So, in other words, we really shouldn't need to define the
encoding at all.

This suggests to me that we're adding a bunch of complications that we
don't necessarily need.  What am I missing here?

Educate me properly and I'll not try to stand in the way of all this...

Thanks,

jon

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Markus Heiser]

Am 07.09.19 um 15:34 schrieb Jonathan Corbet:
> On Thu,  5 Sep 2019 16:57:47 -0300
> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> 
>> The  description at Documentation/process/license-rules.rst is very strict
>> with regards to the position where the SPDX tags should be.
>>
>> In the past several developers and maintainers interpreted it on a
>> more permissive way, placing the SPDX header between lines 1 to 15,
>> with are the ones which the  scripts/spdxcheck.py script verifies.
>>
>> However, recently, devs are becoming more strict about such
>> requirement and want it to strictly follow the rule, with states that
>> the SPDX rule should be at the first line ever on most files, and
>> at the second line for scripts.
>>
>> Well, for Python script, such requirement causes violation to PEP-263,
>> making regressions on scripts that contain encoding lines, as PEP-263
>> also states about the same.
>>
>> This series addresses it.
> 
> So I really don't want to be overly difficult here, but I would like to
> approach this from yet another angle...
> 
>> Patches 1 to 3 fix some Python scripts that violates PEP-263;
> 
> I just checked all of those scripts, and they are all just plain ASCII.
> So it really doesn't matter whether the environment defaults to UTF-8 or
> ASCII here.  So, in other words, we really shouldn't need to define the
> encoding at all.
> 

Thats what I mean [1] .. lets patch the description in the license-rules.rst::

- first line for the OS (shebang)
- second line for environment (python-encoding, editor-mode, ...)
- third and more lines for application (SPDX use) ..

[1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg33240.html

-- Markus --

> This suggests to me that we're adding a bunch of complications that we
> don't necessarily need.  What am I missing here?
> 
> Educate me properly and I'll not try to stand in the way of all this...
> 
> Thanks,
> 
> jon
> 

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Mauro Carvalho Chehab]

Em Sat, 7 Sep 2019 16:36:36 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> Am 07.09.19 um 15:34 schrieb Jonathan Corbet:
> > On Thu,  5 Sep 2019 16:57:47 -0300
> > Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> >   
> >> The  description at Documentation/process/license-rules.rst is very strict
> >> with regards to the position where the SPDX tags should be.
> >>
> >> In the past several developers and maintainers interpreted it on a
> >> more permissive way, placing the SPDX header between lines 1 to 15,
> >> with are the ones which the  scripts/spdxcheck.py script verifies.
> >>
> >> However, recently, devs are becoming more strict about such
> >> requirement and want it to strictly follow the rule, with states that
> >> the SPDX rule should be at the first line ever on most files, and
> >> at the second line for scripts.
> >>
> >> Well, for Python script, such requirement causes violation to PEP-263,
> >> making regressions on scripts that contain encoding lines, as PEP-263
> >> also states about the same.
> >>
> >> This series addresses it.  
> > 
> > So I really don't want to be overly difficult here, but I would like to
> > approach this from yet another angle...
> >   
> >> Patches 1 to 3 fix some Python scripts that violates PEP-263;  
> > 
> > I just checked all of those scripts, and they are all just plain ASCII.
> > So it really doesn't matter whether the environment defaults to UTF-8 or
> > ASCII here.  So, in other words, we really shouldn't need to define the
> > encoding at all.

I'm not a python expert, but, from what I researched, and from what I 
understood from Markus, if a script tries to print an UTF-8 but the
system's encoding is ASCII (or some other encoding), the python script
will crash.

At least on media, we define that some Kernel strings can be UTF-8. 
See, for example the model field at the media_entity struct:

	https://linuxtv.org/downloads/v4l-dvb-apis/kapi/mc-core.html

As stated there:

	"media_entity.model must be filled with the device model name as
	 a NUL-terminated UTF-8 string. The device/model revision must
	 not be stored in this field."

I've no idea if the two perf scripts that contain the encoding data are
meant to print some strings that may be UTF-8 encoding (like those that
we have at the media subsystem), or if it is just that whomever added
were using e-macs and wanted to make his life simpler. As it is better
to be safe then sorry, on patches 2 and 3, I'm assuming the first case.

In any case, we do need the encoding line at Sphinx extensions, 
although there, the shebang line is optional.

In other words, we have those alternatives:

1) Neither shebang nor coding -> SPDX will be at first line;
2) shebang + SPDX -> SPDX will be at the second line;
3) shebang + coding + SPDX -> SPDX will be at the third line;
4) coding + SPDX

   This is something that only makes sense for Sphinx extensions.

   IMHO, I would place SPDX at the second line too, but I *guess* Python
   may accept it at the first line and would still properly evaluate
   coding (as this technically satisfies the text at PEP-263).

> >   
> 
> Thats what I mean [1] .. lets patch the description in the license-rules.rst::
> 
> - first line for the OS (shebang)
> - second line for environment (python-encoding, editor-mode, ...)
> - third and more lines for application (SPDX use) ..
> 
> [1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg33240.html
> 
> -- Markus --
> 
> > This suggests to me that we're adding a bunch of complications that we
> > don't necessarily need.  What am I missing here?
> > 
> > Educate me properly and I'll not try to stand in the way of all this...
> > 
> > Thanks,
> > 
> > jon
> >   



Thanks,
Mauro

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Markus Heiser]

Am 07.09.19 um 18:22 schrieb Mauro Carvalho Chehab:
> Em Sat, 7 Sep 2019 16:36:36 +0200
> Markus Heiser <markus.heiser@darmarit.de> escreveu:
> 
>> Am 07.09.19 um 15:34 schrieb Jonathan Corbet:
>>> On Thu,  5 Sep 2019 16:57:47 -0300
>>> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
>>>    
>>>> The  description at Documentation/process/license-rules.rst is very strict
>>>> with regards to the position where the SPDX tags should be.
>>>>
>>>> In the past several developers and maintainers interpreted it on a
>>>> more permissive way, placing the SPDX header between lines 1 to 15,
>>>> with are the ones which the  scripts/spdxcheck.py script verifies.
>>>>
>>>> However, recently, devs are becoming more strict about such
>>>> requirement and want it to strictly follow the rule, with states that
>>>> the SPDX rule should be at the first line ever on most files, and
>>>> at the second line for scripts.
>>>>
>>>> Well, for Python script, such requirement causes violation to PEP-263,
>>>> making regressions on scripts that contain encoding lines, as PEP-263
>>>> also states about the same.
>>>>
>>>> This series addresses it.
>>>
>>> So I really don't want to be overly difficult here, but I would like to
>>> approach this from yet another angle...
>>>    
>>>> Patches 1 to 3 fix some Python scripts that violates PEP-263;
>>>
>>> I just checked all of those scripts, and they are all just plain ASCII.
>>> So it really doesn't matter whether the environment defaults to UTF-8 or
>>> ASCII here.  So, in other words, we really shouldn't need to define the
>>> encoding at all.
> 
> I'm not a python expert, but, from what I researched, and from what I
> understood from Markus, if a script tries to print an UTF-8 but the
> system's encoding is ASCII (or some other encoding), the python script
> will crash.

An (uncatched) exception is thrown, when writing UTF-8 to a stream which
do not support UTF-8 .. this is not a crash, it mostly indicates that the
developper makes some wrong assumption about the use-case.  There exists
also the possibility to encode the UTF-8 to ASCII and replace unknown
code points in the out-stream, or to catch the exception.

But this was only academical, where do we have such problems in practice?

> At least on media, we define that some Kernel strings can be UTF-8.
> See, for example the model field at the media_entity struct:
> 
> 	https://linuxtv.org/downloads/v4l-dvb-apis/kapi/mc-core.html
> 
> As stated there:
> 
> 	"media_entity.model must be filled with the device model name as
> 	 a NUL-terminated UTF-8 string. The device/model revision must
> 	 not be stored in this field."
> 
> I've no idea if the two perf scripts that contain the encoding data are
> meant to print some strings that may be UTF-8 encoding (like those that
> we have at the media subsystem), or if it is just that whomever added
> were using e-macs and wanted to make his life simpler. As it is better
> to be safe then sorry, on patches 2 and 3, I'm assuming the first case.

Hm, I'am unsure if I understand you correct: Using UTF-8 in the .rst
files are fine .. where do we have scripts generating UTF-8 outputs?
(except the HTML output).

> 
> In any case, we do need the encoding line at Sphinx extensions,
> although there, the shebang line is optional.
> 
> In other words, we have those alternatives:
> 
> 1) Neither shebang nor coding -> SPDX will be at first line;
> 2) shebang + SPDX -> SPDX will be at the second line;
> 3) shebang + coding + SPDX -> SPDX will be at the third line;
> 4) coding + SPDX
> 
>     This is something that only makes sense for Sphinx extensions.
> 
>     IMHO, I would place SPDX at the second line too, but I *guess* Python
>     may accept it at the first line and would still properly evaluate
>     coding (as this technically satisfies the text at PEP-263).

Why you are so restrictive .. what we normal do:

- write a shebang line if this file is called directly from the
   command line .. but we do not need shebangs on py modules which
   are imported from other modules or scripts

- write a encoding line if it is need or helpful / mostly it is helpful
   to know the encoding of a text/code file.

- add a SPDX tag

At the end we will have files with one, two or all three of this lines.
And the oder of this lines is, what I wrote:

>>
>> Thats what I mean [1] .. lets patch the description in the license-rules.rst::
>>
>> - first line for the OS (shebang)
>> - second line for environment (python-encoding, editor-mode, ...)
>> - third and more lines for application (SPDX use) ..
>>
>> [1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg33240.html
>>
>> -- Markus --
>>
>>> This suggests to me that we're adding a bunch of complications that we
>>> don't necessarily need.  What am I missing here?
>>>
>>> Educate me properly and I'll not try to stand in the way of all this...
>>>


It seems like it is not only me who is mising something .. what are
the use-cases we have py-Exceptions, what are the use-cases to be so
restrictive as you described above.

.. or did alice get lost in the cave?

Thanks for your patience with me

-- Markus --

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Mauro Carvalho Chehab]

Em Sat, 7 Sep 2019 19:33:06 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> Am 07.09.19 um 18:22 schrieb Mauro Carvalho Chehab:
> > Em Sat, 7 Sep 2019 16:36:36 +0200
> > Markus Heiser <markus.heiser@darmarit.de> escreveu:
> >   
> >> Am 07.09.19 um 15:34 schrieb Jonathan Corbet:  
> >>> On Thu,  5 Sep 2019 16:57:47 -0300
> >>> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> >>>      
> >>>> The  description at Documentation/process/license-rules.rst is very strict
> >>>> with regards to the position where the SPDX tags should be.
> >>>>
> >>>> In the past several developers and maintainers interpreted it on a
> >>>> more permissive way, placing the SPDX header between lines 1 to 15,
> >>>> with are the ones which the  scripts/spdxcheck.py script verifies.
> >>>>
> >>>> However, recently, devs are becoming more strict about such
> >>>> requirement and want it to strictly follow the rule, with states that
> >>>> the SPDX rule should be at the first line ever on most files, and
> >>>> at the second line for scripts.
> >>>>
> >>>> Well, for Python script, such requirement causes violation to PEP-263,
> >>>> making regressions on scripts that contain encoding lines, as PEP-263
> >>>> also states about the same.
> >>>>
> >>>> This series addresses it.  
> >>>
> >>> So I really don't want to be overly difficult here, but I would like to
> >>> approach this from yet another angle...
> >>>      
> >>>> Patches 1 to 3 fix some Python scripts that violates PEP-263;  
> >>>
> >>> I just checked all of those scripts, and they are all just plain ASCII.
> >>> So it really doesn't matter whether the environment defaults to UTF-8 or
> >>> ASCII here.  So, in other words, we really shouldn't need to define the
> >>> encoding at all.  
> > 
> > I'm not a python expert, but, from what I researched, and from what I
> > understood from Markus, if a script tries to print an UTF-8 but the
> > system's encoding is ASCII (or some other encoding), the python script
> > will crash.  
> 
> An (uncatched) exception is thrown, when writing UTF-8 to a stream which
> do not support UTF-8 .. this is not a crash, it mostly indicates that the
> developper makes some wrong assumption about the use-case.

A not-handled exception is a crash in Python. I've seen python scripts
crash countless times with non-English names.

> There exists
> also the possibility to encode the UTF-8 to ASCII and replace unknown
> code points in the out-stream, or to catch the exception.

Yeah, but getting this right is very painful. I use patchwork since 2013.
It took *years* for it to not crash with non-ASCII chars[1]. That's, btw,
the primary reason why I don't usually use python: with other languages,
an alien char doesn't cause a crash.

[1] I might be wrong, but the last patch I saw addressing an issue
    there was applied this year.

> 
> But this was only academical, where do we have such problems in practice?
> 
> > At least on media, we define that some Kernel strings can be UTF-8.
> > See, for example the model field at the media_entity struct:
> > 
> > 	https://linuxtv.org/downloads/v4l-dvb-apis/kapi/mc-core.html
> > 
> > As stated there:
> > 
> > 	"media_entity.model must be filled with the device model name as
> > 	 a NUL-terminated UTF-8 string. The device/model revision must
> > 	 not be stored in this field."
> > 
> > I've no idea if the two perf scripts that contain the encoding data are
> > meant to print some strings that may be UTF-8 encoding (like those that
> > we have at the media subsystem), or if it is just that whomever added
> > were using e-macs and wanted to make his life simpler. As it is better
> > to be safe then sorry, on patches 2 and 3, I'm assuming the first case.  
> 
> Hm, I'am unsure if I understand you correct: Using UTF-8 in the .rst
> files are fine .. where do we have scripts generating UTF-8 outputs?
> (except the HTML output).

In thesis, perf scripts may be reading strings from the Kernel, with 
might be using UTF-8 encoding.

> 
> > 
> > In any case, we do need the encoding line at Sphinx extensions,
> > although there, the shebang line is optional.
> > 
> > In other words, we have those alternatives:
> > 
> > 1) Neither shebang nor coding -> SPDX will be at first line;
> > 2) shebang + SPDX -> SPDX will be at the second line;
> > 3) shebang + coding + SPDX -> SPDX will be at the third line;
> > 4) coding + SPDX
> > 
> >     This is something that only makes sense for Sphinx extensions.
> > 
> >     IMHO, I would place SPDX at the second line too, but I *guess* Python
> >     may accept it at the first line and would still properly evaluate
> >     coding (as this technically satisfies the text at PEP-263).  
> 
> Why you are so restrictive .. 

No idea. I would actually prefer to just remove the restriction, and let
the SPDX header to be anywhere inside the first comment block inside a 
file [2].

That's basically how this thread started: other developers think 
that it is a good idea to be pedantic. So, be it, but let's then fix
the documentation, as the way it is, it is implicitly forbidding the
addition of encoding lines for Python scripts.

[2] I *suspect* that the restriction was added in order to make
    ./scripts/spdxcheck.py to run faster and to avoid false positives.
    Right now, if the maximum limit is removed (or set to a very high
    value), there will be one false positive:

	Documentation/dev-tools/kselftest.rst

    This doc has a SPDX-like tag at line 230, asking people to add SPDX
    headers on files, but the file itself doesn't have its own SPDX tag. 
	
> what we normal do:
> 
> - write a shebang line if this file is called directly from the
>    command line .. but we do not need shebangs on py modules which
>    are imported from other modules or scripts
> 
> - write a encoding line if it is need or helpful / mostly it is helpful
>    to know the encoding of a text/code file.
> 
> - add a SPDX tag

Yes, but this violates the current documentation, as it doesn't allow the
SPDX tag after line #2.

> 
> At the end we will have files with one, two or all three of this lines.
> And the oder of this lines is, what I wrote:
> 
> >>
> >> Thats what I mean [1] .. lets patch the description in the license-rules.rst::
> >>
> >> - first line for the OS (shebang)
> >> - second line for environment (python-encoding, editor-mode, ...)
> >> - third and more lines for application (SPDX use) ..
> >>
> >> [1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg33240.html
> >>
> >> -- Markus --
> >>  
> >>> This suggests to me that we're adding a bunch of complications that we
> >>> don't necessarily need.  What am I missing here?
> >>>
> >>> Educate me properly and I'll not try to stand in the way of all this...
> >>>  
> 
> 
> It seems like it is not only me who is mising something .. what are
> the use-cases we have py-Exceptions, what are the use-cases to be so
> restrictive as you described above.
> 
> .. or did alice get lost in the cave?
> 
> Thanks for your patience with me
> 
> -- Markus --



Thanks,
Mauro

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Markus Heiser]

Am 07.09.19 um 20:04 schrieb Mauro Carvalho Chehab:
> Em Sat, 7 Sep 2019 19:33:06 +0200
> Markus Heiser <markus.heiser@darmarit.de> escreveu:
>> An (uncatched) exception is thrown, when writing UTF-8 to a stream which
>> do not support UTF-8 .. this is not a crash, it mostly indicates that the
>> developper makes some wrong assumption about the use-case.
> 
> A not-handled exception is a crash in Python. I've seen python scripts
> crash countless times with non-English names.

This has nothing to do with the language, ask the developer of those scripts.

>> There exists
>> also the possibility to encode the UTF-8 to ASCII and replace unknown
>> code points in the out-stream, or to catch the exception.
> 
> Yeah, but getting this right is very painful. I use patchwork since 2013.
> It took *years* for it to not crash with non-ASCII chars[1]. That's, btw,
> the primary reason why I don't usually use python: with other languages,
> an alien char doesn't cause a crash.

Python cares encoded (text) string-types while other languages and
application are just piping bytes to streams .. if you care about the
enconding you need exceptions when one whants write UTF-8 to ASCII out.

Anyway this is a bit of nitpicking / not helping here ..

> 
> [1] I might be wrong, but the last patch I saw addressing an issue
>      there was applied this year.

I alrady postet an example [1]

<snip>
This means your application has to know the encoding of a stream/file.
E.g. we handle the output from of the external Perl script
scripts/kernel-docs by encoding the byte stream from proc-call's
stdout into utf-8:

    out, err = codecs.decode(out, 'utf-8'), codecs.decode(err, 'utf-8')

see patch 
https://github.com/torvalds/linux/commit/86c0f046a8b0c23fca65f77333c233a06c25ef9a

Again, this is talking about application development and has
nothing to do with the encoding of the source files.
<snap>

[1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg33240.html

>>
>> But this was only academical, where do we have such problems in practice?
>>
>>> At least on media, we define that some Kernel strings can be UTF-8.
>>> See, for example the model field at the media_entity struct:
>>>
>>> 	https://linuxtv.org/downloads/v4l-dvb-apis/kapi/mc-core.html
>>>
>>> As stated there:
>>>
>>> 	"media_entity.model must be filled with the device model name as
>>> 	 a NUL-terminated UTF-8 string. The device/model revision must
>>> 	 not be stored in this field."
>>>
>>> I've no idea if the two perf scripts that contain the encoding data are
>>> meant to print some strings that may be UTF-8 encoding (like those that
>>> we have at the media subsystem), or if it is just that whomever added
>>> were using e-macs and wanted to make his life simpler. As it is better
>>> to be safe then sorry, on patches 2 and 3, I'm assuming the first case.
>>
>> Hm, I'am unsure if I understand you correct: Using UTF-8 in the .rst
>> files are fine .. where do we have scripts generating UTF-8 outputs?
>> (except the HTML output).
> 
> In thesis, perf scripts may be reading strings from the Kernel, with
> might be using UTF-8 encoding.
> 
>>
>>>
>>> In any case, we do need the encoding line at Sphinx extensions,
>>> although there, the shebang line is optional.
>>>
>>> In other words, we have those alternatives:
>>>
>>> 1) Neither shebang nor coding -> SPDX will be at first line;
>>> 2) shebang + SPDX -> SPDX will be at the second line;
>>> 3) shebang + coding + SPDX -> SPDX will be at the third line;
>>> 4) coding + SPDX
>>>
>>>      This is something that only makes sense for Sphinx extensions.
>>>
>>>      IMHO, I would place SPDX at the second line too, but I *guess* Python
>>>      may accept it at the first line and would still properly evaluate
>>>      coding (as this technically satisfies the text at PEP-263).
>>
>> Why you are so restrictive ..
> 
> No idea. I would actually prefer to just remove the restriction, and let
> the SPDX header to be anywhere inside the first comment block inside a
> file [2].
> 
> That's basically how this thread started: other developers think
> that it is a good idea to be pedantic. So, be it, but let's then fix
> the documentation, as the way it is, it is implicitly forbidding the
> addition of encoding lines for Python scripts.
> 
> [2] I *suspect* that the restriction was added in order to make
>      ./scripts/spdxcheck.py to run faster and to avoid false positives.
>      Right now, if the maximum limit is removed (or set to a very high
>      value), there will be one false positive:
> 
> 	Documentation/dev-tools/kselftest.rst
> 
>      This doc has a SPDX-like tag at line 230, asking people to add SPDX
>      headers on files, but the file itself doesn't have its own SPDX tag.
> 	
>> what we normal do:
>>
>> - write a shebang line if this file is called directly from the
>>     command line .. but we do not need shebangs on py modules which
>>     are imported from other modules or scripts
>>
>> - write a encoding line if it is need or helpful / mostly it is helpful
>>     to know the encoding of a text/code file.
>>
>> - add a SPDX tag
> 
> Yes, but this violates the current documentation, as it doesn't allow the
> SPDX tag after line #2.

Thats what I mean: The documentation was written with only a small use-cases
in mind .. there is no real need for SPDX to be in line one or two ... lets
fix the documentation as I described before.

Side note: if I can help you with perf or your build systems, don't hesitate
to contact me directly.

-- Markus --

>> At the end we will have files with one, two or all three of this lines.
>> And the oder of this lines is, what I wrote:
>>
>>>>
>>>> Thats what I mean [1] .. lets patch the description in the license-rules.rst::
>>>>
>>>> - first line for the OS (shebang)
>>>> - second line for environment (python-encoding, editor-mode, ...)
>>>> - third and more lines for application (SPDX use) ..
>>>>
>>>> [1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg33240.html
>>>>
>>>> -- Markus --
>>>>   
>>>>> This suggests to me that we're adding a bunch of complications that we
>>>>> don't necessarily need.  What am I missing here?
>>>>>
>>>>> Educate me properly and I'll not try to stand in the way of all this...
>>>>>   
>>
>>
>> It seems like it is not only me who is mising something .. what are
>> the use-cases we have py-Exceptions, what are the use-cases to be so
>> restrictive as you described above.
>>
>> .. or did alice get lost in the cave?
>>
>> Thanks for your patience with me
>>
>> -- Markus --
> 
> 
> 
> Thanks,
> Mauro
> 

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Thomas Gleixner]

On Sat, 7 Sep 2019, Markus Heiser wrote:
> Am 07.09.19 um 20:04 schrieb Mauro Carvalho Chehab:
> > No idea. I would actually prefer to just remove the restriction, and let
> > the SPDX header to be anywhere inside the first comment block inside a
> > file [2].
> 
> > That's basically how this thread started: other developers think
> > that it is a good idea to be pedantic. So, be it, but let's then fix
> > the documentation, as the way it is, it is implicitly forbidding the
> > addition of encoding lines for Python scripts.
> > 
> > [2] I *suspect* that the restriction was added in order to make
> >      ./scripts/spdxcheck.py to run faster and to avoid false positives.
> >      Right now, if the maximum limit is removed (or set to a very high
> >      value), there will be one false positive:

Nope. The intention was to have a well define place and format instead of
everyone and his dog deciding to put it somewhere. SPDX is not intended to
replace the existing licensing mess with some other randomly placed and
formatted licensing mess.

> > > - write a shebang line if this file is called directly from the
> > >     command line .. but we do not need shebangs on py modules which
> > >     are imported from other modules or scripts
> > > 
> > > - write a encoding line if it is need or helpful / mostly it is helpful
> > >     to know the encoding of a text/code file.
> > > 
> > > - add a SPDX tag
> > 
> > Yes, but this violates the current documentation, as it doesn't allow the
> > SPDX tag after line #2.
> 
> Thats what I mean: The documentation was written with only a small use-cases
> in mind .. there is no real need for SPDX to be in line one or two ... lets
> fix the documentation as I described before.

If there is a requirement from the language to have 2 lines right at the
top for conveying information then there is of course no reason to insist
on the SPDX identifier being on line 2.

So the documentation should say:

   The SPDX identifier must be at the first possible line at the top of the
   file which is not occupied by information which is required to be
   immediately at the top of the file by system constraints, e.g. shebang,
   or by the language, e.g. the encoding information for python.

or something to that effect.

Thanks,

	tglx

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Matthew Wilcox]

On Sat, Sep 07, 2019 at 11:17:22PM +0200, Thomas Gleixner wrote:
> On Sat, 7 Sep 2019, Markus Heiser wrote:
> > Am 07.09.19 um 20:04 schrieb Mauro Carvalho Chehab:
> > > No idea. I would actually prefer to just remove the restriction, and let
> > > the SPDX header to be anywhere inside the first comment block inside a
> > > file [2].
> > > [2] I *suspect* that the restriction was added in order to make
> > >      ./scripts/spdxcheck.py to run faster and to avoid false positives.
> > >      Right now, if the maximum limit is removed (or set to a very high
> > >      value), there will be one false positive:
> 
> Nope. The intention was to have a well define place and format instead of
> everyone and his dog deciding to put it somewhere. SPDX is not intended to
> replace the existing licensing mess with some other randomly placed and
> formatted licensing mess.

I find the current style quite unaesthetic:

// SPDX-License-Identifier: GPL-2.0-only
/*
 *  linux/mm/memory.c
 *
 *  Copyright (C) 1991, 1992, 1993, 1994  Linus Torvalds
 */

I'd much rather see

/*
 * SPDX-License-Identifier: GPL-2.0-only
 * Copyright (C) 1991, 1992, 1993, 1994  Linus Torvalds
 */

but I appreciate the desire to force it to be on the first line if at all
possible.

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Thomas Gleixner]

On Sun, 8 Sep 2019, Matthew Wilcox wrote:
> On Sat, Sep 07, 2019 at 11:17:22PM +0200, Thomas Gleixner wrote:
> > On Sat, 7 Sep 2019, Markus Heiser wrote:
> > > Am 07.09.19 um 20:04 schrieb Mauro Carvalho Chehab:
> > > > No idea. I would actually prefer to just remove the restriction, and let
> > > > the SPDX header to be anywhere inside the first comment block inside a
> > > > file [2].
> > > > [2] I *suspect* that the restriction was added in order to make
> > > >      ./scripts/spdxcheck.py to run faster and to avoid false positives.
> > > >      Right now, if the maximum limit is removed (or set to a very high
> > > >      value), there will be one false positive:
> > 
> > Nope. The intention was to have a well define place and format instead of
> > everyone and his dog deciding to put it somewhere. SPDX is not intended to
> > replace the existing licensing mess with some other randomly placed and
> > formatted licensing mess.
> 
> I find the current style quite unaesthetic:
> 
> // SPDX-License-Identifier: GPL-2.0-only
> /*
>  *  linux/mm/memory.c
>  *
>  *  Copyright (C) 1991, 1992, 1993, 1994  Linus Torvalds
>  */
> 
> I'd much rather see
> 
> /*
>  * SPDX-License-Identifier: GPL-2.0-only
>  * Copyright (C) 1991, 1992, 1993, 1994  Linus Torvalds
>  */
> 
> but I appreciate the desire to force it to be on the first line if at all
> possible.

That style is inflicted upon you by Penguin Emperor Decree. :)

Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Sun, 8 Sep 2019, Matthew Wilcox wrote:
> > On Sat, Sep 07, 2019 at 11:17:22PM +0200, Thomas Gleixner wrote:
> > > On Sat, 7 Sep 2019, Markus Heiser wrote:
> > > > Am 07.09.19 um 20:04 schrieb Mauro Carvalho Chehab:
> > > > > No idea. I would actually prefer to just remove the restriction, and let
> > > > > the SPDX header to be anywhere inside the first comment block inside a
> > > > > file [2].
> > > > > [2] I *suspect* that the restriction was added in order to make
> > > > >      ./scripts/spdxcheck.py to run faster and to avoid false positives.
> > > > >      Right now, if the maximum limit is removed (or set to a very high
> > > > >      value), there will be one false positive:
> > > 
> > > Nope. The intention was to have a well define place and format instead of
> > > everyone and his dog deciding to put it somewhere. SPDX is not intended to
> > > replace the existing licensing mess with some other randomly placed and
> > > formatted licensing mess.
> > 
> > I find the current style quite unaesthetic:
> > 
> > // SPDX-License-Identifier: GPL-2.0-only
> > /*
> >  *  linux/mm/memory.c
> >  *
> >  *  Copyright (C) 1991, 1992, 1993, 1994  Linus Torvalds
> >  */
> > 
> > I'd much rather see
> > 
> > /*
> >  * SPDX-License-Identifier: GPL-2.0-only
> >  * Copyright (C) 1991, 1992, 1993, 1994  Linus Torvalds
> >  */
> > 
> > but I appreciate the desire to force it to be on the first line if at all
> > possible.
> 
> That style is inflicted upon you by Penguin Emperor Decree. :)

I'd also say that it's a rather tooling-friendly format which mandates a 
single-line representation, which will be less likely to be morphed into 
a zillion variants like the original boilerplates.

So the Penguin Emperor Decree also makes sense, which helps. ;-)

Thanks,

	Ingo