Upgrade tomcat version to 9.0.115#1327
Upgrade tomcat version to 9.0.115#1327manodyaSenevirathne wants to merge 4 commits intowso2:masterfrom
Conversation
WalkthroughAdds multiple new Maven POMs (11) and two META-INF service provider entries to produce OSGi bundle artifacts: Tomcat 9.0.115.wso2v1 (core, JDK21 variants, EL/JSP/Servlet APIs, Catalina HA), plus Jaeger and Zipkin reporter bundles. Each POM configures bundle metadata, optional dependencies, export/import package rules, and distribution to WSO2 releases. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tomcat-jsp-api/9.0.115.wso2v1/pom.xml (1)
52-52: Consider aligningmaven-bundle-pluginversions across Tomcat 9.0.115.wso2v1 modules.Line 52 uses version
2.3.6, but most other new Tomcat modules use2.4.0. Specifically:tomcat-catalina-ha,tomcat-jsp-api-jdk21, andtomcat-jsp-apiuse2.3.6, while six other modules (tomcat-el-api,tomcat-el-api-jdk21,tomcat-jdk21,tomcat-servlet-api,tomcat-servlet-api-jdk21, andtomcat) use2.4.0. Aligning to a single version will reduce inconsistency in manifest generation across related artifacts.Proposed change (if unintentional)
- <version>2.3.6</version> + <version>2.4.0</version>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tomcat-jsp-api/9.0.115.wso2v1/pom.xml` at line 52, The pom.xml for tomcat-jsp-api currently pins maven-bundle-plugin to <version>2.3.6</version>, causing a mismatch with other modules; update the maven-bundle-plugin version in this POM to 2.4.0 to match the other Tomcat 9.0.115.wso2v1 modules (ensure the <plugin> element referencing maven-bundle-plugin is the one you edit), then run a quick build of modules like tomcat-jsp-api, tomcat-jsp-api-jdk21 and tomcat-catalina-ha to confirm consistent manifest generation across maven-bundle-plugin usage.tomcat-catalina-ha/9.0.115.wso2v1/pom.xml (1)
52-52: Updatemaven-bundle-pluginversion to align with the majority of 9.0.115 modules.This module uses
2.3.6while six out of nine 9.0.115.wso2v1 modules use2.4.0. Althoughtomcat-jsp-apiandtomcat-jsp-api-jdk21also use2.3.6, adopting the prevalent version2.4.0reduces the risk of subtle manifest-generation inconsistencies across orbit bundles.Suggested change
- <version>2.3.6</version> + <version>2.4.0</version>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tomcat-catalina-ha/9.0.115.wso2v1/pom.xml` at line 52, Update the maven-bundle-plugin version element currently set to 2.3.6 to 2.4.0 to match the majority of 9.0.115.wso2v1 modules; locate the <version> element for the maven-bundle-plugin declaration in the POM (the tag containing "maven-bundle-plugin" and its <version> child) and change its value to 2.4.0 so manifest generation is consistent with other modules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tomcat-jdk21/9.0.115.wso2v1/pom.xml`:
- Around line 138-140: The POM pins ECJ to an old incompatible version via the
property version.ecj (currently 4.6.1); update that property to a modern ECJ
release that supports Java 21 (for example set version.ecj to 3.26.0 which is
the Tomcat 9.0.115 baseline) so JSP/ECJ compilation will work under JDK 21 and
align with Tomcat's expected dependency.
---
Nitpick comments:
In `@tomcat-catalina-ha/9.0.115.wso2v1/pom.xml`:
- Line 52: Update the maven-bundle-plugin version element currently set to 2.3.6
to 2.4.0 to match the majority of 9.0.115.wso2v1 modules; locate the <version>
element for the maven-bundle-plugin declaration in the POM (the tag containing
"maven-bundle-plugin" and its <version> child) and change its value to 2.4.0 so
manifest generation is consistent with other modules.
In `@tomcat-jsp-api/9.0.115.wso2v1/pom.xml`:
- Line 52: The pom.xml for tomcat-jsp-api currently pins maven-bundle-plugin to
<version>2.3.6</version>, causing a mismatch with other modules; update the
maven-bundle-plugin version in this POM to 2.4.0 to match the other Tomcat
9.0.115.wso2v1 modules (ensure the <plugin> element referencing
maven-bundle-plugin is the one you edit), then run a quick build of modules like
tomcat-jsp-api, tomcat-jsp-api-jdk21 and tomcat-catalina-ha to confirm
consistent manifest generation across maven-bundle-plugin usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0bf57b41-04c1-4ebf-aaee-360a477fc268
📒 Files selected for processing (11)
tomcat-catalina-ha/9.0.115.wso2v1/pom.xmltomcat-el-api-jdk21/9.0.115.wso2v1/pom.xmltomcat-el-api/9.0.115.wso2v1/pom.xmltomcat-jdk21/9.0.115.wso2v1/pom.xmltomcat-jdk21/9.0.115.wso2v1/src/main/resources/META-INF/services/javax.el.ExpressionFactorytomcat-jsp-api-jdk21/9.0.115.wso2v1/pom.xmltomcat-jsp-api/9.0.115.wso2v1/pom.xmltomcat-servlet-api-jdk21/9.0.115.wso2v1/pom.xmltomcat-servlet-api/9.0.115.wso2v1/pom.xmltomcat/9.0.115.wso2v1/pom.xmltomcat/9.0.115.wso2v1/src/main/resources/META-INF/services/javax.el.ExpressionFactory
| <properties> | ||
| <version.tomcat>9.0.115</version.tomcat> | ||
| <version.ecj>4.6.1</version.ecj> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ECJ 4.6.1 Java version support compatibility JDK
💡 Result:
- Running
ecj-4.6.1.jar(the standalone Eclipse Compiler for Java): the published Maven POM for ECJ 4.6.1 setsmaven.compiler.source/targetto 1.7, meaning the JAR is built for Java 7 bytecode → you need JRE/JDK 7+ to run it. [1] - What Java it can compile (language level): ECJ 4.6.1 corresponds to the Eclipse Neon era and is generally used for up to Java 8 source/target; Java 9 support is not provided by Neon-era tooling (Java 9 support comes with later Eclipse trains). [2]
- Note (batch compiler defaults/constraints): Eclipse’s batch-compiler docs note an implicit default of
-source 1.6 -target 1.6, and that annotation processing support requires a 1.6 VM. [3]
Sources:
[1] Sonatype/Maven Central page showing ECJ 4.6.1 POM snippet (maven.compiler.source/target=1.7). (central.sonatype.com)
[2] Stack Overflow: Eclipse Neon vs Java 9 support (points to needing later Eclipse versions for Java 9). (stackoverflow.com)
[3] Eclipse Help: “Using the batch compiler” (implicit -source/-target 1.6, annotation processing needs 1.6 VM). (help.eclipse.org)
Citations:
- 1: https://central.sonatype.com/artifact/org.eclipse.jdt.core.compiler/ecj/4.6.1
- 2: https://stackoverflow.com/questions/48108155/does-eclipse-neon-support-java-9?utm_source=openai
- 3: https://help.eclipse.org/latest/rtopic/org.eclipse.jdt.doc.user/tasks/task-using_batch_compiler.htm?utm_source=openai
🌐 Web query:
Apache Tomcat 9.0.115 ECJ version dependency
💡 Result:
For Apache Tomcat 9.0.115, the Jasper JSP compiler (tomcat-jasper) has a Maven dependency on:
- ECJ artifact:
org.eclipse.jdt:ecj - ECJ version:
3.26.0(compile dependency) [1]
If you’re looking at the Tomcat binary distribution (the lib/ folder), Tomcat 9 commonly ships an ECJ JAR named ecj-4.20.jar (optional, used when you configure Jasper to use ECJ instead of javac) [2].
Sources:
[1] https://mvnrepository.com/artifact/org.apache.tomcat/tomcat-jasper/9.0.115/dependencies
[2] https://stackoverflow.com/questions/79415289/org-apache-jasper-jasperexception-unable-to-compile-class-for-jsp-facing-this
🏁 Script executed:
# Search for Tomcat 9.0.115 official pom.xml to see what ecj version it uses
curl -s "https://repo.maven.apache.org/maven2/org/apache/tomcat/tomcat/9.0.115/tomcat-9.0.115.pom" | grep -A 5 -B 5 "ecj" || echo "Direct fetch failed, will try alternative"Repository: wso2/orbit
Length of output: 95
🏁 Script executed:
# Check Maven Central for ECJ 4.6.1 release info
curl -s "https://repo1.maven.org/maven2/org/eclipse/jdt/core/compiler/ecj/4.6.1/" | grep -i "release\|date\|version" | head -20Repository: wso2/orbit
Length of output: 36
ECJ 4.6.1 cannot compile code for JDK 21.
Line 140 pins ecj to 4.6.1, which only supports Java 8 language features and lacks Java 9+ support. This will cause JSP compilation failures with JDK 21. Additionally, Tomcat 9.0.115 officially uses ECJ 3.26.0 (a much newer version), so this pinning moves backward from the baseline dependency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tomcat-jdk21/9.0.115.wso2v1/pom.xml` around lines 138 - 140, The POM pins ECJ
to an old incompatible version via the property version.ecj (currently 4.6.1);
update that property to a modern ECJ release that supports Java 21 (for example
set version.ecj to 3.26.0 which is the Tomcat 9.0.115 baseline) so JSP/ECJ
compilation will work under JDK 21 and align with Tomcat's expected dependency.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
zipkin-sender-okhttp3/3.5.1.wso2v2/pom.xml (1)
75-82: Remove the trailing wildcard import fromImport-Package.
*;resolution:=optionalbroadens imports and breaks the established Orbit POM convention in this repo. Keep only explicit imports/negations here.Suggested change
<Import-Package> okhttp3;version="[3.11,5)", okio;version="[1.15,4)", zipkin2;version="[2.27,3)", zipkin2.codec;version="[2.27,3)", - !zipkin2.reporter.*, - *;resolution:=optional + !zipkin2.reporter.* </Import-Package>Based on learnings: Ensure that in Maven bundle POMs (maven-bundle-plugin Felix configuration), the
Import-Packagesection omits the trailing wildcard*as per establishedwso2/orbitconvention.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zipkin-sender-okhttp3/3.5.1.wso2v2/pom.xml` around lines 75 - 82, The Import-Package section currently ends with a wildcard entry "*;resolution:=optional" which contravenes the Orbit POM convention and overly broadens imports; edit the pom's Import-Package list to remove the trailing wildcard entry and only keep the explicit package entries and negations (e.g., retain okhttp3, okio, zipkin2, zipkin2.codec and the "!zipkin2.reporter.*" negation) so that Import-Package contains no "*;resolution:=optional" wildcard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@zipkin-sender-okhttp3/3.5.1.wso2v2/pom.xml`:
- Around line 75-82: The Import-Package section currently ends with a wildcard
entry "*;resolution:=optional" which contravenes the Orbit POM convention and
overly broadens imports; edit the pom's Import-Package list to remove the
trailing wildcard entry and only keep the explicit package entries and negations
(e.g., retain okhttp3, okio, zipkin2, zipkin2.codec and the
"!zipkin2.reporter.*" negation) so that Import-Package contains no
"*;resolution:=optional" wildcard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b0fd19c-4da7-4f5f-aa7b-c4a2bc16327f
📒 Files selected for processing (2)
jaeger-client-jdk21/1.8.0.wso2v2/pom.xmlzipkin-sender-okhttp3/3.5.1.wso2v2/pom.xml
1 participant
Purpose
Summary by CodeRabbit