From 2939a582d75511cd6141df8b6cb2df81302fe909 Mon Sep 17 00:00:00 2001 From: "P. Oscar Boykin" Date: Fri, 2 Feb 2024 15:39:42 -1000 Subject: [PATCH] Improve logging for gradle resolver (#378) Co-authored-by: Oscar Boykin --- .../bazel_deps/GradleLockDependency.scala | 47 ++++++++++--------- .../johnynek/bazel_deps/GradleLockFile.scala | 4 +- .../johnynek/bazel_deps/GradleResolver.scala | 36 ++++++++++---- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/scala/com/github/johnynek/bazel_deps/GradleLockDependency.scala b/src/scala/com/github/johnynek/bazel_deps/GradleLockDependency.scala index d04c17f..5c4832e 100644 --- a/src/scala/com/github/johnynek/bazel_deps/GradleLockDependency.scala +++ b/src/scala/com/github/johnynek/bazel_deps/GradleLockDependency.scala @@ -4,6 +4,7 @@ import cats.data.Validated import io.circe.Decoder.Result import io.circe.{Decoder, Error, HCursor, Json, KeyDecoder, Parser} import io.circe.generic.auto +import org.slf4j.LoggerFactory import scala.util.{Failure, Success, Try} @@ -14,6 +15,8 @@ case class GradleLockDependency( ) object GradleLockDependency { + private val logger = LoggerFactory.getLogger("GradleLockDependency") + sealed trait VersionState case object EqualVersionSpecified extends VersionState case object LeftVersion extends VersionState @@ -24,28 +27,26 @@ object GradleLockDependency { right: Option[String] ): Try[VersionState] = { (left, right) match { - case (None, None) => Success(EqualVersionSpecified) - case (Some(l), None) => Success(LeftVersion) - case (None, Some(r)) => Success(RightVersion) - case (Some(l), Some(r)) if (l == r) => Success(EqualVersionSpecified) - case (Some(l), Some(r)) => { - println( - s"This should probably not be allowed... but we are going to pick a version conflict ${versionConflictPolicy} if we can for $dependencyName between $l, $r" - ) - versionConflictPolicy.resolve( - None, - Set(Version(l), Version(r)) - ) match { - case Validated.Valid(v) => - if (v.asString == l) { - Success(LeftVersion) - } else { - Success(RightVersion) - } - case Validated.Invalid(iv) => - Failure(new Exception(s"Unable ot combine versions, $iv")) - } - } + case (_, _) if (left == right) => Success(EqualVersionSpecified) + case (Some(_), None) => Success(LeftVersion) + case (None, Some(_)) => Success(RightVersion) + case (Some(l), Some(r)) => + versionConflictPolicy + .resolve(None, Set(Version(l), Version(r))) match { + case Validated.Valid(v) => + val vstr = v.asString + logger.debug( + s"Using ${versionConflictPolicy} for $dependencyName with versions $l and $r picking $vstr" + ) + + val ver = + if (vstr == l) LeftVersion + else RightVersion + + Success(ver) + case Validated.Invalid(iv) => + Failure(new Exception(s"For $dependencyName unable to combine versions $l and $r. Error: $iv")) + } } } @@ -68,7 +69,7 @@ object GradleLockDependency { _ <- if (left.project == right.project) unit else Failure( new Exception( - s"Unable to merge due to incompatible project setting, had $left, $right" + s"Unable to merge due to incompatible project setting in $debugName, had $left, $right" ) ) v <- resolveVersions(versionConflictPolicy, debugName.getOrElse("Unknown"))(left.locked, right.locked) diff --git a/src/scala/com/github/johnynek/bazel_deps/GradleLockFile.scala b/src/scala/com/github/johnynek/bazel_deps/GradleLockFile.scala index 8aaa49e..688e9e2 100644 --- a/src/scala/com/github/johnynek/bazel_deps/GradleLockFile.scala +++ b/src/scala/com/github/johnynek/bazel_deps/GradleLockFile.scala @@ -48,7 +48,7 @@ object GradleLockFile { left.testRuntimeClasspath, right.testRuntimeClasspath ) - } yield { + } yield GradleLockFile( annotationProcessor, compileClasspath, @@ -57,9 +57,9 @@ object GradleLockFile { testCompileClasspath, testRuntimeClasspath ) - } } } + def empty = GradleLockFile(None, None, None, None, None, None) def decodeGradleLockFile( diff --git a/src/scala/com/github/johnynek/bazel_deps/GradleResolver.scala b/src/scala/com/github/johnynek/bazel_deps/GradleResolver.scala index 384069d..6af760b 100644 --- a/src/scala/com/github/johnynek/bazel_deps/GradleResolver.scala +++ b/src/scala/com/github/johnynek/bazel_deps/GradleResolver.scala @@ -7,6 +7,7 @@ import java.nio.file.{Path, Paths, Files} import scala.collection.immutable.SortedMap import scala.collection.mutable.{Map => MMap} import scala.util.{Failure, Success, Try} +import org.slf4j.LoggerFactory import cats.implicits._ @@ -17,6 +18,8 @@ class GradleResolver( getShasFn: List[MavenCoordinate] => Try[SortedMap[MavenCoordinate, ResolvedShasValue]] ) extends CustomResolver { + private val logger = LoggerFactory.getLogger("GradleResolver") + def resolverType: ResolverType.Gradle = gradleTpe implicit val tryMergeGradleLock: TryMerge[GradleLockDependency] = @@ -104,8 +107,8 @@ class GradleResolver( private def cleanUpMap( // invariant: depMap is fully connected, all nodes in transitive exist as a key - depMap: Map[String, GradleLockDependency] -): Map[String, GradleLockDependency] = { + depMap: SortedMap[String, GradleLockDependency] +): SortedMap[String, GradleLockDependency] = { // for no content deps there is nothing to fetch/no sha to operate on. def matchNoContentRes(unversioned: String, v: GradleLockDependency): Boolean = @@ -116,10 +119,11 @@ private def cleanUpMap( } // Remove gradle projects, these are source dependencies - val depMapNonProj = depMap.filter(_._2.project != Some(true)) + val depMapNonProj: SortedMap[String, GradleLockDependency] = + depMap.filter(_._2.project != Some(true)) depMapNonProj - .foldLeft(Map.empty[String, GradleLockDependency]) { case (accGraph, (unversioned, thisGLD)) => + .foldLeft(SortedMap.empty[String, GradleLockDependency]) { case (accGraph, (unversioned, thisGLD)) => /* * this method expands a list of transitive deps removing @@ -217,13 +221,22 @@ private def cleanUpMap( val g = Graph.empty[UnversionedCoordinate, Unit] gradleTpe.ignoreDependencyEdge match { case Some(missing) => + logger.info(s"ignoreDependencyEdge has ${missing.size} items") missing.iterator.flatMap { case (s, d) => UnversionedCoordinate.parse(s) - .product(UnversionedCoordinate.parse(d)) - .toOption + .product(UnversionedCoordinate.parse(d)) match { + case Validated.Valid((suv, duv)) => + logger.info(s"ignoring $s -> $d") + Edge(suv, duv, ()) :: Nil + case Validated.Invalid(errs) => + logger.warn(s"could not parse ignoreDependencyEdge: $s -> $d: $errs") + Nil + } } - .foldLeft(g) { case (g, (s, d)) => g.addEdge(Edge(s, d, ()))} - case None => g + .foldLeft(g) { (g, e) => g.addEdge(e) } + case None => + logger.info(s"ignoreDependencyEdge is empty.") + g } } @@ -242,7 +255,7 @@ private def cleanUpMap( ): Try[Graph[MavenCoordinate, Unit]] = assertConnectedDependencyMap(depMap) .map(_ => cleanUpMap(depMap)) - .flatMap { depMap => + .flatMap { (depMap: SortedMap[String, GradleLockDependency]) => type V[+A] = ValidatedNec[(String, String), A] val cache = MMap[String, V[MavenCoordinate]]() @@ -289,7 +302,10 @@ private def cleanUpMap( // but if we ignore the edge, we defensively make sure we add revDep // since this graph is validated, it should be added when it appears // as a key however - if (ignoreEdge(revDep, cnode)) g.addNode(revDep) + if (ignoreEdge(revDep, cnode)) { + logger.debug(s"ignored: $revDep -> $cnode") + g.addNode(revDep) + } else g.addEdge(Edge(revDep, cnode, ())) } }