From e15f0ac8785bc8f8844b1f1a761f2eca10bcdb7a Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Wed, 30 Sep 2026 21:50:52 -0500 Subject: [PATCH 1/2] build: rebuild dependencies built by a different toolchain Dependencies are keyed by name alone, so one built under another environment was reused as-is. Two ways this broke a build, both seen in practice: - LAPACK, HDF5 and Silo built under amdflang were reused after switching to the gnu modules, and post_process failed to link (undefined FIRModule symbols). - The superbuild satisfied FFTW with a library found on the system (an AFAR toolchain on PATH) and installed nothing. In an environment where that library is not visible, simulation could not find FFTW, and since the empty install still counted as installed, nothing ever rebuilt it. After an MFC target is configured, compare the compilers in its CMakeCache.txt with each installed dependency's (resolving symlinks); rebuild any that differ and reconfigure the target. If configure itself fails, the cache still records the compilers, so do the same check there, also rebuilding dependencies that a system library satisfied (empty install), and retry once. --sys-* dependencies and --no-build are left alone. --- toolchain/mfc/build.py | 86 +++++++++++- toolchain/mfc/test_dependency_toolchain.py | 147 +++++++++++++++++++++ 2 files changed, 232 insertions(+), 1 deletion(-) create mode 100644 toolchain/mfc/test_dependency_toolchain.py diff --git a/toolchain/mfc/build.py b/toolchain/mfc/build.py index 307cebf58c..58d62b70af 100644 --- a/toolchain/mfc/build.py +++ b/toolchain/mfc/build.py @@ -639,12 +639,96 @@ def __build_target(target: typing.Union[MFCTarget, str], case: input.MFCInputFil build([dep], case, history) if not target.is_configured(case): - target.configure(case) + try: + target.configure(case) + except MFCException: + # The cache records the compilers even when configure fails, so a + # dependency built by another toolchain, or one a system library + # satisfied in a different environment, is still detectable here. + stale = _stale_dependencies(target, case, include_system_found=True) + if not stale: + raise + _rebuild_dependencies(target, case, history, stale) + + stale = _stale_dependencies(target, case, include_system_found=False) + if stale: + _rebuild_dependencies(target, case, history, stale) target.build(case) target.install(case) +_CMAKE_COMPILER_RE = re.compile(r"^CMAKE_(C|CXX|Fortran)_COMPILER:[A-Z]+=(.+)$") + + +def cmake_cache_compilers(staging_dirpath: str) -> typing.Dict[str, str]: + """Map each language to the compiler path recorded in a CMakeCache.txt.""" + cache_path = os.path.join(staging_dirpath, "CMakeCache.txt") + if not os.path.isfile(cache_path): + return {} + + compilers = {} + with open(cache_path, errors="replace") as f: + for line in f: + match = _CMAKE_COMPILER_RE.match(line.strip()) + if match and not match.group(2).endswith("-NOTFOUND"): + compilers[match.group(1)] = match.group(2) + return compilers + + +def compiler_mismatches(ours: typing.Dict[str, str], theirs: typing.Dict[str, str]) -> typing.List[str]: + # Compare resolved paths: module systems often reach one compiler through several symlinks. + return [lang for lang in ("Fortran", "C", "CXX") if lang in ours and lang in theirs and os.path.realpath(ours[lang]) != os.path.realpath(theirs[lang])] + + +def _all_dependencies(target: MFCTarget) -> typing.List[MFCTarget]: + deps = [] + for dep in target.requires.compute(): + for d in _all_dependencies(dep) + [dep]: + if d.isDependency and d not in deps: + deps.append(d) + return deps + + +def _install_is_empty(target: MFCTarget, case: input.MFCInputFile) -> bool: + return not any(files for _, _, files in os.walk(target.get_install_dirpath(case))) + + +def _stale_dependencies(target: MFCTarget, case: input.MFCInputFile, include_system_found: bool) -> typing.List[typing.Tuple[MFCTarget, str]]: + """Installed dependencies that cannot serve this configured target. + + Dependencies are keyed by name alone, so one built under another environment + (say, a different compiler module) is reused as-is and fails at link time. + A dependency the superbuild satisfied with a system library installs nothing, + and that library may not be visible from the current environment. + """ + ours = cmake_cache_compilers(target.get_staging_dirpath(case)) + stale = [] + for dep in _all_dependencies(target): + if not dep.is_buildable() or not dep.is_installed(case): + continue + theirs = cmake_cache_compilers(dep.get_staging_dirpath(case)) + langs = compiler_mismatches(ours, theirs) + if langs: + stale.append((dep, "; ".join(f"{lang} compiler was {theirs[lang]}, now {ours[lang]}" for lang in langs))) + elif include_system_found and _install_is_empty(dep, case): + stale.append((dep, "it was satisfied by a system library when it was configured")) + return stale + + +def _rebuild_dependencies(target: MFCTarget, case: input.MFCInputFile, history: typing.Set[str], stale: typing.List[typing.Tuple[MFCTarget, str]]): + for dep, reason in stale: + cons.print(f" [bold yellow]Rebuilding[/bold yellow] [magenta]{dep.name}[/magenta] for [magenta]{target.name}[/magenta]: {reason}") + delete_directory(dep.get_staging_dirpath(case)) + delete_directory(dep.get_install_dirpath(case)) + history.discard(dep.name) + + for dep, _ in stale: + __build_target(dep, case, history) + + target.configure(case) + + def get_configured_targets(case: input.MFCInputFile) -> typing.List[MFCTarget]: return [target for target in TARGETS if target.is_configured(case)] diff --git a/toolchain/mfc/test_dependency_toolchain.py b/toolchain/mfc/test_dependency_toolchain.py new file mode 100644 index 0000000000..479c728203 --- /dev/null +++ b/toolchain/mfc/test_dependency_toolchain.py @@ -0,0 +1,147 @@ +"""Tests for detecting installed dependencies that cannot serve the current build. + +Dependencies are keyed by name alone, so a LAPACK built under one compiler module +(amdflang, say) was silently reused after switching to another (gfortran), and +post_process then failed to link. Likewise, a dependency the superbuild satisfied +with a system library installs nothing; in an environment where that library is +not visible, the dependent target cannot configure, and nothing ever rebuilt it. +""" + +import os + +from mfc.build import _stale_dependencies, cmake_cache_compilers, compiler_mismatches + +GFORTRAN = "/opt/gcc/bin/gfortran" +AMDFLANG = "/opt/rocm/bin/amdflang" +GCC = "/opt/gcc/bin/cc" + + +def write_cache(dirpath, fortran=None, c=GCC): + os.makedirs(dirpath, exist_ok=True) + lines = ["# This is the CMakeCache file.", "CMAKE_BUILD_TYPE:STRING=Release"] + if fortran: + lines.append(f"CMAKE_Fortran_COMPILER:FILEPATH={fortran}") + if c: + lines.append(f"CMAKE_C_COMPILER:FILEPATH={c}") + lines.append("CMAKE_CXX_COMPILER:STRING=CMAKE_CXX_COMPILER-NOTFOUND") + with open(os.path.join(dirpath, "CMakeCache.txt"), "w") as f: + f.write("\n".join(lines) + "\n") + + +class Deps: + def __init__(self, deps): + self.deps = deps + + def compute(self): + return self.deps + + +class FakeTarget: + def __init__(self, root, name, is_dependency, deps=(), installed=True, buildable=True, install_files=1): + self.root = root + self.name = name + self.isDependency = is_dependency + self.requires = Deps(list(deps)) + self.installed = installed + self.buildable = buildable + install = self.get_install_dirpath(None) + os.makedirs(install, exist_ok=True) + for i in range(install_files): + with open(os.path.join(install, f"lib{i}.a"), "w") as f: + f.write("x") + + def get_staging_dirpath(self, _case): + return os.path.join(self.root, "staging", self.name) + + def get_install_dirpath(self, _case): + return os.path.join(self.root, "install", self.name) + + def is_installed(self, _case): + return self.installed + + def is_buildable(self): + return self.buildable + + +def test_compilers_are_read_from_the_cache_and_notfound_entries_ignored(tmp_path): + write_cache(str(tmp_path), fortran=GFORTRAN) + assert cmake_cache_compilers(str(tmp_path)) == {"Fortran": GFORTRAN, "C": GCC} + + +def test_a_missing_cache_has_no_compilers(tmp_path): + assert cmake_cache_compilers(str(tmp_path / "nope")) == {} + + +def test_only_languages_both_sides_recorded_are_compared(): + assert compiler_mismatches({"Fortran": GFORTRAN, "C": GCC}, {"Fortran": AMDFLANG}) == ["Fortran"] + assert compiler_mismatches({"Fortran": GFORTRAN}, {"C": GCC}) == [] + assert compiler_mismatches({}, {"Fortran": AMDFLANG}) == [] + + +def test_symlinked_paths_to_one_compiler_match(tmp_path): + real = tmp_path / "amdllvm" + real.write_text("") + link = tmp_path / "amdflang" + link.symlink_to(real) + assert compiler_mismatches({"Fortran": str(real)}, {"Fortran": str(link)}) == [] + + +def test_a_dependency_built_by_another_fortran_compiler_is_stale(tmp_path): + root = str(tmp_path) + lapack = FakeTarget(root, "lapack", True) + fftw = FakeTarget(root, "fftw", True) + post = FakeTarget(root, "post_process", False, deps=[lapack, fftw]) + write_cache(lapack.get_staging_dirpath(None), fortran=AMDFLANG) + write_cache(fftw.get_staging_dirpath(None), fortran=GFORTRAN) + write_cache(post.get_staging_dirpath(None), fortran=GFORTRAN) + + stale = _stale_dependencies(post, None, include_system_found=False) + + assert [dep.name for dep, _ in stale] == ["lapack"] + assert AMDFLANG in stale[0][1] and GFORTRAN in stale[0][1] + + +def test_transitive_dependencies_are_checked(tmp_path): + root = str(tmp_path) + hdf5 = FakeTarget(root, "hdf5", True) + silo = FakeTarget(root, "silo", True, deps=[hdf5]) + post = FakeTarget(root, "post_process", False, deps=[silo]) + write_cache(hdf5.get_staging_dirpath(None), fortran=AMDFLANG) + write_cache(silo.get_staging_dirpath(None), fortran=GFORTRAN) + write_cache(post.get_staging_dirpath(None), fortran=GFORTRAN) + + assert [dep.name for dep, _ in _stale_dependencies(post, None, include_system_found=False)] == ["hdf5"] + + +def test_system_found_dependencies_are_flagged_only_when_asked(tmp_path): + # Same compiler, but the superbuild found FFTW on the system and installed nothing. + root = str(tmp_path) + fftw = FakeTarget(root, "fftw", True, install_files=0) + sim = FakeTarget(root, "simulation", False, deps=[fftw]) + write_cache(fftw.get_staging_dirpath(None), fortran=GFORTRAN) + write_cache(sim.get_staging_dirpath(None), fortran=GFORTRAN) + + assert _stale_dependencies(sim, None, include_system_found=False) == [] + assert [dep.name for dep, _ in _stale_dependencies(sim, None, include_system_found=True)] == ["fftw"] + + +def test_system_provided_and_uninstalled_dependencies_are_left_alone(tmp_path): + root = str(tmp_path) + sys_fftw = FakeTarget(root, "fftw", True, buildable=False, install_files=0) # --sys-fftw + lapack = FakeTarget(root, "lapack", True, installed=False) + post = FakeTarget(root, "post_process", False, deps=[sys_fftw, lapack]) + write_cache(sys_fftw.get_staging_dirpath(None), fortran=AMDFLANG) + write_cache(lapack.get_staging_dirpath(None), fortran=AMDFLANG) + write_cache(post.get_staging_dirpath(None), fortran=GFORTRAN) + + assert _stale_dependencies(post, None, include_system_found=True) == [] + + +def test_matching_toolchains_are_not_stale(tmp_path): + root = str(tmp_path) + lapack = FakeTarget(root, "lapack", True) + post = FakeTarget(root, "post_process", False, deps=[lapack]) + write_cache(lapack.get_staging_dirpath(None), fortran=GFORTRAN) + write_cache(post.get_staging_dirpath(None), fortran=GFORTRAN) + + assert _stale_dependencies(post, None, include_system_found=True) == [] From 39a547945c4c9649b8ecb1c699149563ce9dd6d2 Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Wed, 30 Sep 2026 22:14:32 -0500 Subject: [PATCH 2/2] build: walk dependencies iteratively with a visited set Address review: _all_dependencies recursed without cycle protection. The graph is small and acyclic today, but __build_target guards cycles with its history set, and this helper should not be the one place a cycle hangs the build. --- toolchain/mfc/build.py | 15 ++++++++++----- toolchain/mfc/test_dependency_toolchain.py | 13 +++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/toolchain/mfc/build.py b/toolchain/mfc/build.py index 58d62b70af..ae6692fd47 100644 --- a/toolchain/mfc/build.py +++ b/toolchain/mfc/build.py @@ -682,11 +682,16 @@ def compiler_mismatches(ours: typing.Dict[str, str], theirs: typing.Dict[str, st def _all_dependencies(target: MFCTarget) -> typing.List[MFCTarget]: - deps = [] - for dep in target.requires.compute(): - for d in _all_dependencies(dep) + [dep]: - if d.isDependency and d not in deps: - deps.append(d) + """Every dependency target reachable from target, each once; safe against cycles.""" + deps, seen, stack = [], set(), list(target.requires.compute()) + while stack: + dep = stack.pop() + if dep.name in seen: + continue + seen.add(dep.name) + if dep.isDependency: + deps.append(dep) + stack.extend(dep.requires.compute()) return deps diff --git a/toolchain/mfc/test_dependency_toolchain.py b/toolchain/mfc/test_dependency_toolchain.py index 479c728203..d766996145 100644 --- a/toolchain/mfc/test_dependency_toolchain.py +++ b/toolchain/mfc/test_dependency_toolchain.py @@ -113,6 +113,19 @@ def test_transitive_dependencies_are_checked(tmp_path): assert [dep.name for dep, _ in _stale_dependencies(post, None, include_system_found=False)] == ["hdf5"] +def test_a_dependency_cycle_terminates(tmp_path): + root = str(tmp_path) + a = FakeTarget(root, "a", True) + b = FakeTarget(root, "b", True, deps=[a]) + a.requires = Deps([b]) + post = FakeTarget(root, "post_process", False, deps=[a]) + write_cache(a.get_staging_dirpath(None), fortran=AMDFLANG) + write_cache(b.get_staging_dirpath(None), fortran=GFORTRAN) + write_cache(post.get_staging_dirpath(None), fortran=GFORTRAN) + + assert [dep.name for dep, _ in _stale_dependencies(post, None, include_system_found=False)] == ["a"] + + def test_system_found_dependencies_are_flagged_only_when_asked(tmp_path): # Same compiler, but the superbuild found FFTW on the system and installed nothing. root = str(tmp_path)