From 3f0ec0b1572376d8234ba9e61f710b21e2d0043e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 24 Aug 2026 15:56:59 -0500 Subject: [PATCH] [espidf] Always reconfigure after component discovery (#18730) --- esphome/espidf/toolchain.py | 38 ++++++++---------- tests/unit_tests/test_espidf_toolchain.py | 49 +++++++++++++++++++++++ 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 3c5c4803c2..baf316a4ee 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -398,27 +398,23 @@ def run_compile(config, verbose: bool) -> int: return rc _LOGGER.info("Regenerating CMakeLists.txt with discovered components...") write_project(minimal=False) - # Restamp the reference file has_outdated_files() compares against. - # A reconfigure that only changes properties or plain variables - # (sdkconfig options, the exclusion set) does not rewrite - # CMakeCache.txt, so without this the watched inputs stay newer - # forever and every subsequent build repeats the discovery pass. - # Done after the full write so an interrupt cannot leave a minimal - # CMakeLists behind that is already marked fresh. - cmakecache = CORE.relative_build_path("build/CMakeCache.txt") - if cmakecache.is_file(): - os.utime(cmakecache) - if CORE.testing_mode: - # Reconfigure again so cmake is up to date with the full - # component list before the build's idf.py invocation runs -- - # idf.py build would otherwise re-run cmake and regenerate - # memory.ld, wiping the DRAM/IRAM patches applied below. - # Outside testing mode ninja's own configure-time dep on - # CMakeLists.txt handles the re-run as part of the build step. - rc = run_reconfigure() - if rc != 0: - _LOGGER.error("Reconfigure with discovered components failed") - return rc + # Explicit reconfigure: ninja only re-runs cmake when CMakeLists.txt + # is strictly newer than build.ninja, which fails on coarse-mtime + # filesystems (#18682). Also keeps idf.py from regenerating memory.ld + # in testing mode. + rc = run_reconfigure() + if rc != 0: + _LOGGER.error("Reconfigure with discovered components failed") + return rc + # cmake does not rewrite CMakeCache.txt when only properties change, + # so restamp it or every build repeats discovery. Only after success, + # or a failed reconfigure would be marked fresh. build.ninja is + # restamped too so the cache is not newer and ninja does not + # re-run cmake. + for name in ("build/CMakeCache.txt", "build/build.ninja"): + path = CORE.relative_build_path(name) + if path.is_file(): + os.utime(path) # In testing mode, generate the linker script first, patch DRAM/IRAM sizes, # then build. memory.ld is regenerated by ninja during the build phase, diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index 6c11a74d48..c54daec6a4 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -299,10 +299,13 @@ def test_run_compile_restamps_cmakecache_after_discovery(setup_core: Path) -> No _setup_build(setup_core) config = {CONF_ESPHOME: {}} cmakecache = CORE.relative_build_path("build/CMakeCache.txt") + build_ninja = CORE.relative_build_path("build/build.ninja") cmakecache.parent.mkdir(parents=True, exist_ok=True) cmakecache.write_text("") + build_ninja.write_text("") old = cmakecache.stat().st_mtime - 100 os.utime(cmakecache, (old, old)) + os.utime(build_ninja, (old, old)) with ( patch.object(toolchain, "need_reconfigure", return_value=True), @@ -314,6 +317,8 @@ def test_run_compile_restamps_cmakecache_after_discovery(setup_core: Path) -> No assert toolchain.run_compile(config, verbose=False) == 0 assert cmakecache.stat().st_mtime > old + # build.ninja must not be older than the cache or ninja re-runs cmake + assert build_ninja.stat().st_mtime >= cmakecache.stat().st_mtime def test_run_compile_discovery_without_cmakecache(setup_core: Path) -> None: @@ -334,6 +339,50 @@ def test_run_compile_discovery_without_cmakecache(setup_core: Path) -> None: assert not CORE.relative_build_path("build/CMakeCache.txt").exists() +def test_run_compile_reconfigures_after_full_write_outside_testing_mode( + setup_core: Path, +) -> None: + """The full CMakeLists write is followed by a reconfigure (#18682); a + failure there stops the build and leaves the cache unstamped.""" + _setup_build(setup_core) + config = {CONF_ESPHOME: {}} + cmakecache = CORE.relative_build_path("build/CMakeCache.txt") + cmakecache.parent.mkdir(parents=True, exist_ok=True) + cmakecache.write_text("") + old = cmakecache.stat().st_mtime - 100 + os.utime(cmakecache, (old, old)) + calls: list[tuple] = [] + reconfigures = 0 + + def record_write(minimal: bool = False) -> None: + calls.append(("write_project", minimal)) + + def record_reconfigure() -> int: + nonlocal reconfigures + reconfigures += 1 + calls.append(("run_reconfigure",)) + return 1 if reconfigures == 2 else 0 + + with ( + patch.object(toolchain, "need_reconfigure", return_value=True), + patch("esphome.build_gen.espidf.write_project", side_effect=record_write), + patch.object(toolchain, "run_reconfigure", side_effect=record_reconfigure), + patch.object(toolchain, "run_idf_py", return_value=0) as mock_build, + patch.object(toolchain, "print_summary"), + ): + assert not CORE.testing_mode + assert toolchain.run_compile(config, verbose=False) == 1 + + assert calls == [ + ("write_project", True), + ("run_reconfigure",), + ("write_project", False), + ("run_reconfigure",), + ] + mock_build.assert_not_called() + assert cmakecache.stat().st_mtime == old + + def test_run_compile_passes_compile_process_limit(setup_core: Path) -> None: """compile_process_limit is forwarded to run_idf_py as the job limit.""" _setup_build(setup_core)