[espidf] Always reconfigure after component discovery (#18730)

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,

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)