From 85dcc9d53938f04d8257f7412e48c69e1c667824 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 13:49:22 +0200 Subject: [PATCH 01/10] Cleaning --- core/shared/src/main/scala/coursier/maven/Pom.scala | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/core/shared/src/main/scala/coursier/maven/Pom.scala b/core/shared/src/main/scala/coursier/maven/Pom.scala index 268375234..2d1520347 100644 --- a/core/shared/src/main/scala/coursier/maven/Pom.scala +++ b/core/shared/src/main/scala/coursier/maven/Pom.scala @@ -340,10 +340,6 @@ object Pom { release = text(xmlVersioning, "release", "Release version") .getOrElse("") - versionsOpt = xmlVersioning.children - .find(_.label == "versions") - .map(_.children.filter(_.label == "version").flatMap(_.children.collectFirst{case Text(t) => t})) - lastUpdatedOpt = text(xmlVersioning, "lastUpdated", "Last update date and time") .toOption .flatMap(parseDateTime) @@ -371,7 +367,7 @@ object Pom { text(_, "localCopy", "Snapshot local copy") .toOption ) - .collect{ + .collect { case "true" => true case "false" => false } From 00443c3a46d06a6a3a138a4ebf3f2e15672a8f93 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 14:06:32 +0200 Subject: [PATCH 02/10] Don't use Java >= 7 Exception constructor --- .../coursier/ResolutionException.scala | 10 ++++++---- scripts/travis.sh | 20 ++++++++++++++++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/sbt-coursier/src/main/scala-2.10/coursier/ResolutionException.scala b/sbt-coursier/src/main/scala-2.10/coursier/ResolutionException.scala index 9f8925a91..f083c0ea3 100644 --- a/sbt-coursier/src/main/scala-2.10/coursier/ResolutionException.scala +++ b/sbt-coursier/src/main/scala-2.10/coursier/ResolutionException.scala @@ -4,7 +4,9 @@ final class ResolutionException( val error: ResolutionError ) extends Exception( error.message, - error.cause.orNull, - true, - false // don't keep stack trace around (improves readability from the SBT console) -) + error.cause.orNull +) { + // not using the 4-arg Exception constructor, only available with Java >= 7 + + setStackTrace(Array()) // don't keep stack trace around (improves readability from the SBT console) +} diff --git a/scripts/travis.sh b/scripts/travis.sh index 684b29f94..7e54f8d8d 100755 --- a/scripts/travis.sh +++ b/scripts/travis.sh @@ -110,7 +110,8 @@ testLauncherJava6() { } testSbtCoursierJava6() { - sbt ++${SCALA_VERSION} publishLocal + sbt ++${SCALA_VERSION} coreJVM/publishLocal cache/publishLocal sbt-coursier/publishLocal + git clone https://github.com/alexarchambault/scalacheck-shapeless.git cd scalacheck-shapeless cd project @@ -126,6 +127,23 @@ testSbtCoursierJava6() { openjdk:6-jre \ /bin/bash -c "cd /root/project && /root/bin/sbt update" cd .. + + # ensuring resolution error doesn't throw NoSuchMethodError + mkdir -p foo/project + cd foo + echo 'libraryDependencies += "foo" % "bar" % "1.0"' >> build.sbt + echo 'addSbtPlugin("io.get-coursier" % "sbt-coursier" % "1.0.0-SNAPSHOT")' >> project/plugins.sbt + echo 'sbt.version=0.13.15' >> project/build.properties + docker run -it --rm \ + -v $HOME/.ivy2/local:/root/.ivy2/local \ + -v $(pwd):/root/project \ + -v $(pwd)/../bin:/root/bin \ + -e CI=true \ + openjdk:6-jre \ + /bin/bash -c "cd /root/project && /root/bin/sbt update || true" | tee -a output + grep "coursier.ResolutionException: Encountered 1 error" output + echo "Ok, found ResolutionException in output" + cd .. } clean_plugin_sbt() { From d3b692e1651f91be95ff45c3740251f39ee88627 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 14:34:36 +0200 Subject: [PATCH 03/10] Don't print exclusions by default --- .../main/scala-2.11/coursier/cli/Helper.scala | 28 +++++++++++++--- .../src/main/scala/coursier/util/Print.scala | 32 ++++++++++++++----- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/cli/src/main/scala-2.11/coursier/cli/Helper.scala b/cli/src/main/scala-2.11/coursier/cli/Helper.scala index 128796aab..4cb4da49c 100644 --- a/cli/src/main/scala-2.11/coursier/cli/Helper.scala +++ b/cli/src/main/scala-2.11/coursier/cli/Helper.scala @@ -396,7 +396,14 @@ class Helper( fetchQuiet if (verbosityLevel >= 1) { - errPrintln(s" Dependencies:\n${Print.dependenciesUnknownConfigs(dependencies, Map.empty)}") + errPrintln( + s" Dependencies:\n" + + Print.dependenciesUnknownConfigs( + dependencies, + Map.empty, + printExclusions = verbosityLevel >= 2 + ) + ) if (forceVersions.nonEmpty) { errPrintln(" Force versions:") @@ -522,9 +529,18 @@ class Helper( val depsStr = if (reverseTree || tree) - Print.dependencyTree(dependencies, res, printExclusions = verbosityLevel >= 1, reverse = reverseTree) + Print.dependencyTree( + dependencies, + res, + printExclusions = verbosityLevel >= 1, + reverse = reverseTree + ) else - Print.dependenciesUnknownConfigs(trDeps, projCache) + Print.dependenciesUnknownConfigs( + trDeps, + projCache, + printExclusions = verbosityLevel >= 1 + ) if (printResultStdout) println(depsStr) @@ -554,7 +570,11 @@ class Helper( anyError = true errPrintln( s"\nConflict:\n" + - Print.dependenciesUnknownConfigs(res.conflicts.toVector, projCache) + Print.dependenciesUnknownConfigs( + res.conflicts.toVector, + projCache, + printExclusions = verbosityLevel >= 1 + ) ) } diff --git a/core/shared/src/main/scala/coursier/util/Print.scala b/core/shared/src/main/scala/coursier/util/Print.scala index eb8de860c..26557d6bf 100644 --- a/core/shared/src/main/scala/coursier/util/Print.scala +++ b/core/shared/src/main/scala/coursier/util/Print.scala @@ -4,16 +4,32 @@ import coursier.core.{ Attributes, Dependency, Module, Orders, Project, Resoluti object Print { - def dependency(dep: Dependency): String = { - val exclusionsStr = dep.exclusions.toVector.sorted.map { - case (org, name) => - s"\n exclude($org, $name)" - }.mkString + def dependency(dep: Dependency): String = + dependency(dep, printExclusions = false) - s"${dep.module}:${dep.version}:${dep.configuration}$exclusionsStr" + def dependency(dep: Dependency, printExclusions: Boolean): String = { + + def exclusionsStr = dep + .exclusions + .toVector + .sorted + .map { + case (org, name) => + s"\n exclude($org, $name)" + } + .mkString + + s"${dep.module}:${dep.version}:${dep.configuration}" + (if (printExclusions) exclusionsStr else "") } - def dependenciesUnknownConfigs(deps: Seq[Dependency], projects: Map[(Module, String), Project]): String = { + def dependenciesUnknownConfigs(deps: Seq[Dependency], projects: Map[(Module, String), Project]): String = + dependenciesUnknownConfigs(deps, projects, printExclusions = false) + + def dependenciesUnknownConfigs( + deps: Seq[Dependency], + projects: Map[(Module, String), Project], + printExclusions: Boolean + ): String = { val deps0 = deps.map { dep => dep.copy( @@ -38,7 +54,7 @@ object Print { (dep.module.organization, dep.module.name, dep.module.toString, dep.version) } - deps1.map(dependency).mkString("\n") + deps1.map(dependency(_, printExclusions)).mkString("\n") } private def compatibleVersions(first: String, second: String): Boolean = { From 876129a605f0d7ed7eee08399b78f07bccc31206 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 14:44:39 +0200 Subject: [PATCH 04/10] Cleaning --- .../scala/coursier/test/VersionTests.scala | 82 +++++++------------ 1 file changed, 30 insertions(+), 52 deletions(-) diff --git a/tests/shared/src/test/scala/coursier/test/VersionTests.scala b/tests/shared/src/test/scala/coursier/test/VersionTests.scala index 5b1918f70..14016fdae 100644 --- a/tests/shared/src/test/scala/coursier/test/VersionTests.scala +++ b/tests/shared/src/test/scala/coursier/test/VersionTests.scala @@ -4,21 +4,25 @@ package test import utest._ object VersionTests extends TestSuite { + import core.Version - def compare(first: String, second: String) = Version(first).compare(Version(second)) + def compare(first: String, second: String) = + Version(first).compare(Version(second)) def increasing(versions: String*): Boolean = versions.iterator.sliding(2).withPartial(false).forall{case Seq(a, b) => compare(a, b) < 0 } + val tests = TestSuite { - 'stackOverflow{ + + 'stackOverflow - { val s = "." * 100000 val v = Version(s) assert(v.isEmpty) } - 'empty{ + 'empty - { val v0 = Version("0") val v = Version("") @@ -26,7 +30,7 @@ object VersionTests extends TestSuite { assert(v.isEmpty) } - 'max{ + 'max - { val v21 = Version("2.1") val v22 = Version("2.2") val v23 = Version("2.3") @@ -39,21 +43,15 @@ object VersionTests extends TestSuite { assert(max == v241) } - 'numericOrdering{ - assert(compare("1.2", "1.10") < 0) - } - // Adapted from aether-core/aether-util/src/test/java/org/eclipse/aether/util/version/GenericVersionTest.java // Only one test doesn't pass (see FIXME below) - 'EmptyVersion{ + 'emptyVersion - { assert(compare("0", "" ) == 0) } - - 'NumericOrdering - { + 'numericOrdering - { assert(compare("2", "10" ) < 0) assert(compare("1.2", "1.10" ) < 0) assert(compare("1.0.2", "1.0.10" ) < 0) @@ -63,16 +61,14 @@ object VersionTests extends TestSuite { } - 'Delimiters - { + 'delimiters - { assert(compare("1.0", "1-0" ) == 0) assert(compare("1.0", "1_0" ) == 0) assert(compare("1.a", "1a" ) == 0) } - 'LeadingZerosAreSemanticallyIrrelevant - { + 'leadingZerosAreSemanticallyIrrelevant - { assert(compare("1", "01" ) == 0) assert(compare("1.2", "1.002" ) == 0) assert(compare("1.2.3", "1.2.0003" ) == 0) @@ -80,8 +76,7 @@ object VersionTests extends TestSuite { } - 'TrailingZerosAreSemanticallyIrrelevant - { + 'trailingZerosAreSemanticallyIrrelevant - { assert(compare("1", "1.0.0.0.0.0.0.0.0.0.0.0.0.0" ) == 0) assert(compare("1", "1-0-0-0-0-0-0-0-0-0-0-0-0-0" ) == 0) assert(compare("1", "1.0-0.0-0.0-0.0-0.0-0.0-0.0" ) == 0) @@ -90,8 +85,7 @@ object VersionTests extends TestSuite { } - 'TrailingZerosBeforeQualifierAreSemanticallyIrrelevant - { + 'trailingZerosBeforeQualifierAreSemanticallyIrrelevant - { assert(compare("1.0-ga", "1.0.0-ga" ) == 0) assert(compare("1.0.ga", "1.0.0.ga" ) == 0) assert(compare("1.0ga", "1.0.0ga" ) == 0) @@ -109,8 +103,7 @@ object VersionTests extends TestSuite { } - 'TrailingDelimitersAreSemanticallyIrrelevant - { + 'trailingDelimitersAreSemanticallyIrrelevant - { assert(compare("1", "1............." ) == 0) assert(compare("1", "1-------------" ) == 0) assert(compare("1.0", "1............." ) == 0) @@ -118,8 +111,7 @@ object VersionTests extends TestSuite { } - 'InitialDelimiters - { + 'initialDelimiters - { assert(compare("0.1", ".1" ) == 0) assert(compare("0.0.1", "..1" ) == 0) assert(compare("0.1", "-1" ) == 0) @@ -127,8 +119,7 @@ object VersionTests extends TestSuite { } - 'ConsecutiveDelimiters - { + 'consecutiveDelimiters - { assert(compare("1.0.1", "1..1" ) == 0) assert(compare("1.0.0.1", "1...1" ) == 0) assert(compare("1.0.1", "1--1" ) == 0) @@ -136,20 +127,17 @@ object VersionTests extends TestSuite { } - 'UnlimitedNumberOfVersionComponents - { + 'unlimitedNumberOfVersionComponents - { assert(compare("1.0.1.2.3.4.5.6.7.8.9.0.1.2.10", "1.0.1.2.3.4.5.6.7.8.9.0.1.2.3" ) > 0) } - 'UnlimitedNumberOfDigitsInNumericComponent - { + 'unlimitedNumberOfDigitsInNumericComponent - { assert(compare("1.1234567890123456789012345678901", "1.123456789012345678901234567891" ) > 0) } - 'TransitionFromDigitToLetterAndViceVersaIsEqualivantToDelimiter - { + 'transitionFromDigitToLetterAndViceVersaIsEqualivantToDelimiter - { assert(compare("1alpha10", "1.alpha.10" ) == 0) assert(compare("1alpha10", "1-alpha-10" ) == 0) @@ -158,8 +146,7 @@ object VersionTests extends TestSuite { } - 'WellKnownQualifierOrdering - { + 'wellKnownQualifierOrdering - { assert(compare("1-alpha1", "1-a1" ) == 0) assert(compare("1-alpha", "1-beta" ) < 0) assert(compare("1-beta1", "1-b1" ) == 0) @@ -186,8 +173,7 @@ object VersionTests extends TestSuite { } - 'WellKnownQualifierVersusUnknownQualifierOrdering - { + 'wellKnownQualifierVersusUnknownQualifierOrdering - { assert(compare("1-abc", "1-alpha" ) > 0) assert(compare("1-abc", "1-beta" ) > 0) assert(compare("1-abc", "1-milestone" ) > 0) @@ -198,8 +184,7 @@ object VersionTests extends TestSuite { } - 'WellKnownSingleCharQualifiersOnlyRecognizedIfImmediatelyFollowedByNumber - { + 'wellKnownSingleCharQualifiersOnlyRecognizedIfImmediatelyFollowedByNumber - { assert(compare("1.0a", "1.0" ) > 0) assert(compare("1.0-a", "1.0" ) > 0) assert(compare("1.0.a", "1.0" ) > 0) @@ -229,16 +214,14 @@ object VersionTests extends TestSuite { } - 'UnknownQualifierOrdering - { + 'unknownQualifierOrdering - { assert(compare("1-abc", "1-abcd" ) < 0) assert(compare("1-abc", "1-bcd" ) < 0) assert(compare("1-abc", "1-aac" ) > 0) } - 'CaseInsensitiveOrderingOfQualifiers - { + 'caseInsensitiveOrderingOfQualifiers - { assert(compare("1.alpha", "1.ALPHA" ) == 0) assert(compare("1.alpha", "1.Alpha" ) == 0) @@ -269,8 +252,7 @@ object VersionTests extends TestSuite { } - 'QualifierVersusNumberOrdering - { + 'qualifierVersusNumberOrdering - { assert(compare("1-ga", "1-1" ) < 0) assert(compare("1.ga", "1.1" ) < 0) assert(compare("1-ga", "1.0" ) == 0) @@ -291,8 +273,7 @@ object VersionTests extends TestSuite { - 'MinimumSegment - { + 'minimumSegment - { assert(compare("1.min", "1.0-alpha-1" ) < 0) assert(compare("1.min", "1.0-SNAPSHOT" ) < 0) assert(compare("1.min", "1.0" ) < 0) @@ -305,8 +286,7 @@ object VersionTests extends TestSuite { } - 'MaximumSegment - { + 'maximumSegment - { assert(compare("1.max", "1.0-alpha-1" ) > 0) assert(compare("1.max", "1.0-SNAPSHOT" ) > 0) assert(compare("1.max", "1.0" ) > 0) @@ -319,8 +299,7 @@ object VersionTests extends TestSuite { } - 'VersionEvolution - { + 'versionEvolution - { assert(increasing( "0.9.9-SNAPSHOT", "0.9.9", "0.9.10-SNAPSHOT", "0.9.10", "1.0-alpha-2-SNAPSHOT", "1.0-alpha-2", "1.0-alpha-10-SNAPSHOT", "1.0-alpha-10", "1.0-beta-1-SNAPSHOT", "1.0-beta-1", "1.0-rc-1-SNAPSHOT", "1.0-rc-1", "1.0-SNAPSHOT", "1.0", "1.0-sp-1-SNAPSHOT", "1.0-sp-1")) @@ -337,8 +316,7 @@ object VersionTests extends TestSuite { } -// 'CaseInsensitiveOrderingOfQualifiersIsLocaleIndependent -// { +// 'caseInsensitiveOrderingOfQualifiersIsLocaleIndependent - { // import java.util.Locale // val orig = Locale.getDefault // try { From dba6225ac143a57cc1af2ef1120238daf88d944f Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 14:52:19 +0200 Subject: [PATCH 05/10] Add support for build metadata in versions --- .../main/scala/coursier/core/Version.scala | 104 ++++++++++-------- .../scala/coursier/test/VersionTests.scala | 34 ++++++ 2 files changed, 93 insertions(+), 45 deletions(-) diff --git a/core/shared/src/main/scala/coursier/core/Version.scala b/core/shared/src/main/scala/coursier/core/Version.scala index 08b2c3652..de1df7198 100644 --- a/core/shared/src/main/scala/coursier/core/Version.scala +++ b/core/shared/src/main/scala/coursier/core/Version.scala @@ -29,6 +29,10 @@ object Version { case (BigNumber(a), Number(b)) => a.compare(b) case (Qualifier(_, a), Qualifier(_, b)) => a.compare(b) case (Literal(a), Literal(b)) => a.compareToIgnoreCase(b) + case (BuildMetadata(_), BuildMetadata(_)) => + // Semver § 10: two versions that differ only in the build metadata, have the same precedence. + // Might introduce some non-determinism though :-/ + 0 case _ => val rel0 = compareToEmpty @@ -67,6 +71,10 @@ object Version { val order = -1 override def compareToEmpty = if (value.isEmpty) 0 else 1 } + final case class BuildMetadata(value: String) extends Item { + val order = 1 + override def compareToEmpty = if (value.isEmpty) 0 else 1 + } case object Min extends Item { val order = -8 @@ -97,6 +105,7 @@ object Version { case object Dot extends Separator case object Hyphen extends Separator case object Underscore extends Separator + case object Plus extends Separator case object None extends Separator def apply(s: String): (Item, Stream[(Separator, Item)]) = { @@ -135,6 +144,7 @@ object Version { case '.' => (Dot, s.tail) case '-' => (Hyphen, s.tail) case '_' => (Underscore, s.tail) + case '+' => (Plus, s.tail) case _ => (None, s) } } @@ -143,9 +153,13 @@ object Version { if (s.isEmpty) Stream() else { val (sep, rem0) = parseSeparator(s) - val (item, rem) = parseItem(rem0) - - (sep, item) #:: helper(rem) + sep match { + case Plus => + Stream((sep, BuildMetadata(rem0.mkString))) + case _ => + val (item, rem) = parseItem(rem0) + (sep, item) #:: helper(rem) + } } } @@ -154,51 +168,51 @@ object Version { } } + def postProcess(prevIsNumeric: Option[Boolean], item: Item, tokens0: Stream[(Tokenizer.Separator, Item)]): Stream[Item] = { + val tokens = { + var _tokens = tokens0 + + if (isNumeric(item)) { + val nextNonDotZero = _tokens.dropWhile{case (Tokenizer.Dot, n: Numeric) => n.isEmpty; case _ => false } + if (nextNonDotZero.forall(t => t._1 == Tokenizer.Hyphen || ((t._1 == Tokenizer.Dot || t._1 == Tokenizer.None) && !isNumeric(t._2)))) { // Dot && isNumeric(t._2) + _tokens = nextNonDotZero + } + } + + _tokens + } + + def ifFollowedByNumberElse(ifFollowedByNumber: Item, default: Item) = { + val followedByNumber = tokens.headOption + .exists{ case (Tokenizer.None, num: Numeric) if !num.isEmpty => true; case _ => false } + + if (followedByNumber) ifFollowedByNumber + else default + } + + def next = + if (tokens.isEmpty) Stream() + else postProcess(Some(isNumeric(item)), tokens.head._2, tokens.tail) + + item match { + case Literal("min") => Min #:: next + case Literal("max") => Max #:: next + case Literal("a") => + ifFollowedByNumberElse(qualifiersMap("alpha"), item) #:: next + case Literal("b") => + ifFollowedByNumberElse(qualifiersMap("beta"), item) #:: next + case Literal("m") => + ifFollowedByNumberElse(qualifiersMap("milestone"), item) #:: next + case _ => + item #:: next + } + } + + def isNumeric(item: Item) = item match { case _: Numeric => true; case _ => false } + def items(repr: String): List[Item] = { val (first, tokens) = Tokenizer(repr) - def isNumeric(item: Item) = item match { case _: Numeric => true; case _ => false } - - def postProcess(prevIsNumeric: Option[Boolean], item: Item, tokens0: Stream[(Tokenizer.Separator, Item)]): Stream[Item] = { - val tokens = { - var _tokens = tokens0 - - if (isNumeric(item)) { - val nextNonDotZero = _tokens.dropWhile{case (Tokenizer.Dot, n: Numeric) => n.isEmpty; case _ => false } - if (nextNonDotZero.forall(t => t._1 == Tokenizer.Hyphen || ((t._1 == Tokenizer.Dot || t._1 == Tokenizer.None) && !isNumeric(t._2)))) { // Dot && isNumeric(t._2) - _tokens = nextNonDotZero - } - } - - _tokens - } - - def ifFollowedByNumberElse(ifFollowedByNumber: Item, default: Item) = { - val followedByNumber = tokens.headOption - .exists{ case (Tokenizer.None, num: Numeric) if !num.isEmpty => true; case _ => false } - - if (followedByNumber) ifFollowedByNumber - else default - } - - def next = - if (tokens.isEmpty) Stream() - else postProcess(Some(isNumeric(item)), tokens.head._2, tokens.tail) - - item match { - case Literal("min") => Min #:: next - case Literal("max") => Max #:: next - case Literal("a") => - ifFollowedByNumberElse(qualifiersMap("alpha"), item) #:: next - case Literal("b") => - ifFollowedByNumberElse(qualifiersMap("beta"), item) #:: next - case Literal("m") => - ifFollowedByNumberElse(qualifiersMap("milestone"), item) #:: next - case _ => - item #:: next - } - } - postProcess(None, first, tokens).toList } diff --git a/tests/shared/src/test/scala/coursier/test/VersionTests.scala b/tests/shared/src/test/scala/coursier/test/VersionTests.scala index 14016fdae..c94225959 100644 --- a/tests/shared/src/test/scala/coursier/test/VersionTests.scala +++ b/tests/shared/src/test/scala/coursier/test/VersionTests.scala @@ -43,6 +43,40 @@ object VersionTests extends TestSuite { assert(max == v241) } + 'buildMetadata - { + * - { + assert(compare("1.2", "1.2+foo") < 0) + + // Semver § 10: two versions that differ only in the build metadata, have the same precedence + assert(compare("1.2+bar", "1.2+foo") == 0) + assert(compare("1.2+bar.1", "1.2+bar.2") == 0) + } + + 'shouldNotParseMetadata - { + * - { + val items = Version("1.2+bar.2").items + val expectedItems = Seq( + Version.Number(1), Version.Number(2), Version.BuildMetadata("bar.2") + ) + assert(items == expectedItems) + } + * - { + val items = Version("1.2+bar-2").items + val expectedItems = Seq( + Version.Number(1), Version.Number(2), Version.BuildMetadata("bar-2") + ) + assert(items == expectedItems) + } + * - { + val items = Version("1.2+bar+foo").items + val expectedItems = Seq( + Version.Number(1), Version.Number(2), Version.BuildMetadata("bar+foo") + ) + assert(items == expectedItems) + } + } + } + // Adapted from aether-core/aether-util/src/test/java/org/eclipse/aether/util/version/GenericVersionTest.java // Only one test doesn't pass (see FIXME below) From f7020fd0aded550031c80e674f9ff373c3a47901 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 15:11:39 +0200 Subject: [PATCH 06/10] Only warn on missing credential file --- .../src/main/scala-2.10/coursier/Tasks.scala | 24 ++++++++++++++----- .../missing-credentials/build.sbt | 2 ++ .../missing-credentials/project/plugins.sbt | 11 +++++++++ .../src/main/scala/Main.scala | 1 + .../sbt-coursier/missing-credentials/test | 1 + 5 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/build.sbt create mode 100644 sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/project/plugins.sbt create mode 100644 sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/src/main/scala/Main.scala create mode 100644 sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/test diff --git a/sbt-coursier/src/main/scala-2.10/coursier/Tasks.scala b/sbt-coursier/src/main/scala-2.10/coursier/Tasks.scala index 8c6d7ea0c..62a5d538b 100644 --- a/sbt-coursier/src/main/scala-2.10/coursier/Tasks.scala +++ b/sbt-coursier/src/main/scala-2.10/coursier/Tasks.scala @@ -457,12 +457,24 @@ object Tasks { val useSbtCredentials = coursierUseSbtCredentials.value val authenticationByHost = - if (useSbtCredentials) { - val cred = sbt.Keys.credentials.value.map(sbt.Credentials.toDirect) - cred.map { c => - c.host -> Authentication(c.userName, c.passwd) - }.toMap - } else + if (useSbtCredentials) + sbt.Keys.credentials + .value + .flatMap { + case dc: sbt.DirectCredentials => List(dc) + case fc: sbt.FileCredentials => + sbt.Credentials.loadCredentials(fc.path) match { + case Left(err) => + log.warn(s"$err, ignoring it") + Nil + case Right(dc) => List(dc) + } + } + .map { c => + c.host -> Authentication(c.userName, c.passwd) + } + .toMap + else Map.empty[String, Authentication] val authenticationByRepositoryId = coursierCredentials.value.mapValues(_.authentication) diff --git a/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/build.sbt b/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/build.sbt new file mode 100644 index 000000000..05705fe6c --- /dev/null +++ b/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/build.sbt @@ -0,0 +1,2 @@ +scalaVersion := "2.11.8" +credentials += Credentials(file("nope/nope/nope/nope/nope")) diff --git a/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/project/plugins.sbt b/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/project/plugins.sbt new file mode 100644 index 000000000..152225a9e --- /dev/null +++ b/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/project/plugins.sbt @@ -0,0 +1,11 @@ +{ + val pluginVersion = sys.props.getOrElse( + "plugin.version", + throw new RuntimeException( + """|The system property 'plugin.version' is not defined. + |Specify this property using the scriptedLaunchOpts -D.""".stripMargin + ) + ) + + addSbtPlugin("io.get-coursier" % "sbt-coursier" % pluginVersion) +} diff --git a/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/src/main/scala/Main.scala b/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/src/main/scala/Main.scala new file mode 100644 index 000000000..86ae9e9e3 --- /dev/null +++ b/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/src/main/scala/Main.scala @@ -0,0 +1 @@ +object Main extends App \ No newline at end of file diff --git a/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/test b/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/test new file mode 100644 index 000000000..103bd8d2f --- /dev/null +++ b/sbt-coursier/src/sbt-test/sbt-coursier/missing-credentials/test @@ -0,0 +1 @@ +> update From 869d7c628edcd9315a43f75d87614198146ce6bb Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 15:21:24 +0200 Subject: [PATCH 07/10] Retry downloads if an SSLException is caught --- cache/src/main/scala/coursier/Cache.scala | 90 ++++++++++++++++------- 1 file changed, 62 insertions(+), 28 deletions(-) diff --git a/cache/src/main/scala/coursier/Cache.scala b/cache/src/main/scala/coursier/Cache.scala index 20832aafc..3437bee47 100644 --- a/cache/src/main/scala/coursier/Cache.scala +++ b/cache/src/main/scala/coursier/Cache.scala @@ -189,44 +189,78 @@ object Cache { } finally if (out != null) out.close() } + + private def defaultRetryCount = 3 + + private lazy val retryCount = + sys.props + .get("coursier.sslexception-retry") + .flatMap(s => scala.util.Try(s.toInt).toOption) + .filter(_ >= 0) + .getOrElse(defaultRetryCount) + private def downloading[T]( url: String, file: File, - logger: Option[Logger] + logger: Option[Logger], + retry: Int = retryCount )( f: => FileError \/ T - ): FileError \/ T = - try { - val o = new Object - val prev = urlLocks.putIfAbsent(url, o) - if (prev == null) { - logger.foreach(_.downloadingArtifact(url, file)) + ): FileError \/ T = { - val res = - try \/-(f) - catch { - case nfe: FileNotFoundException if nfe.getMessage != null => - logger.foreach(_.downloadedArtifact(url, success = false)) - -\/(-\/(FileError.NotFound(nfe.getMessage))) - case e: Exception => - logger.foreach(_.downloadedArtifact(url, success = false)) - throw e - } - finally { - urlLocks.remove(url) - } + @tailrec + def helper(retry: Int): FileError \/ T = { - for (res0 <- res) - logger.foreach(_.downloadedArtifact(url, success = res0.isRight)) + val resOpt = + try { + val o = new Object + val prev = urlLocks.putIfAbsent(url, o) - res.merge - } else - -\/(FileError.ConcurrentDownload(url)) - } - catch { case e: Exception => - -\/(FileError.DownloadError(s"Caught $e${Option(e.getMessage).fold("")(" (" + _ + ")")}")) + val res = + if (prev == null) { + logger.foreach(_.downloadingArtifact(url, file)) + + val res = + try \/-(f) + catch { + case nfe: FileNotFoundException if nfe.getMessage != null => + logger.foreach(_.downloadedArtifact(url, success = false)) + -\/(-\/(FileError.NotFound(nfe.getMessage))) + case e: Exception => + logger.foreach(_.downloadedArtifact(url, success = false)) + throw e + } + finally { + urlLocks.remove(url) + } + + for (res0 <- res) + logger.foreach(_.downloadedArtifact(url, success = res0.isRight)) + + res.merge[FileError \/ T] + } else + -\/(FileError.ConcurrentDownload(url)) + + Some(res) + } + catch { + case _: javax.net.ssl.SSLException if retry >= 1 => + // TODO If Cache is made an (instantiated) class at some point, allow to log that exception. + None + case NonFatal(e) => + Some(-\/(FileError.DownloadError(s"Caught $e${Option(e.getMessage).fold("")(" (" + _ + ")")}"))) + } + + resOpt match { + case Some(res) => res + case None => + helper(retry - 1) + } } + helper(retry) + } + private def temporaryFile(file: File): File = { val dir = file.getParentFile val name = file.getName From ad80e1482cca487360a078f14f142ee407e01ee9 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 15:56:59 +0200 Subject: [PATCH 08/10] Accept "groupId", "artifactId", "version" properties --- core/shared/src/main/scala/coursier/core/Resolution.scala | 6 +++++- .../org.apache.directory.shared/shared-ldap/0.9.19 | 8 ++++++++ .../src/test/scala/coursier/test/CentralTests.scala | 7 +++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 tests/shared/src/test/resources/resolutions/org.apache.directory.shared/shared-ldap/0.9.19 diff --git a/core/shared/src/main/scala/coursier/core/Resolution.scala b/core/shared/src/main/scala/coursier/core/Resolution.scala index cc70aa89c..3b0d4129d 100644 --- a/core/shared/src/main/scala/coursier/core/Resolution.scala +++ b/core/shared/src/main/scala/coursier/core/Resolution.scala @@ -376,7 +376,11 @@ object Resolution { // although I can find no mention of them in any manual / spec "pom.groupId" -> project.module.organization, "pom.artifactId" -> project.module.name, - "pom.version" -> project.version + "pom.version" -> project.version, + // Required by some dependencies too (org.apache.directory.shared:shared-ldap:0.9.19 in particular) + "groupId" -> project.module.organization, + "artifactId" -> project.module.name, + "version" -> project.version ) ++ project.properties ++ Seq( "project.groupId" -> project.module.organization, "project.artifactId" -> project.module.name, diff --git a/tests/shared/src/test/resources/resolutions/org.apache.directory.shared/shared-ldap/0.9.19 b/tests/shared/src/test/resources/resolutions/org.apache.directory.shared/shared-ldap/0.9.19 new file mode 100644 index 000000000..e39337d42 --- /dev/null +++ b/tests/shared/src/test/resources/resolutions/org.apache.directory.shared/shared-ldap/0.9.19 @@ -0,0 +1,8 @@ +antlr:antlr:2.7.7:compile +commons-collections:commons-collections:3.2.1:compile +commons-lang:commons-lang:2.4:compile +org.apache.directory.shared:shared-asn1:0.9.19:compile +org.apache.directory.shared:shared-i18n:0.9.19:compile +org.apache.directory.shared:shared-ldap:0.9.19:compile +org.apache.directory.shared:shared-ldap-constants:0.9.19:compile +org.slf4j:slf4j-api:1.5.10:compile diff --git a/tests/shared/src/test/scala/coursier/test/CentralTests.scala b/tests/shared/src/test/scala/coursier/test/CentralTests.scala index 74edbdd48..ef222e65a 100644 --- a/tests/shared/src/test/scala/coursier/test/CentralTests.scala +++ b/tests/shared/src/test/scala/coursier/test/CentralTests.scala @@ -579,6 +579,13 @@ object CentralTests extends TestSuite { assert(urls == expectedZipArtifactUrls) } } + + 'groupIdVersionProperties - { + resolutionCheck( + Module("org.apache.directory.shared", "shared-ldap"), + "0.9.19" + ) + } } } From 13da5e871f28aaa380d63f0268e9b2585b624ba7 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 16:26:49 +0200 Subject: [PATCH 09/10] Don't leave POMs as artifacts in update reports sbt-pack isn't too fine with those in particular --- .../src/main/scala-2.10/coursier/ToSbt.scala | 18 +++++++++--- .../sbt-coursier/no-pom-artifact/build.sbt | 28 +++++++++++++++++++ .../no-pom-artifact/project/plugins.sbt | 11 ++++++++ .../sbt-coursier/no-pom-artifact/test | 1 + 4 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/build.sbt create mode 100644 sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/project/plugins.sbt create mode 100644 sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/test diff --git a/sbt-coursier/src/main/scala-2.10/coursier/ToSbt.scala b/sbt-coursier/src/main/scala-2.10/coursier/ToSbt.scala index 5e3e5e33e..cd7aa7bcc 100644 --- a/sbt-coursier/src/main/scala-2.10/coursier/ToSbt.scala +++ b/sbt-coursier/src/main/scala-2.10/coursier/ToSbt.scala @@ -96,14 +96,23 @@ object ToSbt { def moduleReports( res: Resolution, classifiersOpt: Option[Seq[String]], - artifactFileOpt: (Module, String, Artifact) => Option[File] + artifactFileOpt: (Module, String, Artifact) => Option[File], + keepPomArtifact: Boolean = false ) = { - val depArtifacts = + val depArtifacts0 = classifiersOpt match { case None => res.dependencyArtifacts case Some(cl) => res.dependencyClassifiersArtifacts(cl) } + val depArtifacts = + if (keepPomArtifact) + depArtifacts0 + else + depArtifacts0.filter { + case (_, a) => a.attributes != Attributes("pom", "") + } + val groupedDepArtifacts = grouped(depArtifacts) val versions = res.dependencies.toVector.map { dep => @@ -155,7 +164,8 @@ object ToSbt { resolution: Resolution, configs: Map[String, Set[String]], classifiersOpt: Option[Seq[String]], - artifactFileOpt: (Module, String, Artifact) => Option[File] + artifactFileOpt: (Module, String, Artifact) => Option[File], + keepPomArtifact: Boolean = false ): sbt.UpdateReport = { val configReports = configs.map { @@ -163,7 +173,7 @@ object ToSbt { val configDeps = extends0.flatMap(configDependencies.getOrElse(_, Nil)) val subRes = resolution.subset(configDeps) - val reports = ToSbt.moduleReports(subRes, classifiersOpt, artifactFileOpt) + val reports = ToSbt.moduleReports(subRes, classifiersOpt, artifactFileOpt, keepPomArtifact) new ConfigurationReport( config, diff --git a/sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/build.sbt b/sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/build.sbt new file mode 100644 index 000000000..372da643b --- /dev/null +++ b/sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/build.sbt @@ -0,0 +1,28 @@ + +lazy val noPomCheck = TaskKey[Unit]("noPomCheck") + +noPomCheck := { + + val configReport = update.value + .configuration("compile") + .getOrElse { + throw new Exception( + "compile configuration not found in update report" + ) + } + + val artifacts = configReport + .modules + .flatMap(_.artifacts) + .map(_._1) + + val pomArtifacts = artifacts + .filter { a => + a.`type` == "pom" && a.classifier.isEmpty + } + + for (a <- pomArtifacts) + streams.value.log.error(s"Found POM artifact $a") + + assert(pomArtifacts.isEmpty) +} diff --git a/sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/project/plugins.sbt b/sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/project/plugins.sbt new file mode 100644 index 000000000..8d902e4dc --- /dev/null +++ b/sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/project/plugins.sbt @@ -0,0 +1,11 @@ +{ + val pluginVersion = sys.props.getOrElse( + "plugin.version", + throw new RuntimeException( + """|The system property 'plugin.version' is not defined. + |Specify this property using the scriptedLaunchOpts -D.""".stripMargin + ) + ) + + addSbtPlugin("io.get-coursier" % "sbt-coursier" % pluginVersion) +} \ No newline at end of file diff --git a/sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/test b/sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/test new file mode 100644 index 000000000..67b4dc666 --- /dev/null +++ b/sbt-coursier/src/sbt-test/sbt-coursier/no-pom-artifact/test @@ -0,0 +1 @@ +> noPomCheck From ec0ed108e1c657c2dc61c3947d43a83c437e65e1 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 21 Apr 2017 16:46:50 +0200 Subject: [PATCH 10/10] Add support for relocation --- .../coursier/maven/MavenRepository.scala | 2 +- .../src/main/scala/coursier/maven/Pom.scala | 43 +++++++++++++++++-- .../resolutions/bouncycastle/bctsp-jdk14/138 | 4 ++ .../scala/coursier/test/CentralTests.scala | 7 +++ 4 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 tests/shared/src/test/resources/resolutions/bouncycastle/bctsp-jdk14/138 diff --git a/core/shared/src/main/scala/coursier/maven/MavenRepository.scala b/core/shared/src/main/scala/coursier/maven/MavenRepository.scala index f19e05a22..d20fdb7c4 100644 --- a/core/shared/src/main/scala/coursier/maven/MavenRepository.scala +++ b/core/shared/src/main/scala/coursier/maven/MavenRepository.scala @@ -249,7 +249,7 @@ final case class MavenRepository( for { xml <- \/.fromEither(compatibility.xmlParse(str)) _ <- if (xml.label == "project") \/-(()) else -\/("Project definition not found") - proj <- Pom.project(xml) + proj <- Pom.project(xml, relocationAsDependency = true) } yield proj def artifactFor(url: String) = diff --git a/core/shared/src/main/scala/coursier/maven/Pom.scala b/core/shared/src/main/scala/coursier/maven/Pom.scala index 2d1520347..9b0f43ca3 100644 --- a/core/shared/src/main/scala/coursier/maven/Pom.scala +++ b/core/shared/src/main/scala/coursier/maven/Pom.scala @@ -140,7 +140,13 @@ object Pom { def packagingOpt(pom: Node): Option[String] = text(pom, "packaging", "").toOption - def project(pom: Node): String \/ Project = { + def project(pom: Node): String \/ Project = + project(pom, relocationAsDependency = false) + + def project( + pom: Node, + relocationAsDependency: Boolean + ): String \/ Project = { import Scalaz._ for { @@ -242,10 +248,41 @@ object Pom { case \/-(d) => d } + val finalProjModule = projModule.copy(organization = groupId) + + val relocationDependencyOpt = + if (relocationAsDependency) + pom.children + .find(_.label == "distributionManagement") + .flatMap(_.children.find(_.label == "relocation")) + .map { n => + + // see https://maven.apache.org/guides/mini/guide-relocation.html + + val relocatedGroupId = text(n, "groupId", "").getOrElse(finalProjModule.organization) + val relocatedArtifactId = text(n, "artifactId", "").getOrElse(finalProjModule.name) + val relocatedVersion = text(n, "version", "").getOrElse(version) + + "" -> Dependency( + finalProjModule.copy( + organization = relocatedGroupId, + name = relocatedArtifactId + ), + relocatedVersion, + "", + Set(), + Attributes("", ""), + optional = false, + transitive = true + ) + } + else + None + Project( - projModule.copy(organization = groupId), + finalProjModule, version, - deps.map { + (relocationDependencyOpt.toList ::: deps).map { case (config, dep0) => val dep = extraAttrsMap.get(dep0.moduleVersion).fold(dep0)(attrs => dep0.copy(module = dep0.module.copy(attributes = attrs)) diff --git a/tests/shared/src/test/resources/resolutions/bouncycastle/bctsp-jdk14/138 b/tests/shared/src/test/resources/resolutions/bouncycastle/bctsp-jdk14/138 new file mode 100644 index 000000000..710c52165 --- /dev/null +++ b/tests/shared/src/test/resources/resolutions/bouncycastle/bctsp-jdk14/138 @@ -0,0 +1,4 @@ +bouncycastle:bctsp-jdk14:138:compile +org.bouncycastle:bcmail-jdk14:1.38:compile +org.bouncycastle:bcprov-jdk14:1.38:compile +org.bouncycastle:bctsp-jdk14:1.38:compile diff --git a/tests/shared/src/test/scala/coursier/test/CentralTests.scala b/tests/shared/src/test/scala/coursier/test/CentralTests.scala index ef222e65a..8dceeb69d 100644 --- a/tests/shared/src/test/scala/coursier/test/CentralTests.scala +++ b/tests/shared/src/test/scala/coursier/test/CentralTests.scala @@ -586,6 +586,13 @@ object CentralTests extends TestSuite { "0.9.19" ) } + + 'relocation - { + resolutionCheck( + Module("bouncycastle", "bctsp-jdk14"), + "138" + ) + } } }