Add Java 25 FFM alternates #5927
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…upport - Added FFM/JNI-specific Maven presets for all platforms (GNUC, MSVC, Clang) - Integrated Maven deployment workflows (maven-deploy.yml, test-maven-deployment.yml) - Updated maven-staging.yml to support both FFM and JNI implementations - Updated CLAUDE.md documentation for FFM/JNI Maven workflows - Preserved FFM implementation logic while adding Maven deployment capabilities - Both FFM (hdf5-java-ffm) and JNI (hdf5-java-jni) artifacts can be built and deployed
- Added determine-matrix job to dynamically create build matrix based on inputs
- Replaced 4 separate build jobs (Linux, Windows, macOS x86_64, macOS aarch64) with single matrix-based job
- Matrix builds all platform/implementation combinations in parallel
- java_implementation=both now builds FFM and JNI truly in parallel via matrix
- Updated all downstream jobs to work with matrix artifacts
- Dynamic artifact naming: maven-staging-artifacts-{platform}-{arch}-{impl}
- Simplified artifact download with pattern matching
- All platforms (Linux/GNUC, Windows/MSVC, macOS/Clang) fully supported
- Reduced workflow file size by ~760 lines while adding parallel capability
- macOS platform names already include arch (macOS-x86_64, macOS-aarch64) - Updated artifact suffix logic to avoid duplication - Before: macos-x86_64-x86_64-ffm - After: macos-x86_64-ffm
- Remove dynamic output keys (not supported in matrix jobs) - Extract version from downloaded artifacts instead of job outputs - Update PR comment generation to work without version output - Fixes workflow syntax error preventing CI execution
- Replace non-existent output reference with job result check - Change from needs.build-maven-artifacts.outputs.artifacts-created - To needs.build-maven-artifacts.result == 'success' - Resolves workflow syntax validation error
- Handle push events properly (github.base_ref not available) - Enable Maven testing for all push events - Prevents workflow validation errors on push - Fixes immediate failures in CI runs
- Remove references to deleted jobs (build-maven-artifacts-windows, etc.) - Update needs to use only matrix-based build-maven-artifacts job - Add determine-matrix to dependencies - Resolves: Job depends on unknown job errors
Update validation script to accept hdf5-java-ffm and hdf5-java-jni artifact IDs in addition to hdf5-java. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove complex tarball creation/extraction steps inherited from develop-maven-upload branch. Use source directly like other workflows by checking out to 'hdf5' subdirectory. Removes 3 unnecessary steps: - Set file base name - Create source tarball - Extract source for build 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add 12 workflow presets corresponding to the FFM/JNI configure presets:
- ci-MinShar-{GNUC,MSVC,Clang}-Maven-{FFM,JNI}{,-Snapshot}
This allows using cmake --workflow --preset with FFM/JNI specific builds.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Remove 'path: hdf5' parameter from checkout action to use default behavior. This fixes CMakePresets.json not found error. The checkout action by default checks out to github.workspace which is the correct location for CMake to find CMakePresets.json. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Workflow presets require corresponding build and package presets.
Added 12 build presets and 12 package presets for FFM/JNI variants:
- ci-MinShar-{GNUC,MSVC,Clang}-Maven-{FFM,JNI}{,-Snapshot}
This completes the preset infrastructure for FFM/JNI workflows.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Make test-java-examples-maven job use the same dynamic matrix as build-maven-artifacts so it tests the artifacts that were actually built. This fixes artifact download failures by including the implementation suffix (-ffm, -jni) in the artifact name. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Quote all workspace path variables in bash scripts to handle Windows
backslash paths correctly. Fixes "No such file or directory" errors
when cd-ing to workspace on Windows runners.
Changes:
- Quote ${{ github.workspace }} in cd command (line 300)
- Quote ${{ runner.workspace }} in mkdir command (line 354)
- Quote ${{ runner.workspace }} in find commands (lines 454-455)
- Quote ${{ runner.workspace }} in ls command (line 448)
- Quote ${{ runner.workspace }} in cp command (line 444)
- Quote ${{ runner.workspace }} in validation script call (line 473)
Fixes run HDFGroup#220 (18179453114) Windows build failure.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Quote three more workspace paths in cp commands that were missed in the previous fix. These paths were failing on Windows with the same backslash handling issue. Changes: - Line 414: Quote path in JAR copy command - Line 421: Quote path in SLF4J JAR copy command - Line 437: Quote path in Maven dependency copy command All workspace paths in bash scripts are now properly quoted for cross-platform compatibility. Fixes run HDFGroup#221 Windows artifact collection failure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The Maven dependency simulation was failing because it tried to resolve artifacts from remote repositories before they were deployed. The script now installs artifacts to the local Maven repository first, allowing dependency resolution to succeed. Changes: - Find JAR file matching the POM coordinates - Install artifact to local Maven repository using mvn install:install-file - Then test dependency resolution against local repository - Skip with warning if JAR file not found This allows validation to work for both FFM and JNI artifacts before they are deployed to remote repositories. Fixes Maven dependency simulation failures on all platforms. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…tifacts
Modified the release workflow to properly support parallel FFM and JNI
Maven artifact deployment for all platforms.
Changes to .github/workflows/release.yml:
1. Maven Staging (line 114):
- Added java_implementation: 'both' parameter
- Ensures both FFM and JNI artifacts are built for all platforms
2. Maven Deployment (lines 116-201):
- Replaced old maven-deploy.yml workflow call with new deployment job
- Downloads artifacts from maven-staging workflow
- Deploys artifacts in parallel using matrix strategy
- Supports 8 artifact combinations:
* Linux: FFM + JNI
* Windows: FFM + JNI
* macOS x86_64: FFM + JNI
* macOS aarch64: FFM + JNI
Benefits:
- Release builds now create both FFM and JNI Maven packages
- All 4 platforms supported (Linux, Windows, macOS x86_64, macOS aarch64)
- Parallel deployment for faster release process
- Works with both GitHub Packages and Maven Central
- Maintains backward compatibility with existing release process
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Especially for files like this, it looks like we may be generating interfaces for all sorts of things that don't have an apparent use?
There was a problem hiding this comment.
All the jsrc files are generated from jextract - just pulling out the hdf5_h* files and feature files we actually care about. I suppose you could create a script to remove unneeded files but seems excessive for now.
There was a problem hiding this comment.
Yes, I think the tool is pulling out far more than we actually need, but this is also something that would be mitigated a bit by not checking in these files.
There was a problem hiding this comment.
It was a bit of a shock (but should have been expected) that there would be platform issues. However, I decided that the implementation of the tests, examples and compatibility class should be more important and handled before making things efficient.
| hdf5_h.C_BOOL.withName("open_trace_file"), hdf5_h.C_BOOL.withName("close_trace_file"), | ||
| MemoryLayout.sequenceLayout(1025, hdf5_h.C_CHAR).withName("trace_file_name"), | ||
| hdf5_h.C_BOOL.withName("evictions_enabled"), hdf5_h.C_BOOL.withName("set_initial_size"), | ||
| MemoryLayout.paddingLayout(6), hdf5_h.C_LONG.withName("initial_size"), |
There was a problem hiding this comment.
This makes me think that these files are something that needs to be generated at build time instead of being checked in files
There was a problem hiding this comment.
The ideal way is to create these files at build time however that was beyond the scope at this time. The process could be modified to do this in the future.
There was a problem hiding this comment.
I guess what I mean to say is that even initially it seems needlessly excessive to check in files for other platforms that won't be used during building and also aren't necessarily going to be correct even on the platform they're being built on. These generated files include the particular memory layout of structures for the platform they were generated on, which isn't going to be static. I'd go so far as to say checking these files in is simply the wrong thing to do here.
There was a problem hiding this comment.
Yes, but given the time constraints and the other reqs, there just wasn't enough time. Future work could upgrade this.
java/src-jni/jni/h5aImp.c
Outdated
| #include "h5util.h" | ||
| #include "h5jni.h" | ||
| #include "h5aImp.h" | ||
| #include "../../src-jni/jni/h5util.h" |
There was a problem hiding this comment.
Should probably fix all these renames to set the correct header paths in the build system and not use relative include paths here
There was a problem hiding this comment.
Yes must have been claude!
CMakeBuildOptions.cmake
Outdated
| option (HDF5_BUILD_CPP_LIB "Build HDF5 C++ Library" OFF) | ||
|
|
||
| option (HDF5_BUILD_JAVA "Build Java HDF5 Library" OFF) | ||
| option (HDF5_ENABLE_JNI "Force JNI implementation instead of FFM for Java bindings when Java 24+ is available" OFF) |
There was a problem hiding this comment.
Shouldn't this option be default ON?
There was a problem hiding this comment.
Maybe, I will test it out if the mac CI passes and I know all the CI is working.
Maven artifact generation is already properly handled by the separate maven-staging.yml workflow, which is called by release.yml when deploy_maven=true. The maven-staging.yml workflow: - Uses ci-MinShar-Maven presets (which disable testing) - Builds for all platforms (Linux, Windows, macOS x86_64, macOS aarch64) - Builds both FFM and JNI implementations - Runs in parallel with ctest.yml ctest.yml should focus only on comprehensive testing with ci-StdShar presets. Mixing Maven artifact generation with testing caused Java test failures due to incompatible artifact structures. This reverts commit 53aa605.
Maven artifact generation is handled by the separate maven-staging.yml workflow call (lines 102-114), not by ctest.yml. The ctest.yml workflow should focus solely on comprehensive testing.
- Add java_version: 21 to call-release-bintest - Ensures bintest uses same Java version as build (21) - Fixes: class file has wrong version 65.0, should be 63.0 - Affects: macOS JNI binary tests only Previously, build used Java 21 (auto/runner default) while bintest defaulted to Java 19, causing version mismatch. Now both use Java 21 for consistency. Co-Authored-By: Claude <[email protected]>
- Add Java 21 setup to Windows MSVC CTest build
- Add Java 21 setup to Linux (Ubuntu gcc) CTest build
- Ensures consistent Java version across all platforms
- Matches macOS which already had Java 21 setup
Fixes: Standard platform builds (Linux, Windows, macOS) in release.yml
were failing with 'package hdf.hdf5lib does not exist' errors
Root cause: Windows and Linux builds were using runner default Java (17)
while trying to compile against Java 21 JARs
Solution: Explicitly set up Java 21 on all standard platforms before build
Co-Authored-By: Claude <[email protected]>
- Remove 'Status as of October 26, 2025' references - Remove specific test counts (444 tests) and percentages (56% coverage) - Replace detailed module table with general coverage list - Make platform status more general and maintainable - Focus on module descriptions rather than point-in-time metrics - Simplifies maintenance as test counts and coverage evolve This makes CLAUDE.md suitable for general use without requiring updates for every test count or coverage change.
|
Superseded by #5957 |
github-project-automation
bot
moved this from In progress
to Done
in HDF5 - TRIAGE & TRACK
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enhanced Maven options.
Added Java 25 FFM source.
Created full FFM tests and examples.
Expandedd Presets and CI.
Closes #5847.
Fixes #3652