Adding `--json` to lint #654

linozen · 2023-01-24T16:35:17Z

This should hopefully close #183.

More tests need to be added and more feedback gathered. This is just a first stab at adding this functionality. Maybe we get it done pre-FOSDEM!

Many thanks!

mxmehl · 2023-01-25T12:39:36Z

Thanks a bunch for implementing this! I'd also like to make @alpianon aware of this PR as he asked for the feature IIRC.

Then, let me post some remarks and questions:

My idea behind the source key in licenses and copyrights was that it should show where the information came from: the file directly, an accompanying .license file, or dep5? Currently, this always is file.spdxfile.name
I tested it with a repo containing a lot of issues, and it threw a traceback with --json added:

$ reuse lint --json
reuse.project - WARNING - Could not resolve SPDX License Identifier of LICENSES/InVaLiD.txt, resolving to InVaLiD. Make sure the license is in the license list found at <https://spdx.org/licenses/> or that it starts with 'LicenseRef-', and that it has a file extension.
reuse.report - ERROR - Could not read 'README.md'
PermissionError: [Errno 13] Permission denied: 'README.md'
Traceback (most recent call last):
  File "/home/max/.cache/pypoetry/virtualenvs/reuse-RJlAEOXQ-py3.10/bin/reuse", line 6, in <module>
    sys.exit(main())
  File "/home/max/Git/github/reuse-tool/src/reuse/_main.py", line 283, in main
    return parsed_args.func(parsed_args, project, out)
  File "/home/max/Git/github/reuse-tool/src/reuse/lint.py", line 331, in run
    lint(data, formatter=formatter, out=out)
  File "/home/max/Git/github/reuse-tool/src/reuse/lint.py", line 311, in lint
    out.write(formatter(data))
  File "/home/max/Git/github/reuse-tool/src/reuse/lint.py", line 295, in format_json
    return json.dumps(
  File "/usr/lib/python3.10/json/__init__.py", line 238, in dumps
    **kw).encode(obj)
  File "/usr/lib/python3.10/json/encoder.py", line 201, in encode
    chunks = list(chunks)
  File "/usr/lib/python3.10/json/encoder.py", line 431, in _iterencode
    yield from _iterencode_dict(o, _current_indent_level)
  File "/usr/lib/python3.10/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.10/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.10/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.10/json/encoder.py", line 439, in _iterencode
    yield from _iterencode(o, _current_indent_level)
  File "/usr/lib/python3.10/json/encoder.py", line 429, in _iterencode
    yield from _iterencode_list(o, _current_indent_level)
  File "/usr/lib/python3.10/json/encoder.py", line 325, in _iterencode_list
    yield from chunks
  File "/usr/lib/python3.10/json/encoder.py", line 439, in _iterencode
    yield from _iterencode(o, _current_indent_level)
  File "/usr/lib/python3.10/json/encoder.py", line 436, in _iterencode
    raise ValueError("Circular reference detected")
ValueError: Circular reference detected

To reproduce:

git clone https://github.com/mxmehl/reuse-example.git test && cd test
git checkout so-many-errors
chmod 000 README.md   # also provoke a read error
reuse lint --json

The same error also occurs when linting the Linux kernel for example.

Then, copy from Add --json flag to lint #183 (comment): The files key also shows the SPDX-FileCopyrightText tag in the value key. As per reuse spdx should exclude the tag from FileCopyrightText #536 we consider to remove that from the spdx subcommand, so we may also want to do it here.

Otherwise, it looks good to me so far. It would be interesting to use it against a few concrete test cases.

linozen · 2023-04-05T12:27:51Z

Thanks a bunch for implementing this! I'd also like to make @alpianon aware of this PR as he asked for the feature IIRC.

Then, let me post some remarks and questions:

My idea behind the source key in licenses and copyrights was that it should show where the information came from: the file directly, an accompanying .license file, or dep5? Currently, this always is file.spdxfile.name

This is now fixed by slightly adapting the SpdxInfo namedtuple. We decided that the move from a namedtuple to a dataclass is best handled in #669. We left a note in #669 about where SpdxInfo needs to be adapted.

I tested it with a repo containing a lot of issues, and it threw a traceback with --json added:

$ reuse lint --json
reuse.project - WARNING - Could not resolve SPDX License Identifier of LICENSES/InVaLiD.txt, resolving to InVaLiD. Make sure the license is in the license list found at <https://spdx.org/licenses/> or that it starts with 'LicenseRef-', and that it has a file extension.
reuse.report - ERROR - Could not read 'README.md'
PermissionError: [Errno 13] Permission denied: 'README.md'
Traceback (most recent call last):
  File "/home/max/.cache/pypoetry/virtualenvs/reuse-RJlAEOXQ-py3.10/bin/reuse", line 6, in <module>
    sys.exit(main())
  File "/home/max/Git/github/reuse-tool/src/reuse/_main.py", line 283, in main
    return parsed_args.func(parsed_args, project, out)
  File "/home/max/Git/github/reuse-tool/src/reuse/lint.py", line 331, in run
    lint(data, formatter=formatter, out=out)
  File "/home/max/Git/github/reuse-tool/src/reuse/lint.py", line 311, in lint
    out.write(formatter(data))
  File "/home/max/Git/github/reuse-tool/src/reuse/lint.py", line 295, in format_json
    return json.dumps(
  File "/usr/lib/python3.10/json/__init__.py", line 238, in dumps
    **kw).encode(obj)
  File "/usr/lib/python3.10/json/encoder.py", line 201, in encode
    chunks = list(chunks)
  File "/usr/lib/python3.10/json/encoder.py", line 431, in _iterencode
    yield from _iterencode_dict(o, _current_indent_level)
  File "/usr/lib/python3.10/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.10/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.10/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.10/json/encoder.py", line 439, in _iterencode
    yield from _iterencode(o, _current_indent_level)
  File "/usr/lib/python3.10/json/encoder.py", line 429, in _iterencode
    yield from _iterencode_list(o, _current_indent_level)
  File "/usr/lib/python3.10/json/encoder.py", line 325, in _iterencode_list
    yield from chunks
  File "/usr/lib/python3.10/json/encoder.py", line 439, in _iterencode
    yield from _iterencode(o, _current_indent_level)
  File "/usr/lib/python3.10/json/encoder.py", line 436, in _iterencode
    raise ValueError("Circular reference detected")
ValueError: Circular reference detected

To reproduce:

git clone https://github.com/mxmehl/reuse-example.git test && cd test
git checkout so-many-errors
chmod 000 README.md   # also provoke a read error
reuse lint --json

The same error also occurs when linting the Linux kernel for example.

This should now be fixed but using a custom JSON serializer.

Then, copy from Add --json flag to lint #183 (comment): The files key also shows the SPDX-FileCopyrightText tag in the value key. As per reuse spdx should exclude the tag from FileCopyrightText #536 we consider to remove that from the spdx subcommand, so we may also want to do it here.

IMHO, this is probably best treated in a separate PR as it does not only relate to the JSON output of lint. What do you think?

mxmehl · 2023-04-05T13:44:43Z

Thank you! I can a few first tests and noticed some issues. But first of all, I took the liberty to merge latest master in the branch.

Then, copy from Add --json flag to lint #183 (comment): The files key also shows the SPDX-FileCopyrightText tag in the value key. As per reuse spdx should exclude the tag from FileCopyrightText #536 we consider to remove that from the spdx subcommand, so we may also want to do it here.

IMHO, this is probably best treated in a separate PR as it does not only relate to the JSON output of lint. What do you think?

Yes, that makes sense.

Now, to the issues I found:

I noticed that many outputs are appearing in a random order, e.g. the list of used licenses, both in the JSON and plain text output. Also, the files in the files key would be great to have sorted. It's not critical but a UX issue. Two runs of the program should result in the very same output.
I see some issues with the detection of where a file is described. Perhaps the core problem lies further down than what you touched in this PR, but it's clearly wrong. I made some test cases in my fork:

git clone https://github.com/mxmehl/reuse-example.git test && cd test
git checkout json-tests

cat.jpg

With my edits, cat.jpg is described in both the cat.jpg.license as well as .reuse/dep5. The tool detects the different source, but does not mention them correctly here. Both the "Tester Testerson" copyright and the "Apache-2.0" license are in the dep5 file.

    "img/cat.jpg": {
      "copyrights": [
        {
          "value": "SPDX-FileCopyrightText: 2017 Peter Janzen",
          "source": "img/cat.jpg.license"
        },
        {
          "value": "Tester Testerson",
          "source": "img/cat.jpg.license"
        }
      ],
      "licenses": [
        {
          "value": "CC-BY-4.0",
          "source": "img/cat.jpg.license"
        },
        {
          "value": "Apache-2.0",
          "source": "img/cat.jpg.license"
        }
      ]
    }

However, for dog.jpg, it correctly displays the origin as dep5, but for this image there is no .license file.

.gitignore

That's even more complicated. The file itself contains only copyright info, the .license file a license and copyright, and dep5 a copyright and some invalid licensing info. This is the result:

    ".gitignore": {
      "copyrights": [
        {
          "value": "SPDX-FileCopyrightText: 2019 Jane Doe <jane@example.com>",
          "source": ".gitignore.license"
        }
      ],
      "licenses": [
        {
          "value": "CC0-1.0",
          "source": ".gitignore.license"
        }
      ]
    }

As you see, the copyright in the file itself is ignored. I think this is intentional as we once decided that such a file can override all contents from the file itself to circumvent false-positive info. We may want to rethink that.

However, for some reason, the dep5 info is completely ignored here. I cannot explain why this is different from the cat.jpg case.

linozen · 2023-04-06T06:51:52Z

Thanks for your feedback @mxmehl!

I noticed that many outputs are appearing in a random order, e.g. the list of used licenses, both in the JSON and plain text output. Also, the files in the files key would be great to have sorted. It's not critical but a UX issue. Two runs of the program should result in the very same output.

Yes, this was absolutely an issue. The last two commits should have fixed that for JSON and plaintext output respectively.

I see some issues with the detection of where a file is described. Perhaps the core problem lies further down than what you touched in this PR, but it's clearly wrong. I made some test cases in my fork.

Yes, I also expect the problem to have deeper roots then this PR. In a sense, we're now exposing REUSE internals (e.g. where a license/copyright information was gathered from) that were not exposed previously. Before JSON output, the tool only really needed to know whether a file had license and copyright information attached to it and whether is info was valid. It didn't really matter where it came from. Now with the much more verbose JSON output we're facing the following issue:

It is unclear what to do when license and copyright information is described in two sources, i.e. a .license file and a dep5 file and which one takes precedence. This is related to this docs issue about precedence and also this issue about the undesired merging of copyright information from different sources.

My approach to this would be:

Clearly define precedence in the the spec. Option 3 as outlined by @mxmehl in the issue seems to be most popular. It is this:

file.license
explicit in adjacent yaml (so file directly)
explicit in faraway
file header
glob in adjacent yaml (file covered e.g. as part of *.png)
glob in faraway yaml

IMHO, we would need to include DEP5 here as well as we're not hard deprecating its use.

Implement precedence in this PR (keeping in mind that we don't have REUSE.yaml yet. The code where that happens is here and it should not be too hard to (a) disable the merging behaviour described in this issue and (b) implement the precedence logic to be implemented until we have REUSE.yaml.

What do you think?

carmenbianca

A lot of remarks. I like the bones of this PR. There's just a lot of nitpicking 😅

src/reuse/report.py

tests/test_main.py

src/reuse/lint.py

src/reuse/project.py

Adding functions to retrieve and format the data from the `ProjectReport` class.

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>

src/reuse/lint.py

floriansnow

LGTM! Would you please also check if the copyright information is updated in all files you contributed to? It looked to me like some may not have changed, but perhaps those were renamed files.

src/reuse/__init__.py

src/reuse/lint.py

src/reuse/__init__.py

This commit adds a new enum `SourceType` with three possible values to indicate if the source type is a `.license file`, `file header` or `DEP5 file`. It then updates the usage of `source_type` by replacing the string type with the new `SourceType` enum type. This improves readability and makes the code more maintainable.

linozen · 2023-06-01T08:54:24Z

LGTM! Would you please also check if the copyright information is updated in all files you contributed to? It looked to me like some may not have changed, but perhaps those were renamed files.

Since I worked during my work hours, I'm fine with FSFE holding the copyright.

linozen requested review from mxmehl and carmenbianca and removed request for mxmehl January 24, 2023 16:35

This was referenced Feb 8, 2023

Implement support for SPDX-FileContributor #669

Merged

discuss supporting other reuse lint report output formats (exporters) #676

Closed

mxmehl mentioned this pull request Mar 8, 2023

Add a "Recommendations" section in lint output based on found issues #698

Merged

mxmehl mentioned this pull request Apr 6, 2023

Add support for file comments in spdx #625

Open

carmenbianca requested changes Apr 7, 2023

View reviewed changes

carmenbianca mentioned this pull request Apr 9, 2023

Type checking with mypy #726

Merged

carmenbianca requested changes Apr 10, 2023

View reviewed changes

src/reuse/lint.py Show resolved Hide resolved

src/reuse/project.py Show resolved Hide resolved

carmenbianca mentioned this pull request Apr 10, 2023

Provide json lint schema file #728

Open

carmenbianca force-pushed the lint-json branch from 72e33c8 to 3bd95bb Compare May 11, 2023 09:44

linozen added 13 commits May 11, 2023 11:53

Rework lint

cb94206

Adding functions to retrieve and format the data from the `ProjectReport` class.

Add --json` argument

3ae8af7

Slim down data dictionary creation

2e6805e

Rework plaintext formatter

5a03e9e

Move JSON formatter and re-add lint function

21d0026

Remove old output generation functions

0b473f0

Adapt tests

74fd00e

Add new tests

fb32ead

Satisfy pylint

ff3d3c3

Satisfy isort

cfcac8e

Make showing of license (not path) work on Windows

d749d51

Satisfy pylint (again)

5dcd872

Add changelog entry

e03fb3e

carmenbianca and others added 10 commits May 21, 2023 16:24

Make lint version a global constant

23fa4cc

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>

Add tool version to json output

8fa808a

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>

Refactoring precedence for license path

c94b5a0

Add two --json specific test to tests/test_lint.py

7493ed1

Adapting two tests to new precedence behaviour

bb3217a

Adding a warning when information is overriden

f2f1ef0

Import Path instead of PosixPath

cd820e1

Use StringIO object instead of normal string object

6c51545

Make sure that some keys are on top of sorted output

d1c2146

Make summary_contents a dictionary

c8baa6e

carmenbianca reviewed May 26, 2023

View reviewed changes

src/reuse/lint.py Outdated Show resolved Hide resolved

linozen added 6 commits May 26, 2023 17:02

Simplify lint --quiet

983b0c4

Move dictionary sorting and test it

d7af2e2

Rename SpdxInfo to ReuseInfo

33456cb

Rename ReuseInfo.license_path to ReuseInfo.source_path

962521f

Also encode source_type in ReuseInfo

852b3fb

Rename spdx_info_of to reuse_info_of

322dd8a

linozen mentioned this pull request May 31, 2023

Remove potentially superfluous attributes in _File class #753

Closed

linozen requested review from carmenbianca, nicorikken and floriansnow May 31, 2023 08:45

floriansnow approved these changes May 31, 2023

View reviewed changes

src/reuse/__init__.py Show resolved Hide resolved

src/reuse/lint.py Show resolved Hide resolved

carmenbianca reviewed May 31, 2023

View reviewed changes

src/reuse/__init__.py Outdated Show resolved Hide resolved

linozen requested a review from carmenbianca June 1, 2023 08:55

linozen merged commit 7362734 into main Jun 1, 2023

carmenbianca mentioned this pull request Jun 10, 2023

Some followup work on the --json PR #759

Merged

mxmehl deleted the lint-json branch June 13, 2023 13:06

carmenbianca mentioned this pull request Jun 25, 2023

Make sure the sources are correct when information is aggregated #787

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding `--json` to lint #654

Adding `--json` to lint #654

linozen commented Jan 24, 2023

mxmehl commented Jan 25, 2023

linozen commented Apr 5, 2023 •

edited by mxmehl

Loading

mxmehl commented Apr 5, 2023

linozen commented Apr 6, 2023

carmenbianca left a comment

floriansnow left a comment •

edited

Loading

linozen commented Jun 1, 2023

Adding --json to lint #654

Adding --json to lint #654

Conversation

linozen commented Jan 24, 2023

mxmehl commented Jan 25, 2023

linozen commented Apr 5, 2023 • edited by mxmehl Loading

mxmehl commented Apr 5, 2023

cat.jpg

.gitignore

linozen commented Apr 6, 2023

carmenbianca left a comment

Choose a reason for hiding this comment

floriansnow left a comment • edited Loading

Choose a reason for hiding this comment

linozen commented Jun 1, 2023

Adding `--json` to lint #654

Adding `--json` to lint #654

linozen commented Apr 5, 2023 •

edited by mxmehl

Loading

floriansnow left a comment •

edited

Loading