-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Enhanced version.sh and version.bat Output #919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| eval "\"$_RUNJAVA\"" "$JAVA_OPTS" \ | ||
| -classpath "\"$CATALINA_HOME/lib/catalina.jar\"" \ | ||
| -classpath "\"$CATALINA_HOME/bin/tomcat-juli.jar:$CATALINA_HOME/lib/*\"" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes also the boot classpath...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on why you think that? The change was made in the version command's classpath which doesn't affect tomcat startup since the start command uses $CLASSPATH. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you didn't miss anything. I expect that this change was deliberate. Ideally that would inlude rather the classpath from catalina.properties to be consistent. But if you think this is enough, I am fine with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I could use that I guess. I only added what was needed for the new logic though. Let's leave this open and see if others have an opinion since you're neutral. Thanks for the feedback.
…d third-party library versions
cea11af to
6c55a57
Compare
|
Force pushed to fix checkstyle failings since validate is off by default :( |
| TCN_RECOMMENDED_MAJOR + "." + TCN_RECOMMENDED_MINOR + "." + TCN_RECOMMENDED_PV; | ||
| } | ||
| return null; | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe logging a warning here to determine where the exception came from is helpful, but theoretically it should never happen.
| if (JreCompat.isJre22Available()) { | ||
| try { | ||
| Class<?> openSSLLibraryClass = | ||
| Class.forName("org.apache.tomcat.util.net.openssl.panama.OpenSSLLibrary"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I consider making the forName and getMethod calls use a variable rather than a string to make it easier to update if the classes or packages change? Or are these internal APIs generally not changed?
| return false; | ||
| } | ||
|
|
||
| private static String getJarVersion(File jarFile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could probably optimize this method a bit and use a loop rather than the two ifs to make it look a bit nicer. Thoughts?
Change Summary
This changeset enhances the output
version.shandversion.batto display comprehensive dependency and library version information that is available to tomcat for use (read: not configured in the server.xml), making it easier to verify which versions are available without the potentially costly action of grepping the logs after the fact. The update allows a single command to retrieve all the important version information a user may need without starting tomcat. The output is also an automation friendly parseable output for scripts and monitoring tools, and allows for a quick inventory of libraries that could be used by the instance.I've added six public methods total across three classes to expose version info and reduce the need for as much reflection, introduced 14 new tests which covers all of the changes in ServerInfo, and ensured that the change is compatible with Java 8+ and ready for backporting all the way to Tomcat 9.0.x with no changes required.
Files Modified
bin/catalina.shandbin/catalina.batversioncommand to include:tomcat-juli.jar(for logging support)$CATALINA_HOME/lib/*(for APR/FFM classes)-Dcatalina.homeand-Dcatalina.basesystem properties to have access to third-party jarsversioncommand, not normal startupjava/org/apache/catalina/core/AprLifecycleListener.javagetInstalledTcnVersion()- Returns Tomcat Native version stringgetInstalledAprVersion()- Returns APR version stringgetInstalledOpenSslVersion()- Returns OpenSSL version string (via APR)getTcnVersionWarning()- Returns version warning if installed version is outdatedjava/org/apache/catalina/core/OpenSSLLifecycleListener.javagetInstalledOpenSslVersion()to expose FFM OpenSSL versionOpenSSLLibrary.getVersionString()for native version string to avoid compile time depsjava/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.javagetVersionString()to expose native OpenSSL version stringjava/org/apache/catalina/util/ServerInfo.javagetConstructor().newInstance()instead of Java 9+getDeclaredConstructor()for Java 8 compattest/org/apache/catalina/util/TestServerInfo.javaisTomcatCoreJar())getJarVersion())captureServerInfoOutput()to eliminate test code duplicationConsumer<Manifest>for test JAR creation (Java 8 compatible)Assume.assumeTrue()to skip gracefully when native libraries unavailablewebapps/docs/changelog.xmlExample Output
Before:
After:
New Information in Output
APR and Tomcat Native Detection (when available)
FFM OpenSSL Detection (when available)
Third-Party Libraries
lib/