NoticeTemplate reporter fails if no scanner archive storage is configured

[sschuberth]

If no storage is configured to archive the found license files, then no original license texts (and copyright holders) can be obtained by the NoticeTemplate reporter. This currently causes the reporter to fail like

SEVERE: Error executing FreeMarker template
FreeMarker template error:
Java method "org.ossreviewtoolkit.model.licenses.ResolvedLicenseFile.readFile()" threw an exception when invoked on org.ossreviewtoolkit.model.licenses.ResolvedLicenseFile object "ResolvedLicenseFile(provenance=Provenance(downloadTime=2020-09-23T20:16:44.490769Z, sourceArtifact=null, vcsInfo=VcsInfo(type=Git, url=https://github.com/JetBrains/kotlin.git, revision=7fe66d3456a2e51e5bf228497e81b28a6367766c, resolvedRevision=7fe66d3456a2e51e5bf228497e81b28a6367766c, path=), originalVcsInfo=VcsInfo(type=Git, url=https://github.com/JetBrains/kotlin.git, revision=, resolvedRevision=null, path=)), licenses=[], path=license, file=/tmp/ort10253785273173582224archive/license)"; see cause exception in the Java stack trace.

----
FTL stack trace ("~" means nesting-related):
	- Failed at: ${licenseFile.readFile()}  [in template "default.ftl" at line 82, column 1]

but it should probably fail more gracefully.

[sschuberth]

@mnonnenmacher can you help with this?

[mnonnenmacher]

If the archiver is null the template should not try to read any license files. My assumption is that there is an archive, but it does not contain the expected files. This happens if the patterns configured for the archiver do not match the patterns used when creating the archive. With "no storage is configured", do you mean the default storage was used?

[sschuberth]

With "no storage is configured", do you mean the default storage was used?

Yes, exactly. I don't have (or ever had) the val archive: FileArchiverConfiguration configured, so it always was null.

[mnonnenmacher]

This problem would also happen if you run the scanner and reporter on different machines without configuring a remote storage. I think we should not ignore missing license files or just print a warning, as this might go unnoticed. What do you think of adding a reporter option like:

• -O NoticeTemplate.ignoreMissingLicenseFiles=true: To not fail if a license file is missing.
• or -O NoticeTemplate.includeLicenseFiles=false: To disable the inclusion of license files.

[sschuberth]

This problem would also happen if you run the scanner and reporter on different machines without configuring a remote storage.

Ah, which probably is the default case when using the stock Jenkinsfile as each step runs on a separate node.

I think we should not ignore missing license files or just print a warning, as this might go unnoticed.

Don't get me wrong, I'm fine with the reporter to fail in this case. I just believe it could do so more gracefully by showing a more user-friendly message with more background information.

[sschuberth]

What about falling back to the "normalized" / standard license text and printing a warning if no license text is found in an archive?

[mnonnenmacher]

I just believe it could do so more gracefully by showing a more user-friendly message with more background information.

The output you posted above contains the text "see cause exception in the Java stack trace", but the Java stack trace is missing. Can you add that? Because the actual exception in ResolvedLicenseFile.readFile() is missing, and we can only change that, but not how Freemarker is printing error information.

What about falling back to the "normalized" / standard license text and printing a warning if no license text is found in an archive?

I don't like printing a warning, because you only see it if you read the logs which is something rarely done if a job succeeds. It should be a user choice, if you want the license files to be included, it should fail if the required files are not available. That's why I proposed the two options above. ignoreMissingLicenseFiles could also be called doNotFailOnMissingLicenseFile.

Maybe we should change the default template to be one that does not include license files?

[sschuberth]

but the Java stack trace is missing. Can you add that?

Here you go:

Java stack trace (for programmers):
----
freemarker.core._TemplateModelException: [... Exception message was already printed; see it above ...]
  at freemarker.ext.beans._MethodUtil.newInvocationTemplateModelException(_MethodUtil.java:291)
  at freemarker.ext.beans._MethodUtil.newInvocationTemplateModelException(_MethodUtil.java:254)
  at freemarker.ext.beans.SimpleMethodModel.exec(SimpleMethodModel.java:78)
  at freemarker.core.MethodCall._eval(MethodCall.java:62)
  at freemarker.core.Expression.eval(Expression.java:101)
  at freemarker.core.DollarVariable.calculateInterpolatedStringOrMarkup(DollarVariable.java:100)
  at freemarker.core.DollarVariable.accept(DollarVariable.java:63)
  at freemarker.core.Environment.visit(Environment.java:370)
  at freemarker.core.IteratorBlock$IterationContext.executedNestedContentForCollOrSeqListing(IteratorBlock.java:321)
  at freemarker.core.IteratorBlock$IterationContext.executeNestedContent(IteratorBlock.java:271)
  at freemarker.core.IteratorBlock$IterationContext.accept(IteratorBlock.java:244)
  at freemarker.core.Environment.visitIteratorBlock(Environment.java:644)
  at freemarker.core.IteratorBlock.acceptWithResult(IteratorBlock.java:108)
  at freemarker.core.IteratorBlock.accept(IteratorBlock.java:94)
  at freemarker.core.Environment.visit(Environment.java:334)
  at freemarker.core.Environment.visit(Environment.java:376)
  at freemarker.core.IteratorBlock$IterationContext.executedNestedContentForCollOrSeqListing(IteratorBlock.java:321)
  at freemarker.core.IteratorBlock$IterationContext.executeNestedContent(IteratorBlock.java:271)
  at freemarker.core.IteratorBlock$IterationContext.accept(IteratorBlock.java:244)
  at freemarker.core.Environment.visitIteratorBlock(Environment.java:644)
  at freemarker.core.IteratorBlock.acceptWithResult(IteratorBlock.java:108)
  at freemarker.core.IteratorBlock.accept(IteratorBlock.java:94)
  at freemarker.core.Environment.visit(Environment.java:334)
  at freemarker.core.Environment.visit(Environment.java:340)
  at freemarker.core.Environment.process(Environment.java:313)
  at freemarker.template.Template.process(Template.java:383)
  at org.ossreviewtoolkit.reporter.reporters.NoticeTemplateReporter.generateReport(NoticeTemplateReporter.kt:113)
  at org.ossreviewtoolkit.commands.ReporterCommand.run(ReporterCommand.kt:257)
  at com.github.ajalt.clikt.parsers.Parser.parse(Parser.kt:171)
  at com.github.ajalt.clikt.parsers.Parser.parse(Parser.kt:180)
  at com.github.ajalt.clikt.parsers.Parser.parse(Parser.kt:16)
  at com.github.ajalt.clikt.core.CliktCommand.parse(CliktCommand.kt:395)
  at com.github.ajalt.clikt.core.CliktCommand.parse$default(CliktCommand.kt:392)
  at com.github.ajalt.clikt.core.CliktCommand.main(CliktCommand.kt:410)
  at com.github.ajalt.clikt.core.CliktCommand.main(CliktCommand.kt:435)
  at org.ossreviewtoolkit.OrtMainKt.main(OrtMain.kt:194)
Caused by: java.io.FileNotFoundException: /tmp/ort15844618501012269741archive/license (Is a directory)
  at java.base/java.io.FileInputStream.open0(Native Method)
  at java.base/java.io.FileInputStream.open(Unknown Source)
  at java.base/java.io.FileInputStream.<init>(Unknown Source)
  at kotlin.io.FilesKt__FileReadWriteKt.readText(FileReadWrite.kt:125)
  at kotlin.io.FilesKt__FileReadWriteKt.readText$default(FileReadWrite.kt:125)
  at org.ossreviewtoolkit.model.licenses.ResolvedLicenseFile.readFile(ResolvedLicenseFileInfo.kt:81)
  at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
  at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
  at java.base/java.lang.reflect.Method.invoke(Unknown Source)
  at freemarker.ext.beans.BeansWrapper.invokeMethod(BeansWrapper.java:1552)
  at freemarker.ext.beans.SimpleMethodModel.exec(SimpleMethodModel.java:73)
  ... 33 more

Doesn't the path "/tmp/ort15844618501012269741archive/license" also look odd as there is no "/" before "archive"?

I don't like printing a warning, because you only see it if you read the logs which is something rarely done if a job succeeds.

When I wrote "print" I actually meant an OrtIssue in the result, with level "warning".

That's why I proposed the two options above.

Maybe better fallBackToStandardLicenseTexts?

Maybe we should change the default template to be one that does not include license files?

I don't think that's necessary; it was the main point for the new NOTICE generator to use the literal / unmodified license texts. User's who don't want / need this could use the summary NOTICE file, which does not require the archive, correct?

[sschuberth]

I cannot reproduce this anymore.