Clearer warnings in license review

build/build/src/engine/context.rs

@@ -349,12 +349,6 @@ impl RunContext {
                 tasks.push("engine-runner/buildNativeImage");
             }
 
-            if TARGET_OS != OS::Windows {
-                // FIXME [mwu] apparently this is broken on Windows because of the line endings
-                // mismatch
-                tasks.push("verifyLicensePackages");
-            }
-
             if self.config.build_project_manager_package() {
                 tasks.push("buildProjectManagerDistribution");
             }
@@ -565,8 +559,14 @@ impl RunContext {
             runner_sanity_test(&self.repo_root, Some(enso_java)).await?;
         }
 
+        // Verify the status of the License Review Report
+        if TARGET_OS != OS::Windows {
+            // FIXME [mwu] apparently this is broken on Windows because of the line endings
+            // mismatch
+            sbt.call_arg("verifyLicensePackages").await?;
+        }
 
-        // Verify License Packages in Distributions
+        // Verify Integrity of Generated License Packages in Distributions
         // FIXME apparently this does not work on Windows due to some CRLF issues?
         if self.config.verify_packages && TARGET_OS != OS::Windows {
             for package in ret.packages() {

distribution/engine/THIRD-PARTY/NOTICE

@@ -66,11 +66,6 @@ The license file can be found at `licenses/APACHE2.0`.
 Copyright notices related to this dependency can be found in the directory `com.google.guava.guava-32.0.0-jre`.
 
 
-'listenablefuture', licensed under the The Apache Software License, Version 2.0, is distributed with the engine.
-The license file can be found at `licenses/APACHE2.0`.
-Copyright notices related to this dependency can be found in the directory `com.google.guava.listenablefuture-9999.0-empty-to-avoid-conflict-with-guava`.
-
-
 'j2objc-annotations', licensed under the Apache License, Version 2.0, is distributed with the engine.
 The license file can be found at `licenses/APACHE2.0`.
 Copyright notices related to this dependency can be found in the directory `com.google.j2objc.j2objc-annotations-2.8`.

distribution/lib/Standard/Google_Api/0.0.0-dev/THIRD-PARTY/NOTICE

@@ -91,11 +91,6 @@ The license file can be found at `licenses/APACHE2.0`.
 Copyright notices related to this dependency can be found in the directory `com.google.guava.guava-32.1.3-jre`.
 
 
-'listenablefuture', licensed under the The Apache Software License, Version 2.0, is distributed with the Google_Api.
-The license file can be found at `licenses/APACHE2.0`.
-Copyright notices related to this dependency can be found in the directory `com.google.guava.listenablefuture-9999.0-empty-to-avoid-conflict-with-guava`.
-
-
 'google-http-client', licensed under the The Apache Software License, Version 2.0, is distributed with the Google_Api.
 The license file can be found at `licenses/APACHE2.0`.
 Copyright notices related to this dependency can be found in the directory `com.google.http-client.google-http-client-1.43.3`.

project/GatherLicenses.scala

@@ -44,9 +44,8 @@ object GatherLicenses {
         s"It consists of the following sbt project roots:" +
         s" ${projectNames.mkString(", ")}"
       )
-      val (sbtInfo, sbtWarnings) =
+      val (sbtInfo, sbtDiagnostics) =
         SbtLicenses.analyze(distribution.sbtComponents, log)
-      sbtWarnings.foreach(log.warn(_))
 
       val allInfo = sbtInfo // TODO [RW] add Rust frontend result here (#1187)
 
@@ -59,32 +58,42 @@ object GatherLicenses {
           s"${dependency.url}"
         )
         val defaultAttachments = defaultBackend.run(dependency.sources)
-        val attachments =
-          if (defaultAttachments.nonEmpty) defaultAttachments
+        val WithDiagnostics(attachments, attachmentDiagnostics) =
+          if (defaultAttachments.nonEmpty) WithDiagnostics(defaultAttachments)
           else GithubHeuristic(dependency, log).run()
-        (dependency, attachments)
+        (dependency, attachments, attachmentDiagnostics)
       }
 
-      val summary          = DependencySummary(processed)
-      val distributionRoot = configRoot / distribution.artifactName
-      val WithWarnings(processedSummary, summaryWarnings) =
+      val forSummary            = processed.map(t => (t._1, t._2))
+      val processingDiagnostics = processed.flatMap(_._3)
+      val summary               = DependencySummary(forSummary)
+      val distributionRoot      = configRoot / distribution.artifactName
+      val WithDiagnostics(processedSummary, summaryDiagnostics) =
         Review(distributionRoot, summary).run()
-      val allWarnings = sbtWarnings ++ summaryWarnings
+      val allDiagnostics =
+        sbtDiagnostics ++ processingDiagnostics ++ summaryDiagnostics
       val reportDestination =
         targetRoot / s"${distribution.artifactName}-report.html"
 
-      sbtWarnings.foreach(log.warn(_))
-      if (summaryWarnings.size > 10)
-        log.warn(
-          s"There are too many warnings (${summaryWarnings.size}) to " +
-          s"display. Please inspect the generated report."
-        )
-      else allWarnings.foreach(log.warn(_))
+      val (warnings: Seq[Diagnostic.Warning], errors: Seq[Diagnostic.Error]) =
+        Diagnostic.partition(allDiagnostics)
+
+      if (warnings.nonEmpty) {
+        log.warn(s"Found ${warnings.size} non-fatal warnings in the report:")
+        warnings.foreach(notice => log.warn(notice.message))
+      }
+
+      if (errors.isEmpty) {
+        log.info("No fatal errors found in the report.")
+      } else {
+        log.error(s"Found ${errors.size} fatal errors in the report:")
+        errors.foreach(problem => log.error(problem.message))
+      }
 
       Report.writeHTML(
         distribution,
         processedSummary,
-        allWarnings,
+        allDiagnostics,
         reportDestination
       )
       log.info(
@@ -96,10 +105,10 @@ object GatherLicenses {
       ReportState.write(
         distributionRoot / stateFileName,
         distribution,
-        summaryWarnings.length
+        errors.size
       )
       log.info(s"Re-generated distribution notices at `$packagePath`.")
-      if (summaryWarnings.nonEmpty) {
+      if (errors.nonEmpty) {
         log.warn(
           "The distribution notices were regenerated, but there are " +
           "not-reviewed issues within the report. The notices are probably " +
@@ -176,7 +185,7 @@ object GatherLicenses {
       .getOrElse(
         throw LegalReviewException(
           s"Report at $distributionConfig is not available. " +
-          s"Make sure to run `enso/gatherLicenses`."
+          s"Make sure to run `enso/gatherLicenses` or `openLegalReviewReport`."
         )
       )
 

project/src/main/scala/licenses/DependencySummary.scala

@@ -2,9 +2,10 @@ package src.main.scala.licenses
 
 import sbt.IO
 import src.main.scala.licenses.report.{
+  Diagnostic,
   LicenseReview,
   PackageNotices,
-  WithWarnings
+  WithDiagnostics
 }
 
 /** Contains a sequence of dependencies and any attachments found.
@@ -122,37 +123,54 @@ object ReviewedSummary {
 
   /** Returns a list of warnings that indicate missing reviews or other issues.
     */
-  def warnAboutMissingReviews(summary: ReviewedSummary): WithWarnings[Unit] = {
-    val warnings = summary.dependencies.flatMap { dep =>
-      val warnings = collection.mutable.Buffer[String]()
-      val name     = dep.information.moduleInfo.toString
+  def warnAboutMissingReviews(
+    summary: ReviewedSummary
+  ): WithDiagnostics[Unit] = {
+    val diagnostics = summary.dependencies.flatMap { dep =>
+      val diagnostics = collection.mutable.Buffer[Diagnostic]()
+      val name        = dep.information.moduleInfo.toString
 
       val missingFiles = dep.files.filter(_._2 == AttachmentStatus.NotReviewed)
       if (missingFiles.nonEmpty) {
-        warnings.append(
-          s"${missingFiles.size} files are not reviewed in $name."
+        diagnostics.append(
+          Diagnostic.Error(
+            s"${missingFiles.size} files are not reviewed in $name."
+          )
         )
       }
       val missingCopyrights =
         dep.copyrights.filter(_._2 == AttachmentStatus.NotReviewed)
       if (missingCopyrights.nonEmpty) {
-        warnings.append(
-          s"${missingCopyrights.size} copyrights are not reviewed in $name."
+        diagnostics.append(
+          Diagnostic.Error(
+            s"${missingCopyrights.size} copyrights are not reviewed in $name."
+          )
         )
       }
 
       val includedInfos =
         (dep.files.map(_._2) ++ dep.copyrights.map(_._2)).filter(_.included)
       if (includedInfos.isEmpty) {
-        warnings.append(
-          s"No files or copyright information are included for $name."
+        diagnostics.append(
+          Diagnostic.Error(
+            s"No files or copyright information are included for $name. " +
+            s"Generally every dependency should have _some_ copyright info, so " +
+            s"this suggests all our heuristics failed. " +
+            s"Please find the information manually and add it using `files-add` " +
+            s"or `copyright-add`. Even if the dependency is in public domain, " +
+            s"it may be good to include some information about its source."
+          )
         )
       }
 
       dep.licenseReview match {
         case LicenseReview.NotReviewed =>
-          warnings.append(
-            s"License ${dep.information.license.name} for $name is not reviewed."
+          diagnostics.append(
+            Diagnostic.Error(
+              s"Default license ${dep.information.license.name} for $name is " +
+              s"used, but that license is not reviewed " +
+              s"(need to add an entry to `reviewed-licenses`)."
+            )
           )
         case LicenseReview.Default(
               defaultPath,
@@ -163,10 +181,17 @@ object ReviewedSummary {
               case Some(includedLicense) =>
                 val licenseContent = IO.read(defaultPath.toFile)
                 if (licenseContent.strip != includedLicense.content) {
-                  warnings.append(
-                    s"A license file was discovered in $name that is different " +
-                    s"from the default license file that is associated with its " +
-                    s"license ${dep.information.license.name}."
+                  diagnostics.append(
+                    Diagnostic.Error(
+                      s"A license file was discovered in $name that is different " +
+                      s"from the default license file that is associated with its " +
+                      s"license ${dep.information.license.name}, " +
+                      s"but a custom license was not expected. " +
+                      s"If this custom license should override the default one, " +
+                      s"create a `custom-license` config file. " +
+                      s"If both files are expected to be included, " +
+                      s"create an empty `default-and-custom-license` file."
+                    )
                   )
                 }
               case None =>
@@ -178,14 +203,16 @@ object ReviewedSummary {
           val fileWillBeIncludedAsCopyrightNotices =
             filename == PackageNotices.gatheredNoticesFilename
           if (!fileIsIncluded && !fileWillBeIncludedAsCopyrightNotices) {
-            warnings.append(
-              s"License for $name is set to custom file `$filename`, but no such file is attached."
+            diagnostics.append(
+              Diagnostic.Error(
+                s"License for $name is set to custom file `$filename`, but no such file is attached."
+              )
             )
           }
       }
 
-      warnings
+      diagnostics
     }
-    WithWarnings.justWarnings(warnings)
+    WithDiagnostics.justDiagnostics(diagnostics)
   }
 }

project/src/main/scala/licenses/backend/GithubHeuristic.scala

@@ -1,17 +1,17 @@
 package src.main.scala.licenses.backend
 
 import java.nio.file.Path
-
 import sbt.Logger
 import sbt.io.syntax.url
+import src.main.scala.licenses.report.{Diagnostic, WithDiagnostics}
 import src.main.scala.licenses.{
   AttachedFile,
   Attachment,
   DependencyInformation,
   PortablePath
 }
 
-import scala.sys.process._
+import scala.sys.process.*
 import scala.util.control.NonFatal
 
 /** Tries to find copyright mentions in the GitHub project homepage and any
@@ -28,11 +28,11 @@ case class GithubHeuristic(info: DependencyInformation, log: Logger) {
     *
     * It proceeds only if the project has an URL that seems to point to GitHub.
     */
-  def run(): Seq[Attachment] = {
+  def run(): WithDiagnostics[Seq[Attachment]] = {
     info.url match {
       case Some(url) if url.contains("github.com") =>
         tryDownloadingAttachments(url.replace("http://", "https://"))
-      case _ => Seq()
+      case _ => WithDiagnostics(Seq(), Seq())
     }
   }
 
@@ -41,15 +41,23 @@ case class GithubHeuristic(info: DependencyInformation, log: Logger) {
     *
     * Any found files are fetched and saved into the results.
     */
-  def tryDownloadingAttachments(address: String): Seq[Attachment] =
+  def tryDownloadingAttachments(
+    address: String
+  ): WithDiagnostics[Seq[Attachment]] =
     try {
       val homePage    = url(address).cat.!!
       val branchRegex = """"defaultBranch":"([^"]*?)"""".r("branch")
       val branch      = branchRegex.findFirstMatchIn(homePage).map(_.group("branch"))
       branch match {
         case None =>
-          log.warn(s"Cannot find default branch for $address")
-          Seq()
+          WithDiagnostics(
+            Seq(),
+            Seq(
+              Diagnostic.Error(
+                s"GitHub heuristic failure: Cannot find default branch for $address"
+              )
+            )
+          )
         case Some(branch) =>
           val fileRegex =
             """\{"name":"([^"]*?)","path":"([^"]*?)","contentType":"file"\}"""
@@ -59,7 +67,7 @@ case class GithubHeuristic(info: DependencyInformation, log: Logger) {
             .map(m => (m.group("name"), m.group("path")))
             .filter(p => mayBeRelevant(p._1))
             .toList
-          matches.flatMap { case (_, path) =>
+          val results = matches.map { case (_, path) =>
             val rawHref = address + "/raw/" + branch + "/" + path
             // This path is reconstructed to match the 'legacy' format for compatibility with older versions of the review settings.
             // It has the format <org>/<repo>/blob/<branch>/<path>
@@ -68,26 +76,41 @@ case class GithubHeuristic(info: DependencyInformation, log: Logger) {
               .stripSuffix("/") + "/blob/" + branch + "/" + path
             try {
               val content = url(rawHref).cat.!!
-              Seq(
-                AttachedFile(
-                  PortablePath.of(internalPath),
-                  content,
-                  origin = Some(address)
+              WithDiagnostics(
+                Seq(
+                  AttachedFile(
+                    PortablePath.of(internalPath),
+                    content,
+                    origin = Some(address)
+                  )
                 )
               )
             } catch {
               case NonFatal(error) =>
-                log.warn(
-                  s"Found file $rawHref but cannot download it: $error"
+                WithDiagnostics(
+                  Seq(),
+                  Seq(
+                    Diagnostic.Error(
+                      s"GitHub heuristic failure: " +
+                      s"Found file $rawHref but cannot download it: $error"
+                    )
+                  )
                 )
-                Seq()
             }
           }
+          results.flip.map(_.flatten)
       }
     } catch {
       case NonFatal(error) =>
-        log.warn(s"GitHub backend for ${info.packageName} failed with $error")
-        Seq()
+        WithDiagnostics(
+          Seq(),
+          Seq(
+            Diagnostic.Error(
+              s"GitHub heuristic failure: " +
+              s"processing ${info.packageName} failed with error: $error"
+            )
+          )
+        )
     }
 
   /** Decides if the file may be relevant and should be included in the result.