From 7157af57cf19f89ea16e223407e00fd1ca201084 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Tue, 9 Sep 2014 11:04:20 -0400 Subject: [PATCH 1/8] Add failing test for ivy/maven cross-over configuration issue. --- .../cross-ivy-maven/build.sbt | 15 +++++++++++++++ .../mvn-repo/bad/mvn/1.0/mvn-1.0.jar | 0 .../mvn-repo/bad/mvn/1.0/mvn-1.0.pom | 15 +++++++++++++++ .../dependency-management/cross-ivy-maven/test | 1 + 4 files changed, 31 insertions(+) create mode 100644 sbt/src/sbt-test/dependency-management/cross-ivy-maven/build.sbt create mode 100644 sbt/src/sbt-test/dependency-management/cross-ivy-maven/mvn-repo/bad/mvn/1.0/mvn-1.0.jar create mode 100644 sbt/src/sbt-test/dependency-management/cross-ivy-maven/mvn-repo/bad/mvn/1.0/mvn-1.0.pom create mode 100644 sbt/src/sbt-test/dependency-management/cross-ivy-maven/test 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 From b226f6511aa96f837aa279b04d922effab44ed70 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Tue, 9 Sep 2014 17:45:02 -0400 Subject: [PATCH 2/8] Fix Maven configuration mappings in Ivy. 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". Includes a test demonstrating the issue. --- .../ivy/plugins/parser/m2/EvilHackery.scala | 93 +++++++++++++++++++ ivy/src/main/scala/sbt/CustomPomParser.scala | 6 +- .../project/TestProject.scala | 13 ++- .../exclude-transitive/test | 1 + 4 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/EvilHackery.scala diff --git a/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/EvilHackery.scala b/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/EvilHackery.scala new file mode 100644 index 000000000..f76637404 --- /dev/null +++ b/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/EvilHackery.scala @@ -0,0 +1,93 @@ +package org.apache.ivy.plugins.parser.m2 + +import org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor; + +/** + * Helper evil hackery method to ensure Maven configurations + Ivy i + */ +object EvilHackery { + + val REPLACEMENT_MAVEN_MAPPINGS = { + // Here we copy paste from Ivy + val REPLACEMENT_MAPPINGS = new java.util.HashMap[String, PomModuleDescriptorBuilder.ConfMapper] + + REPLACEMENT_MAPPINGS.put("compile", new PomModuleDescriptorBuilder.ConfMapper { + def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean) { + if (isOptional) { + dd.addDependencyConfiguration("optional", "compile(*)") + // NOTE - This is the problematic piece we're dropping. EVIL!!!! + dd.addDependencyConfiguration("optional", "master(compile)") + } else { + dd.addDependencyConfiguration("compile", "compile(*)") + // NOTE - This is the problematic piece we're dropping, as `master` is not special cased. + 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(*)") + // Remove evil hackery + dd.addDependencyConfiguration("optional", "master(compile)") + } else { + dd.addDependencyConfiguration("provided", "compile(*)") + dd.addDependencyConfiguration("provided", "provided(*)") + dd.addDependencyConfiguration("provided", "runtime(*)") + // Remove evil hackery + 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(*)") + // Remove evil hackery + dd.addDependencyConfiguration("optional", "master(compile)") + } else { + dd.addDependencyConfiguration("runtime", "compile(*)") + dd.addDependencyConfiguration("runtime", "runtime(*)") + // Remove evil hackery + dd.addDependencyConfiguration("runtime", "master(compile)") + } + } + }) + + REPLACEMENT_MAPPINGS.put("test", new PomModuleDescriptorBuilder.ConfMapper { + def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean) { + dd.addDependencyConfiguration("test", "runtime(*)") + // Remove evil hackery + dd.addDependencyConfiguration("test", "master(compile)") + } + }) + + REPLACEMENT_MAPPINGS.put("system", new PomModuleDescriptorBuilder.ConfMapper { + def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean) { + // Hacked + dd.addDependencyConfiguration("system", "master(compile)") + } + }) + + REPLACEMENT_MAPPINGS + } + + def init(): Unit = { + // SUPER EVIL INITIALIZATION + 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..e19f01a9c 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.{ EvilHackery, 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. + EvilHackery.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/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 From bdd3d01f6e9bc56e7da2d39040c39207d8046920 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Wed, 10 Sep 2014 07:56:19 -0400 Subject: [PATCH 3/8] Fixes from review, better names and internal code documentation. --- ...> ReplaceMavenConfigurationMappings.scala} | 44 ++++++++++++++----- ivy/src/main/scala/sbt/CustomPomParser.scala | 4 +- 2 files changed, 34 insertions(+), 14 deletions(-) rename ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/{EvilHackery.scala => ReplaceMavenConfigurationMappings.scala} (55%) diff --git a/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/EvilHackery.scala b/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/ReplaceMavenConfigurationMappings.scala similarity index 55% rename from ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/EvilHackery.scala rename to ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/ReplaceMavenConfigurationMappings.scala index f76637404..68a9dad40 100644 --- a/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/EvilHackery.scala +++ b/ivy/src/main/scala/org/apache/ivy/plugins/parser/m2/ReplaceMavenConfigurationMappings.scala @@ -3,42 +3,62 @@ package org.apache.ivy.plugins.parser.m2 import org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor; /** - * Helper evil hackery method to ensure Maven configurations + Ivy i + * 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 EvilHackery { +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(*)") - // NOTE - This is the problematic piece we're dropping. EVIL!!!! + // 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(*)") - // NOTE - This is the problematic piece we're dropping, as `master` is not special cased. + // 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(*)") - // Remove evil hackery + // 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(*)") - // Remove evil hackery + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. dd.addDependencyConfiguration("provided", "master(compile)") } } @@ -49,12 +69,12 @@ object EvilHackery { if (isOptional) { dd.addDependencyConfiguration("optional", "compile(*)") dd.addDependencyConfiguration("optional", "provided(*)") - // Remove evil hackery + // 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(*)") - // Remove evil hackery + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. dd.addDependencyConfiguration("runtime", "master(compile)") } } @@ -63,14 +83,14 @@ object EvilHackery { REPLACEMENT_MAPPINGS.put("test", new PomModuleDescriptorBuilder.ConfMapper { def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean) { dd.addDependencyConfiguration("test", "runtime(*)") - // Remove evil hackery + // 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) { - // Hacked + // FIX - Here we take a mroe conservative approach of depending on the compile configuration if master isn't there. dd.addDependencyConfiguration("system", "master(compile)") } }) @@ -79,7 +99,7 @@ object EvilHackery { } def init(): Unit = { - // SUPER EVIL INITIALIZATION + // 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() diff --git a/ivy/src/main/scala/sbt/CustomPomParser.scala b/ivy/src/main/scala/sbt/CustomPomParser.scala index e19f01a9c..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.{ EvilHackery, 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 @@ -29,7 +29,7 @@ final class CustomPomParser(delegate: ModuleDescriptorParser, transform: (Module object CustomPomParser { // Evil hackery to override the default maven pom mappings. - EvilHackery.init() + 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." From 8451caf93a691067978b0b49907cec505e38e7f8 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Wed, 10 Sep 2014 12:36:05 -0400 Subject: [PATCH 4/8] Bump version for next sbt release. --- project/Sbt.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Sbt.scala b/project/Sbt.scala index 5b2453dfb..2930f13a0 100644 --- a/project/Sbt.scala +++ b/project/Sbt.scala @@ -17,7 +17,7 @@ object Sbt extends Build { s"all control/$task collections/$task io/$task completion/$task" def buildSettings = Seq( organization := "org.scala-sbt", - version := "0.13.6-SNAPSHOT", + version := "0.13.7-SNAPSHOT", publishArtifact in packageDoc := false, scalaVersion := "2.10.4", publishMavenStyle := false, From 3d981177df699f30b9e363504d88d290185e4720 Mon Sep 17 00:00:00 2001 From: Antonio Cunei Date: Wed, 10 Sep 2014 02:36:37 +0200 Subject: [PATCH 5/8] Adding test for broken "set every", see #1430 To test, use: > scripted tests/set-every --- sbt/src/sbt-test/tests/set-every/build.sbt | 16 ++++++++++++++++ sbt/src/sbt-test/tests/set-every/test | 3 +++ 2 files changed, 19 insertions(+) create mode 100644 sbt/src/sbt-test/tests/set-every/build.sbt create mode 100644 sbt/src/sbt-test/tests/set-every/test diff --git a/sbt/src/sbt-test/tests/set-every/build.sbt b/sbt/src/sbt-test/tests/set-every/build.sbt new file mode 100644 index 000000000..bd8e9b438 --- /dev/null +++ b/sbt/src/sbt-test/tests/set-every/build.sbt @@ -0,0 +1,16 @@ +val a = project.settings(version := "2.8.1") + +val trySetEvery = taskKey[Unit]("Tests \"set every\"") + +trySetEvery := { + val s = state.value + val extracted = Project.extract(s) + import extracted._ + val allProjs = structure.allProjectRefs + val Some(aProj) = allProjs.find(_.project == "a") + val aVer = (version in aProj get structure.data).get + if (aVer != "1.0") { + println("Version of project a: " + aVer + ", expected: 1.0") + error("\"set every\" did not change the version of all projects.") + } +} diff --git a/sbt/src/sbt-test/tests/set-every/test b/sbt/src/sbt-test/tests/set-every/test new file mode 100644 index 000000000..8fa56c46e --- /dev/null +++ b/sbt/src/sbt-test/tests/set-every/test @@ -0,0 +1,3 @@ +> set every version := '"1.0"' +> trySetEvery + From 9aa0985f4be0d654bf07a84e80ff8faec02fa653 Mon Sep 17 00:00:00 2001 From: Antonio Cunei Date: Thu, 11 Sep 2014 02:04:17 +0200 Subject: [PATCH 6/8] This commit reverts part of 322f6de6551665cade7d56b532348ea5dc3d54db The implementation of Relation should in theory make no difference whether an element is unmapped, or whether it is mapped to an empty set. One of the changes in 322f6de6551665cade7d56b532348ea5dc3d54db introduced an optimization to the '+' operation on Relations that, in theory, should have made no difference to the semantic. The result of that optimization is that some mappings of the form "elem -> Set()" are no longer inserted in the forwardMap of the Relation. Unfortunately, the change resulted in the breakage of #1430, causing "set every" to behave incorrectly. There must be, somewhere in the code, a test on the presence of a key rather than an access via .get(), or some other access that bypasses the supposed semantic equivalence described above. I spent several hours trying to track down exactly the offending test, without success. By undoing the relevant change in 322f6de6551665cade, "set every" works again. That however offers no guarantee that everything else will keep working correctly; the underlying quirk in the code that depends on this supposedly inessential detail is also still lurking in the code, which is less than ideal. --- util/relation/src/main/scala/sbt/Relation.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/relation/src/main/scala/sbt/Relation.scala b/util/relation/src/main/scala/sbt/Relation.scala index 987aafb14..dcd38fa90 100644 --- a/util/relation/src/main/scala/sbt/Relation.scala +++ b/util/relation/src/main/scala/sbt/Relation.scala @@ -129,7 +129,7 @@ private final class MRelation[A, B](fwd: Map[A, Set[B]], rev: Map[B, Set[A]]) ex def +(pair: (A, B)) = this + (pair._1, Set(pair._2)) def +(from: A, to: B) = this + (from, to :: Nil) - def +(from: A, to: Traversable[B]) = if (to.isEmpty) this else + def +(from: A, to: Traversable[B]) = new MRelation(add(fwd, from, to), (rev /: to) { (map, t) => add(map, t, from :: Nil) }) def ++(rs: Traversable[(A, B)]) = ((this: Relation[A, B]) /: rs) { _ + _ } From 6033106a2e4b34e88e87d03cb57f65bd0ae6f327 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Thu, 11 Sep 2014 11:59:38 -0400 Subject: [PATCH 7/8] Add notes and add note about notes in Contributing. --- CONTRIBUTING.md | 16 ++++++++++++++++ notes/0.13.7/cross-ivy-maven.md | 8 ++++++++ 2 files changed, 24 insertions(+) create mode 100644 notes/0.13.7/cross-ivy-maven.md 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/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] From 9b94a16b73fd708a9fe33cf8c1548d62634a4c07 Mon Sep 17 00:00:00 2001 From: Antonio Cunei Date: Fri, 12 Sep 2014 20:51:04 +0200 Subject: [PATCH 8/8] Undone the revert on the optimization, and fixed setAll() The optimization, and therefore the change in the behavior of Relation, is now needed by the class Logic, and cannot be reverted. This patch (written by Josh) therefore changes the implementation of setAll() so that _1s is no longer used. --- main/src/main/scala/sbt/SettingCompletions.scala | 4 ++-- util/relation/src/main/scala/sbt/Relation.scala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/main/src/main/scala/sbt/SettingCompletions.scala b/main/src/main/scala/sbt/SettingCompletions.scala index f3668de79..6b94c4c6b 100644 --- a/main/src/main/scala/sbt/SettingCompletions.scala +++ b/main/src/main/scala/sbt/SettingCompletions.scala @@ -27,7 +27,7 @@ private[sbt] object SettingCompletions { { import extracted._ val r = relation(extracted.structure, true) - val allDefs = r._1s.toSeq + val allDefs = Def.flattenLocals(Def.compiled(extracted.structure.settings, true)(structure.delegates, structure.scopeLocal, implicitly[Show[ScopedKey[_]]])).map(_._1) val projectScope = Load.projectScope(currentRef) def resolve(s: Setting[_]): Seq[Setting[_]] = Load.transformSettings(projectScope, currentRef.build, rootProject, s :: Nil) def rescope[T](setting: Setting[T]): Seq[Setting[_]] = @@ -353,4 +353,4 @@ private[sbt] object SettingCompletions { classOf[Long], classOf[String] ) -} \ No newline at end of file +} diff --git a/util/relation/src/main/scala/sbt/Relation.scala b/util/relation/src/main/scala/sbt/Relation.scala index dcd38fa90..987aafb14 100644 --- a/util/relation/src/main/scala/sbt/Relation.scala +++ b/util/relation/src/main/scala/sbt/Relation.scala @@ -129,7 +129,7 @@ private final class MRelation[A, B](fwd: Map[A, Set[B]], rev: Map[B, Set[A]]) ex def +(pair: (A, B)) = this + (pair._1, Set(pair._2)) def +(from: A, to: B) = this + (from, to :: Nil) - def +(from: A, to: Traversable[B]) = + def +(from: A, to: Traversable[B]) = if (to.isEmpty) this else new MRelation(add(fwd, from, to), (rev /: to) { (map, t) => add(map, t, from :: Nil) }) def ++(rs: Traversable[(A, B)]) = ((this: Relation[A, B]) /: rs) { _ + _ }