build: base dependency and configure skips on real state, not CMakeCache.txt

[sbryngelson]

Fixes #1689. One file, +21/−7. Not compiler- or platform-specific — separated from the CCE 21 port (#1694) because it stands on its own.

Merge order

This one has no file overlap with any other open PR and can merge at any time. Merging it first is the most useful, because anyone building or hand-testing #1688 / #1694 on Frontier will otherwise hit the bug this fixes.

Suggested order for the CCE 21 work as a whole:

order	PR	why
1	#1696 (this)	independent; unblocks hand-testing the rest
2	#1688 QBMM hoist	self-contained, correct on any compiler; removes 275 lines of move-only diff from #1694 on rebase
3	#1694 CCE 21 port	rebase after #1688; its m_qbmm.fpp hunk then drops out

⚠️ #1694 and #1679 conflict — see #1694's description. Not this PR's problem, but it affects the order of everything else.

The problem

MFCTarget.is_configured() decides whether to skip configure and whether to skip a dependency entirely, based only on CMakeCache.txt existing:

def is_configured(self, case: Case) -> bool:
    # We assume that if the CMakeCache.txt file exists, then the target is
    # configured. (this isn't perfect, but it's good enough for now)
    return os.path.isfile(os.sep.join([self.get_staging_dirpath(case), "CMakeCache.txt"]))

CMake writes that file before it generates the build system, and long before anything is installed. Two callers draw stronger conclusions than it supports.

Configure gets skipped. An interrupted configure leaves a cache with no Makefile. Every later build then skips configure and dies with a message that names the target, not the cause:

gmake: Makefile: No such file or directory
gmake: *** No rule to make target 'Makefile'.  Stop.
Error: Failed to build the post_process target.

A dependency gets skipped permanently. The comment states the assumption outright:

# Dependencies are pinned to fixed versions. If already configured
# (built & installed by a prior --deps-only step), skip entirely

configured ⇒ built & installed is false for any dependency build that failed. The superbuild creates the staging dir and cache before it downloads, so a clone that fails — very easy to hit, since compute nodes have no outbound network — leaves the cache behind with nothing installed. From then on MFC skips that dependency forever, CMAKE_PREFIX_PATH points at an empty install tree, and the user sees an error from somewhere else entirely:

CMake Error at .../FindPackageHandleStandardArgs.cmake:233 (message):
  Could NOT find SILO (missing: SILO_LIBRARY SILO_INCLUDE_DIR)

or, for hipfort, a silent fallback to ROCm's system copy — which on ROCm 7.2.0 is itself incomplete (missing hipfort-hipfft-targets.cmake) and produces a baffling error deep inside /opt/rocm-7.2.0. A related report on #1692 had ./mfc.sh build --deps-only return rc=0 with no hipfort installed, surfacing later as HIPFFTPLANMANY has no explicit type in M_FFTW.

The fix

Ask the question each caller actually means:

• ready to build — require the generator's build file (build.ninja or Makefile) alongside the cache
• already installed — require install_manifest.txt, which CMake writes only after a successful install

Verification

install_manifest.txt is present for all five dependencies (fftw, hdf5, silo, lapack, hipfort) after a normal build, checked across two independent worktrees — so the stricter predicate does not cause spurious rebuilds.

It also discriminated correctly on a real broken tree, which is the case that matters:

dependency	cache	manifest	action
silo	present	absent	rebuilt ✅
lapack	present	absent	rebuilt ✅
hipfort	present	absent	rebuilt ✅
hdf5, fftw	present	present	correctly skipped ✅

Four correct skips, three correct rebuilds, no manual intervention. ./mfc.sh precheck passes 7/7.

Behaviour change worth reviewing

A dependency provisioned by hand, outside MFC's own install path, will no longer be recognised and will be rebuilt. That is arguably correct — MFC manages its own dependencies — but it converts a silent-wrong-answer case into a loud-failure one, and on a compute node that rebuild will fail for lack of network rather than succeed. Anyone relying on hand-provisioned deps should know.

Why this is worth taking on its own

Over one porting effort this single predicate destroyed six batch jobs, produced a false negative in a compiler-flag experiment (a link reported as failing when it had never been attempted), broke a control run, and silently voided two full benchmark campaigns. In every instance the visible error implicated the compiler, a missing system package, or a slow machine — never the skip logic. The cost is not the failure itself but that it consistently points investigation at the wrong layer.

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR fixes incorrect “skip” logic in the build tooling by distinguishing between (a) a target being configured and ready to build vs (b) a dependency being successfully installed, preventing broken or partial dependency/configure states from being cached indefinitely.

Changes:

• Strengthen is_configured() to require both CMakeCache.txt and a generator build file (build.ninja or Makefile).
• Add is_installed() using install_manifest.txt as the indicator for successful dependency install.
• Update dependency-skip logic to gate on is_installed() instead of is_configured().

[sbryngelson]

Thanks — took 1 and 2, declining 3 with a finding.

1. Hardcoded build.ninja/Makefile. Fair. Fixed in 5dcf00e: read CMAKE_GENERATOR from the cache and check that generator's output file. An unrecognized generator falls back to the cache alone, i.e. the pre-existing behaviour — so this is never stricter than before, only more accurate for the generators it knows. Not reachable today (all 28 staging directories here report CMAKE_GENERATOR:INTERNAL=Unix Makefiles, and MFC never passes -G), but cheap, and the failure mode it removes would present as an unexplained re-configure on every build.

2. install_manifest.txt under multi-config generators. Also fixed in 5dcf00e — prefix/suffix match, so install_manifest_<config>.txt counts. Likewise not reachable today (all 31 manifests here are the unqualified name).

3. Opt-out for hand-provisioned dependencies. Declining, because the hook you're describing already exists and is dead code. is_buildable() reads ARG(f"sys_{self.name}", False) at build.py:401, but no CLI argument named sys_<dep> is ever defined, so the lookup always returns the False default. Adding a second opt-out beside a non-functional one would make the situation worse, and wiring up the existing one is a separate change with its own design question (per-dependency flags vs. one list) that shouldn't ride along on a bugfix.

Worth noting the practical impact is smaller than it looks: this PR makes the skip depend on is_installed(), which is satisfied by an install manifest. A hand-provisioned dependency that was ever installed through the normal path keeps its manifest and is still skipped. The case that regresses is one installed entirely out-of-band — and that case was equally broken before, just in the other direction.

Happy to open a follow-up for the dead sys_<dep> hook if you want it wired up.

---

Note on the force-push: the branch briefly carried three junk commits from an unrelated bug — the toolchain unit tests were committing into the real repo when run from the pre-commit hook, one of those commits deleting 2090 files. Branch is repaired; root cause and fix are in #1697.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.04%. Comparing base (782a1fd) to head (5dcf00e).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1696   +/-   ##
=======================================
  Coverage   61.04%   61.04%           
=======================================
  Files          83       83           
  Lines       20978    20978           
  Branches     3099     3099           
=======================================
  Hits        12807    12807           
  Misses       6126     6126           
  Partials     2045     2045           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.