ci: propagate dispatched Libneo revision - #507
Conversation
There was a problem hiding this comment.
Review verdict: Request changes
Summary: PR #507 pins libneo to commit afa0e42, drops the NetCDF/HDF5 fallback in favor of the required fortio::fortio target, propagates a dispatched libneo_ref through CI, and renames the libneo test-toggle cache variable. The direction is sound and consistent, but the new CI/regression propagation is silently defeated by a duplicate -DLIBNEO_REF.
Findings:
-
[major] Makefile:77 + .github/workflows/main.yml:119-130 — a dispatched libneo revision is silently ignored for regression/golden tests.
make test-regression LIBNEO_REF=Xsets FLAGS to-DLIBNEO_REF=X(Makefile:9), then the target appends-DLIBNEO_REF=$(GOLDEN_LIBNEO_REF)(line 77), yielding two-DLIBNEO_REFflags; CMake resolves last-one-wins, so the golden SHA always overrides the dispatched ref. The CI step added in commit 4aaf84d is therefore ineffective, and anymake test-regression LIBNEO_REF=<candidate>also silently builds against the golden SHA. Fix: apply the golden override only whenLIBNEO_REFwas not given on the command line. -
[minor] cmake/Util.cmake:98 — default libneo revision is hardcoded to
afa0e42, replacingget_branch_or_main, which is now dead code; feature-branch builds no longer match a same-named libneo branch. Remove the unused function and document the pinning policy. -
[minor] cmake/Util.cmake:66,101 —
LIBNEO_ENABLE_TESTSrenamed toLIBNEO_BUILD_TESTING; couldn't verify against libneo (no network). Confirm libneo's CMake option name before merge. -
[minor] CMakeLists.txt:180-186 — removing the NetCDF fallback for a hard
fortio::fortiorequirement is coherent with the pin, but users overriding to older libneo revisions now get a hard failure instead of the fallback; deliberate compatibility cut.
Verdict: Request changes — the dispatched libneo_ref propagation for the regression/golden suite (the purpose of the CI change) is defeated by the duplicate -DLIBNEO_REF in the test-regression target.
Ensures the full reverse-dependency workflow tests the dispatched Libneo SHA in all build paths, including the editable Python package and deterministic regression reconfiguration. Previously those stages silently fell back to SIMPLE's default Libneo pin.
Verification:
actionlint .github/workflows/main.ymlpasses. Bareforeaches a pre-existing fpm duplicate-module bootstrap failure before tests.This PR is intentionally ready-for-review and must remain open/unmerged.