kitodo-production
kitodo-production copied to clipboard
Relocate dependencies
Relocate some XML based dependencies as needed for switch to Java 17 and Jakarta support without changing existing code.
@thomaslow would you mind taking a look at this? I know you said you would not have time to do reviews for Kitodo at the moment we last talked, but since this is a smaller PR concerning a topic you have already invested some time investigating I think you'd be the best suited for this and perhaps might do an exception?
Hi Henning, Arved asked me to review your pull request, because I previously experimented with upgrading Kitodo-Production to Java 17 and Tomcat 10. I do have a preliminary branch that allows to start Kitodo-Production with Java 17 and Tomcat 10 (untested unit and integration tests). However, I mixed changes related to Java 17 and Tomcat 10. I don't know which of those changes can be applied safely for Java 17 while keeping backwards compatibility to Java 11.
Anyway, I found this Eclipse reference that lists all Maven Package changes to the Jakarta-Namespace. Maybe you find that helpful.
@thomaslow : thanks for your input and your link which should be helpful. This pull request is independent of the Java 11-> 17 and Tomcat 9 -> 10 issue but is part of it.
My approach was the following:
step one: update / relocate all the dependencies which can be moved to the new groupId and / or arctifactId where the development of the dependencies is going on which must be done time by time. step two: do the javax -> jakarta namespace renaming which is possible with Java 11. I don't know if this is possible with all the used dependencies or not step three: do the Java 11 -> 17 needed update including the needed dependency updates step four: do the Tomcat 9 -> 10 update
Step 3 and 4 can maybe switched but it is even possible that the javax to jakarta migration need even the Java 11 to 17 update if one or more dependencies made trouble.
I did myself an approach for the javax to jakarta migration (see https://github.com/kitodo/kitodo-production/compare/master...slub:kitodo-production:javax_to_jakarta) but I'm got some issues with Hibernate and I stopped the migration at this point as Matthias is doing the Hibernate Search integration right now and I don't want to made him more work to integrate my javax -> jakarta changes into his work.
Hi Henning, are you still working on these changes or do you force-push your commits to be up-to-date with master?
If I'm working on this this pull request would not visible or on draft mode. I'm only refreshing all my pull request after changes on the main / master branch to keep them up to date.
Thanks for hinting this out @thomaslow !
I will try to adjust this PR als dublicated / similar jar files are bad (jaxb-api) and if a dependency is already provided by an other this is not needed (even Maven is not using it).
@thomaslow : I updated my PR according to your suggestions. If one or both additional commits are not wanted I would remove them.
Your exclusion rule for javax.xml.bind:jaxb-api:2.3.1
works for me. Now, the final war file only contains the newer jakarta jar.
I'm not sure whether the explicit pom dependency declaration for jakarta.xml.bind-api:2.3.3
should be removed or not. What is Kitodo's general strategy here? Removing it means that there is one less dependency declaration, meaning pom files would be more compact and there would be one less specific artifact version, which has to be maintained. On the other hand, the source code explicitly imports classes from this package. Because of that, I think it would generally be a good idea to keep this dependency in pom files, even though it would not be strictly necessary due to the transitive dependency via glassfish or hibernate.
The recommended strategy of the official maven-dependency-plugin
is that all artifacts whose classes are explicitly imported in source code should also be listed as dependencies in the pom file, even if they are included transitively via other artifacts. The corresponding maven command mvn dependency:analyze
reports theses missing dependencies declarations as warnings, now also including jakarta.xml.bind-api:2.3.3
, amongst many other similar warnings.
@solth What do you think?
Updating org.jvnet.jaxb
to version 2.0.9
including the replacement to use the commons-lang for generating the equals
and hash
methods will reduce the error counting in CodeQL for this generated classes as this new generation way will generate Javadoc for this methods. I discovered this a few days ago. So this pull request did not only adjust the used dependencies it will reduce the CodeQL discovered issues.
Your exclusion rule for
javax.xml.bind:jaxb-api:2.3.1
works for me. Now, the final war file only contains the newer jakarta jar.I'm not sure whether the explicit pom dependency declaration for
jakarta.xml.bind-api:2.3.3
should be removed or not. What is Kitodo's general strategy here? Removing it means that there is one less dependency declaration, meaning pom files would be more compact and there would be one less specific artifact version, which has to be maintained. On the other hand, the source code explicitly imports classes from this package. Because of that, I think it would generally be a good idea to keep this dependency in pom files, even though it would not be strictly necessary due to the transitive dependency via glassfish or hibernate.The recommended strategy of the official
maven-dependency-plugin
is that all artifacts whose classes are explicitly imported in source code should also be listed as dependencies in the pom file, even if they are included transitively via other artifacts. The corresponding maven commandmvn dependency:analyze
reports theses missing dependencies declarations as warnings, now also includingjakarta.xml.bind-api:2.3.3
, amongst many other similar warnings.@solth What do you think?
Thanks a lot for the explanation @thomaslow I understand that both approaches have their advantages (streamlining pom files vs. explicit dependencies for directly used packages and classes) and would agree with you that we should stick with keeping those dependencies in the pom file, especially when this is an officially recommended strategy.
Thanks a lot for the explanation @thomaslow I understand that both approaches have their advantages (streamlining pom files vs. explicit dependencies for directly used packages and classes) and would agree with you that we should stick with keeping those dependencies in the pom file, especially when this is an officially recommended strategy.
@solth : Did I understand you correct, that I should revert my commit https://github.com/kitodo/kitodo-production/pull/6020/commits/f46777d80d9ce4eadc919d1ef338f726832a44cb in total? If so then your commenting is obsolete or?
@solth : Did I understand you correct, that I should revert my commit f46777d in total? If so then your commenting is obsolete or?
@henning-gerhardt yes, please revert that commit.
@solth : Did I understand you correct, that I should revert my commit f46777d in total? If so then your commenting is obsolete or?
@henning-gerhardt yes, please revert that commit.
Done. I must rebase the branch in any way.