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

Populate the LicenseConcluded field in the SPDX report #623

Merged
merged 14 commits into from
Apr 6, 2023

Conversation

pietroalbini
Copy link
Contributor

@pietroalbini pietroalbini commented Nov 10, 2022

This PR fixes #586 by adding the --add-license-concluded flag to the reuse spdx command.

When that flag is provided, REUSE will now calculate the LicenseConcluded field in the SPDX report rather than setting it to NOASSERT, by ANDing all the expressions related to the file. This will allow tools consuming the SPDX report to know the actual licensing of each file, rather than having to rely on the lossy/incorrect LicenseInfoInFile fields.

Passing the flag will result in a slightly noticeable slowdown in large repositories: on rust-lang/rust, without the flag reuse spdx takes 4.13s, while with the flag it takes 4.72s.

To ensure compliance with the SPDX spec, this also adds the --creator-person and --creator-organization flags to reuse spdx. Optional by default, at least one of the flags will be required when passing --add-license-concluded, as requested in the issue.

Since there is no consensus yet in #586 whether to make this the default behavior or not, populating the field and gating that behind a CLI flag are in two different commits, so if you all decide to populate LicenseConcluded by default we can simply drop the last commit. This PR does not change LicenseInfoInFile in any way.

Also, the first commit makes running tests bearable on my machine, since I configured git to require signing all commits with my gpg key, which requires a physical touch on my YubiKey every time a commit is created 😅

@tenzap
Copy link

tenzap commented Nov 10, 2022

Will it work also when there are exceptions like in

SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0

@pietroalbini
Copy link
Contributor Author

Yes, this also works when there are exceptions! To be extra sure, I added an exception to the new test I added 🙂

@pietroalbini
Copy link
Contributor Author

Added reuse spdx --creator-person=NAME --creator-organization=NAME as requested in #586 (comment). The command will continue working without the flags, but at least one of them will be required when passing --add-license-concluded.

@alpianon
Copy link

hi @pietroalbini we at Oniro Project just realized that we would really need this functionality, it's great that you are already developing it, thanks! :)

FYI I'm proposing to add another spdx-related feature (support for comments, conversion of comments from dep5 to spdx, and possibly other comment-related features), see #625 . If you have any idea or feedback on that, feel free to comment there

@mxmehl
Copy link
Member

mxmehl commented Jan 4, 2023

Great work, thanks for implementing all of @silverhook's suggestions!

src/reuse/report.py Outdated Show resolved Hide resolved
src/reuse/spdx.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.

2 small suggestions but overall great work! I can see this coming in handy in many cases, thanks for contributing it.

pietroalbini and others added 14 commits February 17, 2023 22:32
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
@nicorikken
Copy link
Member

⚠️ Please don't merge this PR just yet, @carmenbianca has added some improvements on top if this work at #690

@pietroalbini
Copy link
Contributor Author

@nicorikken added @carmenbianca's commits to this PR. Sorry about not being able to push directly to the PR, that's a limitation of my employer's GitHub organization.

@linozen linozen merged commit 0804dc4 into fsfe:main Apr 6, 2023
@pietroalbini pietroalbini deleted the pa-spdx-concluded-license branch April 6, 2023 12:07
@y0urself
Copy link

y0urself commented Jun 2, 2023

Is there any chance these arguments will get into a release anytime soon?

@linozen
Copy link
Member

linozen commented Jun 2, 2023

Yes, we're planning a release in the next two weeks.

@y0urself
Copy link

y0urself commented Jun 3, 2023

Thank you for the answer! :)

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.

reuse spdx: also output the full SPDX-License-Identifier
8 participants