Skip to content

Map a declared license that refers to "LICENSE.txt" to NONE #3347

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sschuberth
Copy link
Member

The string "LICENSE.txt" is not a declared license, and the contents of
"LICENSE.txt" would be scanned by the scanner, so map it to NONE here.

Seen in https://github.com/Knio/dominate/blob/2.3.1/setup.py#L46.

Signed-off-by: Sebastian Schuberth [email protected]

The string "LICENSE.txt" is not a declared license, and the contents of
"LICENSE.txt" would be scanned by the scanner, so map it to NONE here.

Seen in https://github.com/Knio/dominate/blob/2.3.1/setup.py#L46.

Signed-off-by: Sebastian Schuberth <[email protected]>
@@ -299,6 +299,7 @@
"LGPL, version 3.0": LGPL-2.1-only
"LGPL/MIT": LGPL-3.0-only OR MIT
"LGPLv3 or later": LGPL-3.0-or-later
"LICENSE.txt": NONE
Copy link
Member

@fviernau fviernau Nov 19, 2020

Choose a reason for hiding this comment

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

commit: The argumentation for mapping this to "NONE" does apply also to other references, like the URLs which we do not map to "NONE". It seems inconsistent to me. What do you think?

_edit:
Maybe the above didn't make sense as URLs are pointing to location not in the repo. But how about that:

Would the rule of thumb become "map any declared license which references a file in the sources to "NONE"" ? I wonder if it's ok to make that choice for our users, in particular having such entry would make it impossible for the users to handling it differently.

Also we now have an heuristic for determining from the detected licenses which LICENSE files do actually apply.
If the heuristic says that "LICENSE.txt" does not apply, then it will be filtered out and thus the assumption made in the commit - that it would be covered by the scan - would not hold.

What do you think?
_

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, that somewhat relates to the discussion we had before, that we ideally should only have universally valid mappings in here. Strictly speaking, a mapping like

"https://github.com/dotnet/corefx/blob/master/LICENSE.TXT": MIT

would not be universally valid in my view, because master could move and the referred LICENSE.TXT file be changed. But for convenience, we so far accepted this "risk", assuming that it's unlikely for a project like dotnet to change it's license, and at least we do know where which project we talk about.

But the "LICENSE.txt": NONE case is different in that we do not even know which project we are talking about. There is no way how we could conveniently map "LICENSE.txt" to anything meaningful, like we can for "https://github.com/dotnet/corefx/blob/master/LICENSE.TXT".

Copy link
Member Author

Choose a reason for hiding this comment

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

Also we now have an heuristic for determining from the detected licenses which LICENSE files do actually apply.

I'm not quite sure what "heuristic" you talk about... do you mean the glob matching of license files?

If the heuristic says that "LICENSE.txt" does not apply, then it will be filtered out and thus the assumption made in the commit - that it would be covered by the scan - would not hold.

That's a theoretical issue, yes. But maybe just a matter of me rewording the commit message in that regard. Generally, I don't see another way than relying on the detected license here, because the declared license is non-telling.

Copy link
Member

@fviernau fviernau Nov 19, 2020

Choose a reason for hiding this comment

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

I'm not quite sure what "heuristic" you talk about...

See https://github.com/oss-review-toolkit/ort/blob/master/model/src/main/kotlin/ScanResult.kt#L62-L65

That's a theoretical issue, yes.

Why that?

I don't see another way than relying on the detected license here, because the declared license is non-telling.

Did you consider a curation with a declared_license_mapping as solution?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why that?

Why what? Why it's an issue, or why it's theoretical?

I just wanted to confirm your view that it is an issue, yes, but that the likeliness that it hits us in a bad way is probably not that big. Anyway, not having this mapping would not make this theoretical issue go away. That's why I said that only my wording in the commit message is maybe a bit misleading here.

Did you consider a curation with a declared_license_mapping as solution?

IMO that's too specific. "LICENSE.txt" never is a meaningful declared license, so I do not see why we should limit the mapping to a specific package with a curation.

Going a step back by looking at this PR again and phrasing the question differently, why were you fine with adding

"OSI Approved": NONE

as a mapping, but here you're not fine with adding

"LICENSE.txt": NONE

as a mapping. What's the difference for you?

Copy link
Member

Choose a reason for hiding this comment

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

Why what? Why it's an issue, or why it's theoretical?

theoretical

IMO that's too specific. "LICENSE.txt" never is a meaningful declared license, so I do not see why we should limit the mapping to a specific package with a curation.

I think choosing the solution to map it to "NONE" is valid. Probably I would also choose it as I like the risk / effort trade-off.
I just wanted to point out that other trade-offs would be valid as well and maybe be preferred by other users.

"OSI Approved": NONE...What's the difference for you?

For "LICENSE.txt" to me it seemed 100% clear to what this refers to. E.g. the license file.
For "OSI Approved" it seemed not 100% clear to what license this refers to. But I do actually agree with you
that "OSI Approved" should be treated consistently with "LICENSE.txt".

Anyhow, re-considering the risk related to the entries in the file I'm fine with merging also this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out we cannot have this mapping easily: According to our rules, it should be added to simple-license-mapping (not to declared-license-mapping like I did here), but in simple-license-mapping the values need to be of type SpdxLicense, which NONE isn't.

I'm not sure how to go forward now. So far we've avoided to explicitly add NONE and NOASSERTION to SpdxLicense, and I'm not sure whether we should add it now.

Any opinions?

@sschuberth sschuberth added the on hold Pull requests that cannot currently be merged label Mar 24, 2021
@sschuberth sschuberth marked this pull request as draft October 21, 2022 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold Pull requests that cannot currently be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants