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

License entry from dep5 file not respected for some files #229

Closed
sschuberth opened this issue May 14, 2020 · 24 comments
Closed

License entry from dep5 file not respected for some files #229

sschuberth opened this issue May 14, 2020 · 24 comments

Comments

@sschuberth
Copy link

Our automated reuse lint check fails for this PR although all files in that directory are set to have a different license.

Still, reuse only complains about CAL-1.0 and CAL-1.0-Combined-Work-Exception.

Any idea why that is?

@carmenbianca
Copy link
Member

Ah, the latest version of reuse (0.10.0) has an outdated SPDX license list shipped with it. I'll release a 0.10.1 that has an updated license list.

@sschuberth
Copy link
Author

Thanks, I thought something like that, but why does it matter in this case? If I put a directory under a fixed license via dep5 it should basically be ignored by reuse, or?

@carmenbianca
Copy link
Member

The tool also verifies whether the used licenses are valid. This is handy to prevent accidents such as SPDX-License-Identifier: GPL-3.9-or-later, which obviously doesn't exist.

@sschuberth
Copy link
Author

Well, I agree it makes sense to verify that the License specified in dep5 (in this case CC0-1.0) is valid. But why does reuse also complain about the contents of files in a directory it should ignore licensing-wise?

@carmenbianca
Copy link
Member

Uploaded https://pypi.org/project/reuse/0.10.1/, only change is an updated license list :)

But why does reuse also complain about the contents of files in a directory it should ignore licensing-wise?

Ahh, I think I get it. It doesn't actually ignore the files when dep5 is used—it draws from both sources. This is a little clunky admittedly. I vaguely remember discussing this design decision with someone. I'd love to figure out a way to make that less clunky.

@sschuberth
Copy link
Author

Uploaded https://pypi.org/project/reuse/0.10.1/, only change is an updated license list :)

Thanks for the quick fix! But I still get:

ort (spdx-license-list-3-8) $ reuse --version
reuse 0.10.1
ort (spdx-license-list-3-8) $ reuse lint
# MISSING LICENSES

'CAL-1.0' found in:
* spdx-utils/src/main/resources/licenses/CAL-1.0
* spdx-utils/src/main/resources/licenses/CAL-1.0-Combined-Work-Exception

'CAL-1.0-Combined-Work-Exception' found in:
* spdx-utils/src/main/resources/licenses/CAL-1.0
* spdx-utils/src/main/resources/licenses/CAL-1.0-Combined-Work-Exception


# SUMMARY

* Bad licenses:
* Deprecated licenses:
* Licenses without file extension:
* Missing licenses: CAL-1.0, CAL-1.0-Combined-Work-Exception
* Unused licenses:
* Used licenses: Apache-2.0, CC-BY-3.0, CC0-1.0
* Read errors: 0
* Files with copyright information: 2416 / 2416
* Files with license information: 2416 / 2416

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

So, looks like the BAD LICENSES error is gone, but the MISSING LICENSES error remains.

This is a little clunky admittedly.

Indeed it is 😉 But thanks for being open about it!

@carmenbianca
Copy link
Member

Oh, it has the tag inside of the license file itself. I don't have a quick fix for that immediately. Normally the license would be in LICENSES/, which is completely ignored. And the tool kind of explicitly doesn't have an "ignore" mechanism because that would undermine its purpose. This really shouldn't cause problems, but it makes the tool super ill-fitted for programs that focus on compliance. To make the tool compliant with itself, I've had to do a little bit of hackery.

The simple workaround is to create a CAL-1.0.license file, which overrides the file contents. But that may not be an ideal solution.

Other solutions require this program to be amended:

  • Override with dep5 instead of using both sources.
  • Add an ignore mechanism after all.
  • Detect this specific scenario and deal with it?

The most low-effort of these is to add a work-around for this very specific issue: If the file name is CAL-1.0[.ext], ignore its contents. This doesn't fix the class of issues, but does remove your blocker, and puts the pressure off coming up with a solution swiftly.

@mxmehl Do you have thoughts on this issue? Context: ORT ships a directory with all SPDX licenses. One of the SPDX licenses contains SPDX-License-Identifier: [itself]. The directory containing the licenses is whitelisted in .reuse/dep5, but the tool checks both .reuse/dep5 and the file contents.

@sschuberth
Copy link
Author

And the tool kind of explicitly doesn't have an "ignore" mechanism because that would undermine its purpose.

That makes sense, and I agree there shouldn't be an explicit ignore mechanism. But I still believe reuse shouldn't look at the contents of files in directories that have been given a (valid) license via dep5. Which basically matches your first proposal, right?

@carmenbianca
Copy link
Member

Correct.

@sschuberth
Copy link
Author

@mxmehl Do you have thoughts on this issue? Context: ORT ships a directory with all SPDX licenses. One of the SPDX licenses contains SPDX-License-Identifier: [itself]. The directory containing the licenses is whitelisted in .reuse/dep5, but the tool checks both .reuse/dep5 and the file contents.

@mxmehl, did you have a chance to discuss this with @carmenbianca?

@mxmehl
Copy link
Member

mxmehl commented May 19, 2020

Sorry, Github's new notification system still confuses me sometimes...

For this specific case, I quite like the idea of the dep5 file overriding file content. However, in the general design, I usually prefer that the closer a definition is to the file, the more weight it should have.

However, as we can see in this case, file content can be ambiguous and misleading, so I would suggest the following order: .license > dep5 > file content. My rationale: the .license file is closer to the file, and is completely under REUSE's control, so there should be no misleading content. The dep5 file would be the next possibility to define overrides.

However, it's not logical in the sense that this would not be a preference chain nearest -> farthest or the other way round, so I'm split here.

@sschuberth
Copy link
Author

Sorry, Github's new notification system still confuses me sometimes...

Yeah, I hate it, too 😉

However, it's not logical in the sense that this would not be a preference chain nearest -> farthest or the other way round, so I'm split here.

I believe it is consistent if you only look at the "scope" of the overrides, leaving the file itself out of the chain: A broader override (dep5) has lower priority than a narrower override (.license).

@carmenbianca
Copy link
Member

I've had scarce little time to do REUSE things past week. I'll look into this really soon.

carmenbianca pushed a commit to carmenbianca/reuse-tool that referenced this issue May 25, 2020
These licenses contain SPDX tags referring to themselves. This causes
issues in <fsfe#229>. Ignoring them
is a workaround.

Signed-off-by: Carmen Bianca Bakker <[email protected]>
carmenbianca pushed a commit to carmenbianca/reuse-tool that referenced this issue May 25, 2020
These licenses contain SPDX tags referring to themselves. This causes
issues in <fsfe#229>. Ignoring them
is a workaround.

Signed-off-by: Carmen Bianca Bakker <[email protected]>
@carmenbianca
Copy link
Member

Released 0.11.0 that should be a stopgap solution until I can implement the proper order of precedence as suggested above.

@sschuberth
Copy link
Author

Thanks, version 0.11.0 worked for me!

@sschuberth
Copy link
Author

sschuberth commented May 28, 2020

@carmenbianca
Copy link
Member

@sschuberth Sorry for the super late response. Released v0.11.1 with a workaround. I'm a little scarce on time.

@sschuberth
Copy link
Author

Thanks @carmenbianca, one again your patched version 0.11.1 worked.

@sschuberth
Copy link
Author

However, I just realized the REUSE web site itself seems to use an older version than 0.11.1 as ORT is marked as not compliant there 😭

@carmenbianca
Copy link
Member

@mxmehl ?

Also I'll be able to review a lot of the open issues in a few weeks or so. I am currently in the process of writing my thesis, so that's eating into a lot of my time.

@mxmehl
Copy link
Member

mxmehl commented Jun 23, 2020

Sorry, the API server had to be updated to use the latest reuse tool version which I forgot after the 0.11.1 release. Looks good now ;)

@mxmehl mxmehl closed this as completed Jun 23, 2020
@sschuberth
Copy link
Author

Thanks! You might want to keep this issue open, though, because the 0.11.1 release only contains a work-around for the real issue AFAIU.

@oddhack
Copy link

oddhack commented Aug 17, 2020

I ran into a variant of this in #253 (sorry, I see several loosely related issues around this topic so am somewhat repeating myself). ISTM the order of matching should be .license, if present ; imbedded in file, if present and parseable; and matched by dep5, if neither of the above. A tricky situation is if there's a wildcard in dep5 which captures a file that has its own license information. Generally you would want to use the information in the file but there might be cases where dep5 should override. Rather than invent a syntax for that and complexify things further, ISTM that so long as this situation is rare, the recommendation should be to add a .license file when an override of the file's own licensing information is required.

@mxmehl
Copy link
Member

mxmehl commented Mar 16, 2021

I'll close this in favour of fsfe/reuse-docs#70 where we work on defining the precendence of REUSE information. As the spec changes, it will be worked in the helper tool at the same time.

@mxmehl mxmehl closed this as completed Mar 16, 2021
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

No branches or pull requests

4 participants