Share the CMake pch consumer block, apply review suggestions

esphome/build_gen/espidf.py

@@ -291,30 +291,13 @@ target_link_options(${{COMPONENT_LIB}} PUBLIC
 
 
 def _pch_cmake() -> str:
-    """The src component's precompiled-header block (C++ TUs only).
+    """Consumer block appended to the component CMakeLists.
 
-    The -include stays relative (resolved from the compiler cwd, the build
-    dir); an absolute path would poison ccache keys.
+    Strict inverts: a per-process consumer rejection reds the build.
+    Baked at generation: a knob flip takes effect when the CMakeLists is
+    rewritten (every esphome compile); a hand-run idf.py keeps the old one
     """
-    if not pch_enabled():
-        return ""
-    # Strict inverts: a per-process consumer rejection reds the build.
-    # Baked at generation: a knob flip takes effect when the CMakeLists is
-    # rewritten (every esphome compile); a hand-run idf.py keeps the old one
-    escalation = pch.pch_consumer_escalation()
-    return f"""
-# ESPHome precompiled header (see esphome/build_helpers/pch.py).
-# OBJECT_DEPENDS is on the header, not the .gch: pch-baked headers drop
-# out of TU depfiles, and prepare_pch() touches the header on rebuild.
-target_compile_options(${{COMPONENT_LIB}} PRIVATE
-    "$<$<COMPILE_LANGUAGE:CXX>:-Winvalid-pch>"
-    "$<$<COMPILE_LANGUAGE:CXX>:{escalation}>"
-    "$<$<COMPILE_LANGUAGE:CXX>:-include>"
-    "$<$<COMPILE_LANGUAGE:CXX>:{PCH_HEADER_NAME}>"
-)
-set_source_files_properties(${{app_sources}} PROPERTIES
-    OBJECT_DEPENDS "${{CMAKE_BINARY_DIR}}/{PCH_HEADER_NAME}")
-"""
+    return pch.pch_cmake_consumer("${COMPONENT_LIB}", "${app_sources}")
 
 
 def discard_pch() -> None:

esphome/build_helpers/pch.py

@@ -156,6 +156,33 @@ def pch_consumer_escalation() -> str:
     return "-Werror=invalid-pch" if pch_strict() else "-Wno-error=invalid-pch"
 
 
+def pch_cmake_consumer(target: str, sources_var: str) -> str:
+    """Emit the CMake block making ``target``'s C++ sources consume the
+    pch; empty when disabled. Shared by every CMake-based backend so the
+    consumer contract (flags, relative include, header dependency)
+    cannot drift between them.
+
+    OBJECT_DEPENDS is on the header, not the .gch: pch-baked headers drop
+    out of TU depfiles, and prepare_pch() touches the header on rebuild.
+    The -include stays relative (resolved from the compiler cwd, the build
+    dir); an absolute path would poison ccache keys.
+    """
+    if not pch_enabled():
+        return ""
+    escalation = pch_consumer_escalation()
+    return f"""
+# ESPHome precompiled header (see esphome/build_helpers/pch.py)
+target_compile_options({target} PRIVATE
+    "$<$<COMPILE_LANGUAGE:CXX>:-Winvalid-pch>"
+    "$<$<COMPILE_LANGUAGE:CXX>:{escalation}>"
+    "$<$<COMPILE_LANGUAGE:CXX>:-include>"
+    "$<$<COMPILE_LANGUAGE:CXX>:{PCH_HEADER_NAME}>"
+)
+set_source_files_properties({sources_var} PROPERTIES
+    OBJECT_DEPENDS "${{CMAKE_BINARY_DIR}}/{PCH_HEADER_NAME}")
+"""
+
+
 def ccache_pch_env() -> dict[str, str]:
     """Settings ccache needs to cache compiles that consume the .gch;
     empty unless this build actually emitted one. User-set values win.

esphome/components/nrf52/__init__.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import asyncio
+from contextlib import suppress
 import logging
 from pathlib import Path
 import re
@@ -13,7 +14,7 @@ from esphome.build_helpers.pch import (
     PCH_DEFAULT_HEADERS,
     PCH_HEADER_NAME,
     mark_pch_emitted,
-    pch_consumer_escalation,
+    pch_cmake_consumer,
     pch_enabled,
     pch_header_text,
 )
@@ -814,24 +815,8 @@ def _generate_cmake_lists() -> bool:
             ")",
         ]
 
-    if pch_enabled():
-        # ESPHome precompiled header (see esphome/build_helpers/pch.py).
-        # OBJECT_DEPENDS is on the header, not the .gch: pch-baked headers
-        # drop out of TU depfiles, and prepare_pch() touches the header on
-        # rebuild. The relative -include resolves from the compiler cwd
-        # (the build dir); an absolute path would poison ccache keys.
-        escalation = pch_consumer_escalation()
-        lines += [
-            "",
-            "target_compile_options(app PRIVATE",
-            '  "$<$<COMPILE_LANGUAGE:CXX>:-Winvalid-pch>"',
-            f'  "$<$<COMPILE_LANGUAGE:CXX>:{escalation}>"',
-            '  "$<$<COMPILE_LANGUAGE:CXX>:-include>"',
-            f'  "$<$<COMPILE_LANGUAGE:CXX>:{PCH_HEADER_NAME}>"',
-            ")",
-            "set_source_files_properties(${APP_SOURCES} PROPERTIES",
-            f'  OBJECT_DEPENDS "${{CMAKE_BINARY_DIR}}/{PCH_HEADER_NAME}")',
-        ]
+    if consumer := pch_cmake_consumer("app", "${APP_SOURCES}"):
+        lines += consumer.rstrip("\n").splitlines()
 
     if link_flags:
         lines += [
@@ -871,7 +856,19 @@ def _prepare_pch(app_dir: Path) -> None:
     write_file_if_changed(
         app_dir / PCH_HEADER_NAME, pch_header_text(PCH_DEFAULT_HEADERS)
     )
-    autoconf = next(app_dir.glob("zephyr/include/generated/**/autoconf.h"), None)
+    # New layout first (Zephyr >= 3.4 nests under zephyr/); fixed candidates
+    # keep the .sum identity deterministic and skip walking generated/
+    autoconf = next(
+        (
+            candidate
+            for candidate in (
+                app_dir / "zephyr" / "include" / "generated" / "zephyr" / "autoconf.h",
+                app_dir / "zephyr" / "include" / "generated" / "autoconf.h",
+            )
+            if candidate.exists()
+        ),
+        None,
+    )
     if autoconf is None:
         # Fail closed: autoconf.h is the .sum's Kconfig identity
         _LOGGER.warning("No autoconf.h found; compiling without the pch")
@@ -970,7 +967,7 @@ def run_compile(args, config: ConfigType) -> bool:
                 stream_output=True,
                 cwd=str(paths["framework_path"]),
             ):
-                raise EsphomeError("nRF52 native build failed")
+                raise EsphomeError("nRF52 native build configure failed")
             # The pch includes zephyr/kernel.h, whose syscall headers are
             # generated at build time (same target the clang-tidy flow uses)
             if not run_command_ok(
@@ -985,7 +982,7 @@ def run_compile(args, config: ConfigType) -> bool:
                 stream_output=True,
                 cwd=str(paths["framework_path"]),
             ):
-                raise EsphomeError("nRF52 native build failed")
+                raise EsphomeError("nRF52 Zephyr header generation failed")
 
     # An optional speedup must never abort the build
     app_dir = _app_build_dir(build_dir)
@@ -998,6 +995,9 @@ def run_compile(args, config: ConfigType) -> bool:
         pch.discard_pch(app_dir)
         if strict:
             raise
+        # Best effort: OBJECT_DEPENDS needs the header even without a pch
+        with suppress(OSError):
+            (app_dir / PCH_HEADER_NAME).touch()
         _LOGGER.warning(
             "Precompiled header setup failed; compiling without it", exc_info=True
         )

tests/unit_tests/build_helpers/test_pch.py

@@ -232,6 +232,34 @@ def test_pch_strict(
     assert pch.pch_strict() is expected
 
 
+def test_pch_cmake_consumer_substitutes_target_and_sources(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.delenv("ESPHOME_PCH_ENABLE", raising=False)
+    monkeypatch.delenv("ESPHOME_PCH_STRICT", raising=False)
+    block = pch.pch_cmake_consumer("app", "${APP_SOURCES}")
+    assert "target_compile_options(app PRIVATE" in block
+    assert '"$<$<COMPILE_LANGUAGE:CXX>:-Winvalid-pch>"' in block
+    assert "-Wno-error=invalid-pch" in block
+    assert '"$<$<COMPILE_LANGUAGE:CXX>:esphome_pch.h>"' in block
+    assert "set_source_files_properties(${APP_SOURCES} PROPERTIES" in block
+    assert 'OBJECT_DEPENDS "${CMAKE_BINARY_DIR}/esphome_pch.h"' in block
+
+
+def test_pch_cmake_consumer_strict_escalates(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setenv("ESPHOME_PCH_STRICT", "1")
+    assert "-Werror=invalid-pch" in pch.pch_cmake_consumer("app", "${APP_SOURCES}")
+
+
+def test_pch_cmake_consumer_empty_when_disabled(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setenv("ESPHOME_PCH_ENABLE", "0")
+    assert pch.pch_cmake_consumer("app", "${APP_SOURCES}") == ""
+
+
 def test_pch_degraded_raises_only_in_strict(
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:

tests/unit_tests/components/nrf52/test_pch.py

@@ -221,17 +221,30 @@ class TestRunCompilePhases:
     def test_generated_headers_failure_raises(self, compile_ctx) -> None:
         run_cmd, prepare, _ = compile_ctx
         run_cmd.side_effect = [True, False]
-        with pytest.raises(EsphomeError, match="nRF52 native build failed"):
+        with pytest.raises(EsphomeError, match="header generation failed"):
             self._run()
         assert not prepare.called
 
     def test_cmake_phase_failure_raises(self, compile_ctx) -> None:
         run_cmd, prepare, _ = compile_ctx
         run_cmd.side_effect = [False]
-        with pytest.raises(EsphomeError, match="nRF52 native build failed"):
+        with pytest.raises(EsphomeError, match="configure failed"):
             self._run()
         assert not prepare.called
 
+    def test_ccache_pch_env_reaches_west(self, compile_ctx) -> None:
+        run_cmd, _, _ = compile_ctx
+        run_cmd.side_effect = [False]
+        # clear=True also drops ambient CCACHE_*/ESPHOME_PCH_* overrides
+        with (
+            patch.dict("os.environ", {}, clear=True),
+            pytest.raises(EsphomeError, match="configure failed"),
+        ):
+            self._run()
+        env = run_cmd.call_args.kwargs["env"]
+        assert env["CCACHE_PCH_EXTSUM"] == "true"
+        assert env["CCACHE_SLOPPINESS"] == "pch_defines,time_macros"
+
     def test_settled_db_skips_cmake_phase(self, compile_ctx) -> None:
         run_cmd, prepare, build_dir = compile_ctx
         build_dir.mkdir(parents=True)
@@ -275,13 +288,18 @@ class TestRunCompilePhases:
     def test_prepare_failure_never_aborts_the_build(
         self, compile_ctx, caplog: pytest.LogCaptureFixture
     ) -> None:
-        run_cmd, prepare, _ = compile_ctx
+        run_cmd, prepare, build_dir = compile_ctx
+        build_dir.mkdir(parents=True)
+        # Keep the pristine wipe from dropping the dir the fallback touches
+        (build_dir / "CMakeCache.txt").write_text("")
         prepare.side_effect = RuntimeError("boom")
         run_cmd.side_effect = [True, True, False]
         with pytest.raises(EsphomeError, match="nRF52 native build failed"):
             self._run()
         assert run_cmd.call_count == 3
         assert "Precompiled header setup failed" in caplog.text
+        # The fallback still satisfies OBJECT_DEPENDS
+        assert (build_dir / "esphome_pch.h").is_file()
 
     def test_prepare_failure_strict_raises(
         self, monkeypatch: pytest.MonkeyPatch, compile_ctx