From bb258f42a632f044d40d7d92b0bcd6ffb2fe9701 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Fri, 10 Feb 2017 02:43:26 -0500 Subject: [PATCH] Maven version range improvement Previously, when the dependency resolver (Ivy) encountered a Maven version range such as `[1.3.0,)` it would go out to the Internet to find the latest version. This would result to a surprising behavior where the eventual version keeps changing over time *even when there's a version of the library that satisfies the range condition*. This changes to some Maven version ranges would be replaced with its lower bound so that when a satisfactory version is found in the dependency graph it will be used. You can disable this behavior using the JVM flag `-Dsbt.modversionrange=false`. Fixes #2954 Ref #2291 / #2953 --- .gitignore | 1 + .../librarymanagement/CustomPomParser.scala | 16 +++- .../internal/librarymanagement/MakePom.scala | 50 +---------- .../librarymanagement/VersionRange.scala | 83 +++++++++++++++++++ .../librarymanagement/VersionRangeSpec.scala | 19 +++++ 5 files changed, 120 insertions(+), 49 deletions(-) create mode 100644 librarymanagement/src/main/scala/sbt/internal/librarymanagement/VersionRange.scala create mode 100644 librarymanagement/src/test/scala/sbt/internal/librarymanagement/VersionRangeSpec.scala diff --git a/.gitignore b/.gitignore index 3cedeeb25..96c1348b1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ tmp/ target/ +__pycache__ diff --git a/librarymanagement/src/main/scala/sbt/internal/librarymanagement/CustomPomParser.scala b/librarymanagement/src/main/scala/sbt/internal/librarymanagement/CustomPomParser.scala index fbcf53649..a8336662e 100644 --- a/librarymanagement/src/main/scala/sbt/internal/librarymanagement/CustomPomParser.scala +++ b/librarymanagement/src/main/scala/sbt/internal/librarymanagement/CustomPomParser.scala @@ -162,6 +162,17 @@ object CustomPomParser { def isIdentity = false } + // TODO: It would be better if we can make dd.isForce to `false` when VersionRange.isVersionRange is `true`. + private[this] def stripVersionRange(dd: DependencyDescriptor): DependencyDescriptor = + VersionRange.stripMavenVersionRange(dd.getDependencyRevisionId.getRevision) match { + case Some(newVersion) => + val id = dd.getDependencyRevisionId + val newId = ModuleRevisionId.newInstance(id.getOrganisation, id.getName, id.getBranch, newVersion, id.getExtraAttributes) + transform(dd, _ => newId) + case None => dd + } + private[sbt] lazy val versionRangeFlag = sys.props.get("sbt.modversionrange") map { _.toLowerCase == "true" } getOrElse true + import collection.JavaConverters._ def addExtra(properties: Map[String, String], dependencyExtra: Map[ModuleRevisionId, Map[String, String]], parser: ModuleDescriptorParser, md: ModuleDescriptor): ModuleDescriptor = { @@ -187,7 +198,10 @@ object CustomPomParser { IvySbt.addExtraNamespace(dmd) val withExtra = md.getDependencies map { dd => addExtra(dd, dependencyExtra) } - val unique = IvySbt.mergeDuplicateDefinitions(withExtra) + val withVersionRangeMod: Seq[DependencyDescriptor] = + if (versionRangeFlag) withExtra map { stripVersionRange } + else withExtra + val unique = IvySbt.mergeDuplicateDefinitions(withVersionRangeMod) unique foreach dmd.addDependency for (ed <- md.getInheritedDescriptors) dmd.addInheritedDescriptor(new DefaultExtendsDescriptor(md, ed.getLocation, ed.getExtendsTypes)) diff --git a/librarymanagement/src/main/scala/sbt/internal/librarymanagement/MakePom.scala b/librarymanagement/src/main/scala/sbt/internal/librarymanagement/MakePom.scala index 2103cc6f0..5544d0d52 100644 --- a/librarymanagement/src/main/scala/sbt/internal/librarymanagement/MakePom.scala +++ b/librarymanagement/src/main/scala/sbt/internal/librarymanagement/MakePom.scala @@ -26,56 +26,10 @@ import sbt.io.IO object MakePom { /** True if the revision is an ivy-range, not a complete revision. */ - def isDependencyVersionRange(revision: String): Boolean = { - (revision endsWith "+") || - (revision contains "[") || - (revision contains "]") || - (revision contains "(") || - (revision contains ")") - } + def isDependencyVersionRange(revision: String): Boolean = VersionRange.isVersionRange(revision) /** Converts Ivy revision ranges to that of Maven POM */ - def makeDependencyVersion(revision: String): String = { - def plusRange(s: String, shift: Int = 0) = { - def pow(i: Int): Int = if (i > 0) 10 * pow(i - 1) else 1 - val (prefixVersion, lastVersion) = (s + "0" * shift).reverse.split("\\.", 2) match { - case Array(revLast, revRest) => - (revRest.reverse + ".", revLast.reverse) - case Array(revLast) => ("", revLast.reverse) - } - val lastVersionInt = lastVersion.toInt - s"[${prefixVersion}${lastVersion},${prefixVersion}${lastVersionInt + pow(shift)})" - } - val startSym = Set(']', '[', '(') - val stopSym = Set(']', '[', ')') - val DotPlusPattern = """(.+)\.\+""".r - val DotNumPlusPattern = """(.+)\.(\d+)\+""".r - val NumPlusPattern = """(\d+)\+""".r - val maxDigit = 5 - try { - revision match { - case "+" => "[0,)" - case DotPlusPattern(base) => plusRange(base) - // This is a heuristic. Maven just doesn't support Ivy's notions of 1+, so - // we assume version ranges never go beyond 5 siginificant digits. - case NumPlusPattern(tail) => (0 until maxDigit).map(plusRange(tail, _)).mkString(",") - case DotNumPlusPattern(base, tail) => (0 until maxDigit).map(plusRange(base + "." + tail, _)).mkString(",") - case rev if rev endsWith "+" => sys.error(s"dynamic revision '$rev' cannot be translated to POM") - case rev if startSym(rev(0)) && stopSym(rev(rev.length - 1)) => - val start = rev(0) - val stop = rev(rev.length - 1) - val mid = rev.substring(1, rev.length - 1) - (if (start == ']') "(" else start) + mid + (if (stop == '[') ")" else stop) - case _ => revision - } - } catch { - case e: NumberFormatException => - // TODO - if the version doesn't meet our expectations, maybe we just issue a hard - // error instead of softly ignoring the attempt to rewrite. - //sys.error(s"Could not fix version [$revision] into maven style version") - revision - } - } + def makeDependencyVersion(revision: String): String = VersionRange.fromIvyToMavenVersion(revision) } class MakePom(val log: Logger) { import MakePom._ diff --git a/librarymanagement/src/main/scala/sbt/internal/librarymanagement/VersionRange.scala b/librarymanagement/src/main/scala/sbt/internal/librarymanagement/VersionRange.scala new file mode 100644 index 000000000..bb73c882a --- /dev/null +++ b/librarymanagement/src/main/scala/sbt/internal/librarymanagement/VersionRange.scala @@ -0,0 +1,83 @@ +package sbt +package internal +package librarymanagement + +object VersionRange { + /** True if the revision is an ivy-range, not a complete revision. */ + def isVersionRange(revision: String): Boolean = { + (revision endsWith "+") || + (revision contains "[") || + (revision contains "]") || + (revision contains "(") || + (revision contains ")") + } + + // Assuming Ivy is used to resolve conflict, this removes the version range + // when it is open-ended to avoid dependency resolution hitting the Internet to get the latest. + // See https://github.com/sbt/sbt/issues/2954 + def stripMavenVersionRange(version: String): Option[String] = + if (isVersionRange(version)) { + val noSpace = version.replaceAllLiterally(" ", "") + noSpace match { + case MavenVersionSetPattern(open1, x1, comma, x2, close1, rest) => + // http://maven.apache.org/components/enforcer/enforcer-rules/versionRanges.html + (open1, Option(x1), Option(comma), Option(x2), close1) match { + case (_, None, _, Some(x2), "]") => Some(x2) + // a good upper bound is unknown + case (_, None, _, Some(x2), ")") => None + case (_, Some(x1), _, None, _) => Some(x1) + case _ => None + } + case _ => None + } + } else None + + /** Converts Ivy revision ranges to that of Maven POM */ + def fromIvyToMavenVersion(revision: String): String = { + def plusRange(s: String, shift: Int = 0) = { + def pow(i: Int): Int = if (i > 0) 10 * pow(i - 1) else 1 + val (prefixVersion, lastVersion) = (s + "0" * shift).reverse.split("\\.", 2) match { + case Array(revLast, revRest) => + (revRest.reverse + ".", revLast.reverse) + case Array(revLast) => ("", revLast.reverse) + } + val lastVersionInt = lastVersion.toInt + s"[${prefixVersion}${lastVersion},${prefixVersion}${lastVersionInt + pow(shift)})" + } + val DotPlusPattern = """(.+)\.\+""".r + val DotNumPlusPattern = """(.+)\.(\d+)\+""".r + val NumPlusPattern = """(\d+)\+""".r + val maxDigit = 5 + try { + revision match { + case "+" => "[0,)" + case DotPlusPattern(base) => plusRange(base) + // This is a heuristic. Maven just doesn't support Ivy's notions of 1+, so + // we assume version ranges never go beyond 5 siginificant digits. + case NumPlusPattern(tail) => (0 until maxDigit).map(plusRange(tail, _)).mkString(",") + case DotNumPlusPattern(base, tail) => (0 until maxDigit).map(plusRange(base + "." + tail, _)).mkString(",") + case rev if rev endsWith "+" => sys.error(s"dynamic revision '$rev' cannot be translated to POM") + case rev if startSym(rev(0)) && stopSym(rev(rev.length - 1)) => + val start = rev(0) + val stop = rev(rev.length - 1) + val mid = rev.substring(1, rev.length - 1) + (if (start == ']') "(" else start) + mid + (if (stop == '[') ")" else stop) + case _ => revision + } + } catch { + case e: NumberFormatException => + // TODO - if the version doesn't meet our expectations, maybe we just issue a hard + // error instead of softly ignoring the attempt to rewrite. + //sys.error(s"Could not fix version [$revision] into maven style version") + revision + } + } + + def hasMavenVersionRange(version: String): Boolean = + if (version.length <= 1) false + else startSym(version(0)) && stopSym(version(version.length - 1)) + + private[this] val startSym = Set(']', '[', '(') + private[this] val stopSym = Set(']', '[', ')') + private[this] val MavenVersionSetPattern = """([\]\[\(])([\w\.\-]+)?(,)?([\w\.\-]+)?([\]\[\)])(,.+)?""".r +} diff --git a/librarymanagement/src/test/scala/sbt/internal/librarymanagement/VersionRangeSpec.scala b/librarymanagement/src/test/scala/sbt/internal/librarymanagement/VersionRangeSpec.scala new file mode 100644 index 000000000..e8bd99028 --- /dev/null +++ b/librarymanagement/src/test/scala/sbt/internal/librarymanagement/VersionRangeSpec.scala @@ -0,0 +1,19 @@ +package sbt +package internal +package librarymanagement + +class VersionRangeSpec extends UnitSpec { + "Version range" should "strip 1.0 to None" in stripTo("1.0", None) + it should "strip (,1.0] to 1.0" in stripTo("(,1.0]", Some("1.0")) + it should "strip (,1.0) to None" in stripTo("(,1.0)", None) + it should "strip [1.0] to 1.0" in stripTo("[1.0]", Some("1.0")) + it should "strip [1.0,) to 1.0" in stripTo("[1.0,)", Some("1.0")) + it should "strip (1.0,) to 1.0" in stripTo("(1.0,)", Some("1.0")) + it should "strip (1.0,2.0) to None" in stripTo("(1.0,2.0)", None) + it should "strip [1.0,2.0] to None" in stripTo("[1.0,2.0]", None) + it should "strip (,1.0],[1.2,) to 1.0" in stripTo("(,1.0],[1.2,)", Some("1.0")) + it should "strip (,1.1),(1.1,) to None" in stripTo("(,1.1),(1.1,)", None) + + def stripTo(s: String, expected: Option[String]) = + assert(VersionRange.stripMavenVersionRange(s) == expected) +}