scripts/spdxcheck.py: Always open files in binary mode
PATCH 1/2 scripts/spdxcheck.py: Always open files in binary mode
From: Thierry Reding <[email protected]>
The spdxcheck script currently falls over when confronted with a binary file (such as Documentation/logo.gif). To avoid that, always open files in binary mode and decode line-by-line, ignoring encoding errors.
One tricky case is when piping data into the script and reading it from standard input. By default, standard input will be opened in text mode, so we need to reopen it in binary mode.
--- scripts/spdxcheck.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
scripts/spdxcheck.py +4 -2
@@ -168,6 +168,7 @@ class id_parser(object): self.curline = 0 try: for line in fd: + line = line.decode(locale.getpreferredencoding(False), errors='ignore') self.curline += 1 if self.curline > maxlines: break
@@ -249,12 +250,13 @@ if __name__ == '__main__': try: if len(args.path) and args.path[0] == '-': - parser.parse_lines(sys.stdin, args.maxlines, '-') + stdin = os.fdopen(sys.stdin.fileno(), 'rb') + parser.parse_lines(stdin, args.maxlines, '-') else: if args.path: for p in args.path: if os.path.isfile(p): - parser.parse_lines(open(p), args.maxlines, p) + parser.parse_lines(open(p, 'rb'), args.maxlines, p) elif os.path.isdir(p): scan_git_subtree(repo.head.reference.commit.tree, p) else:
Signature · 1 line
2.19.1
PATCH 2/2 scripts: Add spdxcheck.py self test
From: Thierry Reding <[email protected]>
Add a script that will run spdxcheck.py through a couple of self tests to simplify validation in the future. The tests are run for both Python 2 and Python 3 to make sure all changes to the script remain compatible across both versions.
The script tests a regular text file (Makefile) for basic sanity checks and then runs it on a binary file (Documentation/logo.gif) to make sure it works in both cases. It also tests opening files passed on the command line as well as piped files read from standard input. Finally a run on the complete tree will be performed to catch any other potential issues.
--- scripts/spdxcheck-test.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100755 scripts/spdxcheck-test.sh
scripts/spdxcheck-test.sh new file +12 -0
@@ -0,0 +1,12 @@ +#!/bin/sh + +for PYTHON in python2 python3; do + # run check on a text and a binary file + for FILE in Makefile Documentation/logo.gif; do + $PYTHON scripts/spdxcheck.py $FILE + $PYTHON scripts/spdxcheck.py - < $FILE + done + + # run check on complete tree to catch any other issues + $PYTHON scripts/spdxcheck.py > /dev/null +done
Signature · 1 line
2.19.1
Hi,
Quoted 46 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The spdxcheck script currently falls over when confronted with a binary file (such as
Documentation/logo.gif). To avoid that, always open files in binary mode and decode line-by-line, ignoring encoding errors.One tricky case is when piping data into the script and reading it from standard input. By default, standard input will be opened in text mode, so we need to reopen it in binary mode.
Signed-off-by Thierry Reding <[email protected]>--- scripts/spdxcheck.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)scripts/spdxcheck.py +4
-2@@ -168,6 +168,7 @@ class id_parser(object): self.curline = 0 try: for line in fd: + line = line.decode(locale.getpreferredencoding(False), errors='ignore') self.curline += 1 if self.curline > maxlines: break@@ -249,12 +250,13 @@ if __name__ == '__main__': try: if len(args.path) and args.path[0] == '-': - parser.parse_lines(sys.stdin, args.maxlines, '-') + stdin = os.fdopen(sys.stdin.fileno(), 'rb') + parser.parse_lines(stdin, args.maxlines, '-') else: if args.path: for p in args.path: if os.path.isfile(p): - parser.parse_lines(open(p), args.maxlines, p) + parser.parse_lines(open(p, 'rb'), args.maxlines, p) elif os.path.isdir(p): scan_git_subtree(repo.head.reference.commit.tree, p) else:Signature · 1 line
2.19.1
It might be worth noting this fixes commit 6f4d29df66ac ("scripts/spdxcheck.py: make python3 compliant") and also Cc this for stable since 6f4d29df66ac got backported to v4.19. While that commit did indeed make the script work with Python 3 for piping data, it broke Python 2 and made its way to stable.
Regards,
Jeremy
Quoted 2 lines · Jeremy Cline
It might be worth noting this fixes commit
6f4d29df66ac("scripts/spdxcheck.py: make python3 compliant")
umm, how does it do that?
Quoted 4 lines
and also Cc this for
stable since6f4d29df66acgot backported to v4.19. While that commit did indeed make the script work with Python 3 for piping data, it broke Python 2 and made its way to stable.
I think attempting to use spdxcheck on binary files is not useful in the first place.
Quoted 6 lines · Joe Perches
Quoted 2 lines · Jeremy Cline
It might be worth noting this fixes commit
6f4d29df66ac("scripts/spdxcheck.py: make python3 compliant")umm, how does it do that?
Quoted 7 lines
Quoted 4 lines
and also Cc this for
stable since6f4d29df66acgot backported to v4.19. While that commit did indeed make the script work with Python 3 for piping data, it broke Python 2 and made its way to stable.I think attempting to use spdxcheck on binary files is not useful in the first place.
Agreed. However, if run without arguments the tool will walk the entire tree and inevitably run into binary files. I realize that this is kind of a lame way of dealing with binary files, but the alternatives would be much more complicated and involve dealing with mimetypes and such.
Thierry
Quoted 8 lines · Jeremy Cline
Hi,
Quoted 5 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The spdxcheck script currently falls over when confronted with a binary file (such as
Documentation/logo.gif). To avoid that, always open files in binary mode and decode line-by-line, ignoring encoding errors.
I suggest pointing out that the breakage only happens with python3 and results in a UnicodeDecodeError.
Quoted 38 lines
Quoted 38 lines
One tricky case is when piping data into the script and reading it from standard input. By default, standard input will be opened in text mode, so we need to reopen it in binary mode.
Signed-off-by Thierry Reding <[email protected]>--- scripts/spdxcheck.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)scripts/spdxcheck.py +4
-2@@ -168,6 +168,7 @@ class id_parser(object): self.curline = 0 try: for line in fd: + line = line.decode(locale.getpreferredencoding(False), errors='ignore') self.curline += 1 if self.curline > maxlines: break@@ -249,12 +250,13 @@ if __name__ == '__main__': try: if len(args.path) and args.path[0] == '-': - parser.parse_lines(sys.stdin, args.maxlines, '-') + stdin = os.fdopen(sys.stdin.fileno(), 'rb') + parser.parse_lines(stdin, args.maxlines, '-') else: if args.path: for p in args.path: if os.path.isfile(p): - parser.parse_lines(open(p), args.maxlines, p) + parser.parse_lines(open(p, 'rb'), args.maxlines, p) elif os.path.isdir(p): scan_git_subtree(repo.head.reference.commit.tree, p) else:
Quoted 8 lines
Quoted 2 lines
2.19.1
It might be worth noting this fixes commit
6f4d29df66ac("scripts/spdxcheck.py: make python3 compliant") and also Cc this for stable since6f4d29df66acgot backported to v4.19. While that commit did indeed make the script work with Python 3 for piping data, it broke Python 2 and made its way to stable.
It didn't break for me. Can you provide details about how and when it broke for you?
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
Quoted 60 lines · Uwe Kleine-König
Quoted 8 lines · Jeremy Cline
Hi,
Quoted 5 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The spdxcheck script currently falls over when confronted with a binary file (such as
Documentation/logo.gif). To avoid that, always open files in binary mode and decode line-by-line, ignoring encoding errors.I suggest pointing out that the breakage only happens with python3 and results in a UnicodeDecodeError.
Quoted 38 lines
Quoted 38 lines
One tricky case is when piping data into the script and reading it from standard input. By default, standard input will be opened in text mode, so we need to reopen it in binary mode.
Signed-off-by Thierry Reding <[email protected]>--- scripts/spdxcheck.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)scripts/spdxcheck.py +4
-2@@ -168,6 +168,7 @@ class id_parser(object): self.curline = 0 try: for line in fd: + line = line.decode(locale.getpreferredencoding(False), errors='ignore') self.curline += 1 if self.curline > maxlines: break@@ -249,12 +250,13 @@ if __name__ == '__main__': try: if len(args.path) and args.path[0] == '-': - parser.parse_lines(sys.stdin, args.maxlines, '-') + stdin = os.fdopen(sys.stdin.fileno(), 'rb') + parser.parse_lines(stdin, args.maxlines, '-') else: if args.path: for p in args.path: if os.path.isfile(p): - parser.parse_lines(open(p), args.maxlines, p) + parser.parse_lines(open(p, 'rb'), args.maxlines, p) elif os.path.isdir(p): scan_git_subtree(repo.head.reference.commit.tree, p) else:Quoted 8 lines
Quoted 2 lines
2.19.1
It might be worth noting this fixes commit
6f4d29df66ac("scripts/spdxcheck.py: make python3 compliant") and also Cc this for stable since6f4d29df66acgot backported to v4.19. While that commit did indeed make the script work with Python 3 for piping data, it broke Python 2 and made its way to stable.
Quoted 2 lines
It didn't break for me. Can you provide details about how and when it broke for you?
I was wrong about it being Python 2 that broke, sorry about that. 6f4d29df66ac broke Python 3 when you run it against a sub-tree because scan_git_tree() opens the files in binary mode, but then find is run with a text string:
$ python3 scripts/spdxcheck.py net/
FAIL: argument should be integer or bytes-like object, not 'str'
Traceback (most recent call last):
File "scripts/spdxcheck.py", line 259, in <module>
scan_git_subtree(repo.head.reference.commit.tree, p)
File "scripts/spdxcheck.py", line 211, in scan_git_subtree
scan_git_tree(tree)
File "scripts/spdxcheck.py", line 206, in scan_git_tree
parser.parse_lines(fd, args.maxlines, el.path)
File "scripts/spdxcheck.py", line 175, in parse_lines
if line.find("SPDX-License-Identifier:") < 0:
TypeError: argument should be integer or bytes-like object, not 'str'The reason I opened things in binary mode when I started adding Python 3 support was because not all files were valid UTF-8 (and some were binary) so I decoded the text line-by-line and ignored any decoding errors for simplicity's sake.
Regards,
Jeremy
Quoted 22 lines · Jeremy Cline
Quoted 2 lines · Uwe Kleine-König
It didn't break for me. Can you provide details about how and when it broke for you?
I was wrong about it being Python 2 that broke, sorry about that.
6f4d29df66acbroke Python 3 when you run it against a sub-tree becausescan_git_tree()opens the files in binary mode, but then find is run with a text string:$ python3 scripts/spdxcheck.py net/ FAIL: argument should be integer or bytes-like object, not 'str' Traceback (most recent call last): File "scripts/spdxcheck.py", line 259, in <module> scan_git_subtree(repo.head.reference.commit.tree, p) File "scripts/spdxcheck.py", line 211, in scan_git_subtree scan_git_tree(tree) File "scripts/spdxcheck.py", line 206, in scan_git_tree parser.parse_lines(fd, args.maxlines, el.path) File "scripts/spdxcheck.py", line 175, in parse_lines if line.find("SPDX-License-Identifier:") < 0: TypeError: argument should be integer or bytes-like object, not 'str'
Quoted 4 lines
The reason I opened things in binary mode when I started adding Python 3 support was because not all files were valid UTF-8 (and some were binary) so I decoded the text line-by-line and ignored any decoding errors for simplicity's sake.
OK I understand. The problem is that there are inconsistencies in handling files as binaries or not that already existed before 6f4d29df66ac. Different code paths result in a different type for line depending on how fd was opened. I fixed the cases where fd was opened as text file and broke the cases where it was opened as binary.
So changing this to consistently using binary mode (as the patch by Thierry does) seems the right thing to do.
Thanks
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |