search

ome / ome-common-java

Java library providing common functionality for all OME Java components
Other
2 stars 19 forks source link

Update to the newest stable release of kryo #50

Closed ctrueden closed 4 years ago

ctrueden commented 4 years ago

From 2.24.0 to 3.0.0, the kryo project changed its groupId from com.esotericsoftware.kryo to com.esotericsoftware. Therefore, from Maven's perspective, both of these artifacts can be present on the classpath at the same time, creating duplicate class conflicts. Unfortunately, this scenario is happening when attempting to combine org.openmicroscopy:ome-common:6.0.4 (which wants kryo 2.24.0) with graphics.scenery:scenery:0.7.0-beta-7 (which wants kryo 4.0.2).

One solution to this dilemma is to update the version of kryo here to 4.0.2, which is what this PR does. The code still compiles, and tests pass, although I am not certain how thorough (if at all) the kryo-related functionality is exercised.

What do you think? Is this a good way forward?

ctrueden commented 4 years ago

Hmm, the CI failures don't appear related to this change. If there is anything I can do to help, let me know.

sbesson commented 4 years ago

@ctrueden thanks for opening this discussion. The Travis issue you reported above was indeed independent and got fixed separately.

We briefly discussed this contribution and how to review it with @ome/formats and @ome/omero. The kryo dependency is crucial at the level of OMERO where initialized Bio-Formats readers are serialized on disk. Due to the deepness of the stack, testing dependency upgrades have revealed to be quite involved in the past.

Immediately, this PR will be included into our next daily CI builds so that we can have a first assessment of its impact across all Bio-Formats readers as well as in a deployed OMERO server.

ctrueden commented 4 years ago

Thanks for pursuing it, @sbesson. I appreciate the importance of testing changes like this carefully. For the time being, I updated pom-scijava to ignore overlap in the com.esotericsoftware classes. This merely hides the problem, not solving it—but at least it enables people to ostensibly combine dependencies relying on both old and new kryo at the same time.

sbesson commented 4 years ago

Unfortunately, while testing a combined build of all Bio-Formats components, I saw that the old 2.24.0 version was still pulled. This is because the kryo dependency is independently declared in Bio-Formats

https://github.com/ome/bioformats/blob/ed992ebc2cb07d04fcb49485540af0babd1baa05/pom.xml#L52 https://github.com/ome/bioformats/blob/ed992ebc2cb07d04fcb49485540af0babd1baa05/components/formats-bsd/pom.xml#L71-L75

and as per the Maven dependency resolution mechanism, it will override the version defined in ome-common.

ctrueden commented 4 years ago

@sbesson I filed ome/bioformats#3557 updating kryo to 4.0.2 there as well.

as per the Maven dependency resolution mechanism, it will override the version defined in ome-common.

No, it won't, because the groupId changed. So you'll get both versions of kryo on the classpath! The solution is to stamp out usage of the old kryo across the entire OME software stack.