Better handling of artifact types

Fixes https://github.com/alexarchambault/coursier/issues/318

cli/src/main/scala-2.11/coursier/cli/Bootstrap.scala

@@ -11,6 +11,8 @@ import caseapp._
 import coursier.cli.util.Zip
 
 case class Bootstrap(
+  @Recurse
+    artifactOptions: ArtifactOptions,
   @Recurse
     options: BootstrapOptions
 ) extends App {
@@ -78,6 +80,7 @@ case class Bootstrap(
         def subFiles0 = helper.fetch(
           sources = false,
           javadoc = false,
+          artifactTypes = artifactOptions.artifactTypes,
           subset = isolatedDeps.getOrElse(target, Seq.empty).toSet
         )
 
@@ -96,11 +99,19 @@ case class Bootstrap(
     if (options.standalone)
       (
         Seq.empty[String],
-        helper.fetch(sources = false, javadoc = false)
+        helper.fetch(
+          sources = false,
+          javadoc = false,
+          artifactTypes = artifactOptions.artifactTypes
+        )
       )
     else
       (
-        helper.artifacts(sources = false, javadoc = false).map(_.url),
+        helper.artifacts(
+          sources = false,
+          javadoc = false,
+          artifactTypes = artifactOptions.artifactTypes
+        ).map(_.url),
         Seq.empty[File]
       )
 

cli/src/main/scala-2.11/coursier/cli/Fetch.scala

@@ -12,9 +12,13 @@ case class Fetch(
     options: FetchOptions
 ) extends App {
 
-  val helper = new Helper(options.common, remainingArgs, ignoreErrors = options.force)
+  val helper = new Helper(options.common, remainingArgs, ignoreErrors = options.artifactOptions.force)
 
-  val files0 = helper.fetch(sources = options.sources, javadoc = options.javadoc)
+  val files0 = helper.fetch(
+    sources = options.sources,
+    javadoc = options.javadoc,
+    artifactTypes = options.artifactOptions.artifactTypes
+  )
 
   val out =
     if (options.classpath)

cli/src/main/scala-2.11/coursier/cli/Helper.scala

@@ -492,6 +492,7 @@ class Helper(
   def artifacts(
     sources: Boolean,
     javadoc: Boolean,
+    artifactTypes: Set[String],
     subset: Set[Dependency] = null
   ): Seq[Artifact] = {
 
@@ -514,25 +515,34 @@ class Helper(
 
     val res0 = Option(subset).fold(res)(res.subset)
 
-    if (classifier0.nonEmpty || sources || javadoc) {
+    val artifacts0 =
-      var classifiers = classifier0
+      if (classifier0.nonEmpty || sources || javadoc) {
-      if (sources)
+        var classifiers = classifier0
-        classifiers = classifiers :+ "sources"
+        if (sources)
-      if (javadoc)
+          classifiers = classifiers :+ "sources"
-        classifiers = classifiers :+ "javadoc"
+        if (javadoc)
+          classifiers = classifiers :+ "javadoc"
 
-      res0.classifiersArtifacts(classifiers.distinct)
+        res0.dependencyClassifiersArtifacts(classifiers.distinct).map(_._2)
-    } else
+      } else
-      res0.artifacts
+        res0.dependencyArtifacts.map(_._2)
+
+    if (artifactTypes("*"))
+      artifacts0
+    else
+      artifacts0.filter { artifact =>
+        artifactTypes(artifact.`type`)
+      }
   }
 
   def fetch(
     sources: Boolean,
     javadoc: Boolean,
+    artifactTypes: Set[String],
     subset: Set[Dependency] = null
   ): Seq[File] = {
 
-    val artifacts0 = artifacts(sources, javadoc, subset)
+    val artifacts0 = artifacts(sources, javadoc, artifactTypes, subset)
 
     val logger =
       if (verbosityLevel >= 0)
@@ -588,7 +598,15 @@ class Helper(
 
   lazy val (parentLoader, filteredFiles) = {
 
-    val files0 = fetch(sources = false, javadoc = false)
+    // FIXME That shouldn't be hard-coded this way...
+    // This whole class ought to be rewritten more cleanly.
+    val artifactTypes = Set("jar")
+
+    val files0 = fetch(
+      sources = false,
+      javadoc = false,
+      artifactTypes = artifactTypes
+    )
 
     if (isolated.isolated.isEmpty)
       (baseLoader, files0)
@@ -603,6 +621,7 @@ class Helper(
           val isolatedFiles = fetch(
             sources = false,
             javadoc = false,
+            artifactTypes = artifactTypes,
             subset = isolatedDeps.getOrElse(target, Seq.empty).toSet
           )
 

cli/src/main/scala-2.11/coursier/cli/Options.scala

@@ -168,6 +168,33 @@ case class IsolatedLoaderOptions(
 
 }
 
+object ArtifactOptions {
+  def defaultArtifactTypes = Set("jar", "bundle")
+}
+
+case class ArtifactOptions(
+  @Help("Artifact types that should be retained (e.g. jar, src, doc, etc.) - defaults to jar,bundle")
+  @Value("type1,type2,...")
+  @Short("A")
+    artifactType: List[String],
+  @Help("Fetch artifacts even if the resolution is errored")
+    force: Boolean
+) {
+  lazy val artifactTypes = {
+    val types0 = artifactType
+      .flatMap(_.split(','))
+      .filter(_.nonEmpty)
+      .toSet
+
+    if (types0.isEmpty)
+      ArtifactOptions.defaultArtifactTypes
+    else if (types0("*"))
+      Set("*")
+    else
+      types0
+  }
+}
+
 case class FetchOptions(
   @Help("Fetch source artifacts")
   @Short("S")
@@ -178,8 +205,8 @@ case class FetchOptions(
   @Help("Print java -cp compatible output")
   @Short("p")
     classpath: Boolean,
-  @Help("Fetch artifacts even if the resolution is errored")
+  @Recurse
-    force: Boolean,
+    artifactOptions: ArtifactOptions,
   @Recurse
     common: CommonOptions
 )
@@ -245,6 +272,8 @@ case class SparkSubmitOptions(
   @Help("Maximum idle time of spark-submit (time with no output). Exit early if no output from spark-submit for more than this duration. Set to 0 for unlimited. (Default: 0)")
   @Value("seconds")
     maxIdleTime: Int,
+  @Recurse
+    artifactOptions: ArtifactOptions,
   @Recurse
     common: CommonOptions
 )

cli/src/main/scala-2.11/coursier/cli/SparkSubmit.scala

@@ -70,8 +70,11 @@ case class SparkSubmit(
     extraJars = rawExtraJars
   )
   val jars =
-    helper.fetch(sources = false, javadoc = false) ++
+    helper.fetch(
-      options.extraJars.map(new File(_))
+      sources = false,
+      javadoc = false,
+      artifactTypes = options.artifactOptions.artifactTypes
+    ) ++ options.extraJars.map(new File(_))
 
   val (scalaVersion, sparkVersion) =
     if (options.sparkVersion.isEmpty)
@@ -170,6 +173,7 @@ case class SparkSubmit(
     sparkVersion,
     options.noDefaultSubmitDependencies,
     options.submitDependencies.flatMap(_.split(",")).filter(_.nonEmpty),
+    options.artifactOptions.artifactTypes,
     options.common
   )
 

cli/src/main/scala-2.11/coursier/cli/spark/Assembly.scala

@@ -140,14 +140,15 @@ object Assembly {
     sparkVersion: String,
     noDefault: Boolean,
     extraDependencies: Seq[String],
-    options: CommonOptions
+    options: CommonOptions,
+    artifactTypes: Set[String] = Set("jar")
   ): Either[String, (File, Seq[File])] = {
 
     val base = if (noDefault) Seq() else sparkAssemblyDependencies(scalaVersion, sparkVersion)
     val helper = new Helper(options, extraDependencies ++ base)
 
-    val artifacts = helper.artifacts(sources = false, javadoc = false)
+    val artifacts = helper.artifacts(sources = false, javadoc = false, artifactTypes = artifactTypes)
-    val jars = helper.fetch(sources = false, javadoc = false)
+    val jars = helper.fetch(sources = false, javadoc = false, artifactTypes = artifactTypes)
 
     val checksums = artifacts.map { a =>
       val f = a.checksumUrls.get("SHA-1") match {

cli/src/main/scala-2.11/coursier/cli/spark/Submit.scala

@@ -11,6 +11,7 @@ object Submit {
     sparkVersion: String,
     noDefault: Boolean,
     extraDependencies: Seq[String],
+    artifactTypes: Set[String],
     common: CommonOptions
   ): Seq[File] = {
 
@@ -43,7 +44,11 @@ object Submit {
       (if (noDefault) Nil else defaultDependencies) ++ extraDependencies
     )
 
-    helper.fetch(sources = false, javadoc = false) ++ extraCp
+    helper.fetch(
+      sources = false,
+      javadoc = false,
+      artifactTypes = artifactTypes
+    ) ++ extraCp
   }
 
   def mainClassName = "org.apache.spark.deploy.SparkSubmit"

core/shared/src/main/scala/coursier/core/Definitions.scala

@@ -197,7 +197,10 @@ final case class Artifact(
   attributes: Attributes,
   changing: Boolean,
   authentication: Option[Authentication]
-)
+) {
+  def `type`: String = attributes.`type`
+  def classifier: String = attributes.classifier
+}
 
 object Artifact {
   trait Source {

core/shared/src/main/scala/coursier/ivy/IvyRepository.scala

@@ -72,10 +72,9 @@ case class IvyRepository(
               case None =>
                 project.publications.collect {
                   case (conf, p)
-                    if (conf == "*" ||
+                    if conf == "*" ||
-                        conf == dependency.configuration ||
+                       conf == dependency.configuration ||
-                        project.allConfigurations.getOrElse(dependency.configuration, Set.empty).contains(conf)
+                       project.allConfigurations.getOrElse(dependency.configuration, Set.empty).contains(conf) =>
-                       ) && p.classifier.isEmpty =>
                     p
                 }
               case Some(classifiers) =>

tests/jvm/src/it/scala/coursier/test/IvyLocalTests.scala

@@ -29,7 +29,7 @@ object IvyLocalTests extends TestSuite {
           extraRepo = extraRepo
         ))
 
-        val artifacts = res.artifacts.map(_.url)
+        val artifacts = res.dependencyArtifacts.filter(_._2.`type` == "jar").map(_._2.url)
         val anyJavadoc = artifacts.exists(_.contains("-javadoc"))
         val anySources = artifacts.exists(_.contains("-sources"))
 

tests/jvm/src/test/resources/test-repo/http/ivy.abc.com/com.example/a_2.11/0.1.0-SNAPSHOT/ivys/ivy.xml

@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ivy-module version="2.0" xmlns:e="http://ant.apache.org/ivy/extra">
+  <info organisation="com.example" module="a_2.11" revision="0.1.0-SNAPSHOT" status="integration" publication="20161030134939">
+    <description/>
+  </info>
+  <configurations>
+    
+    <conf name="pom" visibility="public" description=""/>
+    <conf extends="runtime" name="test" visibility="public" description=""/>
+    <conf name="provided" visibility="public" description=""/>
+    
+    <conf name="docs" visibility="public" description=""/>
+    <conf name="optional" visibility="public" description=""/>
+    <conf name="compile" visibility="public" description=""/>
+    
+    
+    <conf name="sources" visibility="public" description=""/>
+    <conf extends="compile" name="runtime" visibility="public" description=""/>
+    
+  </configurations>
+  <publications>
+    <artifact name="a_2.11" type="jar" ext="jar" conf="compile"/>
+    <artifact name="a_2.11" type="pom" ext="pom" conf="pom"/>
+    <artifact e:classifier="javadoc" name="a_2.11" type="doc" ext="jar" conf="compile">
+</artifact>
+    <artifact e:classifier="sources" name="a_2.11" type="src" ext="jar" conf="compile">
+</artifact>
+    <artifact e:classifier="tests-javadoc" name="a_2.11" type="doc" ext="jar" conf="test">
+</artifact>
+    <artifact e:classifier="tests-sources" name="a_2.11" type="src" ext="jar" conf="test">
+</artifact>
+    <artifact e:classifier="tests" name="a_2.11" type="jar" ext="jar" conf="test"/>
+  </publications>
+  <dependencies>
+    
+    
+    
+    
+    <dependency org="org.scala-lang" name="scala-library" rev="2.11.8" conf="compile->default(compile)">
+</dependency>
+  </dependencies>
+</ivy-module>

tests/jvm/src/test/resources/test-repo/http/ivy.abc.com/com.example/a_2.11/0.1.0-SNAPSHOT/ivys/ivy.xml.md5

@@ -0,0 +1 @@
+6cc6e8ae02e9c9ba222c05aa5b0ea838

tests/jvm/src/test/resources/test-repo/http/ivy.abc.com/com.example/a_2.11/0.1.0-SNAPSHOT/ivys/ivy.xml.sha1

@@ -0,0 +1 @@
+8b338542251660a1e91c560b77c53dd5870d3ff5

tests/jvm/src/test/scala/coursier/test/IvyTests.scala

@@ -1,6 +1,6 @@
 package coursier.test
 
-import coursier.Module
+import coursier.{ Attributes, Dependency, Module }
 import coursier.ivy.IvyRepository
 
 import utest._
@@ -46,10 +46,56 @@ object IvyTests extends TestSuite {
         extraRepo = Some(sbtRepo)
       )
 
-      * - CentralTests.withArtifact(mod, ver, extraRepo = Some(sbtRepo)) { artifact =>
+      * - CentralTests.withArtifact(mod, ver, "jar", extraRepo = Some(sbtRepo)) { artifact =>
         assert(artifact.url == expectedArtifactUrl)
       }
     }
+
+    'testArtifacts - {
+
+      val dep = Dependency(
+        Module("com.example", "a_2.11"),
+        "0.1.0-SNAPSHOT",
+        transitive = false,
+        attributes = Attributes()
+      )
+
+      val repoBase = getClass.getResource("/test-repo/http/ivy.abc.com").toString.stripSuffix("/") + "/"
+
+      val repo = IvyRepository.fromPattern(
+        repoBase +: coursier.ivy.Pattern.default,
+        dropInfoAttributes = true
+      )
+
+      val mainJarUrl = repoBase + "com.example/a_2.11/0.1.0-SNAPSHOT/jars/a_2.11.jar"
+      val testJarUrl = repoBase + "com.example/a_2.11/0.1.0-SNAPSHOT/jars/a_2.11-tests.jar"
+
+      * - CentralTests.withArtifacts(
+        dep = dep,
+        artifactType = "jar",
+        extraRepo = Some(repo)
+      ) {
+        case Seq(artifact) =>
+          assert(artifact.url == mainJarUrl)
+        case other =>
+          throw new Exception(s"Unexpected number of artifacts\n${other.mkString("\n")}")
+      }
+
+      * - CentralTests.withArtifacts(
+        dep = dep.copy(configuration = "test"),
+        artifactType = "jar",
+        extraRepo = Some(repo)
+      ) {
+        case Seq(artifact1, artifact2) =>
+          val urls = Set(
+            artifact1.url,
+            artifact2.url
+          )
+          assert(urls == Set(mainJarUrl, testJarUrl))
+        case other =>
+          throw new Exception(s"Unexpected number of artifacts\n${other.mkString("\n")}")
+      }
+    }
   }
 
 }

tests/shared/src/test/scala/coursier/test/CentralTests.scala

@@ -100,14 +100,12 @@ object CentralTests extends TestSuite {
   def withArtifact[T](
     module: Module,
     version: String,
+    artifactType: String,
     extraRepo: Option[Repository] = None
   )(
     f: Artifact => T
-  ): Future[T] = async {
+  ): Future[T] =
-    val dep = Dependency(module, version, transitive = false, attributes = Attributes())
+    withArtifacts(module, version, artifactType, extraRepo) {
-    val res = await(resolve(Set(dep), extraRepo = extraRepo))
-
-    res.artifacts match {
       case Seq(artifact) =>
         f(artifact)
       case other =>
@@ -116,10 +114,41 @@ object CentralTests extends TestSuite {
             "Artifacts:\n" + other.map("  " + _).mkString("\n")
         )
     }
+
+  def withArtifacts[T](
+    module: Module,
+    version: String,
+    artifactType: String,
+    extraRepo: Option[Repository] = None
+  )(
+    f: Seq[Artifact] => T
+  ): Future[T] = {
+    val dep = Dependency(module, version, transitive = false, attributes = Attributes())
+    withArtifacts(dep, artifactType, extraRepo)(f)
   }
 
-  def ensureArtifactHasExtension(module: Module, version: String, extension: String): Future[Unit] =
+  def withArtifacts[T](
-    withArtifact(module, version) { artifact =>
+    dep: Dependency,
+    artifactType: String,
+    extraRepo: Option[Repository]
+  )(
+    f: Seq[Artifact] => T
+  ): Future[T] = async {
+    val res = await(resolve(Set(dep), extraRepo = extraRepo))
+
+    assert(res.errors.isEmpty)
+    assert(res.conflicts.isEmpty)
+    assert(res.isDone)
+
+    val artifacts = res.dependencyArtifacts.map(_._2).filter { a =>
+      a.`type` == artifactType
+    }
+
+    f(artifacts)
+  }
+
+  def ensureHasArtifactWithExtension(module: Module, version: String, artifactType: String, extension: String): Future[Unit] =
+    withArtifact(module, version, artifactType) { artifact =>
       assert(artifact.url.endsWith("." + extension))
     }
 
@@ -275,7 +304,7 @@ object CentralTests extends TestSuite {
 
       * - resolutionCheck(mod, version)
 
-      * - withArtifact(mod, version) { artifact =>
+      * - withArtifact(mod, version, "jar") { artifact =>
         assert(artifact.url == expectedArtifactUrl)
       }
     }
@@ -301,27 +330,30 @@ object CentralTests extends TestSuite {
     'packaging - {
       'aar - {
         // random aar-based module found on Central
-        ensureArtifactHasExtension(
+        ensureHasArtifactWithExtension(
           Module("com.yandex.android", "speechkit"),
           "2.5.0",
+          "aar",
           "aar"
         )
       }
 
       'bundle - {
         // has packaging bundle - ensuring coursier gives its artifact the .jar extension
-        ensureArtifactHasExtension(
+        ensureHasArtifactWithExtension(
           Module("com.google.guava", "guava"),
           "17.0",
+          "bundle",
           "jar"
         )
       }
 
       'mavenPlugin - {
         // has packaging maven-plugin - ensuring coursier gives its artifact the .jar extension
-        ensureArtifactHasExtension(
+        ensureHasArtifactWithExtension(
           Module("org.bytedeco", "javacpp"),
           "1.1",
+          "maven-plugin",
           "jar"
         )
       }