diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f9c54e533..c9fa68c64 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -84,6 +84,22 @@ Whether implementing a new feature, fixing a bug, or modifying documentation, pl Binary compatible changes will be backported to a previous series (currently, 0.12.x) at the time of the next stable release. See below for instructions on building sbt from source. +All pull requests are required to include a "Notes" file which documents the change. This file should reside in the +directory: + + + notes/ + / + .md + +Notes files should have the following contents: + +* Bullet item description under one of the following sections: + - `### Bug fixes` + - `### Improvements` + - `### Fixes with compatibility implications` +* Complete section describing new features. + Documentation ------------- diff --git a/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/ReplaceMavenConfigurationMappings.scala b/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/ReplaceMavenConfigurationMappings.scala new file mode 100644 index 000000000..68a9dad40 --- /dev/null +++ b/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/ReplaceMavenConfigurationMappings.scala @@ -0,0 +1,113 @@ +package org.apache.ivy.plugins.parser.m2 + +import org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor; + +/** + * It turns out there was a very subtle, and evil, issue sitting the Ivy/maven configuration, and it + * related to dependency mapping. A mapping of `foo->bar(*)` means that the local configuration + * `foo` depends on the remote configuration `bar`, if it exists, or *ALL CONFIGURATIONS* if `bar` + * does not exist. Since the default Ivy configuration mapping was using the random `master` + * configuration, which AFAICT is NEVER specified, just an assumed default, this would cause leaks + * between maven + ivy projects. + * + * i.e. if a maven POM depends on a module denoted by an ivy.xml file, then you'd wind up accidentally + * bleeding ALL the ivy module's configurations into the maven module's configurations. + * + * This fix works around the issue, by assuming that if there is no `master` configuration, than the + * maven default of `compile` is intended. As sbt forces generated `ivy.xml` files to abide by + * maven conventions, this works in all of our test cases. The only scenario where it wouldn't work + * is those who have custom ivy.xml files *and* have pom.xml files which rely on those custom ivy.xml files, + * a very unlikely situation where the workaround is: "define a master configuration". + * + * Also see: http://ant.apache.org/ivy/history/2.3.0/ivyfile/dependency.html + * and: http://svn.apache.org/repos/asf/ant/ivy/core/tags/2.3.0/src/java/org/apache/ivy/plugins/parser/m2/PomModuleDescriptorBuilder.java + * + * + */ +object ReplaceMavenConfigurationMappings { + + val REPLACEMENT_MAVEN_MAPPINGS = { + // Here we copy paste from Ivy + val REPLACEMENT_MAPPINGS = new java.util.HashMap[String, PomModuleDescriptorBuilder.ConfMapper] + + // NOTE - This code is copied from org.apache.ivy.plugins.parser.m2.PomModuleDescriptorBuilder + // except with altered default configurations... + REPLACEMENT_MAPPINGS.put("compile", new PomModuleDescriptorBuilder.ConfMapper { + def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean) { + if (isOptional) { + dd.addDependencyConfiguration("optional", "compile(*)") + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. + dd.addDependencyConfiguration("optional", "master(compile)") + } else { + dd.addDependencyConfiguration("compile", "compile(*)") + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. + dd.addDependencyConfiguration("compile", "master(compile)") + dd.addDependencyConfiguration("runtime", "runtime(*)") + } + } + }) + REPLACEMENT_MAPPINGS.put("provided", new PomModuleDescriptorBuilder.ConfMapper { + def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean) { + if (isOptional) { + dd.addDependencyConfiguration("optional", "compile(*)") + dd.addDependencyConfiguration("optional", "provided(*)") + dd.addDependencyConfiguration("optional", "runtime(*)") + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. + dd.addDependencyConfiguration("optional", "master(compile)") + } else { + dd.addDependencyConfiguration("provided", "compile(*)") + dd.addDependencyConfiguration("provided", "provided(*)") + dd.addDependencyConfiguration("provided", "runtime(*)") + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. + dd.addDependencyConfiguration("provided", "master(compile)") + } + } + }) + + REPLACEMENT_MAPPINGS.put("runtime", new PomModuleDescriptorBuilder.ConfMapper { + def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean) { + if (isOptional) { + dd.addDependencyConfiguration("optional", "compile(*)") + dd.addDependencyConfiguration("optional", "provided(*)") + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. + dd.addDependencyConfiguration("optional", "master(compile)") + } else { + dd.addDependencyConfiguration("runtime", "compile(*)") + dd.addDependencyConfiguration("runtime", "runtime(*)") + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. + dd.addDependencyConfiguration("runtime", "master(compile)") + } + } + }) + + REPLACEMENT_MAPPINGS.put("test", new PomModuleDescriptorBuilder.ConfMapper { + def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean) { + dd.addDependencyConfiguration("test", "runtime(*)") + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. + dd.addDependencyConfiguration("test", "master(compile)") + } + }) + + REPLACEMENT_MAPPINGS.put("system", new PomModuleDescriptorBuilder.ConfMapper { + def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean) { + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. + dd.addDependencyConfiguration("system", "master(compile)") + } + }) + + REPLACEMENT_MAPPINGS + } + + def init(): Unit = { + // Here we mutate a static final field, because we have to AND because it's evil. + try { + val map = PomModuleDescriptorBuilder.MAVEN2_CONF_MAPPING.asInstanceOf[java.util.Map[String, PomModuleDescriptorBuilder.ConfMapper]] + map.clear() + map.putAll(REPLACEMENT_MAVEN_MAPPINGS) + } catch { + case e: Exception => + // TODO - Log that Ivy may not be configured correctly and you could have maven/ivy issues. + throw new RuntimeException("FAILURE to install Ivy maven hooks. Your ivy-maven interaction may suffer resolution errors", e) + } + } +} \ No newline at end of file diff --git a/ivy/src/main/scala/sbt/CustomPomParser.scala b/ivy/src/main/scala/sbt/CustomPomParser.scala index bcb8a2476..7bc878b2b 100644 --- a/ivy/src/main/scala/sbt/CustomPomParser.scala +++ b/ivy/src/main/scala/sbt/CustomPomParser.scala @@ -4,7 +4,7 @@ import org.apache.ivy.core.module.id.ModuleRevisionId import org.apache.ivy.core.module.descriptor.{ DefaultArtifact, DefaultExtendsDescriptor, DefaultModuleDescriptor, ModuleDescriptor } import org.apache.ivy.core.module.descriptor.{ DefaultDependencyDescriptor, DependencyDescriptor } import org.apache.ivy.plugins.parser.{ ModuleDescriptorParser, ModuleDescriptorParserRegistry, ParserSettings } -import org.apache.ivy.plugins.parser.m2.{ PomModuleDescriptorBuilder, PomModuleDescriptorParser } +import org.apache.ivy.plugins.parser.m2.{ ReplaceMavenConfigurationMappings, PomModuleDescriptorBuilder, PomModuleDescriptorParser } import org.apache.ivy.plugins.repository.Resource import org.apache.ivy.plugins.namespace.NamespaceTransformer import org.apache.ivy.util.extendable.ExtendableItem @@ -27,6 +27,10 @@ final class CustomPomParser(delegate: ModuleDescriptorParser, transform: (Module override def getMetadataArtifact(mrid: ModuleRevisionId, res: Resource) = delegate.getMetadataArtifact(mrid, res) } object CustomPomParser { + + // Evil hackery to override the default maven pom mappings. + ReplaceMavenConfigurationMappings.init() + /** The key prefix that indicates that this is used only to store extra information and is not intended for dependency resolution.*/ val InfoKeyPrefix = "info." val ApiURLKey = "info.apiURL" diff --git a/notes/0.13.7/cross-ivy-maven.md b/notes/0.13.7/cross-ivy-maven.md new file mode 100644 index 000000000..1dc45d06d --- /dev/null +++ b/notes/0.13.7/cross-ivy-maven.md @@ -0,0 +1,8 @@ + [1586]: https://github.com/sbt/sbt/pull/1586 + [@jsuereth]: https://github.com/jsuereth + + +### Fixes with compatibility implications + +* Maven artifact dependencies now limit their transitive dependencies to "compile" rather than "every configuration" + if no `master` configuration is found. [#1586][1586] by [@jsuereth][@jsuereth] diff --git a/sbt/src/sbt-test/dependency-management/cross-ivy-maven/build.sbt b/sbt/src/sbt-test/dependency-management/cross-ivy-maven/build.sbt new file mode 100644 index 000000000..60a2d2c01 --- /dev/null +++ b/sbt/src/sbt-test/dependency-management/cross-ivy-maven/build.sbt @@ -0,0 +1,15 @@ + +val repoFile = file("mvn-repo") + +resolvers += "bad-mvn-repo" at repoFile.toURI.toURL.toString + +libraryDependencies += "bad" % "mvn" % "1.0" + +TaskKey[Unit]("check") := { + val cp = (fullClasspath in Compile).value + def isTestJar(n: String): Boolean = + (n contains "scalacheck") || + (n contains "specs2") + val testLibs = cp map (_.data.getName) filter isTestJar + assert(testLibs.isEmpty, s"Compile Classpath has test libs:\n * ${testLibs.mkString("\n * ")}") +} \ No newline at end of file diff --git a/sbt/src/sbt-test/dependency-management/cross-ivy-maven/mvn-repo/bad/mvn/1.0/mvn-1.0.jar b/sbt/src/sbt-test/dependency-management/cross-ivy-maven/mvn-repo/bad/mvn/1.0/mvn-1.0.jar new file mode 100644 index 000000000..e69de29bb diff --git a/sbt/src/sbt-test/dependency-management/cross-ivy-maven/mvn-repo/bad/mvn/1.0/mvn-1.0.pom b/sbt/src/sbt-test/dependency-management/cross-ivy-maven/mvn-repo/bad/mvn/1.0/mvn-1.0.pom new file mode 100644 index 000000000..2821446dd --- /dev/null +++ b/sbt/src/sbt-test/dependency-management/cross-ivy-maven/mvn-repo/bad/mvn/1.0/mvn-1.0.pom @@ -0,0 +1,15 @@ + + + 4.0.0 + bad + mvn + jar + 1.0 + + + org.scala-sbt + completion + 0.13.5 + + + diff --git a/sbt/src/sbt-test/dependency-management/cross-ivy-maven/test b/sbt/src/sbt-test/dependency-management/cross-ivy-maven/test new file mode 100644 index 000000000..a5912a391 --- /dev/null +++ b/sbt/src/sbt-test/dependency-management/cross-ivy-maven/test @@ -0,0 +1 @@ +> check \ No newline at end of file diff --git a/sbt/src/sbt-test/dependency-management/exclude-transitive/project/TestProject.scala b/sbt/src/sbt-test/dependency-management/exclude-transitive/project/TestProject.scala index c5f3887ff..7e2796e5b 100644 --- a/sbt/src/sbt-test/dependency-management/exclude-transitive/project/TestProject.scala +++ b/sbt/src/sbt-test/dependency-management/exclude-transitive/project/TestProject.scala @@ -16,9 +16,14 @@ object TestProject extends Build private def check(transitive: Boolean) = (dependencyClasspath in Compile) map { downloaded => val jars = downloaded.size - if(transitive) - if(jars <= 2) error("Transitive dependencies not downloaded") else () - else - if(jars > 2) error("Transitive dependencies downloaded (" + downloaded.files.mkString(", ") + ")") else () + if(transitive) { + if (jars <= 2) + sys.error(s"Transitive dependencies not downloaded, found:\n * ${downloaded.mkString("\n * ")}") + else () + } else { + if (jars > 2) + sys.error(s"Transitive dependencies not downloaded, found:\n * ${downloaded.mkString("\n * ")}") + else () + } } } diff --git a/sbt/src/sbt-test/dependency-management/exclude-transitive/test b/sbt/src/sbt-test/dependency-management/exclude-transitive/test index b7200b24b..511952ae3 100644 --- a/sbt/src/sbt-test/dependency-management/exclude-transitive/test +++ b/sbt/src/sbt-test/dependency-management/exclude-transitive/test @@ -1,3 +1,4 @@ +> debug # load the project definition with transitive dependencies enabled # and check that they are not downloaded #$ pause