Skip to content

Update to the newest stable release of kryo#50

Merged
sbesson merged 2 commits into
ome:masterfrom
ctrueden:update-kryo
Jul 24, 2020
Merged

Update to the newest stable release of kryo#50
sbesson merged 2 commits into
ome:masterfrom
ctrueden:update-kryo

Conversation

@ctrueden

@ctrueden ctrueden commented May 8, 2020

Copy link
Copy Markdown
Member

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 added 2 commits May 8, 2020 17:43
This eliminates the following warning from the Maven CLI tool:

  [WARNING] The project org.openmicroscopy:ome-common:jar:6.0.3-SNAPSHOT
  uses prerequisites which is only intended for maven-plugin projects
  but not for non maven-plugin projects. For such purposes you should
  use the maven-enforcer-plugin. See
  https://maven.apache.org/enforcer/enforcer-rules/requireMavenVersion.html
@ctrueden

ctrueden commented May 8, 2020

Copy link
Copy Markdown
Member Author

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

ctrueden added a commit to scijava/pom-scijava that referenced this pull request May 8, 2020
ctrueden added a commit to scijava/pom-scijava that referenced this pull request May 8, 2020
ctrueden added a commit to scijava/pom-scijava that referenced this pull request May 8, 2020
ctrueden added a commit to scijava/pom-scijava that referenced this pull request May 8, 2020
@sbesson sbesson mentioned this pull request May 9, 2020
@sbesson sbesson closed this May 11, 2020
@sbesson sbesson reopened this May 11, 2020
@sbesson

sbesson commented May 11, 2020

Copy link
Copy Markdown
Member

@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

ctrueden commented May 11, 2020

Copy link
Copy Markdown
Member Author

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

sbesson commented May 11, 2020

Copy link
Copy Markdown
Member

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

ctrueden commented May 11, 2020

Copy link
Copy Markdown
Member Author

@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.

ctrueden added a commit to scijava/pom-scijava that referenced this pull request May 11, 2020
ctrueden added a commit to scijava/pom-scijava that referenced this pull request May 11, 2020
ctrueden added a commit to scijava/pom-scijava that referenced this pull request May 23, 2020
@sbesson sbesson merged commit 1ef1a53 into ome:master Jul 24, 2020
@ctrueden ctrueden deleted the update-kryo branch July 24, 2020 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants