Merge branch 'esp32-idf-pch' into platformio-pch-rp2

esphome/build_gen/espidf.py

@@ -1,6 +1,5 @@
 """ESP-IDF direct build generator for ESPHome."""
 
-import hashlib
 import json
 import logging
 import os
@@ -41,7 +40,9 @@ _LOGGER = logging.getLogger(__name__)
 # rest. Deliberately hard-coded: frequency-derived sets measured no better
 # and kept selecting headers that cannot compile standalone (X-macro,
 # platform-variant). Every entry must be safe to include first in an
-# empty TU.
+# empty TU. Caveat: application.h/automation.h become ambiently visible,
+# so a TU missing those #includes still builds here but not on other
+# platforms; ESPHOME_PCH_ENABLE=0 restores the strict view.
 _PCH_HEADERS = (
     PCH_CORE_HEADER,
     "esphome/core/component.h",
@@ -58,7 +59,7 @@ _PCH_BUILD_HEADER = f"build/{PCH_HEADER_NAME}"
 # Compile-command tokens dropped when retargeting a TU's flags at the
 # prefix header: source/output/depfile flags with an argument, and the
 # argument-less depfile flags (the pch compile must not touch depfiles)
-_PCH_STRIP_FLAGS_WITH_ARG = frozenset({"-include", "-o", "-c", "-MT", "-MF", "-MQ"})
+_PCH_STRIP_FLAGS_WITH_ARG = frozenset({"-o", "-c", "-MT", "-MF", "-MQ"})
 _PCH_STRIP_FLAGS = frozenset({"-MD", "-MMD", "-MP", "-MM", "-M"})
 _CXX_SOURCE_SUFFIXES = (".cpp", ".cc", ".cxx")
 
@@ -354,7 +355,8 @@ def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str]
             (build_dir / "compile_commands.json").read_text(encoding="utf-8")
         )
     except (OSError, json.JSONDecodeError) as err:
-        _LOGGER.debug("No usable compile database, skipping pch: %s", err)
+        # Configure already succeeded, so an unusable DB is a real anomaly
+        _LOGGER.warning("No usable compile database, skipping pch: %s", err)
         return None
     # Windows compile DBs use backslashes; normalize both sides
     src_prefix = str(CORE.relative_src_path()).replace("\\", "/")
@@ -368,10 +370,10 @@ def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str]
         None,
     )
     if entry is None:
-        _LOGGER.debug("No src C++ entry in the compile database, skipping pch")
+        _LOGGER.warning("No src C++ entry in the compile database, skipping pch")
         return None
     tokens = expand_response_files(
-        split_command(entry["command"]), Path(entry.get("directory", build_dir))
+        split_command(entry.get("command", "")), Path(entry.get("directory", build_dir))
     )
     # A DB recorded with ccache enabled prefixes the compiler with the
     # launcher; the .gch must be compiled directly
@@ -385,6 +387,13 @@ def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str]
             continue
         if tok in _PCH_STRIP_FLAGS:
             continue
+        if tok == "-include":
+            # Drop only the injected prefix; user force-includes must reach
+            # the .gch compile or GCC rejects it over the macro mismatch
+            inc = next(arg_it, "")
+            if not inc.endswith(PCH_HEADER_NAME):
+                args.extend(("-include", inc))
+            continue
         args.append(tok)
     return [*args, "-x", "c++-header", "-c", str(header), "-o", str(gch)]
 
@@ -394,8 +403,9 @@ def prepare_pch() -> None:
 
     Runs right before ninja, after every reconfigure, so the flags in
     compile_commands.json and the sdkconfig are the settled ones. The .sum
-    doubles as the freshness stamp; a failed .gch compile falls back to
-    the plain header include.
+    doubles as the freshness stamp and folds in the compile command, so a
+    flag-only change rebuilds the .gch. A failed compile falls back to the
+    plain header include.
     """
     if not pch_enabled():
         return
@@ -403,12 +413,27 @@ def prepare_pch() -> None:
     header = CORE.relative_build_path(_PCH_BUILD_HEADER)
     gch = Path(f"{header}.gch")
     sum_path = Path(f"{gch}.sum")
+    cmd = _pch_compile_command(build_dir, header, gch)
+    if cmd is None:
+        # Freshness cannot be validated; a leftover .gch must not be consumed
+        gch.unlink(missing_ok=True)
+        sum_path.unlink(missing_ok=True)
+        return
+    sdkconfig_path = CORE.relative_build_path(f"sdkconfig.{CORE.name}")
     try:
-        sdkconfig = CORE.relative_build_path(f"sdkconfig.{CORE.name}").read_text(
-            encoding="utf-8"
+        sdkconfig = sdkconfig_path.read_text(encoding="utf-8")
+    except OSError as err:
+        # Folding the error in keeps distinct unreadable states from colliding
+        _LOGGER.warning(
+            "Could not read %s for the pch checksum: %s", sdkconfig_path, err
         )
-    except OSError:
-        sdkconfig = "no-sdkconfig"
+        sdkconfig = f"unreadable:{err}"
+    # Build-path stripped so identical configs hash identically across devices
+    cmd_id = (
+        " ".join(cmd)
+        .replace(str(Path(CORE.build_path).resolve()), "")
+        .replace(str(CORE.build_path), "")
+    )
     checksum = pch_checksum(
         CORE.relative_src_path(),
         _PCH_HEADERS,
@@ -418,6 +443,7 @@ def prepare_pch() -> None:
             sdkconfig,
             *get_project_compile_flags(),
             *get_project_cxx_compile_flags(),
+            cmd_id,
         ),
     )
     if (
@@ -426,21 +452,15 @@ def prepare_pch() -> None:
         and sum_path.read_text(encoding="utf-8").strip() == checksum
     ):
         return
-    cmd = _pch_compile_command(build_dir, header, gch)
-    if cmd is None:
-        # The checksum is stale; a leftover .gch must not be consumed
-        gch.unlink(missing_ok=True)
-        sum_path.unlink(missing_ok=True)
-        return
-    # Keyed on the checksum and the compile command: a failure caused by
-    # the command alone must retry when the command changes
-    marker_key = f"{checksum} {hashlib.sha256(' '.join(cmd).encode()).hexdigest()}"
     failed_marker = Path(f"{gch}.failed")
     if (
         failed_marker.is_file()
-        and failed_marker.read_text(encoding="utf-8").strip() == marker_key
+        and failed_marker.read_text(encoding="utf-8").strip() == checksum
     ):
-        _LOGGER.debug("Pch previously failed for these inputs; skipping")
+        _LOGGER.info(
+            "Precompiled header disabled after an earlier failure; delete %s to retry",
+            failed_marker,
+        )
         return
     try:
         result = subprocess.run(
@@ -452,7 +472,12 @@ def prepare_pch() -> None:
         elif not gch.is_file():
             error = "compiler produced no .gch"
     except (OSError, subprocess.SubprocessError) as err:
-        error = str(err)
+        # Transient (timeout, spawn/IO): warn and retry next build, no marker
+        _LOGGER.warning("Precompiled header compile did not run: %s", err)
+        gch.unlink(missing_ok=True)
+        sum_path.unlink(missing_ok=True)
+        os.utime(header)
+        return
     if error is not None:
         _LOGGER.warning(
             "Precompiled header failed; compiling without it: %s", error[:400]
@@ -460,7 +485,7 @@ def prepare_pch() -> None:
         gch.unlink(missing_ok=True)
         sum_path.unlink(missing_ok=True)
         # Skip retries until a header/flag/sdkconfig/command change
-        failed_marker.write_text(marker_key + "\n", encoding="utf-8")
+        failed_marker.write_text(checksum + "\n", encoding="utf-8")
         os.utime(header)
         return
     failed_marker.unlink(missing_ok=True)

esphome/build_helpers/idedata.py

@@ -198,10 +198,15 @@ def parse_entry(
 
     it = iter(tokens[1:])
     for tok in it:
-        if tok in ("-c", "-o", "-include"):
-            # Drop the flag and its argument; the injected relative
-            # -include esphome_pch.h does not resolve outside the build dir
-            next(it, None)
+        if tok in ("-c", "-o"):
+            next(it, None)  # drop the flag and its argument (input/output)
+            continue
+        if tok == "-include":
+            # Drop only the injected pch include, whose relative path does
+            # not resolve outside the build dir; keep other force-includes
+            inc = next(it, "")
+            if not inc.endswith("esphome_pch.h"):
+                cxx_flags.extend(("-include", inc))
         elif tok.startswith("-D"):
             # ``.strip()`` handles tokens like ``-D CONFIGURED=1`` (a single
             # quoted arg with a space after -D) that some flags arrive as.

esphome/espidf/toolchain.py

@@ -528,10 +528,16 @@ def run_compile(config, verbose: bool) -> int:
             return result.returncode
         _patch_memory_segments()
 
-    # After every reconfigure so compile_commands and sdkconfig are settled
+    # After every reconfigure so compile_commands and sdkconfig are settled.
+    # An optional speedup must never abort the build
     from esphome.build_gen.espidf import prepare_pch
 
-    prepare_pch()
+    try:
+        prepare_pch()
+    except Exception as err:  # noqa: BLE001  # pylint: disable=broad-exception-caught
+        _LOGGER.warning(
+            "Precompiled header setup failed; compiling without it: %s", err
+        )
 
     # Build
     args = []

tests/unit_tests/build_gen/test_espidf.py

@@ -667,20 +667,27 @@ def test_prepare_pch_failure_writes_marker_and_skips_retry(tmp_path: Path) -> No
     assert (dev / "build" / "esphome_pch.h.gch.failed").exists()
 
 
-def test_prepare_pch_spawn_oserror_degrades(tmp_path: Path) -> None:
+def test_prepare_pch_spawn_oserror_is_transient(tmp_path: Path) -> None:
+    """Spawn/IO failures retry on the next build instead of latching."""
     from esphome.build_gen.espidf import prepare_pch
 
     dev = _make_pch_device(tmp_path, "dev_o")
     CORE.build_path = dev
+    calls = []
+
+    def raising(cmd, **kwargs):
+        calls.append(cmd)
+        raise OSError("no such compiler")
+
     with (
         patch.object(CORE, "name", "test"),
-        patch(
-            "esphome.build_gen.espidf.subprocess.run",
-            side_effect=OSError("no such compiler"),
-        ),
+        patch("esphome.build_gen.espidf.subprocess.run", side_effect=raising),
     ):
         prepare_pch()
-    assert (dev / "build" / "esphome_pch.h.gch.failed").exists()
+        prepare_pch()
+    assert not (dev / "build" / "esphome_pch.h.gch.failed").exists()
+    assert not (dev / "build" / "esphome_pch.h.gch.sum").exists()
+    assert len(calls) == 2
 
 
 def test_prepare_pch_disabled_is_noop(
@@ -810,3 +817,53 @@ def test_component_cmakelists_pch_object_depends() -> None:
 
     content = get_component_cmakelists()
     assert 'OBJECT_DEPENDS "${CMAKE_BINARY_DIR}/esphome_pch.h"' in content
+
+
+def test_prepare_pch_command_change_invalidates_sum(tmp_path: Path) -> None:
+    """A flag-only change in the compile DB must rebuild the .gch."""
+    from esphome.build_gen.espidf import prepare_pch
+
+    dev = _make_pch_device(tmp_path, "dev_c")
+    CORE.build_path = dev
+    gch = dev / "build" / "esphome_pch.h.gch"
+
+    def fake_compile(cmd, **kwargs):
+        gch.write_bytes(b"gch")
+        return subprocess.CompletedProcess(cmd, 0, "", "")
+
+    with (
+        patch.object(CORE, "name", "test"),
+        patch("esphome.build_gen.espidf.subprocess.run", side_effect=fake_compile),
+    ):
+        prepare_pch()
+        first = (dev / "build" / "esphome_pch.h.gch.sum").read_text()
+        db = dev / "build" / "compile_commands.json"
+        db.write_text(db.read_text().replace("-DX=1", "-DX=2"))
+        prepare_pch()
+    assert (dev / "build" / "esphome_pch.h.gch.sum").read_text() != first
+
+
+def test_prepare_pch_keeps_user_force_includes(tmp_path: Path) -> None:
+    from esphome.build_gen.espidf import _pch_compile_command
+
+    dev = _make_pch_device(tmp_path, "dev_u")
+    CORE.build_path = dev
+    build = dev / "build"
+    src_file = str(dev / "src" / "esphome" / "a.cpp")
+    build.joinpath("compile_commands.json").write_text(
+        json.dumps(
+            [
+                {
+                    "directory": str(build),
+                    "command": (
+                        "g++ -include user.h -include esphome_pch.h "
+                        f"-o a.obj -c {src_file}"
+                    ),
+                    "file": src_file,
+                }
+            ]
+        )
+    )
+    cmd = _pch_compile_command(build, build / "esphome_pch.h", build / "x.gch")
+    assert "user.h" in cmd
+    assert "esphome_pch.h" not in " ".join(cmd[:-3])

tests/unit_tests/test_espidf_framework.py

@@ -1991,3 +1991,12 @@ def test_ccache_env_opt_in_with_usable_binary(
         env = _ccache_env()
     assert env["IDF_CCACHE_ENABLE"] == "1"
     assert not [r for r in caplog.records if r.levelno >= logging.WARNING]
+
+
+def test_ccache_env_exports_pch_settings(tmp_path: Path) -> None:
+    # The pch cannot cache under ccache without these
+    p1, p2, p3 = _ccache_patches(tmp_path, "/usr/bin/ccache", tmp_path / "build")
+    with patch.dict("os.environ", {}, clear=True), p1, p2, p3:
+        env = _ccache_env()
+    assert env["CCACHE_SLOPPINESS"] == "pch_defines,time_macros"
+    assert env["CCACHE_PCH_EXTSUM"] == "true"

tests/unit_tests/test_espidf_toolchain.py

@@ -667,3 +667,22 @@ def test_get_core_framework_version_from_core_data():
 
     CORE.data = {KEY_ESP32: {KEY_IDF_VERSION: cv.Version(5, 5, 4)}}
     assert toolchain._get_core_framework_version() == "5.5.4"
+
+
+def test_run_compile_invokes_prepare_pch_and_survives_failure(
+    setup_core: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """The pch hook runs before the build and a failure never aborts it."""
+    monkeypatch.setenv("ESPHOME_PCH_ENABLE", "1")
+    _setup_build(setup_core)
+
+    with (
+        patch.object(toolchain, "need_reconfigure", return_value=False),
+        patch.object(toolchain, "run_idf_py", return_value=0),
+        patch.object(toolchain, "print_summary"),
+        patch(
+            "esphome.build_gen.espidf.prepare_pch", side_effect=RuntimeError("boom")
+        ) as prepare,
+    ):
+        assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) == 0
+    prepare.assert_called_once()