From eda892cafd4e9753186d9176b885f01da31218c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 16:46:17 +0100 Subject: [PATCH 01/15] rename --- project/GatherLicenses.scala | 2 +- .../scala/licenses/DependencySummary.scala | 6 +- .../main/scala/licenses/report/Review.scala | 32 +++++------ .../licenses/report/WithDiagnostics.scala | 56 +++++++++++++++++++ .../scala/licenses/report/WithWarnings.scala | 56 ------------------- 5 files changed, 76 insertions(+), 76 deletions(-) create mode 100644 project/src/main/scala/licenses/report/WithDiagnostics.scala delete mode 100644 project/src/main/scala/licenses/report/WithWarnings.scala diff --git a/project/GatherLicenses.scala b/project/GatherLicenses.scala index 9573dc844346..dd4f5bf1c743 100644 --- a/project/GatherLicenses.scala +++ b/project/GatherLicenses.scala @@ -67,7 +67,7 @@ object GatherLicenses { val summary = DependencySummary(processed) val distributionRoot = configRoot / distribution.artifactName - val WithWarnings(processedSummary, summaryWarnings) = + val WithDiagnostics(processedSummary, summaryWarnings) = Review(distributionRoot, summary).run() val allWarnings = sbtWarnings ++ summaryWarnings val reportDestination = diff --git a/project/src/main/scala/licenses/DependencySummary.scala b/project/src/main/scala/licenses/DependencySummary.scala index 988950a47a1f..6114ff576910 100644 --- a/project/src/main/scala/licenses/DependencySummary.scala +++ b/project/src/main/scala/licenses/DependencySummary.scala @@ -4,7 +4,7 @@ import sbt.IO import src.main.scala.licenses.report.{ LicenseReview, PackageNotices, - WithWarnings + WithDiagnostics } /** Contains a sequence of dependencies and any attachments found. @@ -122,7 +122,7 @@ object ReviewedSummary { /** Returns a list of warnings that indicate missing reviews or other issues. */ - def warnAboutMissingReviews(summary: ReviewedSummary): WithWarnings[Unit] = { + def warnAboutMissingReviews(summary: ReviewedSummary): WithDiagnostics[Unit] = { val warnings = summary.dependencies.flatMap { dep => val warnings = collection.mutable.Buffer[String]() val name = dep.information.moduleInfo.toString @@ -186,6 +186,6 @@ object ReviewedSummary { warnings } - WithWarnings.justWarnings(warnings) + WithDiagnostics.justDiagnostics(warnings) } } diff --git a/project/src/main/scala/licenses/report/Review.scala b/project/src/main/scala/licenses/report/Review.scala index cc619bbbeb92..cc9156f84566 100644 --- a/project/src/main/scala/licenses/report/Review.scala +++ b/project/src/main/scala/licenses/report/Review.scala @@ -71,7 +71,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { /** Runs the review process, returning a [[ReviewedDependency]] which includes * information from the [[DependencySummary]] enriched with review statuses. */ - def run(): WithWarnings[ReviewedSummary] = + def run(): WithDiagnostics[ReviewedSummary] = for { reviews <- dependencySummary.dependencies.map { case (information, attachments) => @@ -94,7 +94,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { */ private def warnAboutMissingDependencies( existingPackageNames: Seq[String] - ): WithWarnings[Unit] = { + ): WithDiagnostics[Unit] = { val foundConfigurations = listFiles(root).filter(_.isDirectory) val expectedFileNames = existingPackageNames ++ Seq(Paths.filesAdd, Paths.reviewedLicenses) @@ -104,7 +104,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { s"Found legal review configuration for package ${p.getName}, " + s"but no such dependency has been found. Perhaps it has been removed?" ) - WithWarnings.justWarnings(warnings) + WithDiagnostics.justDiagnostics(warnings) } /** Finds a header defined in the settings or @@ -157,7 +157,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { private def reviewDependency( info: DependencyInformation, attachments: Seq[Attachment] - ): WithWarnings[ReviewedDependency] = { + ): WithDiagnostics[ReviewedDependency] = { val packageRoot = root / info.packageName val (files, copyrights) = splitAttachments(attachments) val copyrightsDeduplicated = @@ -182,7 +182,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { private def reviewFiles( packageRoot: File, files: Seq[AttachedFile] - ): WithWarnings[Seq[(AttachedFile, AttachmentStatus)]] = { + ): WithDiagnostics[Seq[(AttachedFile, AttachmentStatus)]] = { def keyForFile(file: AttachedFile): String = file.path.toString val keys = files.map(keyForFile) for { @@ -214,7 +214,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { private def reviewCopyrights( packageRoot: File, copyrights: Seq[CopyrightMention] - ): WithWarnings[Seq[(CopyrightMention, AttachmentStatus)]] = { + ): WithDiagnostics[Seq[(CopyrightMention, AttachmentStatus)]] = { def keyForMention(copyrightMention: CopyrightMention): String = copyrightMention.content.strip val keys = copyrights.map(keyForMention) @@ -260,11 +260,11 @@ case class Review(root: File, dependencySummary: DependencySummary) { private def reviewLicense( packageRoot: File, info: DependencyInformation - ): WithWarnings[LicenseReview] = + ): WithDiagnostics[LicenseReview] = readFile(packageRoot / Paths.customLicense) match { case Some(content) => val customFilename = content.strip() - WithWarnings(LicenseReview.Custom(customFilename)) + WithDiagnostics(LicenseReview.Custom(customFilename)) case None => val directory = root / Paths.reviewedLicenses val fileName = Review.normalizeName(info.license.name) @@ -274,7 +274,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { readFile(settingPath) .map { content => if (content.isBlank) { - WithWarnings( + WithDiagnostics( LicenseReview.NotReviewed, Seq(s"License review file $settingPath is empty.") ) @@ -283,7 +283,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { val path = Path.of(content.strip()) val bothDefaultAndCustom = (packageRoot / Paths.defaultAndCustomLicense).exists() - WithWarnings( + WithDiagnostics( LicenseReview.Default( path, allowAdditionalCustomLicenses = bothDefaultAndCustom @@ -291,7 +291,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { ) } catch { case e: InvalidPathException => - WithWarnings( + WithDiagnostics( LicenseReview.NotReviewed, Seq( s"License review file $settingPath is malformed: $e" @@ -299,14 +299,14 @@ case class Review(root: File, dependencySummary: DependencySummary) { ) } } - .getOrElse(WithWarnings(LicenseReview.NotReviewed)) + .getOrElse(WithDiagnostics(LicenseReview.NotReviewed)) case Array(_, _*) => - WithWarnings( + WithDiagnostics( LicenseReview.NotReviewed, Seq(s"Multiple copies of file $settingPath with differing case.") ) case Array() => - WithWarnings( + WithDiagnostics( LicenseReview.NotReviewed, Seq(s"License review file $settingPath is missing.") ) @@ -328,7 +328,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { fileName: String, expectedLines: Seq[String], packageRoot: File - ): WithWarnings[Seq[String]] = { + ): WithDiagnostics[Seq[String]] = { val lines = readLines(packageRoot / fileName) val unexpectedLines = lines.filter(l => !expectedLines.contains(l)) val warnings = unexpectedLines.map(l => @@ -337,7 +337,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { s"update? Please remove it from the file and make sure that the report " + s"contains all necessary elements after this change." ) - WithWarnings(lines, warnings) + WithDiagnostics(lines, warnings) } /** Reads the file as a [[String]]. diff --git a/project/src/main/scala/licenses/report/WithDiagnostics.scala b/project/src/main/scala/licenses/report/WithDiagnostics.scala new file mode 100644 index 000000000000..6b269b6e053e --- /dev/null +++ b/project/src/main/scala/licenses/report/WithDiagnostics.scala @@ -0,0 +1,56 @@ +package src.main.scala.licenses.report + +/** A simple monad for storing diagnostics related to a result. + */ +case class WithDiagnostics[+A](value: A, diagnostics: Seq[String] = Seq()) { + + /** Returns a result with a mapped value and the same diagnostics. + */ + def map[B](f: A => B): WithDiagnostics[B] = WithDiagnostics(f(value), diagnostics) + + /** Combines two computations returning diagnostics, preserving diagnostics from + * both of them. + */ + def flatMap[B](f: A => WithDiagnostics[B]): WithDiagnostics[B] = { + val result = f(value) + WithDiagnostics(result.value, diagnostics ++ result.diagnostics) + } +} + +object WithDiagnostics { + implicit class SeqWithDiagnosticsSyntax[+A](seq: Seq[WithDiagnostics[A]]) { + + /** Turns a sequence of [[WithDiagnostics]] instances into a single + * [[WithDiagnostics]] that contains the sequence of values and combined + * diagnostics. + */ + def flip: WithDiagnostics[Seq[A]] = + WithDiagnostics(seq.map(_.value), seq.flatMap(_.diagnostics)) + } + + implicit class SeqLikeSyntax[A](seq: WithDiagnostics[Seq[A]]) { + + /** A shorthand syntax to concatenate two sequences wrapped with diagnostics, + * combining their diagnostics. + */ + def ++(other: WithDiagnostics[Seq[A]]): WithDiagnostics[Seq[A]] = + for { + lhs <- seq + rhs <- other + } yield lhs ++ rhs + + /** A shorthand syntax to concatenate an ordinary sequence to a sequence + * with diagnostics. + */ + def ++(other: Seq[A]): WithDiagnostics[Seq[A]] = + for { + lhs <- seq + } yield lhs ++ other + } + + /** Creates a [[WithDiagnostics]] containing Unit and a provided sequence of + * diagnostics. + */ + def justDiagnostics(diagnostics: Seq[String]): WithDiagnostics[Unit] = + WithDiagnostics((), diagnostics) +} diff --git a/project/src/main/scala/licenses/report/WithWarnings.scala b/project/src/main/scala/licenses/report/WithWarnings.scala deleted file mode 100644 index aeebede21673..000000000000 --- a/project/src/main/scala/licenses/report/WithWarnings.scala +++ /dev/null @@ -1,56 +0,0 @@ -package src.main.scala.licenses.report - -/** A simple monad for storing warnings related to a result. - */ -case class WithWarnings[+A](value: A, warnings: Seq[String] = Seq()) { - - /** Returns a result with a mapped value and the same warnings. - */ - def map[B](f: A => B): WithWarnings[B] = WithWarnings(f(value), warnings) - - /** Combines two computations returning warnings, preserving warnings from - * both of them. - */ - def flatMap[B](f: A => WithWarnings[B]): WithWarnings[B] = { - val result = f(value) - WithWarnings(result.value, warnings ++ result.warnings) - } -} - -object WithWarnings { - implicit class SeqWithWarningsSyntax[+A](seq: Seq[WithWarnings[A]]) { - - /** Turns a sequence of [[WithWarnings]] instances into a single - * [[WithWarnings]] that contains the sequence of values and combined - * warnings. - */ - def flip: WithWarnings[Seq[A]] = - WithWarnings(seq.map(_.value), seq.flatMap(_.warnings)) - } - - implicit class SeqLikeSyntax[A](seq: WithWarnings[Seq[A]]) { - - /** A shorthand syntax to concatenate two sequences wrapped with warnings, - * combining their warnings. - */ - def ++(other: WithWarnings[Seq[A]]): WithWarnings[Seq[A]] = - for { - lhs <- seq - rhs <- other - } yield lhs ++ rhs - - /** A shorthand syntax to concatenate an ordinary sequence to a sequence - * with warnings. - */ - def ++(other: Seq[A]): WithWarnings[Seq[A]] = - for { - lhs <- seq - } yield lhs ++ other - } - - /** Creates a [[WithWarnings]] containing Unit and a provided sequence of - * warnings. - */ - def justWarnings(warnings: Seq[String]): WithWarnings[Unit] = - WithWarnings((), warnings) -} From f099f2b1f6efd906127264b2c8151f47523c2f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 17:31:19 +0100 Subject: [PATCH 02/15] Clear distinction between problems and notices --- project/GatherLicenses.scala | 31 +++++++------- .../scala/licenses/DependencySummary.scala | 40 +++++++++---------- .../scala/licenses/frontend/SbtLicenses.scala | 26 +++++------- .../scala/licenses/report/Diagnostic.scala | 20 ++++++++++ .../scala/licenses/report/HTMLWriter.scala | 1 + .../main/scala/licenses/report/Report.scala | 39 ++++++++++-------- .../main/scala/licenses/report/Review.scala | 20 +++++----- .../licenses/report/WithDiagnostics.scala | 4 +- 8 files changed, 101 insertions(+), 80 deletions(-) create mode 100644 project/src/main/scala/licenses/report/Diagnostic.scala diff --git a/project/GatherLicenses.scala b/project/GatherLicenses.scala index dd4f5bf1c743..49cee5f9e023 100644 --- a/project/GatherLicenses.scala +++ b/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) @@ -67,24 +66,28 @@ object GatherLicenses { val summary = DependencySummary(processed) val distributionRoot = configRoot / distribution.artifactName - val WithDiagnostics(processedSummary, summaryWarnings) = + val WithDiagnostics(processedSummary, summaryDiagnostics) = Review(distributionRoot, summary).run() - val allWarnings = sbtWarnings ++ summaryWarnings + val allDiagnostics = sbtDiagnostics ++ 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 (notices: Seq[Diagnostic.Notice], problems: Seq[Diagnostic.Problem]) = + Diagnostic.partition(allDiagnostics) + + notices.foreach(notice => log.warn(notice.message)) + + if (problems.isEmpty) { + log.info("No problems found in the report.") + } else { + log.error(s"Found ${problems.size} in the report:") + problems.foreach(problem => log.error(problem.message)) + } Report.writeHTML( distribution, processedSummary, - allWarnings, + allDiagnostics, reportDestination ) log.info( @@ -96,10 +99,10 @@ object GatherLicenses { ReportState.write( distributionRoot / stateFileName, distribution, - summaryWarnings.length + problems.size ) log.info(s"Re-generated distribution notices at `$packagePath`.") - if (summaryWarnings.nonEmpty) { + if (problems.nonEmpty) { log.warn( "The distribution notices were regenerated, but there are " + "not-reviewed issues within the report. The notices are probably " + diff --git a/project/src/main/scala/licenses/DependencySummary.scala b/project/src/main/scala/licenses/DependencySummary.scala index 6114ff576910..40d809bbd1fd 100644 --- a/project/src/main/scala/licenses/DependencySummary.scala +++ b/project/src/main/scala/licenses/DependencySummary.scala @@ -1,11 +1,7 @@ package src.main.scala.licenses import sbt.IO -import src.main.scala.licenses.report.{ - LicenseReview, - PackageNotices, - WithDiagnostics -} +import src.main.scala.licenses.report.{Diagnostic, LicenseReview, PackageNotices, WithDiagnostics} /** Contains a sequence of dependencies and any attachments found. */ @@ -123,36 +119,36 @@ object ReviewedSummary { /** Returns a list of warnings that indicate missing reviews or other issues. */ def warnAboutMissingReviews(summary: ReviewedSummary): WithDiagnostics[Unit] = { - val warnings = summary.dependencies.flatMap { dep => - val warnings = collection.mutable.Buffer[String]() + 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.Problem(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.Problem(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.Problem(s"No files or copyright information are included for $name. Generally every dependency should have _some_ copyright info, so this suggests all our heuristics failed. Please find the information manually and add it using `files-add` or `copyright-add`. Even if the dependency is public domain, 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.Problem(s"Default license ${dep.information.license.name} for $name is used, but that license is not reviewed (need to add an entry to `reviewed-licenses`).") ) case LicenseReview.Default( defaultPath, @@ -163,10 +159,10 @@ 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 " + + diagnostics.append( + Diagnostic.Problem(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"license ${dep.information.license.name}, but a custom license was not expected. If this custom license should override the default one, create a `custom-license` config file. If both files are expected to be included, create an empty `default-and-custom-license` file.") ) } case None => @@ -178,14 +174,14 @@ 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.Problem(s"License for $name is set to custom file `$filename`, but no such file is attached.") ) } } - warnings + diagnostics } - WithDiagnostics.justDiagnostics(warnings) + WithDiagnostics.justDiagnostics(diagnostics) } } diff --git a/project/src/main/scala/licenses/frontend/SbtLicenses.scala b/project/src/main/scala/licenses/frontend/SbtLicenses.scala index 3b05d50bd10c..a530ce96fc05 100644 --- a/project/src/main/scala/licenses/frontend/SbtLicenses.scala +++ b/project/src/main/scala/licenses/frontend/SbtLicenses.scala @@ -1,20 +1,16 @@ package src.main.scala.licenses.frontend import java.nio.file.Path - import sbtlicensereport.license.{DepLicense, DepModuleInfo} import org.apache.ivy.core.resolve.IvyNode import sbt.Compile import sbt.internal.util.ManagedLogger import sbt.io.IO import sbt.librarymanagement.ConfigRef -import src.main.scala.licenses.{ - DependencyInformation, - SBTDistributionComponent, - SourceAccess -} +import src.main.scala.licenses.report.Diagnostic +import src.main.scala.licenses.{DependencyInformation, SBTDistributionComponent, SourceAccess} -import scala.collection.JavaConverters._ +import scala.collection.JavaConverters.* /** Defines the algorithm for discovering dependency metadata. */ @@ -44,8 +40,8 @@ object SbtLicenses { def analyze( components: Seq[SBTDistributionComponent], log: ManagedLogger - ): (Seq[DependencyInformation], Seq[String]) = { - val results: Seq[(Seq[Dependency], Vector[Path], Seq[String])] = + ): (Seq[DependencyInformation], Seq[Diagnostic]) = { + val results: Seq[(Seq[Dependency], Vector[Path], Seq[Diagnostic])] = components.map { component => val report = component.licenseReport.orig val ivyDeps = @@ -73,12 +69,12 @@ object SbtLicenses { Dependency(dep, depNode, sources) } - val warnings = + val diagnostics = if (component.licenseReport.licenses.isEmpty) - Seq(s"License report for component ${component.name} is empty.") + Seq(Diagnostic.Problem(s"License report for component ${component.name} is empty.")) else Seq() - (deps, sourceArtifacts, warnings) + (deps, sourceArtifacts, diagnostics) } val distinctDependencies = @@ -98,12 +94,12 @@ object SbtLicenses { val missingWarnings = for { dep <- relevantDeps if dep.sources.isEmpty - } yield s"Could not find sources for ${dep.moduleInfo}" + } yield Diagnostic.Notice(s"Could not find sources for ${dep.moduleInfo}") val unexpectedWarnings = for { source <- distinctSources if !distinctDependencies.exists(_.sourcesJARPaths.contains(source)) - } yield s"Found a source $source that does not belong to any known " + - s"dependencies, perhaps the algorithm needs updating?" + } yield Diagnostic.Notice(s"Found a source $source that does not belong to any known " + + s"dependencies, perhaps the algorithm needs updating?") val reportsWarnings = results.flatMap(_._3) (relevantDeps, missingWarnings ++ unexpectedWarnings ++ reportsWarnings) diff --git a/project/src/main/scala/licenses/report/Diagnostic.scala b/project/src/main/scala/licenses/report/Diagnostic.scala new file mode 100644 index 000000000000..5028f31d33a9 --- /dev/null +++ b/project/src/main/scala/licenses/report/Diagnostic.scala @@ -0,0 +1,20 @@ +package src.main.scala.licenses.report + +sealed trait Diagnostic { + def message: String +} + +object Diagnostic { + + /** A notice indicating some unexpected event that should be noted but does not stop the report from being accepted. */ + case class Notice(message: String) extends Diagnostic + + /** A problem with the license review that has to be resolved before the report can be accepted. */ + case class Problem(message: String) extends Diagnostic + + def partition(diagnostics: Seq[Diagnostic]): (Seq[Notice], Seq[Problem]) = { + val notices = diagnostics.collect { case n: Notice => n } + val problems = diagnostics.collect { case p: Problem => p } + (notices, problems) + } +} diff --git a/project/src/main/scala/licenses/report/HTMLWriter.scala b/project/src/main/scala/licenses/report/HTMLWriter.scala index 45c27dfd01a0..0e2868aaac15 100644 --- a/project/src/main/scala/licenses/report/HTMLWriter.scala +++ b/project/src/main/scala/licenses/report/HTMLWriter.scala @@ -227,6 +227,7 @@ object Style { override def toString: String = "color:red" } + /** Makes the text gray. */ case object Gray extends Style { diff --git a/project/src/main/scala/licenses/report/Report.scala b/project/src/main/scala/licenses/report/Report.scala index 28b63cf00984..c5f3a9ede27c 100644 --- a/project/src/main/scala/licenses/report/Report.scala +++ b/project/src/main/scala/licenses/report/Report.scala @@ -19,14 +19,14 @@ object Report { * * @param description description of the distribution * @param summary reviewed summary of findings - * @param warnings sequence of warnings + * @param diagnostics sequence of diagnostics * @param destination location of the generated HTML file */ def writeHTML( - description: DistributionDescription, - summary: ReviewedSummary, - warnings: Seq[String], - destination: File + description: DistributionDescription, + summary: ReviewedSummary, + diagnostics: Seq[Diagnostic], + destination: File ): Unit = { IO.createDirectory(destination.getParentFile) val writer = HTMLWriter.toFile(destination) @@ -40,21 +40,26 @@ object Report { s"${description.rootComponentsNames.mkString(", ")}." ) - if (warnings.isEmpty) { - writer.writeParagraph("There are no warnings.", Style.Green) - } else { - writer.writeParagraph( - s"There are ${warnings.size} warnings!", - Style.Bold, - Style.Red - ) + val (notices: Seq[Diagnostic.Notice], problems: Seq[Diagnostic.Problem]) = + Diagnostic.partition(diagnostics) + + if (notices.nonEmpty) { + writer.writeSubHeading("Notices") + writer.writeList(notices.map { notice => () => + writer.writeText(notice.message) + }) } - writeDependencySummary(writer, summary) + if (problems.nonEmpty) { + writer.writeSubHeading(f"There are ${problems.size} problems found in the review.") + writer.writeList(problems.map { problem => () => + writer.writeText(problem.message, Style.Red) + }) + } else { + writer.writeParagraph("No problems found in the review.", Style.Green) + } - writer.writeList(warnings.map { warning => () => - writer.writeText(writer.escape(warning)) - }) + writeDependencySummary(writer, summary) writer.writeCollapsible("NOTICE header", summary.noticeHeader) if (summary.additionalFiles.nonEmpty) { diff --git a/project/src/main/scala/licenses/report/Review.scala b/project/src/main/scala/licenses/report/Review.scala index cc9156f84566..8791911155d2 100644 --- a/project/src/main/scala/licenses/report/Review.scala +++ b/project/src/main/scala/licenses/report/Review.scala @@ -100,11 +100,11 @@ case class Review(root: File, dependencySummary: DependencySummary) { existingPackageNames ++ Seq(Paths.filesAdd, Paths.reviewedLicenses) val unexpectedConfigurations = foundConfigurations.filter(p => !expectedFileNames.contains(p.getName)) - val warnings = unexpectedConfigurations.map(p => - s"Found legal review configuration for package ${p.getName}, " + - s"but no such dependency has been found. Perhaps it has been removed?" + val diagnostics = unexpectedConfigurations.map(p => + Diagnostic.Problem(s"Found legal review configuration for package ${p.getName}, " + + s"but no such dependency has been found. Perhaps it has been removed or renamed (version change)?") ) - WithDiagnostics.justDiagnostics(warnings) + WithDiagnostics.justDiagnostics(diagnostics) } /** Finds a header defined in the settings or @@ -276,7 +276,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { if (content.isBlank) { WithDiagnostics( LicenseReview.NotReviewed, - Seq(s"License review file $settingPath is empty.") + Seq(Diagnostic.Problem(s"License review file $settingPath is empty, but it should contain a path to a license text inside of `license-texts`.")) ) } else try { @@ -294,7 +294,7 @@ case class Review(root: File, dependencySummary: DependencySummary) { WithDiagnostics( LicenseReview.NotReviewed, Seq( - s"License review file $settingPath is malformed: $e" + Diagnostic.Problem(s"License review file $settingPath is malformed: $e") ) ) } @@ -303,12 +303,12 @@ case class Review(root: File, dependencySummary: DependencySummary) { case Array(_, _*) => WithDiagnostics( LicenseReview.NotReviewed, - Seq(s"Multiple copies of file $settingPath with differing case.") + Seq(Diagnostic.Problem(s"Multiple copies of file $settingPath with differing case (the license names are matched case insensitively).")) ) case Array() => WithDiagnostics( LicenseReview.NotReviewed, - Seq(s"License review file $settingPath is missing.") + Seq(Diagnostic.Problem(s"License review file $settingPath is missing. Either review the default license or set a `custom-license` for packages that used it.")) ) } } @@ -332,10 +332,10 @@ case class Review(root: File, dependencySummary: DependencySummary) { val lines = readLines(packageRoot / fileName) val unexpectedLines = lines.filter(l => !expectedLines.contains(l)) val warnings = unexpectedLines.map(l => - s"File $fileName in ${packageRoot.getName} contains entry `$l`, but no " + + Diagnostic.Problem(s"File $fileName in ${packageRoot.getName} contains entry `$l`, but no " + s"such entry has been detected. Perhaps it has disappeared after an " + s"update? Please remove it from the file and make sure that the report " + - s"contains all necessary elements after this change." + s"contains all necessary elements after this change.") ) WithDiagnostics(lines, warnings) } diff --git a/project/src/main/scala/licenses/report/WithDiagnostics.scala b/project/src/main/scala/licenses/report/WithDiagnostics.scala index 6b269b6e053e..bb4d7b4879a5 100644 --- a/project/src/main/scala/licenses/report/WithDiagnostics.scala +++ b/project/src/main/scala/licenses/report/WithDiagnostics.scala @@ -2,7 +2,7 @@ package src.main.scala.licenses.report /** A simple monad for storing diagnostics related to a result. */ -case class WithDiagnostics[+A](value: A, diagnostics: Seq[String] = Seq()) { +case class WithDiagnostics[+A](value: A, diagnostics: Seq[Diagnostic] = Seq()) { /** Returns a result with a mapped value and the same diagnostics. */ @@ -51,6 +51,6 @@ object WithDiagnostics { /** Creates a [[WithDiagnostics]] containing Unit and a provided sequence of * diagnostics. */ - def justDiagnostics(diagnostics: Seq[String]): WithDiagnostics[Unit] = + def justDiagnostics(diagnostics: Seq[Diagnostic]): WithDiagnostics[Unit] = WithDiagnostics((), diagnostics) } From 94ce60d5b8f9fd66f71bf785285af72a245c677f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 17:34:30 +0100 Subject: [PATCH 03/15] scalafmt --- .../scala/licenses/DependencySummary.scala | 41 ++++++++++++++----- .../scala/licenses/frontend/SbtLicenses.scala | 18 ++++++-- .../scala/licenses/report/Diagnostic.scala | 4 +- .../scala/licenses/report/HTMLWriter.scala | 1 - .../main/scala/licenses/report/Report.scala | 26 ++++++------ .../main/scala/licenses/report/Review.scala | 38 ++++++++++++----- .../licenses/report/WithDiagnostics.scala | 3 +- 7 files changed, 90 insertions(+), 41 deletions(-) diff --git a/project/src/main/scala/licenses/DependencySummary.scala b/project/src/main/scala/licenses/DependencySummary.scala index 40d809bbd1fd..be54b0a987b9 100644 --- a/project/src/main/scala/licenses/DependencySummary.scala +++ b/project/src/main/scala/licenses/DependencySummary.scala @@ -1,7 +1,12 @@ package src.main.scala.licenses import sbt.IO -import src.main.scala.licenses.report.{Diagnostic, LicenseReview, PackageNotices, WithDiagnostics} +import src.main.scala.licenses.report.{ + Diagnostic, + LicenseReview, + PackageNotices, + WithDiagnostics +} /** Contains a sequence of dependencies and any attachments found. */ @@ -118,22 +123,28 @@ object ReviewedSummary { /** Returns a list of warnings that indicate missing reviews or other issues. */ - def warnAboutMissingReviews(summary: ReviewedSummary): WithDiagnostics[Unit] = { + 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 name = dep.information.moduleInfo.toString val missingFiles = dep.files.filter(_._2 == AttachmentStatus.NotReviewed) if (missingFiles.nonEmpty) { diagnostics.append( - Diagnostic.Problem(s"${missingFiles.size} files are not reviewed in $name.") + Diagnostic.Problem( + s"${missingFiles.size} files are not reviewed in $name." + ) ) } val missingCopyrights = dep.copyrights.filter(_._2 == AttachmentStatus.NotReviewed) if (missingCopyrights.nonEmpty) { diagnostics.append( - Diagnostic.Problem(s"${missingCopyrights.size} copyrights are not reviewed in $name.") + Diagnostic.Problem( + s"${missingCopyrights.size} copyrights are not reviewed in $name." + ) ) } @@ -141,14 +152,18 @@ object ReviewedSummary { (dep.files.map(_._2) ++ dep.copyrights.map(_._2)).filter(_.included) if (includedInfos.isEmpty) { diagnostics.append( - Diagnostic.Problem(s"No files or copyright information are included for $name. Generally every dependency should have _some_ copyright info, so this suggests all our heuristics failed. Please find the information manually and add it using `files-add` or `copyright-add`. Even if the dependency is public domain, it may be good to include some information about its source.") + Diagnostic.Problem( + s"No files or copyright information are included for $name. Generally every dependency should have _some_ copyright info, so this suggests all our heuristics failed. Please find the information manually and add it using `files-add` or `copyright-add`. Even if the dependency is public domain, it may be good to include some information about its source." + ) ) } dep.licenseReview match { case LicenseReview.NotReviewed => diagnostics.append( - Diagnostic.Problem(s"Default license ${dep.information.license.name} for $name is used, but that license is not reviewed (need to add an entry to `reviewed-licenses`).") + Diagnostic.Problem( + s"Default license ${dep.information.license.name} for $name is used, but that license is not reviewed (need to add an entry to `reviewed-licenses`)." + ) ) case LicenseReview.Default( defaultPath, @@ -160,9 +175,11 @@ object ReviewedSummary { val licenseContent = IO.read(defaultPath.toFile) if (licenseContent.strip != includedLicense.content) { diagnostics.append( - Diagnostic.Problem(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}, but a custom license was not expected. If this custom license should override the default one, create a `custom-license` config file. If both files are expected to be included, create an empty `default-and-custom-license` file.") + Diagnostic.Problem( + 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}, but a custom license was not expected. If this custom license should override the default one, create a `custom-license` config file. If both files are expected to be included, create an empty `default-and-custom-license` file." + ) ) } case None => @@ -175,7 +192,9 @@ object ReviewedSummary { filename == PackageNotices.gatheredNoticesFilename if (!fileIsIncluded && !fileWillBeIncludedAsCopyrightNotices) { diagnostics.append( - Diagnostic.Problem(s"License for $name is set to custom file `$filename`, but no such file is attached.") + Diagnostic.Problem( + s"License for $name is set to custom file `$filename`, but no such file is attached." + ) ) } } diff --git a/project/src/main/scala/licenses/frontend/SbtLicenses.scala b/project/src/main/scala/licenses/frontend/SbtLicenses.scala index a530ce96fc05..b3264676acfb 100644 --- a/project/src/main/scala/licenses/frontend/SbtLicenses.scala +++ b/project/src/main/scala/licenses/frontend/SbtLicenses.scala @@ -8,7 +8,11 @@ import sbt.internal.util.ManagedLogger import sbt.io.IO import sbt.librarymanagement.ConfigRef import src.main.scala.licenses.report.Diagnostic -import src.main.scala.licenses.{DependencyInformation, SBTDistributionComponent, SourceAccess} +import src.main.scala.licenses.{ + DependencyInformation, + SBTDistributionComponent, + SourceAccess +} import scala.collection.JavaConverters.* @@ -71,7 +75,11 @@ object SbtLicenses { val diagnostics = if (component.licenseReport.licenses.isEmpty) - Seq(Diagnostic.Problem(s"License report for component ${component.name} is empty.")) + Seq( + Diagnostic.Problem( + s"License report for component ${component.name} is empty." + ) + ) else Seq() (deps, sourceArtifacts, diagnostics) @@ -98,8 +106,10 @@ object SbtLicenses { val unexpectedWarnings = for { source <- distinctSources if !distinctDependencies.exists(_.sourcesJARPaths.contains(source)) - } yield Diagnostic.Notice(s"Found a source $source that does not belong to any known " + - s"dependencies, perhaps the algorithm needs updating?") + } yield Diagnostic.Notice( + s"Found a source $source that does not belong to any known " + + s"dependencies, perhaps the algorithm needs updating?" + ) val reportsWarnings = results.flatMap(_._3) (relevantDeps, missingWarnings ++ unexpectedWarnings ++ reportsWarnings) diff --git a/project/src/main/scala/licenses/report/Diagnostic.scala b/project/src/main/scala/licenses/report/Diagnostic.scala index 5028f31d33a9..32cf191d88d5 100644 --- a/project/src/main/scala/licenses/report/Diagnostic.scala +++ b/project/src/main/scala/licenses/report/Diagnostic.scala @@ -1,7 +1,7 @@ package src.main.scala.licenses.report sealed trait Diagnostic { - def message: String + def message: String } object Diagnostic { @@ -13,7 +13,7 @@ object Diagnostic { case class Problem(message: String) extends Diagnostic def partition(diagnostics: Seq[Diagnostic]): (Seq[Notice], Seq[Problem]) = { - val notices = diagnostics.collect { case n: Notice => n } + val notices = diagnostics.collect { case n: Notice => n } val problems = diagnostics.collect { case p: Problem => p } (notices, problems) } diff --git a/project/src/main/scala/licenses/report/HTMLWriter.scala b/project/src/main/scala/licenses/report/HTMLWriter.scala index 0e2868aaac15..45c27dfd01a0 100644 --- a/project/src/main/scala/licenses/report/HTMLWriter.scala +++ b/project/src/main/scala/licenses/report/HTMLWriter.scala @@ -227,7 +227,6 @@ object Style { override def toString: String = "color:red" } - /** Makes the text gray. */ case object Gray extends Style { diff --git a/project/src/main/scala/licenses/report/Report.scala b/project/src/main/scala/licenses/report/Report.scala index c5f3a9ede27c..ab7163266ad4 100644 --- a/project/src/main/scala/licenses/report/Report.scala +++ b/project/src/main/scala/licenses/report/Report.scala @@ -23,10 +23,10 @@ object Report { * @param destination location of the generated HTML file */ def writeHTML( - description: DistributionDescription, - summary: ReviewedSummary, - diagnostics: Seq[Diagnostic], - destination: File + description: DistributionDescription, + summary: ReviewedSummary, + diagnostics: Seq[Diagnostic], + destination: File ): Unit = { IO.createDirectory(destination.getParentFile) val writer = HTMLWriter.toFile(destination) @@ -50,14 +50,16 @@ object Report { }) } - if (problems.nonEmpty) { - writer.writeSubHeading(f"There are ${problems.size} problems found in the review.") - writer.writeList(problems.map { problem => () => - writer.writeText(problem.message, Style.Red) - }) - } else { - writer.writeParagraph("No problems found in the review.", Style.Green) - } + if (problems.nonEmpty) { + writer.writeSubHeading( + f"There are ${problems.size} problems found in the review." + ) + writer.writeList(problems.map { problem => () => + writer.writeText(problem.message, Style.Red) + }) + } else { + writer.writeParagraph("No problems found in the review.", Style.Green) + } writeDependencySummary(writer, summary) diff --git a/project/src/main/scala/licenses/report/Review.scala b/project/src/main/scala/licenses/report/Review.scala index 8791911155d2..f4906c57a6ac 100644 --- a/project/src/main/scala/licenses/report/Review.scala +++ b/project/src/main/scala/licenses/report/Review.scala @@ -101,8 +101,10 @@ case class Review(root: File, dependencySummary: DependencySummary) { val unexpectedConfigurations = foundConfigurations.filter(p => !expectedFileNames.contains(p.getName)) val diagnostics = unexpectedConfigurations.map(p => - Diagnostic.Problem(s"Found legal review configuration for package ${p.getName}, " + - s"but no such dependency has been found. Perhaps it has been removed or renamed (version change)?") + Diagnostic.Problem( + s"Found legal review configuration for package ${p.getName}, " + + s"but no such dependency has been found. Perhaps it has been removed or renamed (version change)?" + ) ) WithDiagnostics.justDiagnostics(diagnostics) } @@ -276,7 +278,11 @@ case class Review(root: File, dependencySummary: DependencySummary) { if (content.isBlank) { WithDiagnostics( LicenseReview.NotReviewed, - Seq(Diagnostic.Problem(s"License review file $settingPath is empty, but it should contain a path to a license text inside of `license-texts`.")) + Seq( + Diagnostic.Problem( + s"License review file $settingPath is empty, but it should contain a path to a license text inside of `license-texts`." + ) + ) ) } else try { @@ -294,7 +300,9 @@ case class Review(root: File, dependencySummary: DependencySummary) { WithDiagnostics( LicenseReview.NotReviewed, Seq( - Diagnostic.Problem(s"License review file $settingPath is malformed: $e") + Diagnostic.Problem( + s"License review file $settingPath is malformed: $e" + ) ) ) } @@ -303,12 +311,20 @@ case class Review(root: File, dependencySummary: DependencySummary) { case Array(_, _*) => WithDiagnostics( LicenseReview.NotReviewed, - Seq(Diagnostic.Problem(s"Multiple copies of file $settingPath with differing case (the license names are matched case insensitively).")) + Seq( + Diagnostic.Problem( + s"Multiple copies of file $settingPath with differing case (the license names are matched case insensitively)." + ) + ) ) case Array() => WithDiagnostics( LicenseReview.NotReviewed, - Seq(Diagnostic.Problem(s"License review file $settingPath is missing. Either review the default license or set a `custom-license` for packages that used it.")) + Seq( + Diagnostic.Problem( + s"License review file $settingPath is missing. Either review the default license or set a `custom-license` for packages that used it." + ) + ) ) } } @@ -332,10 +348,12 @@ case class Review(root: File, dependencySummary: DependencySummary) { val lines = readLines(packageRoot / fileName) val unexpectedLines = lines.filter(l => !expectedLines.contains(l)) val warnings = unexpectedLines.map(l => - Diagnostic.Problem(s"File $fileName in ${packageRoot.getName} contains entry `$l`, but no " + - s"such entry has been detected. Perhaps it has disappeared after an " + - s"update? Please remove it from the file and make sure that the report " + - s"contains all necessary elements after this change.") + Diagnostic.Problem( + s"File $fileName in ${packageRoot.getName} contains entry `$l`, but no " + + s"such entry has been detected. Perhaps it has disappeared after an " + + s"update? Please remove it from the file and make sure that the report " + + s"contains all necessary elements after this change." + ) ) WithDiagnostics(lines, warnings) } diff --git a/project/src/main/scala/licenses/report/WithDiagnostics.scala b/project/src/main/scala/licenses/report/WithDiagnostics.scala index bb4d7b4879a5..29aa85702591 100644 --- a/project/src/main/scala/licenses/report/WithDiagnostics.scala +++ b/project/src/main/scala/licenses/report/WithDiagnostics.scala @@ -6,7 +6,8 @@ case class WithDiagnostics[+A](value: A, diagnostics: Seq[Diagnostic] = Seq()) { /** Returns a result with a mapped value and the same diagnostics. */ - def map[B](f: A => B): WithDiagnostics[B] = WithDiagnostics(f(value), diagnostics) + def map[B](f: A => B): WithDiagnostics[B] = + WithDiagnostics(f(value), diagnostics) /** Combines two computations returning diagnostics, preserving diagnostics from * both of them. From 8ea2f7064ebc74ad6ab34e7dee7e0949d095c90a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 17:55:15 +0100 Subject: [PATCH 04/15] include GH problems in the report, not only in logs, and make them fatal (as they were breaking anyway) --- project/GatherLicenses.scala | 17 +++--- .../licenses/backend/GithubHeuristic.scala | 57 +++++++++++++------ 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/project/GatherLicenses.scala b/project/GatherLicenses.scala index 49cee5f9e023..a1f358fa9902 100644 --- a/project/GatherLicenses.scala +++ b/project/GatherLicenses.scala @@ -58,17 +58,20 @@ 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 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 allDiagnostics = sbtDiagnostics ++ summaryDiagnostics + val allDiagnostics = + sbtDiagnostics ++ processingDiagnostics ++ summaryDiagnostics val reportDestination = targetRoot / s"${distribution.artifactName}-report.html" @@ -179,7 +182,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`." ) ) diff --git a/project/src/main/scala/licenses/backend/GithubHeuristic.scala b/project/src/main/scala/licenses/backend/GithubHeuristic.scala index b4425245c1e6..25d3e8c10f7b 100644 --- a/project/src/main/scala/licenses/backend/GithubHeuristic.scala +++ b/project/src/main/scala/licenses/backend/GithubHeuristic.scala @@ -1,9 +1,9 @@ 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, @@ -11,7 +11,7 @@ import src.main.scala.licenses.{ 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.Problem( + 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 //blob// @@ -68,26 +76,39 @@ 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.Problem( + s"GitHub heuristic: 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.Problem( + s"GitHub heuristic: processing ${info.packageName} failed with error: $error" + ) + ) + ) } /** Decides if the file may be relevant and should be included in the result. From 22e0291fa532573903b3b5d672bc33bde85dfbd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 18:05:33 +0100 Subject: [PATCH 05/15] ignore a known Guava dep that was triggering a warning --- .../scala/licenses/frontend/DependencyFilter.scala | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/project/src/main/scala/licenses/frontend/DependencyFilter.scala b/project/src/main/scala/licenses/frontend/DependencyFilter.scala index 1b75e9abcd9e..57ab14889bb8 100644 --- a/project/src/main/scala/licenses/frontend/DependencyFilter.scala +++ b/project/src/main/scala/licenses/frontend/DependencyFilter.scala @@ -11,13 +11,17 @@ import src.main.scala.licenses.DependencyInformation */ object DependencyFilter { - /** Decides if the dependency should be kept for further processing. - */ + /** Decides if the dependency should be kept for further processing. */ def shouldKeep(dependencyInformation: DependencyInformation): Boolean = shouldKeep(dependencyInformation.moduleInfo) - /** Decides if the module should be kept for further processing. - */ + /** Decides if the module should be kept for further processing. */ def shouldKeep(moduleInfo: DepModuleInfo): Boolean = - moduleInfo.organization != "org.enso" + !shouldIgnore(moduleInfo) + + def shouldIgnore(moduleInfo: DepModuleInfo): Boolean = { + val isEnsoModule = moduleInfo.organization == "org.enso" + val isGuavaEmptyPlaceholder = moduleInfo.version == "9999.0-empty-to-avoid-conflict-with-guava" + isEnsoModule || isGuavaEmptyPlaceholder + } } From e5c10732adaa24f51f869bbf9ffdbab8cc920fe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 18:07:29 +0100 Subject: [PATCH 06/15] message --- tools/legal-review-helper/static/inject.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/legal-review-helper/static/inject.js b/tools/legal-review-helper/static/inject.js index 49abf797d151..0c852b621c24 100644 --- a/tools/legal-review-helper/static/inject.js +++ b/tools/legal-review-helper/static/inject.js @@ -42,10 +42,10 @@ function makeHandler(elem, data, file, action) { $(function () { $('body').prepend( - '
This review helper tool does not regenerate the ' + - 'report - to see the changes that are applied using this tool after ' + - 'refreshing the page, you need to regenerate the report using the ' + - '`gatherLicenses` command.
' + '
This review helper tool does not regenerate the ' + + 'report - any changes that are applied using this tool will not be visible after ' + + 'refreshing the page, until you regenerate the report by running ' + + '`openLegalReviewReport` command again.
' ) $('body').append( '
' + 'Loading...
' From 4ec0711cdb1e017a02c73f711686f7507cf19385 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 18:09:27 +0100 Subject: [PATCH 07/15] rephrase --- project/GatherLicenses.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/project/GatherLicenses.scala b/project/GatherLicenses.scala index a1f358fa9902..ab811f251ce1 100644 --- a/project/GatherLicenses.scala +++ b/project/GatherLicenses.scala @@ -78,7 +78,10 @@ object GatherLicenses { val (notices: Seq[Diagnostic.Notice], problems: Seq[Diagnostic.Problem]) = Diagnostic.partition(allDiagnostics) - notices.foreach(notice => log.warn(notice.message)) + if (notices.nonEmpty) { + log.warn(s"Found ${notices.size} non-fatal notices in the report:") + notices.foreach(notice => log.warn(notice.message)) + } if (problems.isEmpty) { log.info("No problems found in the report.") From 185eae1534e571f7a719851513de57d74229026a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 18:12:48 +0100 Subject: [PATCH 08/15] move verifyLicensePackages to the end of the workflow --- build/build/src/engine/context.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/build/build/src/engine/context.rs b/build/build/src/engine/context.rs index 93d576a10bc1..e0d1482e6910 100644 --- a/build/build/src/engine/context.rs +++ b/build/build/src/engine/context.rs @@ -320,12 +320,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"); } @@ -534,8 +528,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() { From 72434293272c30663c73a56cb5e47b1cb7ba1e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 18:13:52 +0100 Subject: [PATCH 09/15] scalafmt --- .../src/main/scala/licenses/frontend/DependencyFilter.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/project/src/main/scala/licenses/frontend/DependencyFilter.scala b/project/src/main/scala/licenses/frontend/DependencyFilter.scala index 57ab14889bb8..8745e042d690 100644 --- a/project/src/main/scala/licenses/frontend/DependencyFilter.scala +++ b/project/src/main/scala/licenses/frontend/DependencyFilter.scala @@ -21,7 +21,8 @@ object DependencyFilter { def shouldIgnore(moduleInfo: DepModuleInfo): Boolean = { val isEnsoModule = moduleInfo.organization == "org.enso" - val isGuavaEmptyPlaceholder = moduleInfo.version == "9999.0-empty-to-avoid-conflict-with-guava" + val isGuavaEmptyPlaceholder = + moduleInfo.version == "9999.0-empty-to-avoid-conflict-with-guava" isEnsoModule || isGuavaEmptyPlaceholder } } From 470354a4041cf642b75f7b30016ab9e2568e0198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 21 Feb 2024 18:35:52 +0100 Subject: [PATCH 10/15] add CSS --- project/src/main/scala/licenses/report/HTMLWriter.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/project/src/main/scala/licenses/report/HTMLWriter.scala b/project/src/main/scala/licenses/report/HTMLWriter.scala index 45c27dfd01a0..166d4fe63412 100644 --- a/project/src/main/scala/licenses/report/HTMLWriter.scala +++ b/project/src/main/scala/licenses/report/HTMLWriter.scala @@ -21,9 +21,10 @@ class HTMLWriter(bufferedWriter: BufferedWriter) { s""" | | + |$title | | - |$title + | |