[GitHub] [incubator-gobblin] vikrambohra commented on a change in pull request #3022: Correct handling of 'special' dependencies in poms

[GitBox <gi...@apache.org>]

vikrambohra commented on a change in pull request #3022:
URL: https://github.com/apache/incubator-gobblin/pull/3022#discussion_r434000422



##########
File path: gobblin-all/build.gradle
##########
@@ -0,0 +1,12 @@
+description = """Artifact that depends on all other, directly consumable artifacts.
+Useful for resolving all Gobblin dependencies that customers need."""
+
+apply plugin: "java"
+
+dependencies {
+    //TODO: list all dependencies that are directly consumable
+    compile project(":gobblin-api")
+    compile project(":gobblin-rest-service:gobblin-rest-api")
+    compile project(path: ':gobblin-rest-service:gobblin-rest-api', configuration: 'restClient')
+    compile project(path: ':gobblin-rest-service:gobblin-rest-api', configuration: 'dataTemplate')
+}

Review comment:
       Please add all gobblin modules provided on slack

##########
File path: gradle/scripts/bintrayPublishing.gradle
##########
@@ -52,6 +52,42 @@ subprojects{
           artifact javadocJar
 
           pom pomAttributes
+
+          pom.withXml {
+            //Ensures that correct dependencies are in the pom when subproject declares a project dependency
+            // with specific target configuration (restClient, dataTemplate, etc.)
+            //Needed because pom model is lossy and does not carry the 'configuration' information
+            def dependenciesNode = it.asNode().dependencies[0]
+            def removed = [] as Set; def added = [] //helps auditing
+            configurations.runtime.allDependencies.each { d ->
+              def confToPom = ['restClient': 'rest-client', 'dataTemplate': 'data-template']
+              if (d instanceof ProjectDependency && confToPom.containsKey(d.targetConfiguration)) {
+                boolean dependsOnMainModule = configurations.runtime.allDependencies.any {
+                  it.name == d.name && it.targetConfiguration == 'default'
+                }
+                if (!dependsOnMainModule) {
+                  //subproject declares a dependency on target configuration (i.e. path: 'gobblin-rest-api', configuration: 'restClient')
+                  // but does not declare a dependency on the 'default' artifact (i.e. 'gobblin-rest-api')
+                  //we need to remove the 'default' artifact from the pom file
+                  def mainModuleNode = dependenciesNode.find { it.artifactId.text() == d.name }
+                  dependenciesNode.remove(mainModuleNode)
+                  removed.add(d.name)
+                }
+
+                //adding explicit dependency on the artifact that corresponds to given target configuration
+                // (i.e. 'gobblin-rest-api-rest-client')
+                def dependencyNode = dependenciesNode.appendNode('dependency')
+                dependencyNode.appendNode('groupId', d.group)
+                def newArtifactId = d.name + "-" + confToPom[d.targetConfiguration]
+                dependencyNode.appendNode('artifactId', newArtifactId)
+                dependencyNode.appendNode('version', d.version)
+                added.add(newArtifactId)
+              }
+            }
+            if (added || removed) {
+              logger.lifecycle("Updated pom dependencies in {}, added: {}, removed: {}", project.path, added, removed)
+            }
+          }

Review comment:
       Thanks for this. It looks clean. Can you verify if the generated pom also adds global dependency exclusions? (scripts/gradle/globalDependencies.gradle)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org