Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Oct 24, 2023

Conversation

mxmehl
Copy link
Member

@mxmehl mxmehl commented Mar 8, 2023

Fix #672

As noted in the referenced issue, some error messages of reuse lint are not really helpful if your experience with REUSE and/or licensing is limited.

This PR attempts to improve the situation by adding a "Recommendations" section after the linting result, explaining the occured issues and how to best solve them (if there is an easy solution).

I'm not sure whether the position I implemented this in the code is ideal, but I liked that I could derive everything from the report object. Depending on what other issues we might want to explain, there might be better solutions, but I didn't invest many thoughts on this yet.

Possible blockers:

  • The help texts should probably also go into the future JSON output. So Adding --json to lint #654 might need to be adapted, or this PR if JSON is added before.

Please find the output below in a repo in which basically everything is broken:

New output of `reuse lint` on a very problematic repo

$ reuse lint
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'
# BAD LICENSES

'InVaLiD' found in:
* LICENSES/InVaLiD.txt


# DEPRECATED LICENSES

The following licenses are deprecated by SPDX:
* GPL-2.0+


# LICENSES WITHOUT FILE EXTENSION

The following licenses have no file extension:
* LICENSES/Apache-2.0


# MISSING LICENSES

'MIT' found in:
* Makefile
* img/cat.jpg
* img/dog.jpg


# UNUSED LICENSES

The following licenses are not used:
* Apache-2.0
* CC-BY-SA-4.0
* GPL-2.0+
* InVaLiD


# READ ERRORS

Could not read:
* README.md


# MISSING COPYRIGHT AND LICENSING INFORMATION

The following files have no copyright and licensing information:
* src/main.c

The following files have no copyright information:
* Makefile

The following files have no licensing information:
* .gitignore


# SUMMARY

* Bad licenses: InVaLiD
* Deprecated licenses: GPL-2.0+
* Licenses without file extension: Apache-2.0
* Missing licenses: MIT
* Unused licenses: Apache-2.0, CC-BY-SA-4.0, GPL-2.0+, InVaLiD
* Used licenses: MIT
* Read errors: 1
* Files with copyright information: 3 / 5
* Files with license information: 3 / 5

Unfortunately, your project is not compliant with version 3.0 of the REUSE Specification :-(


# RECOMMENDATIONS

* Fix bad licenses: At least one license in the LICENSES directory and/or
  provided by 'SPDX-License-Identifier' tags is invalid. They are either not
  valid SPDX license identifiers or do not start with 'LicenseRef-'. FAQ about
  custom licenses: https://reuse.software/faq/#custom-license
* Fix deprecated licenses: At least one of the licenses in the LICENSES
  directory and/or provided by an 'SPDX-License-Identifier' tag or in
  '.reuse/dep5' has been deprecated by SPDX. The current list and their
  respective recommended  new identifiers can be found here:
  https://spdx.org/licenses/#deprecated
* Fix licenses without file extension: At least one license text file in the
  'LICENSES' directory does not have a '.txt' file extension. Please rename the
  file(s) accordingly.
* Fix missing licenses: For at least one of the license identifiers provided by
  the 'SPDX-LicenseIdentifier' tags, there is no corresponding license text file
  in the 'LICENSES' directory. For SPDX license identifiers, you can simply run
  'reuse download --all' to get any missing ones. For custom licenses (starting
  with 'LicenseRef-'), you need to add these files yourself.
* Fix unused licenses: At least one of the license text files in 'LICENSES' is
  not referenced for any file, e.g. by an 'SPDX-License-Identifier' tag. Please
  make sure that you either tag the accordingly licensed files properly, or
  delete the unused license text if you are sure that no file or code snippet is
  licensed as such.
* Fix read errors: At least one of the files in your directory cannot be read by
  the tool. Please check the file permissions. You will find the affected files
  at the top of the output as part of the logged error messages.
* Fix missing copyright/license information: For one or more files, the tool
  cannot find copyright and/or license information. Please add it as a comment
  in the file header, ideally using the 'SPDX-FileCopyrightText' tag. If that's
  not possible, you can use adjacent '.license' files or the '.reuse/dep5' file.

Copy link
Member

@linozen linozen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like it if I could disable this functionality if I don't need it, e.g. behind a flag such as --no-recommends.

Otherwise LGTM.

@mxmehl
Copy link
Member Author

mxmehl commented Apr 6, 2023

I'd like it if I could disable this functionality if I don't need it, e.g. behind a flag such as --no-recommends.

Sure, I added a --no-recommendations flag. Please check whether that's fine for you :)

src/reuse/lint.py Outdated Show resolved Hide resolved
src/reuse/lint.py Outdated Show resolved Hide resolved
src/reuse/lint.py Outdated Show resolved Hide resolved
Copy link
Member

@nicorikken nicorikken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to add this. Not sure about best approach for defining the text in the file and not sure if the --no-recommendations flag is really necessary.

@mxmehl mxmehl requested a review from linozen May 4, 2023 15:51
@floriansnow
Copy link
Contributor

I agree that the flag doesn't help much in the current state and I'm leaning towards leaving it out here and adding some reduced output to a quiet flag or similar (in a separate issue/PR).

@mxmehl
Copy link
Member Author

mxmehl commented May 19, 2023

I agree that the flag doesn't help much in the current state and I'm leaning towards leaving it out here and adding some reduced output to a quiet flag or similar (in a separate issue/PR).

FWIW, we have --quiet for lint but that completely silences the output (which is expected). Again, I'm fine with both keeping it and throwing it out, so @linozen's opinion would be great as he requested it :)

@linozen
Copy link
Member

linozen commented May 26, 2023

I agree that the flag doesn't help much in the current state and I'm leaning towards leaving it out here and adding some reduced output to a quiet flag or similar (in a separate issue/PR).

FWIW, we have --quiet for lint but that completely silences the output (which is expected). Again, I'm fine with both keeping it and throwing it out, so @linozen's opinion would be great as he requested it :)

Let's throw it out if others don't see the value in it. I rather have this merged soon and have no strong opinions about meaning I'm fine either way. So feel free to remove it and merge this.

@mxmehl mxmehl force-pushed the summary-help branch 3 times, most recently from ebd61a6 to 7fce296 Compare June 13, 2023 14:30
@mxmehl
Copy link
Member Author

mxmehl commented Jun 13, 2023

I had to throw around the whole PR because of the JSON feature. Therefore, I put most of the logic into the ProjectReport class in order to make it easily digestable for both format_json() and format_plain() in lint.py.

I hope that matches your expecations. Please feel free to propose improvements for handling the recommendations in the class, I am not as familiar with best practices for classes as I should be.

Note: I added another unsorted key, this time at the bottom. This is because I think that the recommendations resulting from the summary should be the last entry in the dictionary and JSON output. It complicates a few things but IMHO it's worth it.

mxmehl and others added 3 commits October 24, 2023 12:38
Based on errors in the report, print helpful background and possible solutions
for the user as recommended next steps.
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
@carmenbianca carmenbianca merged commit 629ff01 into main Oct 24, 2023
21 checks passed
@carmenbianca carmenbianca deleted the summary-help branch May 3, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the text output of reuse lint more helpful
5 participants