sboorlagadda commented on code in PR #7926:
URL: https://github.com/apache/geode/pull/7926#discussion_r2382867585
##########
geode-assembly/build.gradle:
##########
@@ -387,24 +408,78 @@ tasks.register('gfshDepsJar', Jar) {
}
}
+// Extract legacy Tomcat 6 jars needed only to satisfy Javadoc for old session
manager classes
+// (LifecycleSupport, SerializablePrincipal) without altering runtime
dependencies.
+def legacyTomcatDir = "$buildDir/legacyTomcat"
+tasks.register('extractLegacyTomcatForJavadoc') {
+ description = 'Extracts legacy Tomcat catalina jars for Javadoc symbol
resolution.'
+ outputs.dir(legacyTomcatDir)
+ dependsOn configurations.webServerTomcat6
+ doLast {
+ delete legacyTomcatDir
+ copy {
+ from { zipTree(configurations.webServerTomcat6.singleFile) }
+ // Include the full Tomcat 6 lib set so packages like
org.apache.catalina.ha.session
Review Comment:
Your PR description says you're "replacing earlier brittle workarounds
(class exclusions, legacy extraction reliance) with a stable classpath-only
symbol resolution strategy" - but the implementation still includes both legacy
JAR
extraction AND stub classes. This seems contradictory to the stated goal.
Core Issue
You're solving the same problem in two different ways simultaneously:
1. Creating minimal stub classes for Tomcat symbols
2. Extracting and including actual legacy Tomcat 6 JARs
3. Adding modern Tomcat 9 dependencies
This puts three different versions of the same classes on the Javadoc
classpath, which could lead to unpredictable symbol resolution depending on
classpath ordering.
Key Questions
1. Which approach do you actually need? If the stubs work, why keep the
legacy JAR extraction? If you need the legacy JARs, why create stubs?
2. Have you tested for classpath conflicts? What happens when
SerializablePrincipal exists in both your stub classes and the extracted Tomcat
6 JARs?
3. Why mix Tomcat versions? You're using Tomcat 9 dependencies for
"compatibility" but extracting Tomcat 6 JARs for "legacy symbols" - these APIs
may be incompatible.
Suggested Path Forward
Pick one approach and commit to it:
- Option A: Keep only the stub classes (remove legacy extraction) -
simpler and cleaner
- Option B: Keep only legacy extraction (remove stubs) - but this
contradicts your goal of eliminating "brittle workarounds"
The current dual approach increases complexity rather than reducing it,
which goes against your stated objective of creating a "stable" solution.
What was the specific reason for keeping both approaches? Were the stubs
insufficient on their own?
--
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]
