Skip to content

Commit

Permalink
Merge pull request #38 from eed3si9n/wip/force
Browse files Browse the repository at this point in the history
Respect force, and prioritize direct dependencies
  • Loading branch information
eed3si9n authored Feb 24, 2021
2 parents 0e8a350 + cb258cc commit e1281fb
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import multiversion.resolvers.Sha256
@CommandName("export")
case class ExportCommand(
lint: Boolean = true,
outputPath: Path = Paths.get("3rdparty", "jvm_deps.bzl"),
outputPath: Path = Paths.get("/tmp", "jvm_deps.bzl"),
cache: Option[Path] = None,
@Inline
lintCommand: LintCommand = LintCommand()
Expand Down Expand Up @@ -236,7 +236,9 @@ case class ExportCommand(
val out =
if (outputPath.isAbsolute()) outputPath
else app.env.workingDirectory.resolve(outputPath)
Files.createDirectories(out.getParent())
if (!Files.exists(out.getParent())) {
Files.createDirectories(out.getParent())
}
Files.write(out, rendered.getBytes(StandardCharsets.UTF_8))
ValueResult(out)
}
Expand All @@ -245,8 +247,9 @@ case class ExportCommand(

def lintPostResolution(index: ResolutionIndex): Result[Unit] = {
val errors = for {
(module, deps) <- index.allDependencies.toList
allVersions = deps.map(d => index.reconciledVersion(d))
(module, deps0) <- index.allDependencies.toList
deps = deps0.map(_._1)
allVersions = deps.map { d => index.reconciledVersion(d) }
if allVersions.size > 1
diagnostic <- index.thirdparty.depsByModule.get(module) match {
case Some(declaredDeps) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ final case class ResolutionIndex(
val allVersions = allDependencies
.get(dep.module)
.getOrElse(Nil)
.map(_.toDependencyId)
.map(_._1.toDependencyId)
allVersions.find(v => dependencies.getOrElse(v, Nil).nonEmpty) match {
case Some(v) =>
// remove self-edge
Expand All @@ -77,16 +77,16 @@ final case class ResolutionIndex(
}
}

val allDependencies: collection.Map[Module, collection.Set[Dependency]] = {
val allDependencies: collection.Map[Module, collection.Set[(Dependency, Boolean)]] = {
val result =
mutable.LinkedHashMap.empty[Module, mutable.LinkedHashSet[Dependency]]
mutable.LinkedHashMap.empty[Module, mutable.LinkedHashSet[(Dependency, Boolean)]]
rawArtifacts.foreach {
case ResolvedDependency(_, d, _, _) =>
case ResolvedDependency(config, d, _, _) =>
val buf = result.getOrElseUpdate(
d.module,
mutable.LinkedHashSet.empty
)
buf += d
buf += ((d, config.force))
}
result
}
Expand Down Expand Up @@ -124,7 +124,7 @@ final case class ResolutionIndex(
VersionCompatibility.EarlySemVer
}
versions = reconcileVersions(deps, compat)
dep <- deps
(dep, force) <- deps
reconciledVersion <- versions.get(dep)
if dep.version != reconciledVersion
} yield dep.withoutConfig -> reconciledVersion
Expand Down Expand Up @@ -155,7 +155,7 @@ object ResolutionIndex {

private val overrideTags = Set("-tw")
def resolveVersions(
vers: Set[String],
verForces: Set[(String, Boolean)],
compat: VersionCompatibility
): Set[String] = {
def isUnstable(v: Version): Boolean = {
Expand All @@ -169,36 +169,49 @@ object ResolutionIndex {
def lessThan(v1: Version, v2: Version): Boolean =
(!hasOverride(v1) && hasOverride(v2)) || (v1 < v2)
// The "winners" are the highest selected versions
val winners = mutable.Set.empty[Version]
val versions = vers.map(Version(_))
versions.foreach { version =>
val isCompatible = winners.exists { winner =>
if (isCompat(version.repr, winner.repr, compat)) {
if (lessThan(winner, version) && !isUnstable(version)) {
winners.remove(winner)
winners.add(version)
}
true
} else {
false
val winners = mutable.Set.empty[(Version, Boolean)]
verForces.foreach {
case (v, force) =>
val version = Version(v)
val isCompatible = winners.exists {
case (winner, wforce) =>
if (isCompat(version.repr, winner.repr, compat)) {
if (
(
(lessThan(winner, version) && force == wforce)
|| (force && !wforce)
)
&& !isUnstable(version)
) {
winners.remove((winner, wforce))
winners.add(version -> force)
}
true
} else {
false
}
}
if (!isCompatible) {
winners.add(version -> force)
}
}
if (!isCompatible) {
winners.add(version)
}
}
winners.map(_.repr).toSet
winners.map(_._1.repr).toSet
}

def reconcileVersions(
deps: collection.Set[Dependency],
deps: collection.Set[(Dependency, Boolean)],
compat: VersionCompatibility
): Map[Dependency, String] = {
val winners = resolveVersions(
deps.map {
case (d, fc) =>
(d.version, fc)
}.toSet,
compat
)
val result = (for {
dep <- deps
// I'm not sure why this .toSet is necessary, but without it
// wrong version would win here
winner <- resolveVersions(deps.map(_.version).toSet, compat)
(dep, force) <- deps
winner <- winners
if (dep.version != winner) && isCompat(dep.version, winner, compat)
} yield dep -> winner).toMap
result
Expand Down
6 changes: 5 additions & 1 deletion tests/src/main/scala/tests/BaseSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ abstract class BaseSuite extends MopedSuite(MultiVersion.app) {
val allGenrules: List[String] = List("kind(genrule, @maven//:all)")
def allScalaImportDeps(target: String): List[String] =
List(s"kind(scala_import, allpaths($target, @maven//:all))")

def exportCommand: List[String] =
List("export", "--output-path", "3rdparty/jvm_deps.bzl")

def checkDeps(
name: TestOptions,
deps: String,
Expand All @@ -95,7 +99,7 @@ abstract class BaseSuite extends MopedSuite(MultiVersion.app) {
): Unit = {
test(name) {
checkCommand(
arguments = List("export"),
arguments = exportCommand,
expectedExit = expectedExit,
expectedOutput = expectedOutput,
workingDirectoryLayout = s"""|/3rdparty.yaml
Expand Down
14 changes: 14 additions & 0 deletions tests/src/test/scala/tests/commands/ExportCommandSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@ package tests.commands

class ExportCommandSuite extends tests.BaseSuite {

checkDeps(
"evicted artifacts do not create genrules",
s"""| - dependency: org.slf4j:slf4j-log4j12:1.6.1
| force: true
| - dependency: org.slf4j:slf4j-log4j12:1.6.4
| force: false
|""".stripMargin,
queryArgs = allGenrules,
expectedQuery = """|@maven//:genrules/log4j_log4j_1.2.16
|@maven//:genrules/org.slf4j_slf4j-api_1.6.1
|@maven//:genrules/org.slf4j_slf4j-log4j12_1.6.1
|""".stripMargin
)

checkDeps(
"evicted artifacts do not create genrules",
s"""| - dependency: org.slf4j:slf4j-log4j12:1.6.1
Expand Down
4 changes: 2 additions & 2 deletions tests/src/test/scala/tests/commands/LintCommandSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import tests.BaseSuite
class LintCommandSuite extends BaseSuite {
test("basic") {
checkCommand(
arguments = List("export"),
arguments = exportCommand,
expectedOutput = """|✔ Generated '/workingDirectory/3rdparty/jvm_deps.bzl'
|""".stripMargin,
workingDirectoryLayout = s"""|/3rdparty.yaml
Expand All @@ -25,7 +25,7 @@ class LintCommandSuite extends BaseSuite {

test("classifier") {
checkCommand(
arguments = List("export"),
arguments = exportCommand,
expectedOutput = """|✔ Generated '/workingDirectory/3rdparty/jvm_deps.bzl'
|""".stripMargin,
workingDirectoryLayout = s"""|/3rdparty.yaml
Expand Down
29 changes: 26 additions & 3 deletions tests/src/test/scala/tests/commands/ResolutionTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,38 @@ class ResolutionTest extends tests.BaseSuite {

test("resolveVersions") {
val vs = ResolutionIndex.resolveVersions(
Set("2.11.2", "2.6.7.1", "2.9.9", "2.10.0", "2.10.2", "2.8.4"),
Set(
"2.11.2" -> true,
"2.6.7.1" -> true,
"2.9.9" -> true,
"2.10.0" -> true,
"2.10.2" -> true,
"2.8.4" -> true
),
VersionCompatibility.EarlySemVer
)
assertEquals(vs.head, "2.11.2")
}

test("resolveVersions with and force = True and False") {
val vs = ResolutionIndex.resolveVersions(
Set("2.11.2" -> false, "2.6.7.1" -> true, "2.11.3" -> false),
VersionCompatibility.EarlySemVer
)
assertEquals(vs.head, "2.6.7.1")
}

test("resolveVersions with force = False") {
val vs = ResolutionIndex.resolveVersions(
Set("2.11.2" -> false, "2.6.7.1" -> false),
VersionCompatibility.EarlySemVer
)
assertEquals(vs.head, "2.11.2")
}

def jodaTime(v: String): Dependency =
def jodaTime(v: String): (Dependency, Boolean) =
Dependency(
Module(Organization("joda-time"), ModuleName("joda-time"), Map.empty),
v
)
) -> true
}

0 comments on commit e1281fb

Please sign in to comment.