lukecwik commented on code in PR #24264:
URL: https://github.com/apache/beam/pull/24264#discussion_r1028456273


##########
sdks/java/harness/build.gradle:
##########
@@ -18,35 +18,44 @@
 
 plugins { id 'org.apache.beam.module' }
 
-// We specifically enumerate all the projects that we depend on since
-// the list is used in both defining the included set for the uber jar
-// and also the set of project level dependencies.
-def dependOnShadedProjects = [":model:pipeline", ":model:fn-execution", 
":sdks:java:core"]
-def dependOnProjects = [":sdks:java:fn-execution",
-                        ":runners:core-java", 
":runners:core-construction-java"]
+configurations {
+  provided
+}
+
+dependencies {
+  // provided dependencies are declared early so they can be resolved in 
shadowClosure
+  provided project(path: ":sdks:java:core", configuration: "shadow")
+}
 
 applyJavaNature(
   classesTriggerCheckerBugs: [
     'AssignWindowsRunner': 
'https://github.com/typetools/checker-framework/issues/3794',
     'WindowMergingFnRunner': 
'https://github.com/typetools/checker-framework/issues/3794',
   ],
   automaticModuleName: 'org.apache.beam.fn.harness',
-  validateShadowJar: false,
   testShadowJar: true,
+  shadowJarValidationExcludes: [
+    "io/github/classgraph/**",
+    "nonapi/io/github/classgraph/**",
+    "org/apache/beam/fn/**",
+    "org/apache/beam/model/**",
+    "org/apache/beam/runners/**",
+    "org/apache/beam/sdk/**",
+    "org/checkerframework/**",
+    "org/github/jamm/**",

Review Comment:
   I think jamm should be provided as it needs to be on the boot agent classpath



##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1022,6 +1022,50 @@ class BeamModulePlugin implements Plugin<Project> {
         }
       }
 
+      // Ban these dependencies from all configurations
+      project.configurations.all {

Review Comment:
   Why did you need/want to move these up to here?



##########
sdks/java/harness/build.gradle:
##########
@@ -18,35 +18,44 @@
 
 plugins { id 'org.apache.beam.module' }
 
-// We specifically enumerate all the projects that we depend on since
-// the list is used in both defining the included set for the uber jar
-// and also the set of project level dependencies.
-def dependOnShadedProjects = [":model:pipeline", ":model:fn-execution", 
":sdks:java:core"]
-def dependOnProjects = [":sdks:java:fn-execution",
-                        ":runners:core-java", 
":runners:core-construction-java"]
+configurations {
+  provided
+}
+
+dependencies {
+  // provided dependencies are declared early so they can be resolved in 
shadowClosure
+  provided project(path: ":sdks:java:core", configuration: "shadow")
+}
 
 applyJavaNature(
   classesTriggerCheckerBugs: [
     'AssignWindowsRunner': 
'https://github.com/typetools/checker-framework/issues/3794',
     'WindowMergingFnRunner': 
'https://github.com/typetools/checker-framework/issues/3794',
   ],
   automaticModuleName: 'org.apache.beam.fn.harness',
-  validateShadowJar: false,
   testShadowJar: true,
+  shadowJarValidationExcludes: [
+    "io/github/classgraph/**",
+    "nonapi/io/github/classgraph/**",
+    "org/apache/beam/fn/**",
+    "org/apache/beam/model/**",
+    "org/apache/beam/runners/**",
+    "org/apache/beam/sdk/**",

Review Comment:
   can this be narrowed down to `org/apache/beam/sdk/fn/**`?



##########
sdks/java/harness/build.gradle:
##########
@@ -55,21 +64,19 @@ description = "Apache Beam :: SDKs :: Java :: Harness"
 ext.summary = "This contains the SDK Fn Harness for Beam Java"
 
 dependencies {
-  dependOnShadedProjects.each {
-    implementation project(path: it, configuration: "shadow")
-  }
-  dependOnProjects.each {
-    implementation project(it)
-  }
-  shadow library.java.vendored_guava_26_0_jre
+  implementation project(path: ":model:fn-execution", configuration: "shadow")
+  implementation project(path: ":model:pipeline", configuration: "shadow")

Review Comment:
   I would expect this to be provided



##########
sdks/java/harness/build.gradle:
##########
@@ -18,35 +18,44 @@
 
 plugins { id 'org.apache.beam.module' }
 
-// We specifically enumerate all the projects that we depend on since
-// the list is used in both defining the included set for the uber jar
-// and also the set of project level dependencies.
-def dependOnShadedProjects = [":model:pipeline", ":model:fn-execution", 
":sdks:java:core"]
-def dependOnProjects = [":sdks:java:fn-execution",
-                        ":runners:core-java", 
":runners:core-construction-java"]
+configurations {
+  provided
+}
+
+dependencies {
+  // provided dependencies are declared early so they can be resolved in 
shadowClosure
+  provided project(path: ":sdks:java:core", configuration: "shadow")
+}
 
 applyJavaNature(
   classesTriggerCheckerBugs: [
     'AssignWindowsRunner': 
'https://github.com/typetools/checker-framework/issues/3794',
     'WindowMergingFnRunner': 
'https://github.com/typetools/checker-framework/issues/3794',
   ],
   automaticModuleName: 'org.apache.beam.fn.harness',
-  validateShadowJar: false,
   testShadowJar: true,
+  shadowJarValidationExcludes: [
+    "io/github/classgraph/**",
+    "nonapi/io/github/classgraph/**",
+    "org/apache/beam/fn/**",
+    "org/apache/beam/model/**",
+    "org/apache/beam/runners/**",

Review Comment:
   Can this be narrowed down to the two underlying packages



##########
sdks/java/harness/build.gradle:
##########
@@ -55,21 +64,19 @@ description = "Apache Beam :: SDKs :: Java :: Harness"
 ext.summary = "This contains the SDK Fn Harness for Beam Java"
 
 dependencies {
-  dependOnShadedProjects.each {
-    implementation project(path: it, configuration: "shadow")
-  }
-  dependOnProjects.each {
-    implementation project(it)
-  }
-  shadow library.java.vendored_guava_26_0_jre
+  implementation project(path: ":model:fn-execution", configuration: "shadow")
+  implementation project(path: ":model:pipeline", configuration: "shadow")
+  implementation project(":runners:core-construction-java")
+  implementation project(":runners:core-java")
+  implementation project(":sdks:java:fn-execution")

Review Comment:
   I would expect this to be provided



##########
sdks/java/harness/build.gradle:
##########
@@ -18,35 +18,44 @@
 
 plugins { id 'org.apache.beam.module' }
 
-// We specifically enumerate all the projects that we depend on since
-// the list is used in both defining the included set for the uber jar
-// and also the set of project level dependencies.
-def dependOnShadedProjects = [":model:pipeline", ":model:fn-execution", 
":sdks:java:core"]
-def dependOnProjects = [":sdks:java:fn-execution",
-                        ":runners:core-java", 
":runners:core-construction-java"]
+configurations {
+  provided
+}
+
+dependencies {
+  // provided dependencies are declared early so they can be resolved in 
shadowClosure
+  provided project(path: ":sdks:java:core", configuration: "shadow")
+}
 
 applyJavaNature(
   classesTriggerCheckerBugs: [
     'AssignWindowsRunner': 
'https://github.com/typetools/checker-framework/issues/3794',
     'WindowMergingFnRunner': 
'https://github.com/typetools/checker-framework/issues/3794',
   ],
   automaticModuleName: 'org.apache.beam.fn.harness',
-  validateShadowJar: false,
   testShadowJar: true,
+  shadowJarValidationExcludes: [
+    "io/github/classgraph/**",
+    "nonapi/io/github/classgraph/**",
+    "org/apache/beam/fn/**",

Review Comment:
   Where does this one come from?



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to