Repository navigation
Conversation
* still SNAPSHOT versions for now
mkitti
left a comment
There was a problem hiding this comment.
Downstream check of this PR against the HDF5 2.2.0 work in janelia-jhdf5, on macOS arm64.
It works once janelia-jhdf5 loads its own native library. Built against janelia-jhdf5 with JaneliaSciComp/janelia-jhdf5#13 and #14 (installed locally as 22.03.01-SNAPSHOT) and org.bytedeco:hdf5:2.2.0-1.5.15-SNAPSHOT, mvn test passes 47 tests with 5 skipped. That's identical to master (cbabb1a) with cisd:jhdf5.
Depends on janelia-jhdf5 #13. Against the janelia-jhdf5 stack without #13, 40 of 47 tests error with UnsatisfiedLinkError: 'int hdf.hdf5lib.H5.H5dont_atexit()', because nothing in janelia-jhdf5's main code loaded hdf5_java. #13 fixes that, and #14 fixes the case where an enum such as CharacterEncoding or HDF5DataClass is touched before a factory.
Before this can merge:
- Snapshot dependencies. pom-scijava's
RequireReproducibleBuildsenforcer rule fails onorg.scijava:janelia-jhdf5:22.03.01-SNAPSHOT. The local check used-Denforcer.skip=true. This needs releases of janelia-jhdf5 and of the bytedeco presets. - CI can't resolve the snapshots. The janelia-jhdf5 snapshot isn't published anywhere, and the
org.bytedecosnapshots live athttps://central.sonatype.com/repository/maven-snapshots/, which isn't among this project's repositories. Fine for a draft. - Natives on other platforms. janelia-jhdf5's published pom currently brings only
macosx-arm64natives. JaneliaSciComp/janelia-jhdf5#11 switches it tohdf5-platform; until then, this would fail on Linux, Windows and Intel Macs. - Windows additionally needs bytedeco/javacpp-presets#1814: without it, the two HDF5 Java bindings load separate HDF5 libraries on Windows.
Small suggestions:
- slf4j:
slf4j-apiis only needed becauseorg.bytedeco:hdf5leaves it optional whilehdf.hdf5lib.H5requires it. That's better declared once in janelia-jhdf5 as a regular dependency, so each consumer doesn't have to. Keeping it here meanwhile is fine. - Version overrides: the
javacpp.version/hdf5.versionoverrides must stay in step with whatever janelia-jhdf5 is built against. They can be dropped once pom-scijava manages those versions.
🤖 Generated with Claude Code
n5-hdf5 is the first consumer of janelia-jhdf5 and exercises its public API from a separate project, including loading the native library from library code (JaneliaSciComp#13/JaneliaSciComp#14), which this project's own tests can't check. The new job installs the build, checks out n5-hdf5's janelia-jhdf5 branch (saalfeldlab/n5-hdf5#50) and runs its tests against that version, on the same OS/Java matrix as the main tests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Suggestions to make this draft build in CI while it waits on the releases. These mirror what we did for the other repos in the migration to janelia-jhdf5 (e.g. fiji/HDF5_Vibez#22, bigdataviewer/bigdataviewer-core#228), so the temporary plumbing looks the same everywhere and is easy to find and remove later. 1. <repositories>
<!-- Temporary: org.bytedeco snapshots (javacpp, hdf5) needed by janelia-jhdf5 until they are released. -->
<repository>
<id>central-portal-snapshots</id>
<url>https://central.sonatype.com/repository/maven-snapshots/</url>
<releases><enabled>false</enabled></releases>
<snapshots><enabled>true</enabled></snapshots>
</repository>
</repositories>(If the project already declares 2. # Temporary: janelia-jhdf5 22.03.01-SNAPSHOT is not published anywhere, so build and
# install it into ~/.m2 first. Remove once janelia-jhdf5 is released.
- name: Check out janelia-jhdf5
uses: actions/checkout@v4
with:
repository: JaneliaSciComp/janelia-jhdf5
ref: main
path: janelia-jhdf5
- name: Install janelia-jhdf5 snapshot
working-directory: janelia-jhdf5
run: mvn -B -U install -DskipTests -Djacoco.skip=true -Djavacpp.platform=linux-x86_64
shell: bashand on the build step: env:
# pom-scijava's enforcer rejects SNAPSHOT dependencies.
MAVEN_ARGS: -Denforcer.skip=true
3. Downstream note: since n5-hdf5 currently brings If it helps, I can push these changes to a branch on 🤖 Generated with Claude Code |
waiting on bytedeco/javacpp-presets#1808 and a new release of https://github.com/JaneliaSciComp/janelia-jhdf5/