Re: [PATCH 1/2] scripts/spdxcheck.py: Always open files in binary mode

[Uwe Kleine-König]

On Thu, Dec 13, 2018 at 10:10:52AM -0500, Jeremy Cline wrote:
> On Thu, Dec 13, 2018 at 08:37:08AM +0100, Uwe Kleine-König wrote:
> > 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.

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |