From 3d0bb90bc7dbdaaf436603f4c0764552145d39bf Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 10 Aug 2026 07:37:57 +0000 Subject: [PATCH 1/5] [CI] Check that a second build has nothing to do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building an already built tree twice in a row must be a no-op, apart from etc/gitinfo.txt which the gitinfotxt target rewrites on every build by design. When that is not the case, every incremental build pays for the targets that are needlessly redone plus everything depending on them. Add a null build check that runs on all platforms right after the build: it fingerprints every file in the build tree, builds again, and reports what was written. Doing it this way rather than parsing build tool output keeps it working across Ninja, Makefiles and MSBuild alike. The verdict is only acted on at the very end, after ctest, so that a spurious rebuild does not cost us the test results. 🤖 Done with the help of AI --- .../workflows/root-ci-config/build_root.py | 48 ++++- .../root-ci-config/null_build_check.py | 196 ++++++++++++++++++ 2 files changed, 240 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/root-ci-config/null_build_check.py diff --git a/.github/workflows/root-ci-config/build_root.py b/.github/workflows/root-ci-config/build_root.py index 7e8b2875feef5..80fbd16ec47f2 100755 --- a/.github/workflows/root-ci-config/build_root.py +++ b/.github/workflows/root-ci-config/build_root.py @@ -24,6 +24,7 @@ import time import build_utils +import null_build_check from build_utils import ( calc_options_hash, die, @@ -174,6 +175,10 @@ def main(): build(options, args.buildtype) + # Done before anything else touches the build tree, and reported only at the + # very end so that a spurious rebuild does not cost us the test results. + null_build_ok = check_for_spurious_rebuilds(args.buildtype) + # Build artifacts should only be uploaded for full builds, and only for # "official" branches (master, v?-??-??-patches), i.e. not for pull_request # We also want to upload any successful build, even if it fails testing @@ -201,6 +206,12 @@ def main(): if testing and ctest_returncode != 0: handle_test_failure(ctest_returncode) + if not null_build_ok: + die( + msg="Building an already built ROOT rebuilt files it should not have; " + 'see the "Check for spurious rebuilds" step above' + ) + print_trace() def handle_test_failure(ctest_returncode): @@ -443,20 +454,49 @@ def dump_requested_config(options): print(f"\nBUILD OPTIONS: {options}") -@github_log_group("Build") -def cmake_build(buildtype): +def cmake_build_command(buildtype) -> str: generator_flags = "-- '-verbosity:minimal' '-consoleloggerparameters:summary'" if WINDOWS else "" parallel_jobs = "4" if WINDOWS else str(os.cpu_count()) builddir = os.path.join(WORKDIR, "build") - result = subprocess_with_log(f""" + + return f""" cmake --build '{builddir}' --config '{buildtype}' --parallel '{parallel_jobs}' {generator_flags} - """) + """ + + +@github_log_group("Build") +def cmake_build(buildtype): + result = subprocess_with_log(cmake_build_command(buildtype)) if result != 0: die(result, "Failed to build") +@github_log_group("Check for spurious rebuilds") +def check_for_spurious_rebuilds(buildtype) -> bool: + """Build a second time and check that there was nothing left to do. + + Returns whether the build tree was already up to date, i.e. whether the + second build wrote nothing but the files listed in null_build_check.py. + """ + builddir = os.path.join(WORKDIR, "build") + + def rebuild() -> int: + return subprocess_with_capture(cmake_build_command(buildtype)).returncode + + try: + touched = null_build_check.check_null_build(builddir, rebuild) + except RuntimeError as err: + # Not a spurious rebuild, but something we still want to know about. + build_utils.print_warning(f"Could not check for spurious rebuilds: {err}") + return True + + null_build_check.report(builddir, touched) + + return not touched + + def build(options, buildtype): if not os.path.isdir(os.path.join(WORKDIR, "build")): builddir = os.path.join(WORKDIR, "build") diff --git a/.github/workflows/root-ci-config/null_build_check.py b/.github/workflows/root-ci-config/null_build_check.py new file mode 100644 index 0000000000000..c80a3d14f7cb3 --- /dev/null +++ b/.github/workflows/root-ci-config/null_build_check.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 + +# pylint: disable=missing-function-docstring,line-too-long + +"""Check that building an already built tree does not rebuild anything. + +Generator agnostic: rather than parsing build tool output, it fingerprints every +file in the build tree, builds again, and reports what was written. ROOT's one +accepted exception is etc/gitinfo.txt, which the gitinfotxt target in the top +level CMakeLists.txt rewrites on every build by design. + +Standalone usage: + + python3 null_build_check.py [...] +""" + +import argparse +import collections +import fnmatch +import os +import subprocess +import sys + +# Globs matched against the '/'-separated path relative to the build directory. +# fnmatch's '*' matches '/' too, so unanchored patterns match at any depth. +ALLOWED_PATTERNS = ( + "etc/gitinfo.txt", + # Build tool logs and dependency databases, not build products. + ".ninja_log", + ".ninja_deps", + ".ninja_lock", + ".cmake/api/*", + # Refreshed by every make. + "*/compiler_depend.*", + "*/depend.internal", + "*/depend.make", + "CMakeFiles/progress.marks", + # MSBuild file trackers and up-to-date markers. + "*.tlog", + "*.tlog/*", + "*.lastbuildstate", + "*.unsuccessfulbuild", + "*.CopyComplete", + # MSBuild writes the link recipe of every project on every build, whether + # or not the project is relinked. + "*.recipe", + # The ZERO_CHECK project of the Visual Studio generator rewrites the CMake + # generation stamps on every build to record that CMake need not re-run. A + # CMake run that really did regenerate would still be caught, since it + # rewrites the project files as well. + "CMakeFiles/generate.stamp", + "*/CMakeFiles/generate.stamp", + # ctest leftovers, in case the tree has been tested before. + "Testing/*", +) + +# ExternalProject keeps git clones of externals in the build tree. +PRUNED_DIRS = (".git",) + +MAX_REPORTED = 100 + + +def snapshot(builddir: str) -> dict: + fingerprints = {} + + for dirpath, dirnames, filenames in os.walk(builddir): + dirnames[:] = [name for name in dirnames if name not in PRUNED_DIRS] + + for filename in filenames: + path = os.path.join(dirpath, filename) + try: + stat = os.lstat(path) + except OSError: + continue # vanished under us, or a dangling symlink + key = os.path.relpath(path, builddir).replace(os.sep, "/") + fingerprints[key] = (stat.st_mtime_ns, stat.st_size) + + return fingerprints + + +def is_allowed(path: str) -> bool: + return any(fnmatch.fnmatch(path, pattern) for pattern in ALLOWED_PATTERNS) + + +def compare(before: dict, after: dict) -> list: + touched = [] + + for path, fingerprint in after.items(): + if path not in before: + touched.append((path, "created")) + elif before[path] != fingerprint: + touched.append((path, "modified")) + + for path in before: + if path not in after: + touched.append((path, "deleted")) + + return sorted(entry for entry in touched if not is_allowed(entry[0])) + + +def ninja_explain(builddir: str) -> str: + if not os.path.exists(os.path.join(builddir, "build.ninja")): + return "" + + try: + result = subprocess.run( + ["ninja", "-C", builddir, "-n", "-d", "explain"], + capture_output=True, + text=True, + errors="replace", + check=False, + ) + except OSError: + return "" + + return result.stderr + + +def check_null_build(builddir: str, rebuild) -> list: + """`rebuild` runs `cmake --build builddir` and returns its exit code.""" + + print(f"Recording the state of {builddir}") + before = snapshot(builddir) + print(f"{len(before)} files") + + returncode = rebuild() + if returncode != 0: + raise RuntimeError(f"rebuilding an already built tree failed with exit code {returncode}") + + return compare(before, snapshot(builddir)) + + +def summarize(touched: list) -> list: + """Count the reported paths per file type, most numerous first.""" + + counts = collections.Counter() + + for path, _ in touched: + name = path.rsplit("/", 1)[-1] + stem, dot, extension = name.rpartition(".") + counts["*." + extension if stem and dot else name] += 1 + + return counts.most_common() + + +def report(builddir: str, touched: list) -> None: + if not touched: + print("No spurious rebuilds: building again left the build tree untouched.") + return + + print(f"{len(touched)} file(s) were written by a build that had nothing to do:") + + for path, what in touched[:MAX_REPORTED]: + print(f" {what:<8} {path}") + + if len(touched) > MAX_REPORTED: + print(f" ... and {len(touched) - MAX_REPORTED} more") + # The listing is truncated and sorted by path, so a category that only + # shows up late would go unnoticed without this. + print("\nBy file type:") + for kind, count in summarize(touched): + print(f" {count:>6} {kind}") + + explanation = ninja_explain(builddir) + if explanation: + print("\nWhy ninja thinks there is work left to do:") + print(explanation) + + print(""" +Usual causes are a custom command that does not declare its OUTPUT or BYPRODUCTS, +an output that ends up newer than what it is compared against, or a generated +file embedding a timestamp. If a file really has to be rewritten on every build, +add it to ALLOWED_PATTERNS above.""") + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("builddir", help="a ROOT build directory that is fully built") + parser.add_argument("build_args", nargs="*", help="extra arguments for `cmake --build`, e.g. --config Release") + args = parser.parse_args() + + builddir = os.path.abspath(args.builddir) + command = ["cmake", "--build", builddir, "--parallel", str(os.cpu_count())] + args.build_args + + def rebuild() -> int: + print("+ " + " ".join(command)) + return subprocess.run(command, check=False).returncode + + touched = check_null_build(builddir, rebuild) + report(builddir, touched) + + return 1 if touched else 0 + + +if __name__ == "__main__": + sys.exit(main()) From 5eba763838c0fdb174455bb03408e3219c5d7e16 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 10 Aug 2026 12:05:52 +0000 Subject: [PATCH 2/5] [cmake] Do not touch the dummy source of NO_SOURCES packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ROOT_STANDARD_LIBRARY_PACKAGE(... NO_SOURCES) provided the dummy source file that add_library() needs as the output of a custom command that touches it. All the packages of one directory share that output, and listing one custom command output in several independent targets that build in parallel is not supported; core/clingutils alone has 15 of them. Ninja and Make happen to get away with it, but with MSBuild the touch runs again on every build, so 14 of the 15 STL dictionaries were recompiled and relinked every time, along with everything depending on them. Write the file at configure time instead. Nothing touches it at build time anymore, so it cannot go stale under the targets that compile it. 🤖 Done with the help of AI --- cmake/modules/RootMacros.cmake | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 864f5f6c81a2d..858a0e550dbb2 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -1371,10 +1371,20 @@ function(ROOT_STANDARD_LIBRARY_PACKAGE libname) set(NO_CXXMODULE_FLAG "NO_CXXMODULE") endif() + set(dummy_source) if(ARG_NO_SOURCES) # Workaround bug in CMake by adding a dummy source file if all sources are generated, since # in that case the initial call to add_library() may not list any sources and CMake complains. - add_custom_command(OUTPUT dummy.cxx COMMAND ${CMAKE_COMMAND} -E touch dummy.cxx) + # + # The file is written here at configure time rather than by a custom command on purpose: all + # the packages of one directory share it, and listing one custom command output in several + # independent targets that build in parallel is not supported. With MSBuild the touch ran + # again on every build, so the 15 STL dictionaries of core/clingutils kept recompiling and + # relinking, while Ninja and Make happened to get away with it. + set(dummy_source ${CMAKE_CURRENT_BINARY_DIR}/dummy.cxx) + if(NOT EXISTS ${dummy_source}) + file(WRITE ${dummy_source} "") + endif() endif() if(runtime_cxxmodules) @@ -1390,16 +1400,14 @@ function(ROOT_STANDARD_LIBRARY_PACKAGE libname) endif() if (ARG_OBJECT_LIBRARY) - ROOT_OBJECT_LIBRARY(${libname}Objs ${ARG_SOURCES} - $<$:dummy.cxx>) + ROOT_OBJECT_LIBRARY(${libname}Objs ${ARG_SOURCES} ${dummy_source}) ROOT_LINKER_LIBRARY(${libname} $ LIBRARIES ${ARG_LIBRARIES} DEPENDENCIES ${ARG_DEPENDENCIES} BUILTINS ${ARG_BUILTINS} ) else(ARG_OBJECT_LIBRARY) - ROOT_LINKER_LIBRARY(${libname} ${ARG_SOURCES} - $<$:dummy.cxx> + ROOT_LINKER_LIBRARY(${libname} ${ARG_SOURCES} ${dummy_source} LIBRARIES ${ARG_LIBRARIES} DEPENDENCIES ${ARG_DEPENDENCIES} BUILTINS ${ARG_BUILTINS} From 11a4407106e3e1238f895527a5a90508b2e73c62 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 10 Aug 2026 22:00:09 +0000 Subject: [PATCH 3/5] [cmake] Use copy_if_different in Windows post-build test-fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MSBuild re-executes these post-build events even when the build has nothing to do, and `cmake -E copy` rewrites its destination unconditionally, so every null build modified those files. Ninja and Makefiles are immune because there POST_BUILD steps only run when the target itself relinks. Use `cmake -E copy_if_different` so a no-op event leaves the destination untouched, and replace the data.h POST_BUILD hack with configure_file(... COPYONLY), the convention the sibling test directories already use for such runtime inputs. 🤖 Done with the help of AI --- io/io/test/CMakeLists.txt | 4 ++-- math/mathcore/test/CMakeLists.txt | 4 ++-- test/CMakeLists.txt | 16 ++++++++-------- tree/dataframe/test/CMakeLists.txt | 4 ++-- tree/ntuple/test/CMakeLists.txt | 12 ++++++------ tree/ntupleutil/test/CMakeLists.txt | 4 ++-- tree/treeplayer/test/CMakeLists.txt | 3 +-- 7 files changed, 23 insertions(+), 24 deletions(-) diff --git a/io/io/test/CMakeLists.txt b/io/io/test/CMakeLists.txt index 78fa44aa014d9..913f415edda87 100644 --- a/io/io/test/CMakeLists.txt +++ b/io/io/test/CMakeLists.txt @@ -27,8 +27,8 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RFileTestIncludes configure_file(RFileTestIncludes.hxx . COPYONLY) if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET RFileTestIncludes POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/$/libRFileTestIncludes.dll - ${CMAKE_CURRENT_BINARY_DIR}/libRFileTestIncludes.dll) + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/$/libRFileTestIncludes.dll + ${CMAKE_CURRENT_BINARY_DIR}/libRFileTestIncludes.dll) endif() ROOT_ADD_GTEST(rfile rfile.cxx LIBRARIES RIO Hist ROOTNTuple Tree RFileTestIncludes) if(pyroot) diff --git a/math/mathcore/test/CMakeLists.txt b/math/mathcore/test/CMakeLists.txt index a0b9dc01f8b7c..71b71907c94eb 100644 --- a/math/mathcore/test/CMakeLists.txt +++ b/math/mathcore/test/CMakeLists.txt @@ -61,8 +61,8 @@ ROOT_STANDARD_LIBRARY_PACKAGE(TrackMathCoreUnitDict if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET TrackMathCoreUnitDict POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/$/libTrackMathCoreUnitDict.dll - ${CMAKE_CURRENT_BINARY_DIR}/libTrackMathCoreUnitDict.dll) + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/$/libTrackMathCoreUnitDict.dll + ${CMAKE_CURRENT_BINARY_DIR}/libTrackMathCoreUnitDict.dll) endif() ROOT_ADD_GTEST(stressMathCoreUnit stress/testSMatrix.cxx stress/testGenVector.cxx diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index bbc10b1b3cd80..2eebde410858f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -23,16 +23,16 @@ ROOT_STANDARD_LIBRARY_PACKAGE(Event DEPENDENCIES Hist MathCore) if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET Event POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/$/libEvent.dll - ${CMAKE_CURRENT_BINARY_DIR}/libEvent.dll) + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/$/libEvent.dll + ${CMAKE_CURRENT_BINARY_DIR}/libEvent.dll) if(NOT runtime_cxxmodules) add_custom_command(TARGET Event POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/libEvent_rdict.pcm - ${CMAKE_CURRENT_BINARY_DIR}/$/libEvent_rdict.pcm) + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/libEvent_rdict.pcm + ${CMAKE_CURRENT_BINARY_DIR}/$/libEvent_rdict.pcm) else() add_custom_command(TARGET Event POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/Event.pcm - ${CMAKE_CURRENT_BINARY_DIR}/$/Event.pcm) + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/Event.pcm + ${CMAKE_CURRENT_BINARY_DIR}/$/Event.pcm) endif() endif() ROOT_EXECUTABLE(eventexe MainEvent.cxx LIBRARIES Event RIO Tree TreePlayer Hist Net) @@ -256,8 +256,8 @@ ROOT_STANDARD_LIBRARY_PACKAGE(TrackMathCoreDict DEPENDENCIES Core MathCore RIO GenVector Smatrix) if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET TrackMathCoreDict POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/$/libTrackMathCoreDict.dll - ${CMAKE_CURRENT_BINARY_DIR}/libTrackMathCoreDict.dll) + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/$/libTrackMathCoreDict.dll + ${CMAKE_CURRENT_BINARY_DIR}/libTrackMathCoreDict.dll) endif() ROOT_EXECUTABLE(stressMathCore stressMathCore.cxx LIBRARIES MathCore Hist RIO Tree GenVector Smatrix) ROOT_ADD_TEST(test-stressmathcore COMMAND stressMathCore FAILREGEX "FAILED|Error in" LABELS longtest) diff --git a/tree/dataframe/test/CMakeLists.txt b/tree/dataframe/test/CMakeLists.txt index 490b883fe7cbb..d6085886c7110 100644 --- a/tree/dataframe/test/CMakeLists.txt +++ b/tree/dataframe/test/CMakeLists.txt @@ -127,8 +127,8 @@ ROOT_STANDARD_LIBRARY_PACKAGE(NTupleStruct configure_file(NTupleStruct.hxx . COPYONLY) if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET NTupleStruct POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/$/libNTupleStruct.dll - ${CMAKE_CURRENT_BINARY_DIR}/libNTupleStruct.dll) + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/$/libNTupleStruct.dll + ${CMAKE_CURRENT_BINARY_DIR}/libNTupleStruct.dll) endif() ROOT_GENERATE_DICTIONARY(ClassWithArraysDict ${CMAKE_CURRENT_SOURCE_DIR}/ClassWithArrays.h MODULE datasource_ntuple LINKDEF ClassWithArraysLinkDef.h OPTIONS -inlineInputHeader diff --git a/tree/ntuple/test/CMakeLists.txt b/tree/ntuple/test/CMakeLists.txt index 82d3045d4563d..d3f26e276f66c 100644 --- a/tree/ntuple/test/CMakeLists.txt +++ b/tree/ntuple/test/CMakeLists.txt @@ -15,8 +15,8 @@ ROOT_STANDARD_LIBRARY_PACKAGE(CustomStruct configure_file(CustomStruct.hxx . COPYONLY) if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET CustomStruct POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/$/libCustomStruct.dll - ${CMAKE_CURRENT_BINARY_DIR}/libCustomStruct.dll) + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/$/libCustomStruct.dll + ${CMAKE_CURRENT_BINARY_DIR}/libCustomStruct.dll) endif() ROOT_ADD_GTEST(ntuple_attributes ntuple_attributes.cxx ntuple_test.cxx LIBRARIES ROOTNTuple xxHash::xxHash) @@ -135,9 +135,9 @@ target_compile_features(StreamerFieldXMLDict PRIVATE $) if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET rfield_streamer POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/librfield_streamer_rdict.pcm + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/librfield_streamer_rdict.pcm ${CMAKE_CURRENT_BINARY_DIR}/$/librfield_streamer_rdict.pcm - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/StreamerFieldXMLDict_rdict.pcm + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/StreamerFieldXMLDict_rdict.pcm ${CMAKE_CURRENT_BINARY_DIR}/$/StreamerFieldXMLDict_rdict.pcm) endif() @@ -155,9 +155,9 @@ target_compile_features(SoAFieldXMLDict PRIVATE $) if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET ntuple_soa POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/libntuple_soa_rdict.pcm + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/libntuple_soa_rdict.pcm ${CMAKE_CURRENT_BINARY_DIR}/$/libntuple_soa_rdict.pcm - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/SoAFieldXMLDict_rdict.pcm + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/SoAFieldXMLDict_rdict.pcm ${CMAKE_CURRENT_BINARY_DIR}/$/SoAFieldXMLDict_rdict.pcm) endif() diff --git a/tree/ntupleutil/test/CMakeLists.txt b/tree/ntupleutil/test/CMakeLists.txt index 5b4f38049836e..390161c7bddf8 100644 --- a/tree/ntupleutil/test/CMakeLists.txt +++ b/tree/ntupleutil/test/CMakeLists.txt @@ -15,8 +15,8 @@ ROOT_STANDARD_LIBRARY_PACKAGE(CustomStructUtil configure_file(CustomStructUtil.hxx . COPYONLY) if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET CustomStructUtil POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/$/libCustomStructUtil.dll - ${CMAKE_CURRENT_BINARY_DIR}/libCustomStructUtil.dll) + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/$/libCustomStructUtil.dll + ${CMAKE_CURRENT_BINARY_DIR}/libCustomStructUtil.dll) endif() ROOT_ADD_GTEST(ntuple_importer ntuple_importer.cxx LIBRARIES ROOTNTupleUtil CustomStructUtil) diff --git a/tree/treeplayer/test/CMakeLists.txt b/tree/treeplayer/test/CMakeLists.txt index e4d73183ffcaa..c646594d889a0 100644 --- a/tree/treeplayer/test/CMakeLists.txt +++ b/tree/treeplayer/test/CMakeLists.txt @@ -25,8 +25,7 @@ ROOT_GENERATE_DICTIONARY(CMSDASClassesDict ${CMAKE_CURRENT_SOURCE_DIR}/CMSDASCla ROOT_ADD_GTEST(treeplayer_gh20033 gh20033.cxx CMSDASClassesDict.cxx LIBRARIES TreePlayer) ROOT_ADD_GTEST(treeplayer_leafs leafs.cxx LIBRARIES TreePlayer) -add_custom_command(TARGET treeplayer_leafs POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/data.h data.h) +configure_file(data.h . COPYONLY) ROOT_ADD_GTEST(treeplayer_readerarray_iterator readerarray_iterator.cxx LIBRARIES TreePlayer) From ec99f0de52928f3077b62a61733c3765be232c12 Mon Sep 17 00:00:00 2001 From: DhanashreePetare Date: Wed, 29 Jul 2026 06:32:43 +0000 Subject: [PATCH 4/5] [Build System] Use DEPFILE instead of IMPLICIT_DEPENDS for dictionaries Fixes #21203. ROOT_GENERATE_DICTIONARY() relied on add_custom_command(IMPLICIT_DEPENDS) to rebuild a dictionary when one of its (transitively) included headers changed. IMPLICIT_DEPENDS is only honoured by the Unix Makefiles generator, is essentially broken even there, and cannot express cross-directory dependencies (see #21203, #20907). Incremental builds therefore routinely missed header changes. Replace it with a proper DEPFILE: * rootcling learns a "-MF " option that writes a Makefile-format dependency file listing every header opened while generating the dictionary. * ROOT_GENERATE_DICTIONARY() passes "-MF" and hands the file to add_custom_command(DEPFILE ...), which is supported by the Ninja, Makefiles and IDE generators. Making the generated dependency file correct on every generator/platform (Ninja "deps = gcc", Unix Makefiles, Windows) required more than dumping the FileManager contents. The following points were needed and are the substance of the cross-platform fixes: * Target name. rootcling writes the dictionary to a temporary file and only renames it to its final name on success, so gOptDictionaryFileName already holds the temporary name ("G__X.cxx_tmp_") by the time the depfile is written. The original dictionary name is captured earlier and used as the depfile target instead. * Skip cling's in-memory buffers. The interpreter's FileManager also reports virtual buffers such as "input_line_N" and "<<< cling interactive line includer >>>". These are not real files: the latter contains spaces and angle brackets that corrupt the Makefile-format depfile (the parser splits it into bogus dependencies, which on Windows is compounded by the drive-letter colon), and none of them exist on disk, which would leave the dictionary permanently out of date. They are filtered out by requiring each entry to exist on disk, plus an explicit angle-bracket check. * Emit absolute paths. rootcling may be invoked from a different directory than the one the depfile is later consumed from. ROOT sets CMP0116 to OLD, so CMake does not rewrite the depfile, and a relative entry like "./Foo.hxx" (as produced for some roottest dictionaries) would be resolved against the wrong base and again leave the dictionary perpetually dirty. Every dependency is therefore made absolute. * Escape characters that are special to make (space, tab, '#', ':') and normalise separators to '/' so the file is valid on Windows too. The dependency file uses the ".depfile" extension rather than ".d" to avoid it being mistaken for a compiler-generated dependency file in another context. Validated with a full ROOT + roottest rebuild (Ninja): all dictionaries regenerate exactly once, after which the tree settles to a genuine no-op incremental build with no dictionary left perpetually dirty; touching a header regenerates precisely the dictionaries that include it. This resolves the roottest IO/tree/treeformula and Windows "*-build" failures seen on the draft PR. --- cmake/modules/RootMacros.cmake | 11 ++-- core/dictgen/src/rootcling_impl.cxx | 96 +++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 5 deletions(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 858a0e550dbb2..b811226e9687e 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -685,10 +685,10 @@ function(ROOT_GENERATE_DICTIONARY dictionary) endforeach() #---build the implicit dependencies arguments - # NOTE: only the Makefile generator respects this! - foreach(_dep ${_linkdef} ${_list_of_header_dependencies}) - list(APPEND _implicitdeps CXX ${_dep}) - endforeach() + # NOTE: DEPFILE is used instead of IMPLICIT_DEPENDS because IMPLICIT_DEPENDS + # only works with Unix Makefiles and has issues with cross-directory dependencies. + # DEPFILE works with all generators (Ninja, Unix Makefiles, etc.) + set(depfile_path ${CMAKE_CURRENT_BINARY_DIR}/${dictionary}.depfile) if(ARG_MODULE) set(MODULE_LIB_DEPENDENCY ${ARG_DEPENDENCIES}) @@ -741,7 +741,8 @@ function(ROOT_GENERATE_DICTIONARY dictionary) # make the dictionary generation command depend on the C++ standard, ensuring that the # dictionaries will be rebuilt if the C++ standard is changed in an incremental build. -DR__DUMMY_CXX_STANDARD_${CMAKE_CXX_STANDARD} - IMPLICIT_DEPENDS ${_implicitdeps} + -MF ${depfile_path} + DEPFILE ${depfile_path} DEPENDS ${_list_of_header_dependencies} ${_linkdef} ${ROOTCLINGDEP} ${pcm_dependencies} ${MODULE_LIB_DEPENDENCY} ${ARG_EXTRA_DEPENDENCIES} diff --git a/core/dictgen/src/rootcling_impl.cxx b/core/dictgen/src/rootcling_impl.cxx index caf295b27f703..d2ae3ec534233 100644 --- a/core/dictgen/src/rootcling_impl.cxx +++ b/core/dictgen/src/rootcling_impl.cxx @@ -3709,6 +3709,10 @@ static llvm::cl::list gOptWDiags("W", llvm::cl::Prefix, llvm::cl::ZeroOrMore, llvm::cl::desc("Specify compiler diagnostics options."), llvm::cl::cat(gRootclingOptions)); +static llvm::cl::opt +gOptDepFile("MF", + llvm::cl::desc("Write dependency output to the specified file."), + llvm::cl::cat(gRootclingOptions)); // Really OneOrMore, will be changed in RootClingMain below. static llvm::cl::list gOptDictionaryHeaderFiles(llvm::cl::Positional, llvm::cl::ZeroOrMore, @@ -4517,6 +4521,11 @@ int RootClingMain(int argc, // Check if code goes to stdout or rootcling file std::ofstream fileout; string main_dictname(gOptDictionaryFileName.getValue()); + // Keep the original dictionary output file name (with extension) for the + // dependency file target: `main_dictname` gets its extension stripped below + // and `gOptDictionaryFileName` is turned into a temporary name by the + // tmpCatalog a few lines down. + const std::string dictOutputFileName(gOptDictionaryFileName.getValue()); std::ostream *splitDictStream = nullptr; std::unique_ptr splitDeleter(nullptr); // Store the temp files @@ -4999,6 +5008,93 @@ int RootClingMain(int argc, // make sure the file is closed before committing fileout.close(); + // Write the dependency file if requested (-MF ). It uses the + // Makefile format understood by CMake's DEPFILE and Ninja's "deps = gcc", + // listing every real header that was opened while generating the dictionary + // so that incremental builds pick up changes to transitively included files. + if (!gOptDepFile.empty() && rootclingRetCode == 0 && !dictOutputFileName.empty()) { + std::ofstream depFile(gOptDepFile.c_str()); + if (!depFile) { + ROOT::TMetaUtils::Error(nullptr, + "rootcling: failed to open dependency file %s\n", + gOptDepFile.c_str()); + rootclingRetCode = 1; + } else { + // Escape a path for the Makefile-format dependency file: forward + // slashes (needed on Windows) and backslash-escape the characters that + // are special to make (space, tab, '#', ':'). + auto escapeForDepFile = [](std::string path) { + std::replace(path.begin(), path.end(), '\\', '/'); + std::string escaped; + escaped.reserve(path.size()); + for (char c : path) { + if (c == ' ' || c == '\t' || c == '#' || c == ':') + escaped += '\\'; + escaped += c; + } + return escaped; + }; + + // The target is the final dictionary source file. Note that + // gOptDictionaryFileName has been turned into a temporary name by the + // tmpCatalog, so we use the original name captured earlier. + depFile << escapeForDepFile(dictOutputFileName) << ":"; + + // Collect all files that were read by clang during dictionary + // generation (headers included directly or indirectly). + clang::SourceManager &SM = CI->getSourceManager(); + clang::FileManager &FM = SM.getFileManager(); + + llvm::SmallVector files; + FM.GetUniqueIDMapping(files); + + std::set includedFiles; + for (const auto &FEOpt : files) { + if (!FEOpt) + continue; + llvm::StringRef filename = FEOpt->getName(); + if (filename.empty()) + continue; + // Skip cling's in-memory buffers, which the FileManager also + // reports: "input_line_N", "<<< cling interactive line includer >>>", + // "", "", ... These are not real files; + // some contain spaces or angle brackets that would corrupt the + // dependency file, and all of them would make the dictionary appear + // perpetually out of date. Requiring the entry to exist on disk + // filters them out (together with the explicit angle-bracket check). + if (filename.contains('<') || filename.contains('>')) + continue; + // Make the path absolute so it is unambiguous regardless of the + // working directory: rootcling may run from a different directory + // than the one the dependency file is later consumed from (with + // CMP0116 OLD the depfile is not rewritten, and a relative entry + // like "./Foo.hxx" would be resolved against the wrong base and + // leave the dictionary permanently out of date). + llvm::SmallString<256> absPath(filename); + llvm::sys::fs::make_absolute(absPath); + if (!llvm::sys::fs::exists(absPath)) + continue; + std::string filenameStr(absPath.str()); + // Skip the output dictionary file itself (final or temporary name). + if (filenameStr == dictOutputFileName || filenameStr == gOptDictionaryFileName.getValue()) + continue; + includedFiles.insert(std::move(filenameStr)); + } + + // Each dependency line except the last ends with a backslash. + for (const auto &file : includedFiles) + depFile << " \\\n " << escapeForDepFile(file); + if (!includedFiles.empty()) + depFile << "\n"; + + depFile.close(); + if (!depFile.good()) { + ROOT::TMetaUtils::Error(nullptr, "rootcling: failed to write dependency file %s\n", gOptDepFile.c_str()); + rootclingRetCode = 1; + } + } + } + // Before returning, rename the files if no errors occurred // otherwise clean them to avoid remnants (see ROOT-10015) if(rootclingRetCode == 0) { From a06a9e904104f4476ddf2c038917752f4d594221 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 10 Aug 2026 10:30:20 +0200 Subject: [PATCH 5/5] Use Ninja on alma8 --- .github/workflows/root-ci-config/buildconfig/alma8.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/root-ci-config/buildconfig/alma8.txt b/.github/workflows/root-ci-config/buildconfig/alma8.txt index e559349baaa0d..cd783cbc97b72 100644 --- a/.github/workflows/root-ci-config/buildconfig/alma8.txt +++ b/.github/workflows/root-ci-config/buildconfig/alma8.txt @@ -1,8 +1,10 @@ +CMAKE_GENERATOR=Ninja builtin_civetweb=ON builtin_gtest=ON builtin_nlohmannjson=ON builtin_tbb=ON builtin_vdt=ON curl=OFF +fortran=OFF pythia8=ON use_gsl_cblas=ON