From ae58cf342b17a409a8fd33f2783f572af72599bb Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Sun, 12 Jun 2016 01:27:19 -0400 Subject: [PATCH] Fixes merged ModuleID dropping configuration specification. Fixes #2002/#1500 Given a dependency graph such as: libraryDependencies += "com.google.guava" % "guava-tests" % "18.0" libraryDependencies += "com.google.guava" % "guava-tests" % "18.0" % "test" classifier "tests" previous releases of sbt would drop the Test configuration from the classifier "tests" artifacts, and end up including the test JARs into the Compile configuration instead of the Test configuration, which would result in runtime error. This fix configures the explicit artifacts into the configuration during merge even when it says `"*"`. --- .../scala/sbt/ivyint/MergeDescriptors.scala | 15 ++++--- ivy/src/test/scala/MergeDescriptorSpec.scala | 42 +++++++++++++++++++ notes/0.13.12/merge-descriptor-fix.md | 7 ++++ 3 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 ivy/src/test/scala/MergeDescriptorSpec.scala create mode 100644 notes/0.13.12/merge-descriptor-fix.md diff --git a/ivy/src/main/scala/sbt/ivyint/MergeDescriptors.scala b/ivy/src/main/scala/sbt/ivyint/MergeDescriptors.scala index 413583066..6d0950fd9 100644 --- a/ivy/src/main/scala/sbt/ivyint/MergeDescriptors.scala +++ b/ivy/src/main/scala/sbt/ivyint/MergeDescriptors.scala @@ -38,7 +38,7 @@ private[sbt] object MergeDescriptors { // combines the artifacts, configurations, includes, and excludes for DependencyDescriptors `a` and `b` // that otherwise have equal IDs -private final class MergedDescriptors(a: DependencyDescriptor, b: DependencyDescriptor) extends DependencyDescriptor { +private[sbt] final case class MergedDescriptors(a: DependencyDescriptor, b: DependencyDescriptor) extends DependencyDescriptor { def getDependencyId = a.getDependencyId def isForce = a.isForce def isChanging = a.isChanging @@ -87,11 +87,14 @@ private final class MergedDescriptors(a: DependencyDescriptor, b: DependencyDesc arts map { art => explicitConfigurations(base, art) } private[this] def explicitConfigurations(base: DependencyDescriptor, art: DependencyArtifactDescriptor): DependencyArtifactDescriptor = { - val aConfs = art.getConfigurations - if (aConfs == null || aConfs.isEmpty) - copyWithConfigurations(art, base.getModuleConfigurations) - else - art + val aConfs = Option(art.getConfigurations) map { _.toList } + // In case configuration list is "*", we should still specify the module configuration of the DependencyDescriptor + // otherwise the explicit specified artifacts from one dd can leak over to the other. + // See gh-1500, gh-2002 + aConfs match { + case None | Some(Nil) | Some(List("*")) => copyWithConfigurations(art, base.getModuleConfigurations) + case _ => art + } } private[this] def defaultArtifact(a: DependencyDescriptor): Array[DependencyArtifactDescriptor] = { diff --git a/ivy/src/test/scala/MergeDescriptorSpec.scala b/ivy/src/test/scala/MergeDescriptorSpec.scala new file mode 100644 index 000000000..a977ef112 --- /dev/null +++ b/ivy/src/test/scala/MergeDescriptorSpec.scala @@ -0,0 +1,42 @@ +package sbt + +import sbt.ivyint.MergedDescriptors +import org.specs2._ +import org.apache.ivy.core.module.descriptor.{ DependencyArtifactDescriptor, DependencyDescriptor } + +class MergeDescriptorSpec extends BaseIvySpecification { + def is = args(sequential = true) ^ s2""" + + This is a specification to check the merge descriptor + + Merging duplicate dependencies should + work $e1 + """ + + def guavaTest = ModuleID("com.google.guava", "guava-tests", "18.0", configurations = Some("compile")) + def guavaTestTests = ModuleID("com.google.guava", "guava-tests", "18.0", configurations = Some("test")).classifier("tests") + def defaultOptions = EvictionWarningOptions.default + + import ShowLines._ + + def e1 = { + cleanIvyCache() + val m = module(ModuleID("com.example", "foo", "0.1.0", Some("compile")), + Seq(guavaTest, guavaTestTests), None, UpdateOptions()) + m.withModule(log) { + case (ivy, md, _) => + val deps = md.getDependencies + assert(deps.size == 1) + deps.head match { + case dd @ MergedDescriptors(dd0, dd1) => + val arts = dd.getAllDependencyArtifacts + val a0: DependencyArtifactDescriptor = arts.toList(0) + val a1: DependencyArtifactDescriptor = arts.toList(1) + val configs0 = a0.getConfigurations.toList + val configs1 = a1.getConfigurations.toList + (configs0 must_== List("compile")) and + (configs1 must_== List("test")) + } + } + } +} diff --git a/notes/0.13.12/merge-descriptor-fix.md b/notes/0.13.12/merge-descriptor-fix.md new file mode 100644 index 000000000..715242291 --- /dev/null +++ b/notes/0.13.12/merge-descriptor-fix.md @@ -0,0 +1,7 @@ + [@eed3si9n]: https://github.com/eed3si9n + [1500]: https://github.com/sbt/sbt/issues/1500 + [2002]: https://github.com/sbt/sbt/issues/2002 + +### Bug fixes + +- Fixes merged dependency descriptors dropping configuration specification. [#2002][2005]/[#1500][1500] by [@eed3si9n][@eed3si9n]