Skip to content

CI: Fix the macOS build (zlib for HDF5's CMake config) and make steps fail fast - #994

Merged
gonuke merged 4 commits into
svalinn:developfrom
cyb3ralbert:ci/fix-mac-build-zlib
Jul 31, 2026
Merged

CI: Fix the macOS build (zlib for HDF5's CMake config) and make steps fail fast#994
gonuke merged 4 commits into
svalinn:developfrom
cyb3ralbert:ci/fix-mac-build-zlib

Conversation

@cyb3ralbert

@cyb3ralbert cyb3ralbert commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Description

Two changes to .github/workflows/mac_build_test.yml. The first is what turns the Mac Build/Test job green again; the second is what makes the next failure readable:

  1. zlib — pass -DZLIB_ROOT="$(brew --prefix zlib)" to the DAGMC configure step and install Homebrew's zlib alongside hdf5.
  2. fail fast — the steps use a custom shell (shell: bash -l {0}), for which GitHub does not add -e. Changed to shell: bash -leo pipefail {0} so a failing command stops the step.

No CMake module is touched, so this does not overlap with the pending refactor in #987.

Motivation and Context

Mac Build/Test has been failing on every branch since March 2026 — the last green run was the push to develop on 2026-02-16 (#990). It fails on unrelated branches, so the trigger is a runner-image update rather than anyone's patch — but the fragility it exposed is in this repository's own CMake, see below. This also makes #542 ("We have no ability to verify whether the Mac build is working") effectively true again.

The failure is in the Build DAGMC step:

CMake Error at .../FindPackageHandleStandardArgs.cmake:290 (message):
  Could NOT find ZLIB (missing: ZLIB_LIBRARY) (found version "1.2.12")
Call Stack (most recent call first):
  .../FindZLIB.cmake:242
  .../CMakeFindDependencyMacro.cmake:93 (find_package)
  /opt/homebrew/lib/cmake/hdf5/hdf5-config.cmake:155 (find_dependency)
  .../FindHDF5.cmake:678 (find_package)
  cmake/HDF5_macro.cmake:5 (find_package)
  cmake/FindMOAB.cmake:25 (find_set_HDF5)
  CMakeLists.txt:60 (find_package)

Chain of events on macos-latest (currently macos-26-arm64, CMake 4.4.0):

  • cmake/HDF5_macro.cmake:4 narrows CMAKE_FIND_LIBRARY_SUFFIXES to ${CMAKE_SHARED_LIBRARY_SUFFIX} — i.e. .dylib only — before find_package(HDF5).
  • Homebrew's hdf5-config.cmake calls find_dependency(ZLIB), so FindZLIB runs with that narrowed list.
  • The macOS SDK ships zlib only as a .tbd stub (MacOSX.sdk/usr/lib/libz.tbd), which the narrowed suffix list excludes. The header is still found — hence the misleading found version "1.2.12" — but ZLIB_LIBRARY stays empty and the configure step fails.

Verified on the runner: a plain find_package(ZLIB) there resolves to MacOSX.sdk/usr/lib/libz.tbd with the default suffix list .tbd;.dylib;.so;.a, so nothing is wrong with the image — only with the interaction between the narrowed suffix list and the SDK stub. Homebrew's zlib is a real .dylib, but keg-only and therefore not under $(brew --prefix)/lib, so ZLIB_ROOT has to point at the keg. The reasoning is left as a comment in the workflow.

The second change is what made this expensive to read. With shell: bash -l {0} and no -e, the step kept going after the configure failure, so the tail of the log — the part a human sees first — is:

make: Makefile: No such file or directory
CMake Error: Not a file: /Users/runner/work/DAGMC/DAGMC/bld/cmake_install.cmake

The real error is ~150 lines above that. bash -leo pipefail {0} makes future failures point at their own cause.

Changes

Bug fix, CI only. No source or CMake changes.

Behavior

  • Current: Build DAGMC fails at configure with Could NOT find ZLIB; the step then runs on and reports a missing Makefile.
  • New: the job configures, builds and runs make test to completion.

Runs in my fork, same workflow, same runner:

Other Information

If you would rather fix the cause than the symptom, the line to drop is the suffix narrowing at cmake/HDF5_macro.cmake:4 — with the default suffix list the SDK stub is found and no ZLIB_ROOT is needed. I left it alone on purpose: #987 removes that file, and I would rather not put this fix behind a pending refactor. Happy to do it that way instead if you prefer.

Follows on from @matthewfeickert's CI work in February 2026 (#989, #990, and the Eigen/TestEndianess comments in this workflow), which is what kept this job alive up to that point.

Deliberately left out of this PR, happy to open a follow-up: no workflow in the repository sets a permissions: block, and actions/checkout@v3 is still used in 9 workflows — the mac job log already warns that its Node.js 20 runtime is deprecated and being forced onto Node.js 24.

Changelog file

Entry added under Fixed in doc/CHANGELOG.rst.

@cyb3ralbert

cyb3ralbert commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for approving the CI run, @gonuke.

Mac Build/Test is green: run 30524019376 — MOAB and DAGMC both configure and build, and Testing DAGMC reports 100% tests passed out of 25 (231.7 s). That job had been failing since mid-February, so this is its first passing run on develop in over five months.

The red "Is Changelog up-to-date ?" check is pre-existing and unrelated to this PR. It fails after 6 seconds on the Checkout repository step, before the changelog script ever runs:

Error relocating /__e/node24_alpine/bin/node: pthread_getname_np: symbol not found

changelog_test.yml runs in container: alpine:3.14 and uses actions/checkout@v3, which targets Node 20. Now that Node 20 is deprecated the runner forces Node 24 instead, and that binary cannot relocate against the musl shipped in alpine 3.14.

The same failure shows up on #982 (run 29750280790); the last green run of this check was on 29 March 2026. The changelog entry this repo requires is present here in doc/CHANGELOG.rst, so the script itself would pass.

I'd be glad to fix it in a separate PR: dropping the alpine:3.14 container (it exists to "install latest git", but 3.14 ships git 2.32 — older than what ubuntu-latest already provides) and bumping checkout should be enough. That fits naturally into the workflow-hygiene PR I mentioned in the description, but I'm happy to keep it standalone, or leave it alone entirely if you'd prefer.

@gonuke

gonuke commented Jul 30, 2026

Copy link
Copy Markdown
Member

Thanks for your contribution @cyb3ralbert - I'm fine with the changelog workflow update being adding to this PR, or a separate one, which ever is easiest. We recently confronted this on another project, but I didn't check the extent of this issue across my projects.

@cyb3ralbert

cyb3ralbert commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @gonuke — I've added the changelog workflow fix here, as you suggested.

Two commits: the workflow change itself, and a doc/CHANGELOG.rst entry for it.

The fix drops the container: alpine:3.14 and bumps actions/checkout from v3 to v5:

   changelog_update:
     runs-on: ubuntu-latest
-    container:
-      image: alpine:3.14
 
     name: Is Changelog up-to-date ?
     steps:
-      - name: Install latest git
-        run: |
-          apk add --no-cache bash git openssh
-          git --version
-
       - name: Checkout repository
-        uses: actions/checkout@v3
+        uses: actions/checkout@v5

The Install latest git step goes away with the container — ubuntu-latest already ships a newer git (2.54) than alpine 3.14 did (2.32), which is what that step was working around in the first place. The check's actual logic is untouched. v5 is the first major release of checkout running on Node 24, so this won't come back the next time a Node version is retired; happy to go to v7 instead if you'd prefer to jump to the latest.

Verified green: run 30610951114 on my fork — Is Changelog up-to-date ? passes in 18 seconds. The check still does its job there rather than passing vacuously: that branch carries a real doc/CHANGELOG.rst entry, so the git diff against svalinn/develop is non-empty as intended.

Note this only fixes changelog_test.yml. The other eight workflows still on actions/checkout@v3 aren't affected by this particular failure, since none of them run in an alpine container — I left them alone to keep this PR focused, but they're the natural content of the hygiene PR I mentioned in the description if you want it.

gonuke
gonuke approved these changes Jul 31, 2026
View reviewed changes

@gonuke gonuke left a comment

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.

Thanks for your interest and support @cyb3ralbert !!

@gonuke
gonuke merged commit c968afb into svalinn:develop Jul 31, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gonuke gonuke gonuke approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cyb3ralbert @gonuke