Skip to content

Make JAR dep paths modifiable#6331

Merged
lrknox merged 5 commits intoHDFGroup:developfrom
mattjala:jar_path_cache
Apr 13, 2026
Merged

Make JAR dep paths modifiable#6331
lrknox merged 5 commits intoHDFGroup:developfrom
mattjala:jar_path_cache

Conversation

@mattjala
Copy link
Copy Markdown
Contributor

The build system for the java libraries currently forces dependency JARs (e.g. SLF4j) to always be pulled from their bundled location in java/lib.

This PR changes several JAR path variables to CACHE variables, so that users interested in providing their own path to different JARs can do so via -D overrides. Specifically, the altered variables are

  • HDF5_JAVA_LOGGING_JAR
  • HDF5_JAVA_LOGGING_NOP_JAR
  • HDF5_JAVA_LOGGING_SIMPLE_JAR

This PR also moves the JUnit and Hamcrest JAR paths to similarly modifiable cache variables (HDF5_JAVA_JUNIT_JAR and HDF5_JAVA_HAMCREST_JAR) instead of building them inline in a couple different places.

hyoklee
hyoklee previously approved these changes Mar 30, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes Java dependency JAR locations configurable via CMake cache variables so consumers can override bundled JAR paths with -D...=... at configure time.

Changes:

  • Convert SLF4J dependency JAR path variables to CACHE FILEPATH entries (HDF5_JAVA_LOGGING_JAR, HDF5_JAVA_LOGGING_NOP_JAR, HDF5_JAVA_LOGGING_SIMPLE_JAR).
  • Introduce cache variables for JUnit and Hamcrest JAR paths (HDF5_JAVA_JUNIT_JAR, HDF5_JAVA_HAMCREST_JAR) and use them in Java test classpaths.
  • Update Java test CMake logic to consume the new cache variables instead of hard-coded ${HDF5_JAVA_LIB_DIR}/... paths.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
CMakeLists.txt Defines JAR paths as cache variables to allow -D overrides, adds JUnit/Hamcrest cache vars.
java/test/CMakeLists.txt Uses new JUnit/Hamcrest cache vars in CMAKE_JAVA_INCLUDE_PATH.
java/src-jni/test/CMakeLists.txt Uses new JUnit/Hamcrest cache vars in CMAKE_JAVA_INCLUDE_PATH.
java/jtest/CMakeLists.txt Uses new JUnit/Hamcrest cache vars in CMAKE_JAVA_INCLUDE_PATH (both FFM and non-FFM classpaths).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

hyoklee
hyoklee previously approved these changes Mar 30, 2026
@jhendersonHDF
Copy link
Copy Markdown
Collaborator

Two other comments:

@mattjala
Copy link
Copy Markdown
Contributor Author

The most recent changes propagate the user-specified path changes down to hdf5-config.cmake. I also added the CHANGELOG, and tested a local build using a custom path to dependency JARs, which passed the java ctests.

glennsong09
glennsong09 previously approved these changes Mar 31, 2026
hyoklee
hyoklee previously approved these changes Apr 1, 2026
Copy link
Copy Markdown
Member

@hyoklee hyoklee left a comment

Choose a reason for hiding this comment

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

jhendersonHDF
jhendersonHDF previously approved these changes Apr 1, 2026
@jhendersonHDF
Copy link
Copy Markdown
Collaborator

Just needs conflict resolving

@mattjala mattjala dismissed stale reviews from jhendersonHDF and hyoklee via 84bd3f1 April 3, 2026 15:07
hyoklee
hyoklee previously approved these changes Apr 3, 2026
glennsong09
glennsong09 previously approved these changes Apr 3, 2026
@lrknox lrknox merged commit f010df9 into HDFGroup:develop Apr 13, 2026
167 of 243 checks passed
@github-project-automation github-project-automation bot moved this from To be triaged to Done in HDF5 - TRIAGE & TRACK Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants