From 3e4a65d5ee66f043c2467972bd6e29f48b570715 Mon Sep 17 00:00:00 2001 From: Dorothy Ordogh Date: Thu, 22 Feb 2018 09:06:06 -0800 Subject: [PATCH] Add ability to fetch artifact with a given url (#774) * changes * changes to Attributes * make changes * add test and fix bug * add more tests * fix failing tests * fix parentheses * remove comments I added and add test that's guarenteed not to exist in a repo * remove Attributes third parameter and refactor Parse's moduleVersionConfig to return a tuple of Dependency and a Map of String to String representing extra parameters for the dependency * update some return types and methods in the Helper class * return FallbackDependenciesRepository back to original state * refactor helper * remove url from attributes * fix tests and add cache to extra in build.sbt * remove FallbackDepsRepo from sbt-coursier * add variable for url and call encode in tests * update tests and helper to have proper behavior * remove setting deps to intransitive when url is present and add two tests * add more tests, implement @wisechengyi's comments * fix nits * update ParseTests because some failed * fix tests * incorporate feedback from @alexarchambault and @wisechengyi * update ParseTests to check for returned error vs thrown error * remove one test that is covered in ParseTests * fix nits * add back deleted brackets * return errors in Left without using return statement * revert change because it's broken * fix some positional things * add return statement or else error won't be processed * fix nits. thanks @wisechengyi * Remove return statements --- build.sbt | 2 +- cli/src/main/scala-2.12/BUILD | 1 + .../main/scala-2.12/coursier/cli/Helper.scala | 45 ++- .../cli/CliFetchIntegrationTest.scala | 329 +++++++++++++++++- .../scala-2.12/coursier/cli/CliTestLib.scala | 6 +- .../scala/coursier/ivy/IvyRepository.scala | 4 +- .../src/main/scala/coursier/util/Parse.scala | 239 ++++++++----- extra/src/main/scala/coursier/BUILD | 8 + .../FallbackDependenciesRepository.scala | 0 .../test/scala/coursier/test/ParseTests.scala | 60 ++-- 10 files changed, 570 insertions(+), 124 deletions(-) create mode 100644 extra/src/main/scala/coursier/BUILD rename {sbt-coursier => extra}/src/main/scala/coursier/FallbackDependenciesRepository.scala (100%) diff --git a/build.sbt b/build.sbt index 23e8cb584..140df21b1 100644 --- a/build.sbt +++ b/build.sbt @@ -118,7 +118,7 @@ lazy val bootstrap = project lazy val extra = project .disablePlugins(ScriptedPlugin) .enablePlugins(ShadingPlugin) - .dependsOn(coreJvm) + .dependsOn(coreJvm, cache) .settings( shared, coursierPrefix, diff --git a/cli/src/main/scala-2.12/BUILD b/cli/src/main/scala-2.12/BUILD index ebaa8f3d6..28b891725 100644 --- a/cli/src/main/scala-2.12/BUILD +++ b/cli/src/main/scala-2.12/BUILD @@ -5,6 +5,7 @@ scala_library( "3rdparty/jvm:caseapp", "cache/src/main/scala:cache", "core:core", + "extra/src/main/scala/coursier:fallback-deps-repo", "extra/src/main/scala/coursier/extra:extra", "extra/src/main/scala-2.12/coursier/extra:native", ":util", diff --git a/cli/src/main/scala-2.12/coursier/cli/Helper.scala b/cli/src/main/scala-2.12/coursier/cli/Helper.scala index 84715af3c..4fc2dc97b 100644 --- a/cli/src/main/scala-2.12/coursier/cli/Helper.scala +++ b/cli/src/main/scala-2.12/coursier/cli/Helper.scala @@ -2,7 +2,7 @@ package coursier package cli import java.io.{File, OutputStreamWriter, PrintWriter} -import java.net.{URL, URLClassLoader} +import java.net.{URL, URLClassLoader, URLDecoder} import java.util.concurrent.Executors import java.util.jar.{Manifest => JManifest} @@ -116,7 +116,7 @@ class Helper( repos } - val repositories = repositoriesValidation match { + val standardRepositories = repositoriesValidation match { case Success(repos) => repos case Failure(errors) => @@ -125,14 +125,13 @@ class Helper( ) } - val loggerFallbackMode = !progress && TermDisplay.defaultFallbackMode val (scaladexRawDependencies, otherRawDependencies) = rawDependencies.partition(s => s.contains("/") || !s.contains(":")) - val scaladexDeps: List[Dependency] = + val scaladexDepsWithExtraParams: List[(Dependency, Map[String, String])] = if (scaladexRawDependencies.isEmpty) Nil else { @@ -192,7 +191,7 @@ class Helper( res .collect { case \/-(l) => l } .flatten - .map { case (mod, ver) => Dependency(mod, ver) } + .map { case (mod, ver) => (Dependency(mod, ver), Map[String, String]()) } } val (forceVersionErrors, forceVersions0) = Parse.moduleVersions(forceVersion, scalaVersion) @@ -258,10 +257,10 @@ class Helper( val moduleReq = ModuleRequirements(globalExcludes, localExcludeMap, defaultConfiguration) - val (modVerCfgErrors: Seq[String], normalDeps: Seq[Dependency]) = + val (modVerCfgErrors: Seq[String], normalDepsWithExtraParams: Seq[(Dependency, Map[String, String])]) = Parse.moduleVersionConfigs(otherRawDependencies, moduleReq, transitive=true, scalaVersion) - val (intransitiveModVerCfgErrors: Seq[String], intransitiveDeps: Seq[Dependency]) = + val (intransitiveModVerCfgErrors: Seq[String], intransitiveDepsWithExtraParams: Seq[(Dependency, Map[String, String])]) = Parse.moduleVersionConfigs(intransitive, moduleReq, transitive=false, scalaVersion) prematureExitIf(modVerCfgErrors.nonEmpty) { @@ -273,11 +272,37 @@ class Helper( intransitiveModVerCfgErrors.map(" "+_).mkString("\n") } - val transitiveDeps: Seq[Dependency] = + val transitiveDepsWithExtraParams: Seq[(Dependency, Map[String, String])] = // FIXME Order of the dependencies is not respected here (scaladex ones go first) - scaladexDeps ++ normalDeps + scaladexDepsWithExtraParams ++ normalDepsWithExtraParams - val allDependencies: Seq[Dependency] = transitiveDeps ++ intransitiveDeps + val transitiveDeps: Seq[Dependency] = transitiveDepsWithExtraParams.map(dep => dep._1) + + val allDependenciesWithExtraParams: Seq[(Dependency, Map[String, String])] = + transitiveDepsWithExtraParams ++ intransitiveDepsWithExtraParams + + val allDependencies: Seq[Dependency] = allDependenciesWithExtraParams.map(dep => dep._1) + + // Any dependencies with URIs should not be resolved with a pom so this is a + // hack to add all the deps with URIs to the FallbackDependenciesRepository + // which will be used during the resolve + val depsWithUrls: Map[(Module, String), (URL, Boolean)] = allDependenciesWithExtraParams + .flatMap { + case (dep, extraParams) => + extraParams.get("url").map { url => + dep.moduleVersion -> (new URL(URLDecoder.decode(url, "UTF-8")), true) + } + }.toMap + + val depsWithUrlRepo: FallbackDependenciesRepository = FallbackDependenciesRepository(depsWithUrls) + + // Prepend FallbackDependenciesRepository to the repository list + // so that dependencies with URIs are resolved against this repo + val repositories: Seq[Repository] = Seq(depsWithUrlRepo) ++ standardRepositories + + for (((mod, version), _) <- depsWithUrls if forceVersions.get(mod).exists(_ != version)) + throw new Exception(s"Cannot force a version that is different from the one specified " + + s"for the module ${mod}:${version} with url") val checksums = { val splitChecksumArgs = checksum.flatMap(_.split(',')).filter(_.nonEmpty) diff --git a/cli/src/test/scala-2.12/coursier/cli/CliFetchIntegrationTest.scala b/cli/src/test/scala-2.12/coursier/cli/CliFetchIntegrationTest.scala index 04e18191d..1eac9d23d 100644 --- a/cli/src/test/scala-2.12/coursier/cli/CliFetchIntegrationTest.scala +++ b/cli/src/test/scala-2.12/coursier/cli/CliFetchIntegrationTest.scala @@ -2,7 +2,7 @@ package coursier.cli import java.io._ import java.util.zip.ZipInputStream - +import java.net.URLEncoder.encode import argonaut.Argonaut._ import coursier.cli.util.{DepNode, ReportNode} import caseapp.core.RemainingArgs @@ -10,6 +10,7 @@ import coursier.cli.options._ import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner +import scala.io.Source @RunWith(classOf[JUnitRunner]) class CliFetchIntegrationTest extends FlatSpec with CliTestLib { @@ -380,4 +381,330 @@ class CliFetchIntegrationTest extends FlatSpec with CliTestLib { assert(!node.dependencies.exists(_.coord.startsWith("org.scala-lang:scala-library:2.11."))) } + /** + * Result: + * |└─ org.apache.commons:commons-compress:1.5 + */ + "local dep url" should "have coursier-fetch-test.jar" in withFile() { + (jsonFile, _) => { + withFile("tada", "coursier-fetch-test", ".jar") { + (testFile, _) => { + val path = testFile.getAbsolutePath + val encodedUrl = encode("file://" + path, "UTF-8") + + + val commonOpt = CommonOptions(jsonOutputFile = jsonFile.getPath) + val fetchOpt = FetchOptions(common = commonOpt) + + // fetch with encoded url set to temp jar + Fetch.run( + fetchOpt, + RemainingArgs( + Seq( + "org.apache.commons:commons-compress:1.5,url=" + encodedUrl + ), + Seq() + ) + ) + + val node: ReportNode = getReportFromJson(jsonFile) + + val depNodes: Seq[DepNode] = node.dependencies + .filter(_.coord == "org.apache.commons:commons-compress:1.5") + .sortBy(_.files.head._1.length) + assert(depNodes.length == 1) + + val urlInJsonFile = depNodes.head.files.head._2 + assert(depNodes.head.files.head._1 == "") + assert(urlInJsonFile.contains(path)) + + // open jar and inspect contents + val fileContents = Source.fromFile(urlInJsonFile).getLines.mkString + assert(fileContents == "tada") + } + } + } + } + + /** + * Result: + * |└─ org.apache.commons:commons-compress:1.5 + */ + "external dep url" should "fetch junit-4.12.jar" in withFile() { + (jsonFile, _) => { + val commonOpt = CommonOptions(jsonOutputFile = jsonFile.getPath) + val fetchOpt = FetchOptions(common = commonOpt) + + // encode path to different jar than requested + val externalUrl = encode("http://central.maven.org/maven2/junit/junit/4.12/junit-4.12.jar", "UTF-8") + + // fetch with different artifact url + Fetch.run( + fetchOpt, + RemainingArgs( + Seq( + "org.apache.commons:commons-compress:1.5,url=" + externalUrl + ), + Seq() + ) + ) + + val node: ReportNode = getReportFromJson(jsonFile) + + val depNodes: Seq[DepNode] = node.dependencies + .filter(_.coord == "org.apache.commons:commons-compress:1.5") + .sortBy(_.files.head._1.length) + assert(depNodes.length == 1) + assert(depNodes.head.files.head._1 == "") + assert(depNodes.head.files.head._2.contains("junit/junit/4.12/junit-4.12.jar")) + } + } + + /** + * Result: + * |└─ a:b:c + */ + "external dep url with arbitrary coords" should "fetch junit-4.12.jar" in withFile() { + (jsonFile, _) => { + val commonOpt = CommonOptions(jsonOutputFile = jsonFile.getPath) + val fetchOpt = FetchOptions(common = commonOpt) + + // encode path to different jar than requested + val externalUrl = encode("http://central.maven.org/maven2/junit/junit/4.12/junit-4.12.jar", "UTF-8") + + // arbitrary coords fail to fetch because... coords need to exist in a repo somewhere to work. fix this. + Fetch.run( + fetchOpt, + RemainingArgs( + Seq( + "a:b:c,url=" + externalUrl + ), + Seq() + ) + ) + + val node: ReportNode = getReportFromJson(jsonFile) + + val depNodes: Seq[DepNode] = node.dependencies + .filter(_.coord == "a:b:c") + .sortBy(_.files.head._1.length) + assert(depNodes.length == 1) + assert(depNodes.head.files.head._1 == "") + assert(depNodes.head.files.head._2.contains("junit/junit/4.12/junit-4.12.jar")) + } + } + + /** + * Result: + * |└─ org.apache.commons:commons-compress:1.5 + */ + "external dep url with classifier" should "fetch junit-4.12.jar and classifier gets thrown away" in withFile() { + (jsonFile, _) => { + val commonOpt = CommonOptions(jsonOutputFile = jsonFile.getPath) + val fetchOpt = FetchOptions(common = commonOpt) + + // encode path to different jar than requested + val externalUrl = encode("http://central.maven.org/maven2/junit/junit/4.12/junit-4.12.jar", "UTF-8") + + Fetch.run( + fetchOpt, + RemainingArgs( + Seq( + "org.apache.commons:commons-compress:1.5,url=" + externalUrl + ",classifier=tests" + ), + Seq() + ) + ) + + val node: ReportNode = getReportFromJson(jsonFile) + + val depNodes: Seq[DepNode] = node.dependencies + .filter(_.coord == "org.apache.commons:commons-compress:1.5") + .sortBy(_.files.head._1.length) + assert(depNodes.length == 1) + // classifier doesn't matter when we have a url so it is not listed + assert(depNodes.head.files.head._1 == "") + assert(depNodes.head.files.head._2.contains("junit/junit/4.12/junit-4.12.jar")) + } + } + + /** + * Result: + * |└─ org.apache.commons:commons-compress:1.5 + * |└─ org.codehaus.jackson:jackson-mapper-asl:1.8.8 + * | └─ org.codehaus.jackson:jackson-core-asl:1.8.8 + */ + "external dep url with another dep" should "fetch junit-4.12.jar and jars for jackson-mapper" in withFile() { + (jsonFile, _) => { + val commonOpt = CommonOptions(jsonOutputFile = jsonFile.getPath) + val fetchOpt = FetchOptions(common = commonOpt) + + val externalUrl = encode("http://central.maven.org/maven2/junit/junit/4.12/junit-4.12.jar", "UTF-8") + + Fetch.run( + fetchOpt, + RemainingArgs( + Seq( + "org.apache.commons:commons-compress:1.5,url=" + externalUrl, + "org.codehaus.jackson:jackson-mapper-asl:1.8.8" + ), + Seq() + ) + ) + + val node: ReportNode = getReportFromJson(jsonFile) + + val depNodes: Seq[DepNode] = node.dependencies + assert(depNodes.length == 3) + + val compressNodes = depNodes + .filter(_.coord == "org.apache.commons:commons-compress:1.5") + .sortBy(_.files.head._1.length) + assert(compressNodes.length == 1) + assert(compressNodes.head.files.head._1 == "") + assert(compressNodes.head.files.head._2.contains("junit/junit/4.12/junit-4.12.jar")) + + val jacksonMapperNodes = depNodes + .filter(_.coord == "org.codehaus.jackson:jackson-mapper-asl:1.8.8") + .sortBy(_.files.head._1.length) + assert(jacksonMapperNodes.length == 1) + assert(jacksonMapperNodes.head.files.head._2.contains("org/codehaus/jackson/jackson-mapper-asl/1.8.8/jackson-mapper-asl-1.8.8.jar")) + assert(jacksonMapperNodes.head.dependencies.size == 1) + assert(jacksonMapperNodes.head.dependencies.head == "org.codehaus.jackson:jackson-core-asl:1.8.8") + + val jacksonCoreNodes = depNodes + .filter(_.coord == "org.codehaus.jackson:jackson-core-asl:1.8.8") + .sortBy(_.files.head._1.length) + assert(jacksonCoreNodes.length == 1) + assert(jacksonCoreNodes.head.files.head._2.contains("org/codehaus/jackson/jackson-core-asl/1.8.8/jackson-core-asl-1.8.8.jar")) + } + } + + /** + * Result: + * Error + */ + "external dep url with forced version" should "throw an error" in withFile() { + (jsonFile, _) => { + val commonOpt = CommonOptions( + jsonOutputFile = jsonFile.getPath, + forceVersion = List("org.apache.commons:commons-compress:1.4.1")) + val fetchOpt = FetchOptions(common = commonOpt) + + val externalUrl = encode("http://central.maven.org/maven2/junit/junit/4.12/junit-4.12.jar", "UTF-8") + + assertThrows[Exception]({ + Fetch.run( + fetchOpt, + RemainingArgs( + Seq( + "org.apache.commons:commons-compress:1.5,url=" + externalUrl + ), + Seq() + ) + ) + }) + } + } + + /** + * Result: + * |└─ org.apache.commons:commons-compress:1.5 + */ + "external dep url with the same forced version" should "fetch junit-4.12.jar" in withFile() { + (jsonFile, _) => { + val commonOpt = CommonOptions( + jsonOutputFile = jsonFile.getPath, + forceVersion = List("org.apache.commons:commons-compress:1.5")) + val fetchOpt = FetchOptions(common = commonOpt) + + val externalUrl = encode("http://central.maven.org/maven2/junit/junit/4.12/junit-4.12.jar", "UTF-8") + + Fetch.run( + fetchOpt, + RemainingArgs( + Seq( + "org.apache.commons:commons-compress:1.5,url=" + externalUrl + ), + Seq() + ) + ) + + val node: ReportNode = getReportFromJson(jsonFile) + + val depNodes: Seq[DepNode] = node.dependencies + assert(depNodes.length == 1) + assert(depNodes.head.files.head._1 == "") + assert(depNodes.head.files.head._2.contains("junit/junit/4.12/junit-4.12.jar")) + } + } + + /** + * Result: + * |└─ org.apache.commons:commons-compress:1.4.1 -> 1.5 + */ + "external dep url on higher version" should "fetch junit-4.12.jar" in withFile() { + (jsonFile, _) => { + val commonOpt = CommonOptions(jsonOutputFile = jsonFile.getPath) + val fetchOpt = FetchOptions(common = commonOpt) + + // encode path to different jar than requested + val externalUrl = encode("http://central.maven.org/maven2/junit/junit/4.12/junit-4.12.jar", "UTF-8") + + Fetch.run( + fetchOpt, + RemainingArgs( + Seq( + "org.apache.commons:commons-compress:1.4.1", + "org.apache.commons:commons-compress:1.5,url=" + externalUrl + ), + Seq() + ) + ) + + val node: ReportNode = getReportFromJson(jsonFile) + + val depNodes: Seq[DepNode] = node.dependencies + .filter(_.coord == "org.apache.commons:commons-compress:1.5") + .sortBy(_.files.head._1.length) + assert(depNodes.length == 1) + assert(depNodes.head.files.head._1 == "") + assert(depNodes.head.files.head._2.contains("junit/junit/4.12/junit-4.12.jar")) + } + } + + /** + * Result: + * |└─ org.apache.commons:commons-compress:1.4.1 -> 1.5 + * | └─ org.tukaani:xz:1.2 + */ + "external dep url on lower version" should "fetch higher version" in withFile() { + (jsonFile, _) => { + val commonOpt = CommonOptions(jsonOutputFile = jsonFile.getPath) + val fetchOpt = FetchOptions(common = commonOpt) + + // encode path to different jar than requested + val externalUrl = encode("http://central.maven.org/maven2/junit/junit/4.12/junit-4.12.jar", "UTF-8") + + Fetch.run( + fetchOpt, + RemainingArgs( + Seq( + "org.apache.commons:commons-compress:1.4.1,url=" + externalUrl, + "org.apache.commons:commons-compress:1.5" + ), + Seq() + ) + ) + + val node: ReportNode = getReportFromJson(jsonFile) + + val depNode = node.dependencies.find(_.coord == "org.apache.commons:commons-compress:1.5") + assert(depNode.isDefined) + assert(depNode.get.files.head._2.contains("commons-compress-1.5.jar")) + + assert(depNode.get.dependencies.size == 1) + assert(depNode.get.dependencies.head.contains("org.tukaani:xz:1.2")) + } + } } diff --git a/cli/src/test/scala-2.12/coursier/cli/CliTestLib.scala b/cli/src/test/scala-2.12/coursier/cli/CliTestLib.scala index e36dbf814..dec617f7d 100644 --- a/cli/src/test/scala-2.12/coursier/cli/CliTestLib.scala +++ b/cli/src/test/scala-2.12/coursier/cli/CliTestLib.scala @@ -5,8 +5,10 @@ import java.io.{File, FileWriter} trait CliTestLib { - def withFile(content: String = "")(testCode: (File, FileWriter) => Any) { - val file = File.createTempFile("hello", "world") // create the fixture + def withFile(content: String = "", + fileName: String = "hello", + suffix: String = "world")(testCode: (File, FileWriter) => Any) { + val file = File.createTempFile(fileName, suffix) // create the fixture val writer = new FileWriter(file) writer.write(content) writer.flush() diff --git a/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala b/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala index 16c426ff8..591d7bf34 100644 --- a/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala +++ b/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala @@ -291,7 +291,9 @@ object IvyRepository { for { propertiesPattern <- PropertiesPattern.parse(pattern) - metadataPropertiesPatternOpt <- metadataPatternOpt.fold(Option.empty[PropertiesPattern].right[String])(PropertiesPattern.parse(_).map(Some(_))) + metadataPropertiesPatternOpt <- metadataPatternOpt + .fold(Option.empty[PropertiesPattern].right[String])(PropertiesPattern.parse(_) + .map(Some(_))) pattern <- propertiesPattern.substituteProperties(properties) metadataPatternOpt <- metadataPropertiesPatternOpt.fold(Option.empty[Pattern].right[String])(_.substituteProperties(properties).map(Some(_))) diff --git a/core/shared/src/main/scala/coursier/util/Parse.scala b/core/shared/src/main/scala/coursier/util/Parse.scala index eb2da60a4..b372850b8 100644 --- a/core/shared/src/main/scala/coursier/util/Parse.scala +++ b/core/shared/src/main/scala/coursier/util/Parse.scala @@ -119,20 +119,26 @@ object Parse { @deprecated("use the variant accepting a default scala version", "1.0.0-M13") def moduleVersionConfig(s: String, defaultScalaVersion: String): Either[String, (Module, String, Option[String])] = { - val mvc: Either[String, Dependency] = moduleVersionConfig(s, ModuleRequirements(), transitive = true, defaultScalaVersion) + val mvc: Either[String, (Dependency, Map[String, String])] = + moduleVersionConfig(s, ModuleRequirements(), transitive = true, defaultScalaVersion) mvc match { case Left(x) => Left(x) - case Right(d) => Right(d.module, d.version, Option(d.configuration).filter(_.trim.nonEmpty)) + case Right(depsWithParams) => + val (dep, _) = depsWithParams + Right(dep.module, dep.version, Option(dep.configuration).filter(_.trim.nonEmpty)) } } @deprecated("use the variant accepting a default scala version", "1.0.0-M13") def moduleVersionConfig(s: String): Either[String, (Module, String, Option[String])] = { - val mvc: Either[String, Dependency] = moduleVersionConfig(s, ModuleRequirements(), transitive = true, defaultScalaVersion) + val mvc: Either[String, (Dependency, Map[String, String])] = + moduleVersionConfig(s, ModuleRequirements(), transitive = true, defaultScalaVersion) mvc match { case Left(x) => Left(x) - case Right(d) => Right(d.module, d.version, Option(d.configuration).filter(_.trim.nonEmpty)) + case Right(depsWithParams) => + val (dep, _) = depsWithParams + Right(dep.module, dep.version, Option(dep.configuration).filter(_.trim.nonEmpty)) } } @@ -146,12 +152,13 @@ object Parse { * or * org:name:version:config,attr1=val1,attr2=val2 * - * Currently only "classifier" attribute is used, and others are ignored. + * Currently only the "classifier" and "url attributes are + * used, and others throw errors. */ def moduleVersionConfig(s: String, req: ModuleRequirements, transitive: Boolean, - defaultScalaVersion: String): Either[String, Dependency] = { + defaultScalaVersion: String): Either[String, (Dependency, Map[String, String])] = { // Assume org:name:version,attr1=val1,attr2=val2 // That is ',' has to go after ':'. @@ -159,89 +166,141 @@ object Parse { val attrSeparator = "," val argSeparator = ":" - val strings = s.split(attrSeparator) - val coords = strings.head + val Array(coords, rawAttrs @ _*) = s.split(attrSeparator) - val attrs = strings.drop(1).map({ x => { - if (x.mkString.contains(argSeparator)) { - throw new ModuleParseError(s"'$argSeparator' is not allowed in attribute '$x' in '$s'. Please follow the format " + - s"'org${argSeparator}name[${argSeparator}version][${argSeparator}config]${attrSeparator}attr1=val1${attrSeparator}attr2=val2'") + val attrsOrErrors = rawAttrs + .map { x => + if (x.contains(argSeparator)) + Left(s"'$argSeparator' is not allowed in attribute '$x' in '$s'. Please follow the format " + + s"'org${argSeparator}name[${argSeparator}version][${argSeparator}config]${attrSeparator}attr1=val1${attrSeparator}attr2=val2'") + else + x.split("=") match { + case Array(k, v) => + Right(k -> v) + case _ => + Left(s"Failed to parse attribute '$x' in '$s'. Keyword argument expected such as 'classifier=tests'") + } } - val y = x.split("=") - if (y.length != 2) { - throw new ModuleParseError(s"Failed to parse attribute '$x' in '$s'. Keyword argument expected such as 'classifier=tests'") + + attrsOrErrors + .collectFirst { + case Left(err) => Left(err) } - (y(0), y(1)) + .getOrElse { + + val attrs = attrsOrErrors + .collect { + case Right(attr) => attr + } + .toMap + + val parts = coords.split(":", 5) + + // Only "classifier" and "url" attributes are allowed + val validAttrsKeys = Set("classifier", "url") + + validateAttributes(attrs, s, validAttrsKeys) match { + case Some(err) => Left(err) + case None => + + val attributes = attrs.get("classifier") match { + case Some(c) => Attributes("", c) + case None => Attributes("", "") + } + + val extraDependencyParams: Map[String, String] = attrs.get("url") match { + case Some(url) => Map("url" -> url) + case None => Map() + } + + val localExcludes = req.localExcludes + val globalExcludes = req.globalExcludes + val defaultConfig = req.defaultConfiguration + + val depOrError = parts match { + case Array(org, "", rawName, version, config) => + module(s"$org::$rawName", defaultScalaVersion) + .right + .map(mod => { + Dependency( + mod, + version, + config, + attributes, + transitive = transitive, + exclusions = localExcludes.getOrElse(mod.orgName, Set()) | globalExcludes) + }) + + case Array(org, "", rawName, version) => + module(s"$org::$rawName", defaultScalaVersion) + .right + .map(mod => { + Dependency( + mod, + version, + configuration = defaultConfig, + attributes = attributes, + transitive = transitive, + exclusions = localExcludes.getOrElse(mod.orgName, Set()) | globalExcludes) + }) + + case Array(org, rawName, version, config) => + module(s"$org:$rawName", defaultScalaVersion) + .right + .map(mod => { + Dependency( + mod, + version, + config, + attributes, + transitive = transitive, + exclusions = localExcludes.getOrElse(mod.orgName, Set()) | globalExcludes) + }) + + case Array(org, rawName, version) => + module(s"$org:$rawName", defaultScalaVersion) + .right + .map(mod => { + Dependency( + mod, + version, + configuration = defaultConfig, + attributes = attributes, + transitive = transitive, + exclusions = localExcludes.getOrElse(mod.orgName, Set()) | globalExcludes) + }) + + case _ => + Left(s"Malformed dependency: $s") + } + + depOrError.right.map(dep => (dep, extraDependencyParams)) + } } - }).toMap + } - val parts = coords.split(":", 5) + /** + * Validates the parsed attributes + * + * Currently only "classifier" and "url" are allowed. If more are + * added, they should be passed in via the second parameter + * + * @param attrs Attributes parsed + * @param dep String representing the dep being parsed + * @param validAttrsKeys Valid attribute keys + * @return A string if there is an error, otherwise None + */ + private def validateAttributes(attrs: Map[String, String], + dep: String, + validAttrsKeys: Set[String]): Option[String] = { + val extraAttributes = attrs.keys.toSet.diff(validAttrsKeys) - val attributes = attrs.get("classifier") match { - case Some(c) => Attributes("", c) - case None => Attributes("", "") - } - - val localExcludes = req.localExcludes - val globalExcludes = req.globalExcludes - val defaultConfig = req.defaultConfiguration - - parts match { - case Array(org, "", rawName, version, config) => - module(s"$org::$rawName", defaultScalaVersion) - .right - .map(mod => { - Dependency( - mod, - version, - config, - attributes, - transitive = transitive, - exclusions = localExcludes.getOrElse(mod.orgName, Set()) | globalExcludes) - }) - - case Array(org, "", rawName, version) => - module(s"$org::$rawName", defaultScalaVersion) - .right - .map(mod => { - Dependency( - mod, - version, - configuration = defaultConfig, - attributes = attributes, - transitive = transitive, - exclusions = localExcludes.getOrElse(mod.orgName, Set()) | globalExcludes) - }) - - case Array(org, rawName, version, config) => - module(s"$org:$rawName", defaultScalaVersion) - .right - .map(mod => { - Dependency( - mod, - version, - config, - attributes, - transitive = transitive, - exclusions = localExcludes.getOrElse(mod.orgName, Set()) | globalExcludes) - }) - - case Array(org, rawName, version) => - module(s"$org:$rawName", defaultScalaVersion) - .right - .map(mod => { - Dependency( - mod, - version, - configuration = defaultConfig, - attributes = attributes, - transitive = transitive, - exclusions = localExcludes.getOrElse(mod.orgName, Set()) | globalExcludes) - }) - - case _ => - Left(s"Malformed dependency: $s") - } + if (attrs.size > validAttrsKeys.size || extraAttributes.nonEmpty) + Some(s"The only attributes allowed are: ${validAttrsKeys.mkString(", ")}. ${ + if (extraAttributes.nonEmpty) s"The following are invalid: " + + s"${extraAttributes.map(_ + s" in "+ dep).mkString(", ")}" + }") + else None } @deprecated("use the variant accepting a default scala version", "1.0.0-M13") @@ -258,15 +317,19 @@ object Parse { @deprecated("use the variant accepting a default scala version", "1.0.0-M13") def moduleVersionConfigs(l: Seq[String]): (Seq[String], Seq[(Module, String, Option[String])]) = { - val mvc: (Seq[String], Seq[Dependency]) = moduleVersionConfigs(l, ModuleRequirements(), transitive = true, defaultScalaVersion) + val mvc: (Seq[String], Seq[(Dependency, Map[String, String])]) = + moduleVersionConfigs(l, ModuleRequirements(), transitive = true, defaultScalaVersion) + val errorsAndDeps = (mvc._1, mvc._2.map(d => d._1)) // convert empty config to None - (mvc._1, mvc._2.map(d => (d.module, d.version, Option(d.configuration).filter(_.trim.nonEmpty)))) + (errorsAndDeps._1, errorsAndDeps._2.map(d => (d.module, d.version, Option(d.configuration).filter(_.trim.nonEmpty)))) } @deprecated("use the variant accepting a default scala version", "1.0.0-M13") def moduleVersionConfigs(l: Seq[String], defaultScalaVersion: String): (Seq[String], Seq[(Module, String, Option[String])]) = { - val mvc: (Seq[String], Seq[Dependency]) = moduleVersionConfigs(l, ModuleRequirements(), transitive = true, defaultScalaVersion) - (mvc._1, mvc._2.map(d => (d.module, d.version, Option(d.configuration).filter(_.trim.nonEmpty)))) + val mvc: (Seq[String], Seq[(Dependency, Map[String, String])]) = + moduleVersionConfigs(l, ModuleRequirements(), transitive = true, defaultScalaVersion) + val errorsAndDeps = (mvc._1, mvc._2.map(d => d._1)) + (errorsAndDeps._1, errorsAndDeps._2.map(d => (d.module, d.version, Option(d.configuration).filter(_.trim.nonEmpty)))) } /** @@ -288,7 +351,7 @@ object Parse { def moduleVersionConfigs(l: Seq[String], req: ModuleRequirements, transitive: Boolean, - defaultScalaVersion: String): (Seq[String], Seq[Dependency]) = + defaultScalaVersion: String): (Seq[String], Seq[(Dependency, Map[String, String])]) = valuesAndErrors(moduleVersionConfig(_, req, transitive, defaultScalaVersion), l) def repository(s: String): String \/ Repository = diff --git a/extra/src/main/scala/coursier/BUILD b/extra/src/main/scala/coursier/BUILD new file mode 100644 index 000000000..cebddab94 --- /dev/null +++ b/extra/src/main/scala/coursier/BUILD @@ -0,0 +1,8 @@ +scala_library( + name = "fallback-deps-repo", + dependencies = [ + "core:core", + "cache/src/main/scala:cache", + ], + sources = globs("*.scala"), +) diff --git a/sbt-coursier/src/main/scala/coursier/FallbackDependenciesRepository.scala b/extra/src/main/scala/coursier/FallbackDependenciesRepository.scala similarity index 100% rename from sbt-coursier/src/main/scala/coursier/FallbackDependenciesRepository.scala rename to extra/src/main/scala/coursier/FallbackDependenciesRepository.scala diff --git a/tests/shared/src/test/scala/coursier/test/ParseTests.scala b/tests/shared/src/test/scala/coursier/test/ParseTests.scala index aabdb5991..f919419a5 100644 --- a/tests/shared/src/test/scala/coursier/test/ParseTests.scala +++ b/tests/shared/src/test/scala/coursier/test/ParseTests.scala @@ -20,6 +20,8 @@ object ParseTests extends TestSuite { case _ => false } + val url = "file%3A%2F%2Fsome%2Fencoded%2Furl" + val tests = TestSuite { "bintray-ivy:" - { val obtained = Parse.repository("bintray-ivy:scalameta/maven") @@ -53,7 +55,7 @@ object ParseTests extends TestSuite { "org:name:version" - { Parse.moduleVersionConfig("org.apache.avro:avro:1.7.4", ModuleRequirements(), transitive = true, "2.11.11") match { case Left(err) => assert(false) - case Right(dep) => + case Right((dep, _)) => assert(dep.module.organization == "org.apache.avro") assert(dep.module.name == "avro") assert(dep.version == "1.7.4") @@ -65,7 +67,7 @@ object ParseTests extends TestSuite { "org:name:version:conifg" - { Parse.moduleVersionConfig("org.apache.avro:avro:1.7.4:runtime", ModuleRequirements(), transitive = true, "2.11.11") match { case Left(err) => assert(false) - case Right(dep) => + case Right((dep, _)) => assert(dep.module.organization == "org.apache.avro") assert(dep.module.name == "avro") assert(dep.version == "1.7.4") @@ -77,7 +79,7 @@ object ParseTests extends TestSuite { "single attr" - { Parse.moduleVersionConfig("org.apache.avro:avro:1.7.4:runtime,classifier=tests", ModuleRequirements(), transitive = true, "2.11.11") match { case Left(err) => assert(false) - case Right(dep) => + case Right((dep, _)) => assert(dep.module.organization == "org.apache.avro") assert(dep.module.name == "avro") assert(dep.version == "1.7.4") @@ -86,47 +88,63 @@ object ParseTests extends TestSuite { } } - "multiple attrs" - { - Parse.moduleVersionConfig("org.apache.avro:avro:1.7.4:runtime,classifier=tests,nickname=superman", ModuleRequirements(), transitive = true, "2.11.11") match { + "single attr with url" - { + Parse.moduleVersionConfig("org.apache.avro:avro:1.7.4:runtime,url=" + url, ModuleRequirements(), transitive = true, "2.11.11") match { case Left(err) => assert(false) - case Right(dep) => + case Right((dep, extraParams)) => + assert(dep.module.organization == "org.apache.avro") + assert(dep.module.name == "avro") + assert(dep.version == "1.7.4") + assert(dep.configuration == "runtime") + assert(dep.attributes == Attributes("", "")) + assert(extraParams.isDefinedAt("url")) + assert(extraParams.getOrElse("url", "") == url) + } + } + + "multiple attrs with url" - { + Parse.moduleVersionConfig("org.apache.avro:avro:1.7.4:runtime,classifier=tests,url=" + url, ModuleRequirements(), transitive = true, "2.11.11") match { + case Left(err) => assert(false) + case Right((dep, extraParams)) => assert(dep.module.organization == "org.apache.avro") assert(dep.module.name == "avro") assert(dep.version == "1.7.4") assert(dep.configuration == "runtime") assert(dep.attributes == Attributes("", "tests")) + assert(extraParams.isDefinedAt("url")) + assert(extraParams.getOrElse("url", "") == url) } } "single attr with org::name:version" - { - Parse.moduleVersionConfig("io.get-coursier.scala-native::sandbox_native0.3:0.3.0-coursier-1,attr1=val1", ModuleRequirements(), transitive = true, "2.11.11") match { + Parse.moduleVersionConfig("io.get-coursier.scala-native::sandbox_native0.3:0.3.0-coursier-1,classifier=tests", ModuleRequirements(), transitive = true, "2.11.11") match { case Left(err) => assert(false) - case Right(dep) => + case Right((dep, _)) => assert(dep.module.organization == "io.get-coursier.scala-native") assert(dep.module.name.contains("sandbox_native0.3")) // use `contains` to be scala version agnostic assert(dep.version == "0.3.0-coursier-1") + assert(dep.attributes == Attributes("", "tests")) } } "illegal 1" - { - try { - Parse.moduleVersionConfig("org.apache.avro:avro,1.7.4:runtime,classifier=tests", ModuleRequirements(), transitive = true, "2.11.11") - assert(false) // Parsing should fail but succeeded. - } - catch { - case foo: ModuleParseError => assert(foo.getMessage().contains("':' is not allowed in attribute")) // do nothing - case _: Throwable => assert(false) // Unexpected exception + Parse.moduleVersionConfig("org.apache.avro:avro,1.7.4:runtime,classifier=tests", ModuleRequirements(), transitive = true, "2.11.11") match { + case Left(err) => assert(err.contains("':' is not allowed in attribute")) + case Right(dep) => assert(false) } } "illegal 2" - { - try { - Parse.moduleVersionConfig("junit:junit:4.12,attr", ModuleRequirements(), transitive = true, "2.11.11") - assert(false) // Parsing should fail but succeeded. + Parse.moduleVersionConfig("junit:junit:4.12,attr", ModuleRequirements(), transitive = true, "2.11.11") match { + case Left(err) => assert(err.contains("Failed to parse attribute")) + case Right(dep) => assert(false) } - catch { - case foo: ModuleParseError => assert(foo.getMessage().contains("Failed to parse attribute")) // do nothing - case _: Throwable => assert(false) // Unexpected exception + } + + "illegal 3" - { + Parse.moduleVersionConfig("a:b:c,batman=robin", ModuleRequirements(), transitive = true, "2.11.11") match { + case Left(err) => assert(err.contains("The only attributes allowed are:")) + case Right(dep) => assert(false) } } }