Merge pull request #2012 from sbt/fix/1856

Fix/1856

ivy/src/main/scala/sbt/CustomPomParser.scala

@@ -51,7 +51,15 @@ object CustomPomParser {
   private[this] val TransformedHashKey = "e:sbtTransformHash"
   // A hash of the parameters transformation is based on.
   // If a descriptor has a different hash, we need to retransform it.
-  private[this] val TransformHash: String = hash((unqualifiedKeys ++ JarPackagings).toSeq.sorted)
+  private[this] def makeCoords(mrid: ModuleRevisionId): String = s"${mrid.getOrganisation}:${mrid.getName}:${mrid.getRevision}"
+
+  // We now include the ModuleID in a hash, to ensure that parent-pom transformations don't corrupt child poms.
+  private[this] def MakeTransformHash(md: ModuleDescriptor): String = {
+    val coords: String = makeCoords(md.getModuleRevisionId)
+
+    hash((unqualifiedKeys ++ JarPackagings ++ Set(coords)).toSeq.sorted)
+  }
+
   private[this] def hash(ss: Seq[String]): String = Hash.toHex(Hash(ss.flatMap(_ getBytes "UTF-8").toArray))
 
   // Unfortunately, ModuleDescriptorParserRegistry is add-only and is a singleton instance.
@@ -65,10 +73,11 @@ object CustomPomParser {
     {
       val oldTransformedHashKey = "sbtTransformHash"
       val extraInfo = md.getExtraInfo
+      val MyHash = MakeTransformHash(md)
       // sbt 0.13.1 used "sbtTransformHash" instead of "e:sbtTransformHash" until #1192 so read both
       Option(extraInfo).isDefined &&
         ((Option(extraInfo get TransformedHashKey) orElse Option(extraInfo get oldTransformedHashKey)) match {
-          case Some(TransformHash) => true
+          case Some(MyHash) => true
           case _ => false
         })
     }
@@ -95,9 +104,9 @@ object CustomPomParser {
       val mergeDuplicates = IvySbt.hasDuplicateDependencies(md.getDependencies)
 
       val unqualify = toUnqualify(filtered)
-      if (unqualify.isEmpty && extraDepAttributes.isEmpty && !convertArtifacts && !mergeDuplicates)
+
-        md
+      // Here we always add extra attributes.  There's a scenario where parent-pom information corrupts child-poms with "e:" namespaced xml elements
-      else
+      // and we have to force the every generated xml file to have the appropriate xml namespace
       addExtra(unqualify, extraDepAttributes, parser, md)
     }
   // The <properties> element of the pom is used to store additional metadata, such as for sbt plugins or for the base URL for API docs.
@@ -185,7 +194,7 @@ object CustomPomParser {
 
       for (l <- md.getLicenses) dmd.addLicense(l)
       for ((key, value) <- md.getExtraInfo.asInstanceOf[java.util.Map[String, String]].asScala) dmd.addExtraInfo(key, value)
-      dmd.addExtraInfo(TransformedHashKey, TransformHash) // mark as transformed by this version, so we don't need to do it again
+      dmd.addExtraInfo(TransformedHashKey, MakeTransformHash(md)) // mark as transformed by this version, so we don't need to do it again
       for ((key, value) <- md.getExtraAttributesNamespaces.asInstanceOf[java.util.Map[String, String]].asScala) dmd.addExtraAttributeNamespace(key, value)
       IvySbt.addExtraNamespace(dmd)
 

notes/0.13.9/pom-corruption-fix.md

@@ -0,0 +1,14 @@
+  [@jsuereth]: https://github.com/jsuereth
+
+  [1856]: https://github.com/sbt/sbt/issues/1856
+
+### Fixes with compatibility implications
+
+### Improvements
+
+### Bug fixes
+
+- Fixes a certain class of pom corruption that can occur in the presence of parent-poms.
+
+
+[#1856][1856] by [@jsuereth][@jsuereth]

sbt/src/sbt-test/dependency-management/pom-parent-pom/build.sbt

@@ -10,6 +10,8 @@ resolvers +=
 libraryDependencies +=
   "com.example" % "example-child" % "1.0-SNAPSHOT"
 
+libraryDependencies += "org.apache.geronimo.specs" % "geronimo-jta_1.1_spec" % "1.1.1"
+
 version := "1.0-SNAPSHOT"
 
 
@@ -20,3 +22,18 @@ cleanExampleCache := {
       IO.delete(cacheDir / "com.example")
     }
 }
+
+val checkIvyXml = taskKey[Unit]("Checks the ivy.xml transform was correct")
+
+checkIvyXml := {
+  ivySbt.value.withIvy(streams.value.log) { ivy =>
+    val cacheDir = ivy.getSettings.getDefaultRepositoryCacheBasedir
+    // TODO - Is this actually ok?
+    val xmlFile  =
+      cacheDir / "org.apache.geronimo.specs" / "geronimo-jta_1.1_spec" / "ivy-1.1.1.xml"
+      //cacheDir / "com.example" / "example-child" / "ivy-1.0-SNAPSHOT.xml"
+    val lines = IO.read(xmlFile)
+    if(lines.isEmpty) sys.error(s"Unable to read $xmlFile, could not resolve geronimo...")
+    assert(lines contains "xmlns:e", s"Failed to appropriately modify ivy.xml file for sbt extra attributes!\n$lines")
+  }
+}

sbt/src/sbt-test/dependency-management/pom-parent-pom/test

@@ -1,2 +1,3 @@
 > cleanExampleCache
 > update
+> checkIvyXml