Skip to content

Build combined#6328

Merged
jburel merged 10 commits intoome:developfrom
jburel:build_combined
Aug 18, 2022
Merged

Build combined#6328
jburel merged 10 commits intoome:developfrom
jburel:build_combined

Conversation

@jburel
Copy link
Copy Markdown
Member

@jburel jburel commented Jun 28, 2022

This PR builds openmicroscopy

  • with and without ZarrReader (SNAPSHOT)
  • with and without Bio-Formats (SNAPSHOT)

An error currently occurs with Bio-Formats (SNAPSHOT). The problem is not there with the release version
@dgault is looking into it

@dgault
Copy link
Copy Markdown
Member

dgault commented Jun 29, 2022

This failure looks to be caused by the fact that the ZarrReader is not being built whilst using a SNAPSHOT version and therefore it is unable to locate its underlying dependencies

@jburel
Copy link
Copy Markdown
Member Author

jburel commented Jun 29, 2022

The idea is to be able to build BF only and/or ZarrReader only and not create a dependency between the 2

@dgault
Copy link
Copy Markdown
Member

dgault commented Jun 29, 2022

If Ivy has a dependency for a SNAPSHOT version for ZarrReader then you are going have to build it. Using a deployed release version should be ok however.

@jburel
Copy link
Copy Markdown
Member Author

jburel commented Jun 30, 2022

@dgault the failure happens with the combination BF-SNAPSHOT+ ZarrReader 0.3.0
The combinations

  • BF 6.10.0 + ZarrReader 0.3.0 works
  • BF 6.10.0 + ZarrReader SNAPSHOT works

@dgault
Copy link
Copy Markdown
Member

dgault commented Jul 4, 2022

An ome-poi PR has been opened to bump the commons-logging version which should then match across the stack. This will require a release of the poi component and formats-gpl.

The other remaining discrepancies between BF and OMERO dependencies are as follows:
org.slf4j:slf4j-api:jar
BF: 1.7.6
OMERO: 1.7.30

We can bump slf4j in BF to match 1.7.30 (see ome/bioformats#3844)

org.apache.httpcomponents:httpclient:jar
BF: 4.5.9
OMERO: 4.5.6

commons-codec:commons-codec:jar
BF: 1.11
OMERO: 1.12

Both of these components in BF come from cdm-core:

+- ome:formats-gpl:jar:6.10.0:compile
|  +- edu.ucar:cdm-core:jar:5.3.3:compile
|  |  +- edu.ucar:httpservices:jar:5.3.3:compile
|  |  |  +- org.apache.httpcomponents:httpclient:jar:4.5.9:compile
|  |  |  |  \- commons-codec:commons-codec:jar:1.11:compile

In OMERO the ivy dependency resolution looks like:
httpclient

Organisation Name Revision In Configurations Asked Revision
com.google.http-client google-http-client 1.20.0 compile, runtime 4.0.1
org.openmicroscopy omero-blitz 5.5.12 default, compile, runtime, master 4.5.6
edu.ucar httpservices 5.3.3 compile, runtime 4.5.9
org.apache.calcite.avatica avatica-core 1.15.0 compile, runtime 4.5.6
org.apache.httpcomponents httpmime 4.5.6 compile, runtime 4.5.6

commons-codec

Organisation Name Revision In Configurations Asked Revision
org.apache.httpcomponents httpclient 4.5.6 compile, runtime 1.12
org.apache.solr solr-core 3.1.0 compile, runtime 1.4
org.apache.calcite calcite-core 1.20.0 compile, runtime 1.12

com.fasterxml.jackson.core:jackson-annotations:jar
com.fasterxml.jackson.core:jackson-core:jar
com.fasterxml.jackson.core:jackson-databind:jar
BF: 2.9.6
OMERO: 2.9.8

In BF these components are coming from minio:

+- org.openmicroscopy:ome-common:jar:6.0.9:compile
|  +- io.minio:minio:jar:5.0.2:compile
|  |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.9.6:compile
|  |  +- com.fasterxml.jackson.core:jackson-core:jar:2.9.6:compile
|  |  \- com.fasterxml.jackson.core:jackson-databind:jar:2.9.6:compile

In OMERO the ivy dependency resolution looks like:
jackson-core

Organisation Name Revision In Configurations Asked Revision
io.minio minio 5.0.2 null, compile, runtime 2.9.6
com.fasterxml.jackson.dataformat jackson-dataformat-yaml 2.9.8 compile, runtime 2.9.8
com.fasterxml.jackson.core jackson-databind 2.9.8 compile, runtime 2.9.8
org.apache.calcite calcite-core 1.20.0 compile, runtime 2.9.8
com.esri.geometry esri-geometry-api 2.2.0 compile, runtime 2.9.8
org.apache.calcite.avatica avatica-core 1.15.0 compile, runtime 2.9.8
com.fasterxml.jackson.core jackson-databind 2.9.6 compile, runtime 2.9.6

jackson-databind

Organisation Name Revision In Configurations Asked Revision
io.minio minio 5.0.2 null, compile, runtime 2.9.6
org.apache.calcite calcite-core 1.20.0 compile, runtime 2.9.8
org.apache.calcite.avatica avatica-core 1.15.0 compile, runtime 2.9.8

jackson-annotations

Organisation Name Revision In Configurations Asked Revision
io.minio minio 5.0.2 null, compile, runtime 2.9.6
com.fasterxml.jackson.core jackson-databind 2.9.8 compile, runtime 2.9.8
org.apache.calcite calcite-core 1.20.0 compile, runtime 2.9.8
org.apache.calcite.avatica avatica-core 1.15.0 compile, runtime 2.9.8
com.fasterxml.jackson.core jackson-databind 2.9.6 compile, runtime 2.9.0

@sbesson
Copy link
Copy Markdown
Member

sbesson commented Jul 5, 2022

As per https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.9.7, I think upgrading jackson-databind to 2.9.8 at least would make sense

Copy link
Copy Markdown
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to see that the combined build can now pass without requiring a chain of low-level components release. A few questions:

  • the main donwside of declaring ome-common is that it's another version to track while it is defined transitively as a dependency of omero-blitz/omero-gateway. Is that something that would be done as part of every omero-blitz/omero-gateway bump as long as a new version is available?
  • there are at least two locations where dependencies are declared ivy.xml and components/tools/OMEROJava/ivy.xml. I am confused and unclear on the role of each file especially as omero-blitz/omero-gateway are declared in both places. Should OMEZarrReader also be declared in components/tools/OMEROJava/ivy.xml and would that suffice to solve the issue?

@jburel
Copy link
Copy Markdown
Member Author

jburel commented Jul 28, 2022

An alternative I have considered is only to inject it as part of the GHA build since it is usually not required during a standard build since it comes via blitz/gateway.

@jburel
Copy link
Copy Markdown
Member Author

jburel commented Aug 16, 2022

@sbesson the last set of commits

  • removes the change in ivy file
  • modifies the ivy file in the github action.
    This allows us to test the matrix without having a new dependency to handle in etc/omero.properties

Copy link
Copy Markdown
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, 👍 for being able to test the cross-repository workflow via GitHub Actions build without any modification to the source files used for release

A few inline questions

Comment thread .github/workflows/source_build.yml Outdated
Comment thread .github/workflows/source_build.yml
- name: Set Bio-Formats version
if: matrix.build_bf
run: |
DEPENDENCY="<dependency org=\"org.openmicroscopy\" name=\"ome-common\" rev=\"${{ steps.bf.outputs.ome_common_version }}\">\n<artifact name=\"ome-common\" type=\"jar\" ext=\"jar\"\/>\n<\/dependency>"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember it was used during the investigation but what is the rationale for including the ome-common dependency?

Copy link
Copy Markdown
Member Author

@jburel jburel Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When building using snapshot, the version of ome-common does not get picked up for some reasons.

@jburel jburel merged commit f585ab1 into ome:develop Aug 18, 2022
@jburel jburel deleted the build_combined branch March 10, 2023 19:58
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.

3 participants