Consider relaxing the SPDX licenseInfoInFiles check

[sschuberth]

We currently do

ort/utils/spdx/src/main/kotlin/model/SpdxFile.kt

Lines 189 to 198 in 4629bd7

	// TODO: The check for [licenseInfoInFiles] can be made more strict, but the SPDX specification is not exact
	// enough yet to do this safely.
	licenseInfoInFiles.filterNot {
	it.isSpdxExpressionOrNotPresent(ALLOW_LICENSEREF_EXCEPTIONS)
	}.let { nonSpdxLicenses ->
	require(nonSpdxLicenses.isEmpty()) {
	"The entries in 'licenseInfoInFiles' must each be either an SPDX expression, 'NONE' or " +
	"'NOASSERTION', but found ${nonSpdxLicenses.joinToString { "'$it'" }}."
	}
	}

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?

[mnonnenmacher]

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.

[fviernau]

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"?

[sschuberth]

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.

[rbieniek]

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

[sschuberth]

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]

Maybe something like https://github.com/nesk/akkurate could be used to first separate the SPDX validation from the data class constructors, and then only call the validators if configured to do so.

[sschuberth]

Maybe something like https://github.com/nesk/akkurate could be used to first separate the SPDX validation from the data class constructors, and then only call the validators if configured to do so.

Another alternative validation library is https://github.com/konform-kt/konform which is what ORT Server already uses.