Skip to content

Commit f5608bf

Browse files
johnynekOscar Boykin
johnynek
and
Oscar Boykin
authored
gradle resolver refactors (#376)
* checkpoint some refactors should be no-op * add failing test * show all the loop errors * be fully applicative in writer * Add test of Circular edges error * respond to review comments * fix typo --------- Co-authored-by: Oscar Boykin <oboykin@netflix.com>
1 parent 380da3e commit f5608bf

File tree

15 files changed

+722
-364
lines changed

15 files changed

+722
-364
lines changed

‎src/scala/com/github/johnynek/bazel_deps/AetherResolver.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class AetherResolver(servers: List[MavenServer], resolverCachePath: Path)
7171
t: Class[_],
7272
impl: Class[_],
7373
exception: Throwable
74-
) {
74+
): Unit = {
7575
logger.error(s"could not create service: $t, $impl", exception)
7676
exception.printStackTrace()
7777
}

‎src/scala/com/github/johnynek/bazel_deps/CoursierResolver.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ object CoursierResolver {
3737
}
3838
}
3939

40-
class CoursierResolver(servers: List[DependencyServer], ec: ExecutionContext, runTimeout: Duration, resolverCachePath: Path) extends Resolver[Task] {
40+
class CoursierResolver(servers: List[DependencyServer], ec: ExecutionContext, runTimeout: Duration, resolverCachePath: Path) extends NormalizingResolver[Task] {
4141
// TODO: add support for a local file cache other than ivy
4242
private[this] val repos = LocalRepositories.ivy2Local :: {
4343
val settings = SettingsLoader.settings

‎src/scala/com/github/johnynek/bazel_deps/Decoders.scala‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -159,29 +159,24 @@ object Decoders {
159159

160160
implicit val gradleDecoder =
161161
auto.exportDecoder[ResolverType.Gradle].instance
162+
162163
val baseOptions = auto.exportDecoder[Options].instance
164+
163165
new Decoder[Options] {
164-
override def apply(c: HCursor): Result[Options] = {
165-
baseOptions(c) match {
166-
case Right(b) => {
167-
b.resolverType match {
168-
case Some(x) =>
169-
x match {
170-
case g: ResolverType.Gradle =>
171-
c.get[Option[ResolverType.Gradle]]("resolverOptions").map {
172-
optV =>
173-
optV
174-
.map { inner => b.copy(resolverType = Some(inner)) }
175-
.getOrElse(b)
176-
}
177-
case _ => Right(b)
178-
}
179-
case None => Right(b)
180-
}
166+
override def apply(c: HCursor): Result[Options] =
167+
baseOptions(c).flatMap { b =>
168+
b.resolverType match {
169+
case Some(g: ResolverType.Gradle) =>
170+
// in the baseOptions the resolver type is empty
171+
// here we parse resolverOptions as if it is the default
172+
// decoder (using gradleDecoder) to get the gradle options
173+
c.get[Option[ResolverType.Gradle]]("resolverOptions").map {
174+
case nonEmpty @ Some(_) => b.copy(resolverType = nonEmpty)
175+
case None => b
176+
}
177+
case _ => Right(b)
181178
}
182-
case Left(a) => Left(a)
183179
}
184-
}
185180
}
186181
}
187182

‎src/scala/com/github/johnynek/bazel_deps/DepsModel.scala‎

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ object Model {
136136
combine(_, _).toEither
137137
)
138138
}
139+
140+
val empty: Model =
141+
Model(Dependencies.empty, None, None)
139142
}
140143

141144
case class MavenGroup(asString: String)
@@ -499,16 +502,28 @@ object MavenArtifactId {
499502
)
500503
}
501504

502-
def apply(str: String): MavenArtifactId =
505+
def parse(str: String): ValidatedNel[String, MavenArtifactId] =
503506
str.split(":") match {
504-
case Array(a, p, c) => MavenArtifactId(a, p, Some(c))
505-
case Array(a, p) => MavenArtifactId(a, p, None)
506-
case Array(a) => MavenArtifactId(a, defaultPackaging, None)
507+
case Array(a) => Validated.valid(MavenArtifactId(a, defaultPackaging, None))
508+
case Array(a, p) => Validated.valid(MavenArtifactId(a, p, None))
509+
case Array(a, p, c) => Validated.valid(MavenArtifactId(a, p, Some(c)))
507510
case _ =>
508-
sys.error(
511+
Validated.invalidNel(
509512
s"$str did not match expected format <artifactId>[:<packaging>[:<classifier>]]"
510513
)
511514
}
515+
516+
def apply(str: String): MavenArtifactId =
517+
parse(str) match {
518+
case Validated.Valid(maid) => maid
519+
case Validated.Invalid(errs) =>
520+
sys.error(errs.mkString_("\n"))
521+
}
522+
523+
implicit val orderingMavenArtifactId: Ordering[MavenArtifactId] =
524+
Ordering.by { (aid: MavenArtifactId) =>
525+
(aid.artifactId, aid.packaging, aid.classifier)
526+
}
512527
}
513528

514529
case class MavenCoordinate(
@@ -551,7 +566,7 @@ object MavenCoordinate {
551566
}
552567

553568
def parse(s: String): ValidatedNel[String, MavenCoordinate] =
554-
s.split(":") match {
569+
s.split(":", -1) match {
555570
case Array(g, a, v) =>
556571
Validated.valid(
557572
MavenCoordinate(MavenGroup(g), MavenArtifactId(a), Version(v))
@@ -757,6 +772,21 @@ case class UnversionedCoordinate(group: MavenGroup, artifact: MavenArtifactId) {
757772
s"//external:${toBindingName(namePrefix)}"
758773
}
759774

775+
object UnversionedCoordinate {
776+
implicit val orderingUnversionedCoordinate: Ordering[UnversionedCoordinate] =
777+
Ordering.by { (uvc: UnversionedCoordinate) => (uvc.group.asString, uvc.artifact) }
778+
779+
def parse(s: String): ValidatedNel[String, UnversionedCoordinate] = {
780+
val colon = s.indexOf(':')
781+
if (colon <= 0) Validated.invalidNel(s"invalid MavenGroup in $s")
782+
else {
783+
val group = s.take(colon)
784+
val rest = s.drop(colon + 1)
785+
MavenArtifactId.parse(rest).map(UnversionedCoordinate(MavenGroup(group), _))
786+
}
787+
}
788+
}
789+
760790
case class ProjectRecord(
761791
lang: Language,
762792
version: Option[Version],
@@ -959,7 +989,7 @@ case class Dependencies(
959989
def exportedUnversioned(
960990
u: UnversionedCoordinate,
961991
r: Replacements
962-
): Either[List[(MavenGroup, ArtifactOrProject)], List[
992+
): Either[NonEmptyList[(MavenGroup, ArtifactOrProject)], List[
963993
UnversionedCoordinate
964994
]] =
965995
recordOf(u).flatMap(_.exports) match {
@@ -969,13 +999,19 @@ case class Dependencies(
969999
g: MavenGroup,
9701000
a: ArtifactOrProject
9711001
): Option[UnversionedCoordinate] =
972-
unversionedCoordinatesOf(g, a).orElse(
973-
r.unversionedCoordinatesOf(g, a)
974-
)
1002+
unversionedCoordinatesOf(g, a)
1003+
.orElse(r.unversionedCoordinatesOf(g, a))
9751004

976-
val errs = l.filter { case (g, a) => uv(g, a).isEmpty }
977-
if (errs.nonEmpty) Left(l.toList)
978-
else Right(l.toList.flatMap { case (g, a) => uv(g, a) })
1005+
val errs = l
1006+
.filter { case (g, a) => uv(g, a).isEmpty }
1007+
.toList
1008+
// make deterministic
1009+
.sortBy { case (g, a) => (g.asString, a.asString) }
1010+
1011+
NonEmptyList.fromList(errs) match {
1012+
case None => Right(l.toList.flatMap { case (g, a) => uv(g, a) })
1013+
case Some(errNel) => Left(errNel)
1014+
}
9791015
}
9801016

9811017
private val coordToProj: Map[MavenCoordinate, ProjectRecord] =

‎src/scala/com/github/johnynek/bazel_deps/GradleLockDependency.scala‎

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ object GradleLockDependency {
1919
case object LeftVersion extends VersionState
2020
case object RightVersion extends VersionState
2121

22-
def resolveVersions(versionConflictPolicy: VersionConflictPolicy, dependencyName: String)(
22+
private def resolveVersions(versionConflictPolicy: VersionConflictPolicy, dependencyName: String)(
2323
left: Option[String],
2424
right: Option[String]
2525
): Try[VersionState] = {
@@ -49,49 +49,43 @@ object GradleLockDependency {
4949
}
5050
}
5151

52+
private val unit = Success(())
53+
5254
def mergeGradleLockDeps(versionConflictPolicy: VersionConflictPolicy): TryMerge[GradleLockDependency] =
5355
new TryMerge[GradleLockDependency] {
5456
def tryMerge(
5557
debugName: Option[String],
56-
left: GradleLockDependency,
57-
right: GradleLockDependency
58-
): Try[GradleLockDependency] = {
58+
left: GradleLockDependency,
59+
right: GradleLockDependency
60+
): Try[GradleLockDependency] = {
5961
lazy val mergedDependencies = Some(
60-
(left.transitive.getOrElse(Nil) ++ right.transitive.getOrElse(
61-
Nil
62-
)).sorted.distinct
63-
).filter(_.nonEmpty)
62+
(left.transitive.getOrElse(Nil) ::: right.transitive.getOrElse(Nil))
63+
.distinct
64+
.sorted
65+
).filter(_.nonEmpty)
6466

6567
for {
66-
v <- resolveVersions(versionConflictPolicy, debugName.getOrElse("Unknown"))(left.locked, right.locked)
67-
_ <-
68-
if (left.project == right.project) Success(())
69-
else
70-
Failure(
71-
new Exception(
72-
s"Unable to merge due to incompatible project setting, had $left, $right"
73-
)
74-
)
75-
} yield {
76-
v match {
77-
case EqualVersionSpecified =>
78-
GradleLockDependency(
79-
locked = left.locked,
80-
project = left.project,
81-
transitive = mergedDependencies
68+
_ <- if (left.project == right.project) unit
69+
else Failure(
70+
new Exception(
71+
s"Unable to merge due to incompatible project setting, had $left, $right"
72+
)
8273
)
83-
case LeftVersion => GradleLockDependency(
74+
v <- resolveVersions(versionConflictPolicy, debugName.getOrElse("Unknown"))(left.locked, right.locked)
75+
} yield (v match {
76+
case EqualVersionSpecified | LeftVersion =>
77+
GradleLockDependency(
8478
locked = left.locked,
8579
project = left.project,
8680
transitive = mergedDependencies
87-
)
88-
case RightVersion => GradleLockDependency(
81+
)
82+
case RightVersion =>
83+
GradleLockDependency(
8984
locked = right.locked,
90-
project = right.project,
85+
project = right.project, // note right.project == left.project or we don't reach here
9186
transitive = mergedDependencies
92-
)
93-
}
94-
}
87+
)
88+
})
9589
}
9690
}
9791

0 commit comments

Comments
 (0)