Fixing geoserver dependencies calculation (#3459) #3460
Conversation
|
I don't feel very comfortable with removing the parent to georchestra root pom ... Maybe forcing the problematic dependencies in the gs/pom.xml should make it, FDYT @groldan ? |
See related issue for the context of this commit. Main proposal here, apart from fixing the versions required by GS upstream, is to unplug the geoserver-georchestra root pom from the georchestra root pom. Note: checking the dependencies against an official GS webapp from sourceforge, there are still some differences after build between geor & upstream: ``` upstream: commons-collections-3.2.2.jar geor: commons-collections-3.2.1.jar upstream: slf4j-api-1.6.4.jar geor: slf4j-api-1.7.21.jar upstream: spring-tx-5.1.20.RELEASE.jar geor: spring-tx-4.2.5.RELEASE.jar upstream: xml-apis-1.4.01.jar geor: xml-apis-1.3.04.jar ``` These differences goes beyond my maven understanding, as I cannot track down why they appear in these specific versions in our custom build. Also I cannot get why we have in the georchestra version junit as a dependency into the classpath, but it seems to be a dependency from gt-geojson.
|
I actually don't think that unplugging GS from the root pom is the right way here: It will require to copy/paste some things that was implicetely provided by the georchestra root pom (url to maven repos, some shared properties across modules ...) In more mid/long term, I think we should think about splitting the huge georchestra repositories into several different ones, to make integration of single components easier. |
partially reverts previous commit, reattaching the geoserver module to the georchestra root pom, but fixing the dependencies via the dependenciesManagement section of the geor-geoserver-root pom. Note: trying a build and comparing with an upstream GS 2.18.3 webapp, we still have a difference on hsqldb, but the mistmatched version comes from geotools transitively, so I don't quite get how the upstream version differs, given the fact we should use the same version of GT. At least the sqlite version which was problematic is now in line with upstream (See #3459)
pmauduit
force-pushed
the
fixing-gs-dependencies-calculation-3459
branch
from
72441cc
to
61b5051
Compare
|
rebased, changed strategy (reattaching to geor root pom), and forced pushed. |
|
console failing because in 20.1.x, some LDAP schemas introduced in master (but not yet in our branch) don't exist anymore. |
We need to stick to ldap:20.1.x docker image. Also using a case insensitive matcher for the test is acceptable, as the cn on the orgs ldap branch is a case-insensitive field.
|
Ok, one test stays a bit flacky (had to relaunch 3 times the CI, was able to reproduce locally 1 or 2 times) I cannot explain why, but @groldan : care to review it ? |
There was a problem hiding this comment.
forcing those couple dep versions seems easy enough, but it'll also add some maintenance burden, whenever you want to upgrade the gs version, you have to remember to repeat the whole process and amend, right?
I'd vote for the couple geor specific changes to the geoserver pom that you can just cherry-pick when upgrading to another version, instead of the maintenance burden of figuring out dep mismatches again and patching them in the pom.
That said, go for it, and treat it as a separate issue.
I suggest we go for this one, and go with unplugging the root pom into master. I'll prepare a PR for this. |
Following the approach suggested by @groldan here: #3460 (review) unplugging the georchestra root pom, and fixing the missing bits from the pom. Tests: using make for the different artifacts, analysing the classpath after compilation, looks closer to what an upstream GS from sourceforge bundles.
See issue #3459 for the context of this commit. Main proposal here, apart from fixing the versions required by GS upstream, is to unplug the geoserver-georchestra root pom from the georchestra root pom.
Note: checking the dependencies against an official GS webapp from sourceforge, there are still some differences after build between geor & upstream:
These differences go beyond my maven understanding (asking @groldan for help :)), as I cannot track down why they appear in these specific versions in our custom build.
Also I cannot get why we have in the georchestra version junit as a dependency into the classpath, but it seems to be a dependency from gt-geojson.