From 5758e0d786068c8edf8fadca876ee7323f404396 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 3 Jul 2017 13:08:59 +0200 Subject: [PATCH] Rework property substitution --- .../main/scala/coursier/core/Resolution.scala | 83 +++++++++++-------- tests/metadata | 2 +- .../org.drools/drools-compiler/7.0.0.Final | 13 +++ .../org.jitsi/jitsi-videobridge/1.0-SNAPSHOT | 2 +- .../resolutions/org.nd4j/nd4j-native/0.5.0 | 3 +- .../scala/coursier/test/CentralTests.scala | 7 ++ .../scala/coursier/test/ResolutionTests.scala | 4 +- .../test/scala/coursier/test/package.scala | 27 ++++++ 8 files changed, 100 insertions(+), 41 deletions(-) create mode 100644 tests/shared/src/test/resources/resolutions/org.drools/drools-compiler/7.0.0.Final diff --git a/core/shared/src/main/scala/coursier/core/Resolution.scala b/core/shared/src/main/scala/coursier/core/Resolution.scala index c45646005..5030f171a 100644 --- a/core/shared/src/main/scala/coursier/core/Resolution.scala +++ b/core/shared/src/main/scala/coursier/core/Resolution.scala @@ -117,12 +117,9 @@ object Resolution { } } + @deprecated("Originally intended for internal use only", "1.0.0-RC7") def propertiesMap(props: Seq[(String, String)]): Map[String, String] = - props.foldLeft(Map.empty[String, String]) { - case (acc, (k, v0)) => - val v = substituteProps(v0, acc) - acc + (k -> v) - } + substitute(props).toMap /** * Substitutes `properties` in `dependencies`. @@ -361,26 +358,25 @@ object Resolution { def projectProperties(project: Project): Seq[(String, String)] = { // vague attempt at recovering the POM packaging tag - def packagingOpt = project.publications.collectFirst { + val packagingOpt = project.publications.collectFirst { case ("compile", pub) => pub.`type` } // FIXME The extra properties should only be added for Maven projects, not Ivy ones - val properties0 = Seq( + val properties0 = project.properties ++ Seq( // some artifacts seem to require these (e.g. org.jmock:jmock-legacy:2.5.1) // although I can find no mention of them in any manual / spec "pom.groupId" -> project.module.organization, "pom.artifactId" -> project.module.name, - "pom.version" -> project.version, + "pom.version" -> project.actualVersion, // Required by some dependencies too (org.apache.directory.shared:shared-ldap:0.9.19 in particular) "groupId" -> project.module.organization, "artifactId" -> project.module.name, - "version" -> project.version - ) ++ project.properties ++ Seq( + "version" -> project.actualVersion, "project.groupId" -> project.module.organization, "project.artifactId" -> project.module.name, - "project.version" -> project.version + "project.version" -> project.actualVersion ) ++ packagingOpt.toSeq.map { packaging => "project.packaging" -> packaging } ++ project.parent.toSeq.flatMap { @@ -395,6 +391,11 @@ object Resolution { // loose attempt at substituting properties in each others in properties0 // doesn't try to go recursive for now, but that could be made so if necessary + substitute(properties0) + } + + private def substitute(properties0: Seq[(String, String)]): Seq[(String, String)] = { + val done = properties0 .collect { case kv @ (_, value) if propRegex.findFirstIn(value).isEmpty => @@ -402,10 +403,20 @@ object Resolution { } .toMap - properties0.map { + var didSubstitutions = false + + val res = properties0.map { case (k, v) => - k -> substituteProps(v, done) + val res = substituteProps(v, done) + if (!didSubstitutions) + didSubstitutions = res != v + k -> res } + + if (didSubstitutions) + substitute(res) + else + res } /** @@ -422,7 +433,7 @@ object Resolution { // section numbers in the comments refer to withDependencyManagement - val properties = propertiesMap(projectProperties(project)) + val properties = project.properties.toMap val (actualConfig, configurations) = withParentConfigurations(from.configuration, project.configurations) @@ -791,13 +802,13 @@ final case class Resolution( project.parent.toSet else { - val approxProperties0 = - project.parent - .flatMap(projectCache.get) - .map(_._2.properties) - .fold(project.properties)(project.properties ++ _) + val parentProperties0 = project + .parent + .flatMap(projectCache.get) + .map(_._2.properties.toMap) + .getOrElse(Map()) - val approxProperties = propertiesMap(approxProperties0) ++ projectProperties(project) + val approxProperties = parentProperties0 ++ projectProperties(project) val profileDependencies = profiles( @@ -862,6 +873,11 @@ final case class Resolution( ) } + private def withFinalProperties(project: Project): Project = + project.copy( + properties = projectProperties(project) + ) + /** * Add dependency management / inheritance related items to `project`, * from what's available in cache. @@ -903,14 +919,14 @@ final case class Resolution( // A bit fragile, but seems to work - val approxProperties0 = - project.parent - .filter(projectCache.contains) - .map(projectCache(_)._2.properties) - .fold(project.properties)(_ ++ project.properties) + val parentProperties0 = project + .parent + .flatMap(projectCache.get) + .map(_._2.properties) + .getOrElse(Seq()) // 1.1 (see above) - val approxProperties = propertiesMap(approxProperties0) ++ projectProperties(project) + val approxProperties = parentProperties0.toMap ++ projectProperties(project) val profiles0 = profiles( project, @@ -923,18 +939,13 @@ final case class Resolution( // 1.2 made from Pom.scala (TODO look at the very details?) // 1.3 & 1.4 (if only vaguely so) - val properties0 = - (project.properties /: profiles0) { (acc, p) => - acc ++ p.properties - } - - val project0 = project.copy( - properties = project.parent // belongs to 1.5 & 1.6 - .flatMap(projectCache.get) - .fold(properties0)(_._2.properties ++ properties0) + val project0 = withFinalProperties( + project.copy( + properties = parentProperties0 ++ project.properties ++ profiles0.flatMap(_.properties) // belongs to 1.5 & 1.6 + ) ) - val propertiesMap0 = propertiesMap(projectProperties(project0)) + val propertiesMap0 = project0.properties.toMap val dependencies0 = addDependencies( (project0.dependencies +: profiles0.map(_.dependencies)).map(withProperties(_, propertiesMap0)) diff --git a/tests/metadata b/tests/metadata index ba7f6c18c..9a00b31af 160000 --- a/tests/metadata +++ b/tests/metadata @@ -1 +1 @@ -Subproject commit ba7f6c18c146c992cc4b3cddc40f0687a4d1e349 +Subproject commit 9a00b31af466454fbf2604e871c273ba9c1c9938 diff --git a/tests/shared/src/test/resources/resolutions/org.drools/drools-compiler/7.0.0.Final b/tests/shared/src/test/resources/resolutions/org.drools/drools-compiler/7.0.0.Final new file mode 100644 index 000000000..68b39a241 --- /dev/null +++ b/tests/shared/src/test/resources/resolutions/org.drools/drools-compiler/7.0.0.Final @@ -0,0 +1,13 @@ +com.google.protobuf:protobuf-java:2.6.0:compile +com.thoughtworks.xstream:xstream:1.4.9:compile +commons-codec:commons-codec:1.10:compile +org.antlr:antlr-runtime:3.5:compile +org.drools:drools-compiler:7.0.0.Final:compile +org.drools:drools-core:7.0.0.Final:compile +org.eclipse.jdt.core.compiler:ecj:4.4.2:compile +org.kie:kie-api:7.0.0.Final:compile +org.kie:kie-internal:7.0.0.Final:compile +org.mvel:mvel2:2.3.0.Final:compile +org.slf4j:slf4j-api:1.7.7:compile +xmlpull:xmlpull:1.1.3.1:compile +xpp3:xpp3_min:1.1.4c:compile \ No newline at end of file diff --git a/tests/shared/src/test/resources/resolutions/org.jitsi/jitsi-videobridge/1.0-SNAPSHOT b/tests/shared/src/test/resources/resolutions/org.jitsi/jitsi-videobridge/1.0-SNAPSHOT index 6e4462a68..9ed368f25 100644 --- a/tests/shared/src/test/resources/resolutions/org.jitsi/jitsi-videobridge/1.0-SNAPSHOT +++ b/tests/shared/src/test/resources/resolutions/org.jitsi/jitsi-videobridge/1.0-SNAPSHOT @@ -57,7 +57,7 @@ org.fusesource:sigar:1.6.4:compile org.gnu.inet:libidn:1.15:compile org.hamcrest:hamcrest-core:1.1:compile org.igniterealtime:tinder:1.2.3:compile -org.igniterealtime.smack:smack:3.2.2-jitsi-2-20170425.180418-3:compile +org.igniterealtime.smack:smack:3.2.2-jitsi-2-20170425.180420-2:compile org.igniterealtime.smack:smackx:3.2.2-jitsi-2-20170425.180420-2:compile org.igniterealtime.whack:core:2.0.0:compile org.jitsi:bccontrib:1.0:compile diff --git a/tests/shared/src/test/resources/resolutions/org.nd4j/nd4j-native/0.5.0 b/tests/shared/src/test/resources/resolutions/org.nd4j/nd4j-native/0.5.0 index 83dc7191a..67b3009c0 100644 --- a/tests/shared/src/test/resources/resolutions/org.nd4j/nd4j-native/0.5.0 +++ b/tests/shared/src/test/resources/resolutions/org.nd4j/nd4j-native/0.5.0 @@ -6,6 +6,7 @@ org.apache.commons:commons-math3:3.4.1:compile org.bytedeco:javacpp:1.2.3:compile org.javassist:javassist:3.18.2-GA:compile org.nd4j:nd4j-api:0.5.0:compile +org.nd4j:nd4j-backend-impls:0.5.0:compile org.nd4j:nd4j-buffer:0.5.0:compile org.nd4j:nd4j-common:0.5.0:compile org.nd4j:nd4j-context:0.5.0:compile @@ -13,4 +14,4 @@ org.nd4j:nd4j-native:0.5.0:compile org.nd4j:nd4j-native-api:0.5.0:compile org.projectlombok:lombok:1.16.4:compile org.reflections:reflections:0.9.10:compile -org.slf4j:slf4j-api:1.7.10:compile +org.slf4j:slf4j-api:1.7.10:compile \ No newline at end of file diff --git a/tests/shared/src/test/scala/coursier/test/CentralTests.scala b/tests/shared/src/test/scala/coursier/test/CentralTests.scala index 0fae3072e..cd29d7c91 100644 --- a/tests/shared/src/test/scala/coursier/test/CentralTests.scala +++ b/tests/shared/src/test/scala/coursier/test/CentralTests.scala @@ -340,6 +340,13 @@ abstract class CentralTests extends TestSuite { ) } + 'propertySubstitution - { + resolutionCheck( + Module("org.drools", "drools-compiler"), + "7.0.0.Final" + ) + } + 'artifactIdProperties - { resolutionCheck( Module("cc.factorie", "factorie_2.11"), diff --git a/tests/shared/src/test/scala/coursier/test/ResolutionTests.scala b/tests/shared/src/test/scala/coursier/test/ResolutionTests.scala index ebf24abf6..0f5d2e028 100644 --- a/tests/shared/src/test/scala/coursier/test/ResolutionTests.scala +++ b/tests/shared/src/test/scala/coursier/test/ResolutionTests.scala @@ -259,7 +259,7 @@ object ResolutionTests extends TestSuite { val dep = Dependency(Module("acme", "config"), "1.3.0") val res = await(resolve0( Set(dep) - )).clearFinalDependenciesCache + )).clearFinalDependenciesCache.clearProjectProperties val expected = Resolution( rootDependencies = Set(dep), @@ -276,7 +276,7 @@ object ResolutionTests extends TestSuite { val trDep = Dependency(Module("acme", "play-json"), "2.4.0") val res = await(resolve0( Set(dep) - )).clearFinalDependenciesCache + )).clearFinalDependenciesCache.clearProjectProperties val expected = Resolution( rootDependencies = Set(dep), diff --git a/tests/shared/src/test/scala/coursier/test/package.scala b/tests/shared/src/test/scala/coursier/test/package.scala index 75c32ae88..302ae7eb2 100644 --- a/tests/shared/src/test/scala/coursier/test/package.scala +++ b/tests/shared/src/test/scala/coursier/test/package.scala @@ -6,6 +6,22 @@ package object test { def withCompileScope: Dependency = underlying.copy(configuration = "compile") } + private val projectProperties = Set( + "pom.groupId", + "pom.artifactId", + "pom.version", + "groupId", + "artifactId", + "version", + "project.groupId", + "project.artifactId", + "project.version", + "project.packaging", + "project.parent.groupId", + "project.parent.artifactId", + "project.parent.version" + ) + implicit class ResolutionOps(val underlying: Resolution) extends AnyVal { // The content of these fields is typically not validated in the tests. @@ -22,6 +38,17 @@ package object test { ) def clearFilter: Resolution = underlying.copy(filter = None) + def clearProjectProperties: Resolution = + underlying.copy( + projectCache = underlying + .projectCache + .mapValues { + case (s, p) => + (s, p.copy(properties = p.properties.filter { case (k, _) => !projectProperties(k) })) + } + .iterator + .toMap + ) } object Profile {