Skip to content

Add HDF5 dependency to dev containers#88

Merged
jbeirer merged 1 commit into
mainfrom
main-addHDF5dep
Apr 18, 2026
Merged

Add HDF5 dependency to dev containers#88
jbeirer merged 1 commit into
mainfrom
main-addHDF5dep

Conversation

@jbeirer
Copy link
Copy Markdown
Collaborator

@jbeirer jbeirer commented Apr 18, 2026

This PR installs the HDF5 dependency in the docker containers, which is required in preparation for #84

Summary by CodeRabbit

  • Chores
    • Docker images now include HDF5 library with C++ support enabled (version 1.14.6 by default), supporting HDF5-dependent workflows in containerized environments.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 18, 2026

📝 Walkthrough

Walkthrough

Both Dockerfile configurations now include a new build stage for HDF5 library compilation and installation. A versioned build argument HDF5_VERSION (default 1.14.6) is introduced, followed by source code retrieval, CMake configuration with C++ bindings enabled, compilation, installation to /usr/local, and temporary directory cleanup.

Changes

Cohort / File(s) Summary
Docker HDF5 Build Stage
docker/alma10/Dockerfile, docker/ubuntu24/Dockerfile
Added new build stage that fetches HDF5 source, configures via CMake with C++ bindings enabled, builds, and installs to /usr/local. Includes new build argument ARG HDF5_VERSION=1.14.6 with tools, examples, testing, and static library builds disabled.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A hop, a skip, HDF5 we'll fetch,
From source code's garden, a perfect match,
CMAKE builds it bright, no tools to compile,
Into /usr/local goes our file,
Dockers now blessed with data delight! 📦

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding HDF5 as a dependency to development container configurations (both alma10 and ubuntu24 Dockerfiles).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch main-addHDF5dep

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docker/alma10/Dockerfile`:
- Around line 106-120: The Dockerfile currently builds and installs HDF5 from
source (ARG HDF5_VERSION / RUN git clone ... && cmake ... --install to
/usr/local) while the Alma image also installs the distro hdf5-devel package,
causing duplicate headers/libraries; pick one approach and remove the other:
either delete this source-build RUN block (lines that clone hdf5_src, configure
and install to /usr/local) and rely on the distro hdf5-devel package, or remove
hdf5-devel from root_dependencies.txt and keep the source build (ensuring
CMAKE_INSTALL_PREFIX=/usr/local remains intentional); make the change so only
one HDF5 stack is installed and update any README or build comments to reflect
the chosen path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 960882ef-38ec-487c-9a51-f836e1344850

📥 Commits

Reviewing files that changed from the base of the PR and between 71ed126 and 433f04f.

📒 Files selected for processing (2)
  • docker/alma10/Dockerfile
  • docker/ubuntu24/Dockerfile

Comment thread docker/alma10/Dockerfile
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.
see 125 files with indirect coverage changes

@jbeirer jbeirer merged commit 23577a8 into main Apr 18, 2026
21 checks passed
@jbeirer jbeirer deleted the main-addHDF5dep branch April 18, 2026 13:11
@jbeirer jbeirer restored the main-addHDF5dep branch April 22, 2026 18:42
@jbeirer
Copy link
Copy Markdown
Collaborator Author

jbeirer commented Apr 22, 2026

For some very strange reason, this PR was merged to main but the commit does not appear in the main branch... Re-trying again in #89

@jbeirer jbeirer deleted the main-addHDF5dep branch April 22, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant