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

Support parentheses in license fields #2595

Closed
hsivonen opened this issue Jun 23, 2020 · 10 comments · Fixed by #4293
Closed

Support parentheses in license fields #2595

hsivonen opened this issue Jun 23, 2020 · 10 comments · Fixed by #4293
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works

Comments

@hsivonen
Copy link
Member

The documentation for the Cargo.toml license field says: "Parentheses are not currently supported."

Please add support for paretheses. The use case is to combine the usual Rust dual license with a permissive license used for some part of the crate originating elsewhere. E.g. "(Apache-2.0 OR MIT) AND BSD-3-Clause".

@carols10cents
Copy link
Member

We use the license-exprs crate, so parentheses support would need to be implemented there. Here is the issue for implementing parentheses.

Also note you can specify a license-file and any custom license you would like, so I am closing this issue.

@hsivonen
Copy link
Member Author

hsivonen commented Dec 9, 2021

I filed this in anticipation of encoding_rs having to deal with a upstream change from CC0 to BSD-3-Clause. And, sure enough, using a license file instead of machine-readable SPDX is trouble.

It appears that cargo deny uses the spdx crate. Could you, please, switch crates.io over to that crate to support parentheses?

This issue isn't specific to encoding_rs. ICU4X also points to a custom license file, because the correct SPDX expression would require parentheses.

@Manishearth
Copy link
Member

Manishearth commented Dec 9, 2021

I'll add: a major recurring problem that LICENSE files face is that automated license checking tooling (which exists in basically every large project, including I think Rust itself!) bails on them, which tends to require additional process (allowlisting, also often checking with a lawyer).

I suspect it should not be hard to add support to the license-exprs crate, it uses a nestable enum to represent license requirements. I might take a crack at it.

I took a quick glance at spdx and it's not actually clear to me that it supports parentheses; it seems to represent expressions as straight lists that include "And" and "Or", which doesn't work for nesting.

@sdroege
Copy link

sdroege commented Dec 9, 2021

I took a quick glance at spdx and it's not actually clear to me that it supports parentheses; it seems to represent expressions as straight lists that include "And" and "Or", which doesn't work for nesting.

I didn't test spdx directly but it's what cargo deny seems to use, and cargo deny handles an (A OR B) AND C expression just fine.

@Turbo87
Copy link
Member

Turbo87 commented Dec 9, 2021

the commit message in rust-lang/cargo@7dee65f might be relevant to this discussion.

the deprecation of the / syntax has happened 4 years ago, so I guess it should be okay to remove support for that at this point. this will only be relevant for new versions being published, while existing versions will still keep their existing license value.

https://doc.rust-lang.org/cargo/reference/manifest.html#the-license-and-license-file-fields states that:

crates.io interprets the license field as an SPDX 2.1 license expression. The name must be a known license from the SPDX license list 3.11.

but it looks like https://github.com/EmbarkStudios/spdx/ is currently on v3.14 of the license list, so that would be a behavior change for crates.io.

I would assume that this should be fine for the majority of cases though. Does anyone have an objection on why we shouldn't update that license list?

Regarding the example license expression above, the spdx crate parses (Apache-2.0 OR MIT) AND BSD-3-Clause to Ok(Apache-2.0 MIT OR BSD-3-Clause AND). The crate documentation states "Note that the expression is returned in post fix order.", so I guess that Debug output is correct.

I think the way forward would be:

  • write tests for the current license validation behavior
  • remove support for the non-standard / delimiter
  • migrate from license-exprs to spdx

@Turbo87 Turbo87 changed the title Support parentheses is SPDX Support parentheses in license fields Dec 9, 2021
@Turbo87 Turbo87 added A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works labels Dec 9, 2021
@Turbo87 Turbo87 reopened this Dec 9, 2021
bors added a commit that referenced this issue Dec 12, 2021
utils/license: Add support for parentheses

<img width="317" alt="Bildschirmfoto 2021-12-12 um 18 29 16" src="https://user-images.githubusercontent.com/141300/145722892-c331d94c-d22e-4fcb-a541-7a433724138f.png">

related to #2595
@Jake-Shadle
Copy link

Hey, author of spdx crate here, I noticed this issue and felt I could clarify some things.

  • The full spdx spec is supported, including parens, it's just that the crate's motivating use case was not only parsing of expressions, but evaluation, which is why it stores them in postfix order to make evaluation a simple stack. It also keeps the original source string for display purposes.
  • By default it disallows / and invalid license identifiers, eg apache-2.0 instead of Apache-2.0, but both of these can be enabled if needed via https://docs.rs/spdx/0.7.0/spdx/lexer/enum.ParseMode.html#variant.Lax
  • 3.11 to 3.14 should not be a behavior change, other than allowing new licenses that have been added between those versions. The spdx license list contains numerous deprecated license identifiers, but the crate still allows them, they just have a flag to indicate which ones have been marked as deprecated.

@Turbo87
Copy link
Member

Turbo87 commented Dec 16, 2021

thank you for the clarification, @Jake-Shadle :)

@Turbo87
Copy link
Member

Turbo87 commented Dec 18, 2021

FWIW I've opened EmbarkStudios/spdx#50 to potentially allow us to keep the / operator behavior, while not opting into all the other laxness of the spdx parser.

bors added a commit that referenced this issue Dec 18, 2021
Add tests for license expression validation

related to:
- #2595
@Turbo87
Copy link
Member

Turbo87 commented Dec 18, 2021

#4293 switches crates.io from license-exprs to spdx. reviews would be appreciated 🙏

@Jake-Shadle
Copy link

I've just published https://crates.io/crates/spdx/0.8.0 which contains the improvements made by @Turbo87, let me know if there are any other changes that need to be made to support the crates.io use case!

@bors bors closed this as completed in 77bfde0 Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants