Community
Participate
Working Groups
Created attachment 187797 [details] Patch to support the use of two loader jars for Tomcat 7.0 Due to some recent changes to Tomcat 7's FileDirContext class (the addition of "static" to some inner class declarations inspired by FindBugs), the current loader jar the WTP Tomcat adapter adds to Tomcat will not work with the upcoming Tomcat 7.0.7 and later. When you try to start the 7.0.7 server, the WTPDirContext class in the loader jar with encounter a NoSuchMethodException when it tries to access some inner classes that are now "static". To fix, the loader jar source needs to be recompiled against Tomcat 7.0.7 classes. This means that one loader jar can't work with both Tomcat 7.0.6, and earlier, and Tomcat 7.0.7, and later. Tomcat 7.0.7 is likely to be released within a month, but it's not that uncommon for a version to introduce a new problem causing users to stick with an earlier version a while longer. Thus, it's not clear how much longer Tomcat 7.0.6 will be the primary Tomcat 7.0 version in use. To maintain a positive user experience with this feature, the best approach would be to use two loader jars for Tomcat 7.0 and copy the appropriate version to the Tomcat server based on it's version. Since the changes were based on FindBugs suggestions, and not due to any effort to modify how the FileDirContext class works, I don't expect further changes to Tomcat 7.0.7 to again break the "Serve modules without publishing" feature.
Created attachment 187798 [details] Loader jar compiled against a locally built Tomcat 7.0.7.
Created attachment 187799 [details] Copy of the prior loader jar renamed for use with Tomcat 7.0.6 and earlier
Angel, I would like to fix this in WTP 3.2.3 so this feature will work with the current Tomcat 7.0.6 and continue working whenever Tomcat 7.0.7 is released. In case you agree, here is the PMC info: 1. Explain why you believe this is a stop-ship defect. Though not a problem at this moment, I've confirmed with a local development build of Tomcat 7.0.7 that the "Serve modules without publishing" feature will be broken when users try to use this version, once it is released. The Tomcat 7.0.7 server will fail to start when this feature is enabled. 2. Is there a work-around? If so, why do you believe the work-around is insufficient? There is no practical work-around, other than not using Tomcat 7.0.7 or later if the user wants to use this feature. How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? I have tested these changes in my development workspace and confirmed that the "Serve modules without publishing" feature functions correctly with Tomcat 7.0.5, 7.0.6, and the locally built 7.0.7. No JUnit attached since catching this problem involves testing with developmental versions of Tomcat. Give a brief technical overview. Who has reviewed this fix? The addition of "static" to some inner class declarations in a Tomcat class has caused it to be binary incompatible between Tomcat 7.0.6 and Tomcat 7.0.7. This class is used by the loader jar that the Tomcat adapter added to the Tomcat server to make this feature work. To work, the jar added to the server needs to have been compiled against the version of that class that Tomcat will be using. As part of Bug 333102, special behavior was implemented based on the Tomcat version for Tomcat 7.0 in support of this feature. These changes expand that behavior to include coping one of two versions of the loader jar added to Tomcat based on the tomcat version. At this moment, no one else has reviewed the patch. What is the risk associated with this fix? The risk is minimal and isolated to this feature.
Larry, For getLoaderJarFile(), it looks like you could create some sort of utility method to parse out the string version. One idea is to parse the version number ahead of time, and put it in an array from there you can just check the element number in the array, whether is the minor or major version. The code as it is seems a little hard to read.
Hi Angel, The internal Tomcat version strings have been known to not get set correctly in Tomcat releases from time to time (usually by a "substitute" release manager). They usually get the "<major>.<minor>." right, but what follows can vary. Version string for 7.0.6 is okay, which is the first "stable" version. I know "7.0.5" is okay too, and assume the other earlier "beta" version will have version strings that are okay. If they are not, I'm not worrying about it. Thus, I'm simply trying to detect if the Tomcat 7.0 is 7.0.6 or earlier, regardless of what the version string might contain in the future. Anything not determined to be Tomcat 7.0.6 or earlier is assumed to be newer. As long as the version string for Tomcat 7.0.7 doesn't go out as "7.0.6" I should be okay. :) For the moment, this detection is only needed to determine which loader jar should be used, so I didn't bother to put it in it's own "utility" method. If you think it would help, I could put the parsing code in an isTomcat706OrEarlier() method. Hopefully, I won't ever need to parse out the full version for general use since it's hard to know exactly what I will see in the future. Larry
Comment on attachment 187797 [details] Patch to support the use of two loader jars for Tomcat 7.0 I would prefer we make the code a bit more generic, and perhaps put it as a utility to make the code easier to read, but the code works and if we don't think we can use it somewhere else, then it might just overweight the need for changes.
Approve. Looks like isolated to the Tomcat server adapter. It does not touch any code in the WTP Server Tools framework. We should allow the Tomcat server adapter to adapt to any changes in the Tomcat Server. With regards to the above, I think it is a good idea to have the Tomcat server adapter as a separate project entity that is independent of the WTP life cycle. Like the many other server adapters for open source and proprietary servers do.
Changes released to WTP 3.2.3. I'll delay releasing to HEAD until WTP 3.3 M5 is offically released.
New Gerrit change created: https://git.eclipse.org/r/109023
