[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #3099: [GOBBLIN-1256]Exclude log4j related jars in compile but include those in test

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

autumnust commented on a change in pull request #3099:
URL: https://github.com/apache/incubator-gobblin/pull/3099#discussion_r487298251



##########
File path: gradle/scripts/globalDependencies.gradle
##########
@@ -35,20 +36,36 @@ subprojects {
           compile(externalDependency.guava) {
               force = true
           }
-      }
-          compile(externalDependency.commonsCodec) {
-            force = true
-          }
+        }
+        compile(externalDependency.commonsCodec) {
+          force = true
+        }
+        //Since testCompile inherit from compile, so we cannot use testCompile to import dependency for log4j
+        customTestCompile externalDependency.log4j
+        customTestCompile externalDependency.slf4jToLog4j
 
         // Required to add JDK's tool jar, which is required to run byteman tests.
         testCompile (files(((URLClassLoader) ToolProvider.getSystemToolClassLoader()).getURLs()))
       }
+      if (!project.name.contains('gobblin-aws')) {
+        configurations.compile.dependencies*.each {
+          //exclude this jar because we introduce log4j-over-slf4j, these two jars cannot present at the same time
+          it.exclude group: 'org.slf4j', module: 'slf4j-log4j12'
+          //exclude log4j related jars to provide a clean log environment
+          it.exclude group: 'log4j', module: 'log4j'
+          it.exclude group: 'log4j', module: 'apache-log4j-extras'
+        }
+      }
       all*.exclude group: 'org.apache.calcite', module: 'calcite-avatica' // replaced by org.apache.calcite.avatica:avatica-core
-      //exclude this jar because we introduce log4j-over-slf4j, these two jars cannot present at the same time
-      compileClasspath.exclude group: 'org.slf4j', module: 'slf4j-log4j12'

Review comment:
       I might miss something obvious here: why this line (used for exclusion) is no longer required? 




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