Vivo 1642 dependency updates #109
Conversation
There was a problem hiding this comment.
Changes to the jsp and servlet APIs could break support for versions of Tomcat without making a conscious decision to.
These APIs are not being bundled, but are provided by the container, and we should NOT be updating them unless we are making a change to the minimum supported versions.
| <artifactId>jsp-api</artifactId> | ||
| <version>2.0</version> | ||
| <version>2.2</version> | ||
| <scope>provided</scope> |
There was a problem hiding this comment.
"Provided" APIs - such as JSP and Servlet spec should NOT be updated, unless we are making a conscious decision about the versions of Tomcat that we are supporting, otherwise there is a serious possibility that we could inadvertently break our support.
There was a problem hiding this comment.
Thanks, @grahamtriggs . It looks like Tomcat 5.5 expects JSP 2.0:
http://tomcat.apache.org/whichversion.html
Tomcat 5.5 was EOL'd in 2012:
https://tomcat.apache.org/tomcat-55-eol.html
Tomcat 7 supports JSP 2.2. The latest release of Tomcat is 9. It seems safe to consciously require Tomcat 7 as a minimal requirement for VIVO. Thoughts?
There was a problem hiding this comment.
The 3.1 version of the servlet spec requires Tomcat 8 if I'm understanding http://tomcat.apache.org/whichversion.html correctly. Which I'm fine with as a requirement, though it's perhaps a bit controversial since Tomcat 7 EOL has not been announced.
This PR updates the version from 2.5 to 3.1.0 in installer/webapp/pom.xml, but it looks like the dependency in Vitro/api/pom.xml is already referencing 3.1.0? What's that mean?
There was a problem hiding this comment.
@gneissone : It seems like we should limit our commitment to Tomcat 7, per:
https://jira.duraspace.org/browse/VIVO-1375
..meaning, updating the servlet-spec dependency to 3.0.
Thoughts, @grahamtriggs ?
There was a problem hiding this comment.
Even going back to 1.7.x, the documentation stated Tomcat 7.x in the installation guide.
https://wiki.duraspace.org/display/VIVOv17x/A+simple+installation
So the intention in upgrading the versions in the previous commit was to not exceed the documented version, just to enable features that we hadn't taken advantage of.
I'm not sure whether it was a misreading of the table or a typo (3.1.0 vs 3.0.1) that has ended up with 3.1.0 in the pom.xml.
There are 4 files (3 are test stubs) that have overrides to satisfy definitions in the Servlet 3.1 spec that would have to be adjusted in order to correct the version to 3.0.1.
None of these are actually used though, and the web.xml specifies version 3.0 - so a built application does still deploy and run on Tomcat 7 even having been built against 3.1.0 (and this has been tested in a virtual Ubuntu 14.04 environment).
So we haven't shipped anything that breaks the documented versions, but they should be aligned.
As noted, it's a small change to a few files to bring it back to Servlet 3.0.1. Equally, I'm happy to consider updating our requirements to Tomcat 8.x. There has been a stable release of 8.x / 8.5.x for 5 years now, and as of April, there won't be a supported version of Ubuntu that only has Tomcat 7 in the repositories. And a lot of people install Tomcat manually anyway.
Plus Tomcat 8 / Servlet 3.1 is prerequisite for some of the newer frameworks like Spring WebFlux and full reactive stacks.
There was a problem hiding this comment.
I second the idea of updating the VIVO requirements to Tomcat 8.x.
@grahamtriggs : Is there anything else that needs to be done in this PR to realize that?
..noting that the documentation will need to be updated:
https://wiki.duraspace.org/display/VIVODOC111x/System+Requirements#SystemRequirements-TomcatTomcat7orlater
There was a problem hiding this comment.
If we are updating to Tomcat 8.x / Servlet 3.1.0 (and I'm happy to see this), then we should align JSP API to version 2.3 (as per http://tomcat.apache.org/whichversion.html).
Highly unlikely that this will require any changes in the code, but I won't vouch for that without doing it.
|
This is being closed due to inactivity; might be reopened in the future. |
** Upgrade dependencies in Vitro **
VIVO-1642:
What does this pull request do?
Based on a pass through all the .pom files in Vitro and a comparison with what's available on https://mvnrepository.com, these changes bring all the dependency versions to the highest possible without breaking the build. This is half of the work for card VIVO-1642, the other half being upgrading the dependencies in VIVO itself. Another PR will be made to the develop branch of the VIVO repository when that work is complete.
How should this be tested?
Ensure the build still works, tests pass, and the VIVO app deploys with the updated Vitro dependencies.
Interested parties
@awoods