You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/09/13 21:17:57 UTC

[GitHub] [geode] kirklund opened a new pull request #6863: GEODE-9595: Exclude circular build dependencies

kirklund opened a new pull request #6863:
URL: https://github.com/apache/geode/pull/6863


   Fix circular dependencies that add geode-*module*.jars to the classpath
   of tests in:
   - geode-core
   - geode-cq
   - geode-gfsh
   - geode-maangement
   - geode-wan
   - geode-web-api
   
   The fix involves excluding the geode-*module* itself within dependency
   blocks on other modules that transitively have dependencies on the
   dependent geode-*module*.
   
   Example in geode-core:
   
       integrationTestImplementation(project(':geode-gfsh')) {
           exclude module: 'geode-core'
       }
       integrationTestImplementation(project(':geode-junit')) {
           exclude module: 'geode-core'
       }
       integrationTestImplementation(project(':geode-dunit')) {
           exclude module: 'geode-core'
       }
       integrationTestImplementation(project(':geode-log4j')) {
           exclude module: 'geode-core'
       }
   


-- 
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: notifications-unsubscribe@geode.apache.org

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



[GitHub] [geode] kirklund closed pull request #6863: GEODE-9595: Exclude circular integrationTest classpath dependencies

Posted by GitBox <gi...@apache.org>.
kirklund closed pull request #6863:
URL: https://github.com/apache/geode/pull/6863


   


-- 
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: notifications-unsubscribe@geode.apache.org

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



[GitHub] [geode] kirklund commented on pull request #6863: GEODE-9595: Exclude circular integrationTest classpath dependencies

Posted by GitBox <gi...@apache.org>.
kirklund commented on pull request #6863:
URL: https://github.com/apache/geode/pull/6863#issuecomment-919553219


   Replaced by #6867.


-- 
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: notifications-unsubscribe@geode.apache.org

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



[GitHub] [geode] kirklund commented on a change in pull request #6863: GEODE-9595: Exclude circular build dependencies

Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6863:
URL: https://github.com/apache/geode/pull/6863#discussion_r707777157



##########
File path: geode-core/build.gradle
##########
@@ -233,7 +233,9 @@ dependencies {
         exclude group: '*'
     }
 
-    runtimeOnly(project(':geode-deployment:geode-deployment-legacy'))
+    runtimeOnly(project(':geode-deployment:geode-deployment-legacy')) {
+        exclude module: 'geode-core'

Review comment:
       @rhoughton-pivot You should be pairing with Dale and me then :) This was hard to debug.
   
   I'm trying to back GEODE-9486. AnalyzeSerializables integration tests load the `sanctioned-geode-*-serializables.txt` resource and then generates a failure message to point the person running the test to overwrite the `sanctioned-geode-*-serializables.txt` with the generated actual serializables file. However, the error message was broken. After fixing it, it fails only in the modules that have these circular dependencies. We discovered that when the test runs (only in modules with circular dependencies) the classes and resources are loaded from the geode-*module*/build/libs/geode-*module*.jar which causes the failure message to blow up.
   
   How about we pair from 10am to noon tomorrow?




-- 
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: notifications-unsubscribe@geode.apache.org

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



[GitHub] [geode] rhoughton-pivot commented on a change in pull request #6863: GEODE-9595: Exclude circular build dependencies

Posted by GitBox <gi...@apache.org>.
rhoughton-pivot commented on a change in pull request #6863:
URL: https://github.com/apache/geode/pull/6863#discussion_r707767966



##########
File path: geode-wan/build.gradle
##########
@@ -62,7 +64,9 @@ dependencies {
 
   upgradeTestImplementation(project(':geode-gfsh'))
   upgradeTestImplementation(project(':geode-junit'))
-  upgradeTestImplementation(project(':geode-dunit'))
+  upgradeTestImplementation(project(':geode-dunit')) {

Review comment:
       geode-dunit does not bring in geode-wan

##########
File path: geode-web-api/build.gradle
##########
@@ -104,6 +104,7 @@ dependencies {
   }
   integrationTestImplementation(project(':geode-dunit')) {
     exclude module: 'geode-core'
+    exclude module: 'geode-web-api'

Review comment:
       geode-dunit does not bring in geode-web-api.

##########
File path: geode-core/build.gradle
##########
@@ -233,7 +233,9 @@ dependencies {
         exclude group: '*'
     }
 
-    runtimeOnly(project(':geode-deployment:geode-deployment-legacy'))
+    runtimeOnly(project(':geode-deployment:geode-deployment-legacy')) {
+        exclude module: 'geode-core'

Review comment:
       What is the actual problem being addressed by this change?
   
   `geode-core` needs `geode-deployment:geode-deployment-legacy`, which needs `geode-core` to exist on the classpath. At runtime, this will be true. Why not declare it?

##########
File path: geode-management/build.gradle
##########
@@ -49,7 +49,9 @@ dependencies {
   testCompileOnly(platform(project(':boms:geode-all-bom')))
   testCompileOnly('io.swagger:swagger-annotations')
 
-  integrationTestImplementation(project(':geode-dunit'))
+  integrationTestImplementation(project(':geode-dunit')) {

Review comment:
       geode-dunit needs geode-core, which needs geode-management. What is the actual concern in the testing here? 

##########
File path: geode-wan/build.gradle
##########
@@ -45,7 +45,9 @@ dependencies {
 
 
   distributedTestImplementation(project(':geode-gfsh'))
-  distributedTestImplementation(project(':geode-dunit'))
+  distributedTestImplementation(project(':geode-dunit')) {
+    exclude module: 'geode-wan'

Review comment:
       geode-dunit does not bring in geode-wan
   
   




-- 
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: notifications-unsubscribe@geode.apache.org

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



[GitHub] [geode] kirklund commented on a change in pull request #6863: GEODE-9595: Exclude circular build dependencies

Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6863:
URL: https://github.com/apache/geode/pull/6863#discussion_r707777157



##########
File path: geode-core/build.gradle
##########
@@ -233,7 +233,9 @@ dependencies {
         exclude group: '*'
     }
 
-    runtimeOnly(project(':geode-deployment:geode-deployment-legacy'))
+    runtimeOnly(project(':geode-deployment:geode-deployment-legacy')) {
+        exclude module: 'geode-core'

Review comment:
       You should be pairing with Dale and me then :) This was hard to debug.
   
   I'm trying to back GEODE-9486. AnalyzeSerializables integration tests load the `sanctioned-geode-*-serializables.txt` resource and then generates a failure message to point the person running the test to overwrite the `sanctioned-geode-*-serializables.txt` with the generated actual serializables file. However, the error message was broken. After fixing it, it fails only in the modules that have these circular dependencies. We discovered that when the test runs (only in modules with circular dependencies) the classes and resources are loaded from the geode-*module*/build/libs/geode-*module*.jar which causes the failure message to blow up.
   
   How about we pair from 10am to noon tomorrow?




-- 
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: notifications-unsubscribe@geode.apache.org

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