ort icon indicating copy to clipboard operation
ort copied to clipboard

Consider relaxing the SPDX `licenseInfoInFiles` check

Open sschuberth opened this issue 1 year ago • 5 comments

We currently do

https://github.com/oss-review-toolkit/ort/blob/4629bd7b84c1d7db037faba52f930b01089e373a/utils/spdx/src/main/kotlin/model/SpdxFile.kt#L189-L198

which results in the SPDX report not to be written out at all if a license is not an SPDX expression with non-deprecated SPDX IDs and / or LicenseRef- "exceptions". This can be problematic if e.g. a scanner emits deprecated SPDX IDs, or declared licenses contain some free text license name.

My proposal is to not fail to write the report at all in this case, but just to log an error / warning in this case. This would allow the issue to become visible to the consumers of the SPDX report, instead of forcing them to get everything right even before the report gets written.

What do @mnonnenmacher and @fviernau think here?

sschuberth avatar Dec 20 '23 07:12 sschuberth

I would like if there was a reporter option like failOnInvalidLicense which defaults to false, then users that definitely do not want to have an SPDX file with invalid licenses could still keep the behavior to fail hard. The reason is that we currently do not have a good way to propagate issues from the reporters and any warning or error log is very likely not getting noticed.

mnonnenmacher avatar Dec 20 '23 10:12 mnonnenmacher

Just to be sure we're on the same page. We're talking about strings which do not match the SPDX expression grammar, right? Like whether it should fail for any random string like e.g. "my random string"?

fviernau avatar Dec 27 '23 08:12 fviernau

My goal would be to accept any strings that from the pure grammar syntax represents a valid SPDX expression. I.e. "my random string" should not be valid, but "foo AND bar" should.

~However, I'm right now quite confused that "my random string".toSpdx(ALLOW_ANY) actually passes, although I though it'd not be valid grammar syntax according to our ANTLR rules. Any idea whether that's intended @mnonnenmacher?~

Edit: Scratch the above, it was an issue with Kotest not using up-to-code code in my test. "my random string".toSpdx() does throw SpdxException as expected, but "my AND string".toSpdx() does not.

So bottom line, I propose to replace ALLOW_LICENSEREF_EXCEPTIONS with ALLOW_ANY in the code from the top post.

sschuberth avatar Dec 27 '23 13:12 sschuberth

I find it useful to let the user running ORT to make the choice.

Therefore implementing this a configuration value (with lenient behavior as the default) would IMHO the best way to go

rbieniek avatar Jan 25 '24 09:01 rbieniek

Just a note on the implementation: The validation happens in the SPDX model, not in the reporter. So exposing the validation level as an SPDX reporter specific option requires some refactoring of the SPDX model first to make the validation configurable.

sschuberth avatar Jan 25 '24 09:01 sschuberth