From eb4e73fa5446652ef14ea893b6cff9e4c6a1ac36 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Sat, 23 Jul 2016 17:17:17 +0200 Subject: [PATCH] Better handling of version intervals / hints reconciliation Fixes https://github.com/alexarchambault/coursier/issues/303 --- build.sbt | 8 ++ .../src/main/scala/coursier/core/Parse.scala | 8 +- .../main/scala/coursier/core/Resolution.scala | 25 ++++--- .../main/scala/coursier/core/Versions.scala | 74 ++++++++++++++----- .../malihu-custom-scrollbar-plugin/3.1.5 | 3 + .../scala/coursier/test/CentralTests.scala | 9 +++ .../test/VersionConstraintTests.scala | 33 +++------ .../coursier/test/VersionIntervalTests.scala | 6 +- 8 files changed, 103 insertions(+), 63 deletions(-) create mode 100644 tests/shared/src/test/resources/resolutions/org.webjars.bower/malihu-custom-scrollbar-plugin/3.1.5 diff --git a/build.sbt b/build.sbt index 827671710..4a590b795 100644 --- a/build.sbt +++ b/build.sbt @@ -148,6 +148,14 @@ lazy val core = crossProject Seq( // Since 1.0.0-M13 + // reworked VersionConstraint + ProblemFilters.exclude[MissingClassProblem]("coursier.core.VersionConstraint$Interval"), + ProblemFilters.exclude[MissingClassProblem]("coursier.core.VersionConstraint$Preferred"), + ProblemFilters.exclude[MissingClassProblem]("coursier.core.VersionConstraint$Preferred$"), + ProblemFilters.exclude[MissingClassProblem]("coursier.core.VersionConstraint$Interval$"), + ProblemFilters.exclude[FinalClassProblem]("coursier.core.VersionConstraint"), + ProblemFilters.exclude[IncompatibleResultTypeProblem]("coursier.core.VersionConstraint.repr"), + ProblemFilters.exclude[IncompatibleMethTypeProblem]("coursier.core.VersionConstraint.this"), // Extra `actualVersion` field in `Project` ProblemFilters.exclude[MissingTypesProblem]("coursier.core.Project$"), ProblemFilters.exclude[MissingMethodProblem]("coursier.core.Project.apply"), diff --git a/core/shared/src/main/scala/coursier/core/Parse.scala b/core/shared/src/main/scala/coursier/core/Parse.scala index 8465ca05c..83bcabd6e 100644 --- a/core/shared/src/main/scala/coursier/core/Parse.scala +++ b/core/shared/src/main/scala/coursier/core/Parse.scala @@ -43,12 +43,12 @@ object Parse { } def versionConstraint(s: String): Option[VersionConstraint] = { - def noConstraint = if (s.isEmpty) Some(VersionConstraint.None) else None + def noConstraint = if (s.isEmpty) Some(VersionConstraint.all) else None noConstraint - .orElse(ivyLatestSubRevisionInterval(s).map(VersionConstraint.Interval)) - .orElse(version(s).map(VersionConstraint.Preferred)) - .orElse(versionInterval(s).map(VersionConstraint.Interval)) + .orElse(ivyLatestSubRevisionInterval(s).map(VersionConstraint.interval)) + .orElse(version(s).map(VersionConstraint.preferred)) + .orElse(versionInterval(s).map(VersionConstraint.interval)) } val fallbackConfigRegex = { diff --git a/core/shared/src/main/scala/coursier/core/Resolution.scala b/core/shared/src/main/scala/coursier/core/Resolution.scala index 5241a3335..f2ee7e0ec 100644 --- a/core/shared/src/main/scala/coursier/core/Resolution.scala +++ b/core/shared/src/main/scala/coursier/core/Resolution.scala @@ -140,25 +140,26 @@ object Resolution { * Returns `None` in case of conflict. */ def mergeVersions(versions: Seq[String]): Option[String] = { - val (nonParsedConstraints, parsedConstraints) = - versions - .map(v => v -> Parse.versionConstraint(v)) - .partition(_._2.isEmpty) + + val parseResults = versions.map(v => v -> Parse.versionConstraint(v)) + + val nonParsedConstraints = parseResults.collect { + case (repr, None) => repr + } // FIXME Report this in return type, not this way if (nonParsedConstraints.nonEmpty) Console.err.println( - s"Ignoring unparsed versions: ${nonParsedConstraints.map(_._1)}" + s"Ignoring unparsed versions: $nonParsedConstraints" ) - val intervalOpt = - (Option(VersionInterval.zero) /: parsedConstraints) { - case (acc, (_, someCstr)) => - acc.flatMap(_.merge(someCstr.get.interval)) - } + val parsedConstraints = parseResults.collect { + case (_, Some(c)) => c + } - intervalOpt - .map(_.constraint.repr) + VersionConstraint + .merge(parsedConstraints: _*) + .flatMap(_.repr) } /** diff --git a/core/shared/src/main/scala/coursier/core/Versions.scala b/core/shared/src/main/scala/coursier/core/Versions.scala index 5398f262c..d59ecfec9 100644 --- a/core/shared/src/main/scala/coursier/core/Versions.scala +++ b/core/shared/src/main/scala/coursier/core/Versions.scala @@ -1,5 +1,8 @@ package coursier.core +import scalaz.{ -\/, \/, \/- } +import scalaz.Scalaz.ToEitherOps + case class VersionInterval(from: Option[Version], to: Option[Version], fromIncluded: Boolean, @@ -64,9 +67,9 @@ case class VersionInterval(from: Option[Version], def constraint: VersionConstraint = this match { - case VersionInterval.zero => VersionConstraint.None - case VersionInterval(Some(version), None, true, false) => VersionConstraint.Preferred(version) - case itv => VersionConstraint.Interval(itv) + case VersionInterval.zero => VersionConstraint.all + case VersionInterval(Some(version), None, true, false) => VersionConstraint.preferred(version) + case itv => VersionConstraint.interval(itv) } def repr: String = Seq( @@ -82,23 +85,54 @@ object VersionInterval { val zero = VersionInterval(None, None, fromIncluded = false, toIncluded = false) } -sealed abstract class VersionConstraint( - val interval: VersionInterval, - val repr: String -) +final case class VersionConstraint( + interval: VersionInterval, + preferred: Seq[Version] +) { + def blend: Option[VersionInterval \/ Version] = + if (interval.isValid) { + val preferredInInterval = preferred.filter(interval.contains) + + if (preferredInInterval.isEmpty) + Some(interval.left) + else + Some(preferredInInterval.max.right) + } else + None + + def repr: Option[String] = + blend.map { + case -\/(itv) => + if (itv == VersionInterval.zero) + "" + else + itv.repr + case \/-(v) => v.repr + } +} object VersionConstraint { - /** Currently treated as minimum... */ - final case class Preferred(version: Version) extends VersionConstraint( - VersionInterval(Some(version), Option.empty, fromIncluded = true, toIncluded = false), - version.repr - ) - final case class Interval(interval0: VersionInterval) extends VersionConstraint( - interval0, - interval0.repr - ) - case object None extends VersionConstraint( - VersionInterval.zero, - "" // Once parsed, "(,)" becomes "" because of this - ) + + def preferred(version: Version): VersionConstraint = + VersionConstraint(VersionInterval.zero, Seq(version)) + def interval(interval: VersionInterval): VersionConstraint = + VersionConstraint(interval, Nil) + + val all = VersionConstraint(VersionInterval.zero, Nil) + + def merge(constraints: VersionConstraint*): Option[VersionConstraint] = { + + val intervals = constraints.map(_.interval) + + val intervalOpt = + (Option(VersionInterval.zero) /: intervals) { + case (acc, itv) => + acc.flatMap(_.merge(itv)) + } + + for (interval <- intervalOpt) yield { + val preferreds = constraints.flatMap(_.preferred).distinct + VersionConstraint(interval, preferreds) + } + } } diff --git a/tests/shared/src/test/resources/resolutions/org.webjars.bower/malihu-custom-scrollbar-plugin/3.1.5 b/tests/shared/src/test/resources/resolutions/org.webjars.bower/malihu-custom-scrollbar-plugin/3.1.5 new file mode 100644 index 000000000..2300dfd10 --- /dev/null +++ b/tests/shared/src/test/resources/resolutions/org.webjars.bower/malihu-custom-scrollbar-plugin/3.1.5 @@ -0,0 +1,3 @@ +org.webjars.bower:jquery:3.1.0:compile +org.webjars.bower:jquery-mousewheel:3.1.13:compile +org.webjars.bower:malihu-custom-scrollbar-plugin:3.1.5:compile diff --git a/tests/shared/src/test/scala/coursier/test/CentralTests.scala b/tests/shared/src/test/scala/coursier/test/CentralTests.scala index bc1fef336..fdd8d3d36 100644 --- a/tests/shared/src/test/scala/coursier/test/CentralTests.scala +++ b/tests/shared/src/test/scala/coursier/test/CentralTests.scala @@ -237,6 +237,15 @@ object CentralTests extends TestSuite { ) } + 'versionInterval - { + // Warning: needs to be updated when new versions of org.webjars.bower:jquery and + // org.webjars.bower:jquery-mousewheel are published :-| + resolutionCheck( + Module("org.webjars.bower", "malihu-custom-scrollbar-plugin"), + "3.1.5" + ) + } + 'latestRevision - { * - resolutionCheck( Module("com.chuusai", "shapeless_2.11"), diff --git a/tests/shared/src/test/scala/coursier/test/VersionConstraintTests.scala b/tests/shared/src/test/scala/coursier/test/VersionConstraintTests.scala index 2227e7537..3fa9f5e99 100644 --- a/tests/shared/src/test/scala/coursier/test/VersionConstraintTests.scala +++ b/tests/shared/src/test/scala/coursier/test/VersionConstraintTests.scala @@ -10,45 +10,30 @@ object VersionConstraintTests extends TestSuite { 'parse{ 'empty{ val c0 = Parse.versionConstraint("") - assert(c0 == Some(VersionConstraint.None)) + assert(c0 == Some(VersionConstraint.all)) } 'basicVersion{ val c0 = Parse.versionConstraint("1.2") - assert(c0 == Some(VersionConstraint.Preferred(Version("1.2")))) + assert(c0 == Some(VersionConstraint.preferred(Version("1.2")))) } 'basicVersionInterval{ val c0 = Parse.versionConstraint("(,1.2]") - assert(c0 == Some(VersionConstraint.Interval(VersionInterval(None, Some(Version("1.2")), false, true)))) + assert(c0 == Some(VersionConstraint.interval(VersionInterval(None, Some(Version("1.2")), false, true)))) } } 'repr{ 'empty{ - val s0 = VersionConstraint.None.repr - assert(s0 == "") + val s0 = VersionConstraint.all.repr + assert(s0 == Some("")) } 'preferred{ - val s0 = VersionConstraint.Preferred(Version("2.1")).repr - assert(s0 == "2.1") + val s0 = VersionConstraint.preferred(Version("2.1")).repr + assert(s0 == Some("2.1")) } 'interval{ - val s0 = VersionConstraint.Interval(VersionInterval(None, Some(Version("2.1")), false, true)).repr - assert(s0 == "(,2.1]") - } - } - - 'interval{ - 'empty{ - val s0 = VersionConstraint.None.interval - assert(s0 == VersionInterval.zero) - } - 'preferred{ - val s0 = VersionConstraint.Preferred(Version("2.1")).interval - assert(s0 == VersionInterval(Some(Version("2.1")), None, true, false)) - } - 'interval{ - val s0 = VersionConstraint.Interval(VersionInterval(None, Some(Version("2.1")), false, true)).interval - assert(s0 == VersionInterval(None, Some(Version("2.1")), false, true)) + val s0 = VersionConstraint.interval(VersionInterval(None, Some(Version("2.1")), false, true)).repr + assert(s0 == Some("(,2.1]")) } } } diff --git a/tests/shared/src/test/scala/coursier/test/VersionIntervalTests.scala b/tests/shared/src/test/scala/coursier/test/VersionIntervalTests.scala index 932df2ca8..95118627c 100644 --- a/tests/shared/src/test/scala/coursier/test/VersionIntervalTests.scala +++ b/tests/shared/src/test/scala/coursier/test/VersionIntervalTests.scala @@ -269,17 +269,17 @@ object VersionIntervalTests extends TestSuite { 'none{ val s1 = "(,)" val c1 = Parse.versionInterval(s1).map(_.constraint) - assert(c1 == Some(VersionConstraint.None)) + assert(c1 == Some(VersionConstraint.all)) } 'preferred{ val s1 = "[1.3,)" val c1 = Parse.versionInterval(s1).map(_.constraint) - assert(c1 == Some(VersionConstraint.Preferred(Parse.version("1.3").get))) + assert(c1 == Some(VersionConstraint.preferred(Parse.version("1.3").get))) } 'interval{ val s1 = "[1.3,2.4)" val c1 = Parse.versionInterval(s1).map(_.constraint) - assert(c1 == Some(VersionConstraint.Interval(VersionInterval(Parse.version("1.3"), Parse.version("2.4"), true, false)))) + assert(c1 == Some(VersionConstraint.interval(VersionInterval(Parse.version("1.3"), Parse.version("2.4"), true, false)))) } } }