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

esphome/build_gen/espidf.py

@@ -358,13 +358,17 @@ def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str]
         # Configure already succeeded, so an unusable DB is a real anomaly
         _LOGGER.warning("No usable compile database, skipping pch: %s", err)
         return None
+    if not isinstance(entries, list):
+        _LOGGER.warning("Malformed compile database, skipping pch")
+        return None
     # Windows compile DBs use backslashes; normalize both sides
     src_prefix = str(CORE.relative_src_path()).replace("\\", "/")
     entry = next(
         (
             e
             for e in entries
-            if e.get("file", "").replace("\\", "/").startswith(src_prefix)
+            if isinstance(e, dict)
+            and e.get("file", "").replace("\\", "/").startswith(src_prefix)
             and e.get("file", "").endswith(_CXX_SOURCE_SUFFIXES)
         ),
         None,
@@ -379,6 +383,11 @@ def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str]
     # launcher; the .gch must be compiled directly
     if tokens and is_launcher(tokens[0]):
         tokens = tokens[1:]
+    if not tokens:
+        # An "arguments"-style or empty entry must skip cleanly, not spawn
+        # a compiler-less argv that warns on every build
+        _LOGGER.warning("Compile database entry has no usable command, skipping pch")
+        return None
     args: list[str] = []
     arg_it = iter(tokens)
     for tok in arg_it:
@@ -398,6 +407,22 @@ def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str]
     return [*args, "-x", "c++-header", "-c", str(header), "-o", str(gch)]
 
 
+def discard_pch() -> None:
+    """Remove the pch sidecars so a stale .gch is never consumed.
+
+    Bumps the header only when a .gch was actually removed: TUs compiled
+    against it have incomplete depfiles, while a repeat failure with no
+    .gch must not force a full rebuild every build.
+    """
+    header = CORE.relative_build_path(_PCH_BUILD_HEADER)
+    gch = Path(f"{header}.gch")
+    had_gch = gch.is_file()
+    gch.unlink(missing_ok=True)
+    Path(f"{gch}.sum").unlink(missing_ok=True)
+    if had_gch and header.is_file():
+        os.utime(header)
+
+
 def prepare_pch() -> None:
     """Compile the prefix header's .gch and write its ccache .sum.
 
@@ -416,18 +441,18 @@ def prepare_pch() -> None:
     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)
+        discard_pch()
         return
     sdkconfig_path = CORE.relative_build_path(f"sdkconfig.{CORE.name}")
     try:
         sdkconfig = sdkconfig_path.read_text(encoding="utf-8")
     except OSError as err:
-        # Folding the error in keeps distinct unreadable states from colliding
+        # Path-independent marker: str(err) embeds the per-device path and
+        # would defeat cross-device .sum sharing
         _LOGGER.warning(
             "Could not read %s for the pch checksum: %s", sdkconfig_path, err
         )
-        sdkconfig = f"unreadable:{err}"
+        sdkconfig = f"unreadable:{type(err).__name__}:{err.errno}"
     # Build-path stripped so identical configs hash identically across devices
     cmd_id = (
         " ".join(cmd)
@@ -438,6 +463,8 @@ def prepare_pch() -> None:
         CORE.relative_src_path(),
         _PCH_HEADERS,
         (
+            # The closure is sorted, so root order only enters via the text
+            pch_header_text(_PCH_HEADERS),
             str(idf_version()),
             CORE.cpp_standard or "",
             sdkconfig,
@@ -474,9 +501,7 @@ def prepare_pch() -> None:
     except (OSError, subprocess.SubprocessError) as 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)
+        discard_pch()
         return
     if error is not None:
         _LOGGER.warning(

esphome/build_helpers/idedata.py

@@ -201,9 +201,17 @@ def parse_entry(
         if tok in ("-c", "-o"):
             next(it, None)  # drop the flag and its argument (input/output)
         elif tok == "-include":
-            # Resolve like -I so cached idedata works from any cwd (the pch
-            # include is emitted relative to the build dir)
-            cxx_flags.extend(("-include", _include(next(it, ""))))
+            # -include searches the compile cwd first, then the -I chain, so
+            # only re-anchor paths that really live next to the compile (the
+            # pch); a name meant for the -I chain must stay untouched
+            raw = next(it, "")
+            if not raw:
+                _LOGGER.warning("Dropping -include with no argument")
+            else:
+                resolved = _include(raw)
+                cxx_flags.extend(
+                    ("-include", resolved if Path(resolved).is_file() else raw)
+                )
         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

@@ -1,5 +1,6 @@
 """ESP-IDF direct build API for ESPHome."""
 
+from contextlib import suppress
 from dataclasses import dataclass, field
 import hashlib
 import json
@@ -530,13 +531,17 @@ def run_compile(config, verbose: bool) -> int:
 
     # 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
+    from esphome.build_gen.espidf import discard_pch, prepare_pch
 
     try:
         prepare_pch()
-    except Exception as err:  # noqa: BLE001  # pylint: disable=broad-exception-caught
+    except Exception:  # noqa: BLE001  # pylint: disable=broad-exception-caught
+        # Discard so an unexpected error can never leave a stale .gch that
+        # GCC would silently consume; exc_info keeps the failure diagnosable
+        with suppress(OSError):
+            discard_pch()
         _LOGGER.warning(
-            "Precompiled header setup failed; compiling without it: %s", err
+            "Precompiled header setup failed; compiling without it", exc_info=True
         )
 
     # Build

esphome/platformio/pch.py.script

@@ -5,6 +5,7 @@ import posixpath
 import re
 import shlex
 import subprocess
+import traceback
 
 # pylint: disable=E0602
 Import("env", "projenv")  # noqa: F821
@@ -107,6 +108,11 @@ def _setup_pch() -> None:
         try:
             version = platform.get_package_version(package)
         except KeyError:
+            # Only trust KeyError as "absent" when the package really is not
+            # installed; an unresolved manifest must not hash as a constant
+            if platform.get_package(package) is not None:
+                print(f"ESPHome: skipping precompiled header: no version for {package}")
+                return
             version = None  # absent optional package
         except Exception as err:  # noqa: BLE001
             # Without trustworthy package identity a stale .gch could be
@@ -141,8 +147,15 @@ def _setup_pch() -> None:
         for local in sorted(p for p in inc_dir.rglob("*") if p.is_file()):
             try:
                 data = local.read_bytes()
-            except OSError:
-                data = b"<unreadable>"
+            except OSError as err:
+                print(f"ESPHome: could not read {local} for the pch checksum: {err}")
+                try:
+                    # mtime/size keep a changed-but-unreadable header shifting
+                    # the digest without putting device paths in it
+                    st = local.stat()
+                    data = f"<unreadable:{st.st_mtime_ns}:{st.st_size}>".encode()
+                except OSError:
+                    data = b"<unreadable>"
             digest.update(str(local.relative_to(proj_dir)).encode())
             digest.update(data)
             digest.update(b"\0")
@@ -181,27 +194,30 @@ def _setup_pch() -> None:
             # macOS arm64 rejects it per-process: "had text segment at
             # different address"); probe once so consumers never pay for a
             # pch that every compile would silently reject
-            probe = subprocess.run(  # noqa: PLW1510
-                [
-                    cxx,
-                    *flags,
-                    "-MF",
-                    os.devnull,
-                    "-Winvalid-pch",
-                    "-include",
-                    str(header),
-                    "-fsyntax-only",
-                    "-x",
-                    "c++",
-                    "-",
-                ],
-                cwd=proj_dir,
-                input="",
-                capture_output=True,
-                text=True,
-            )
-            if ".gch" in probe.stderr:
-                error = f"toolchain cannot load the pch: {probe.stderr.strip()}"
+            try:
+                probe = subprocess.run(  # noqa: PLW1510
+                    [
+                        cxx,
+                        *flags,
+                        "-MF",
+                        os.devnull,
+                        "-Winvalid-pch",
+                        "-include",
+                        str(header),
+                        "-fsyntax-only",
+                        "-x",
+                        "c++",
+                        "-",
+                    ],
+                    cwd=proj_dir,
+                    input="",
+                    capture_output=True,
+                    text=True,
+                )
+                if probe.returncode != 0 or ".gch" in probe.stderr:
+                    error = f"toolchain cannot load the pch: {probe.stderr.strip()}"
+            except OSError as err:
+                error = str(err)
         if error is not None:
             print("ESPHome: precompiled header failed; compiling without it")
             print(error)
@@ -230,5 +246,6 @@ def _setup_pch() -> None:
 
 try:
     _setup_pch()
-except Exception as err:  # noqa: BLE001 -- a speedup must never break the build
-    print(f"ESPHome: precompiled header setup failed; compiling without it: {err}")
+except Exception:  # noqa: BLE001 -- a speedup must never break the build
+    print("ESPHome: precompiled header setup failed; compiling without it")
+    traceback.print_exc()

tests/unit_tests/build_gen/test_espidf.py

@@ -4,6 +4,7 @@ from __future__ import annotations
 
 import json
 import logging
+import os
 from pathlib import Path
 import subprocess
 from unittest.mock import patch
@@ -645,6 +646,59 @@ def test_pch_compile_command_variants(tmp_path: Path) -> None:
     ]
 
 
+def test_pch_compile_command_rejects_unusable_entries(tmp_path: Path) -> None:
+    """Malformed DB shapes and command-less entries skip cleanly instead of
+    producing a compiler-less argv retried every build."""
+    from esphome.build_gen.espidf import _pch_compile_command
+
+    build = tmp_path / "build"
+    build.mkdir()
+    header = build / "esphome_pch.h"
+    gch = build / "esphome_pch.h.gch"
+    db = build / "compile_commands.json"
+    src_file = str(tmp_path / "src" / "esphome" / "a.cpp")
+
+    db.write_text(json.dumps({"not": "a list"}))
+    assert _pch_compile_command(build, header, gch) is None
+
+    db.write_text(json.dumps(["just a string"]))
+    assert _pch_compile_command(build, header, gch) is None
+
+    # arguments-style entry (allowed by the spec, unused by CMake)
+    db.write_text(
+        json.dumps([{"arguments": ["g++", "-c", src_file], "file": src_file}])
+    )
+    assert _pch_compile_command(build, header, gch) is None
+
+
+def test_pch_header_list_order_is_in_checksum(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Reordering _PCH_HEADERS keeps the include closure identical, but the
+    generated header text differs, so the .gch must rebuild."""
+    import esphome.build_gen.espidf as espidf_mod
+
+    dev = _make_pch_device(tmp_path, "dev_r")
+    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),
+    ):
+        espidf_mod.prepare_pch()
+        first = (dev / "build" / "esphome_pch.h.gch.sum").read_text()
+        monkeypatch.setattr(
+            espidf_mod, "_PCH_HEADERS", tuple(reversed(espidf_mod._PCH_HEADERS))
+        )
+        espidf_mod.prepare_pch()
+    assert (dev / "build" / "esphome_pch.h.gch.sum").read_text() != first
+
+
 def test_prepare_pch_failure_writes_marker_and_skips_retry(tmp_path: Path) -> None:
     from esphome.build_gen.espidf import prepare_pch
 
@@ -679,6 +733,8 @@ def test_prepare_pch_spawn_oserror_is_transient(tmp_path: Path) -> None:
         calls.append(cmd)
         raise OSError("no such compiler")
 
+    header = dev / "build" / "esphome_pch.h"
+    before = header.stat().st_mtime_ns
     with (
         patch.object(CORE, "name", "test"),
         patch("esphome.build_gen.espidf.subprocess.run", side_effect=raising),
@@ -688,6 +744,31 @@ def test_prepare_pch_spawn_oserror_is_transient(tmp_path: Path) -> None:
     assert not (dev / "build" / "esphome_pch.h.gch.failed").exists()
     assert not (dev / "build" / "esphome_pch.h.gch.sum").exists()
     assert len(calls) == 2
+    # No .gch was ever in play, so the header must not be re-touched into
+    # forcing a full rebuild on every failing build
+    assert header.stat().st_mtime_ns == before
+
+
+def test_prepare_pch_transient_with_stale_gch_bumps_header(tmp_path: Path) -> None:
+    """A stale .gch removed on a transient failure must dirty its consumers."""
+    from esphome.build_gen.espidf import prepare_pch
+
+    dev = _make_pch_device(tmp_path, "dev_s")
+    CORE.build_path = dev
+    gch = dev / "build" / "esphome_pch.h.gch"
+    gch.write_bytes(b"stale")
+    header = dev / "build" / "esphome_pch.h"
+    os.utime(header, (1, 1))
+    with (
+        patch.object(CORE, "name", "test"),
+        patch(
+            "esphome.build_gen.espidf.subprocess.run",
+            side_effect=OSError("no such compiler"),
+        ),
+    ):
+        prepare_pch()
+    assert not gch.exists()
+    assert header.stat().st_mtime_ns > 1_000_000_000
 
 
 def test_prepare_pch_disabled_is_noop(

tests/unit_tests/build_helpers/test_idedata.py

@@ -85,13 +85,13 @@ def test_parse_entry_resolves_relative_includes() -> None:
     assert all(Path(inc).is_absolute() for inc in includes)
 
 
-def test_parse_entry_resolves_force_include_path() -> None:
+def test_parse_entry_resolves_force_include_path(tmp_path: Path) -> None:
     """The pch -include is emitted relative to the build dir; idedata must
     resolve it so cached flags work from any cwd."""
-    directory = f"{ABS}build/proj"
+    (tmp_path / "esphome_pch.h").write_text("")
     entry = _entry(
-        directory,
-        f"{directory}/src/esphome/x.cpp",
+        str(tmp_path),
+        f"{tmp_path}/src/esphome/x.cpp",
         "g++ -include esphome_pch.h -c x.cpp",
     )
 
@@ -100,7 +100,34 @@ def test_parse_entry_resolves_force_include_path() -> None:
     idx = cxx_flags.index("-include")
     resolved = cxx_flags[idx + 1]
     assert Path(resolved).is_absolute()
-    assert resolved.endswith("build/proj/esphome_pch.h")
+    assert resolved == str(tmp_path / "esphome_pch.h").replace("\\", "/")
+
+
+def test_parse_entry_keeps_search_chain_force_include(tmp_path: Path) -> None:
+    """-include names resolved via the -I chain (libretiny's Arduino.h) must
+    not be re-anchored to a nonexistent build-dir path."""
+    entry = _entry(
+        str(tmp_path),
+        f"{tmp_path}/src/esphome/x.cpp",
+        "g++ -include Arduino.h -c x.cpp",
+    )
+
+    _, _, _, cxx_flags = idedata.parse_entry(entry)
+
+    assert cxx_flags[cxx_flags.index("-include") + 1] == "Arduino.h"
+
+
+def test_parse_entry_drops_trailing_force_include(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    entry = _entry(
+        str(tmp_path), f"{tmp_path}/src/esphome/x.cpp", "g++ -c x.cpp -include"
+    )
+
+    _, _, _, cxx_flags = idedata.parse_entry(entry)
+
+    assert "-include" not in cxx_flags
+    assert "no argument" in caplog.text
 
 
 def test_parse_entry_skips_dependency_flags() -> None:

tests/unit_tests/test_espidf_toolchain.py

@@ -675,6 +675,12 @@ def test_run_compile_invokes_prepare_pch_and_survives_failure(
     """The pch hook runs before the build and a failure never aborts it."""
     monkeypatch.setenv("ESPHOME_PCH_ENABLE", "1")
     _setup_build(setup_core)
+    # A stale .gch must be discarded on the failure path, never consumed
+    build = setup_core / "build" / "test" / "build"
+    build.mkdir(parents=True, exist_ok=True)
+    (build / "esphome_pch.h").write_text("")
+    stale_gch = build / "esphome_pch.h.gch"
+    stale_gch.write_bytes(b"stale")
 
     with (
         patch.object(toolchain, "need_reconfigure", return_value=False),
@@ -686,3 +692,4 @@ def test_run_compile_invokes_prepare_pch_and_survives_failure(
     ):
         assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) == 0
     prepare.assert_called_once()
+    assert not stale_gch.exists()

tests/unit_tests/test_platformio_pch_script.py

@@ -26,12 +26,25 @@ class _FakePlatform:
             raise KeyError(name)
         return "1.2.3"
 
+    def get_package(self, name: str) -> object | None:
+        return None
+
 
 class _BrokenPlatform(_FakePlatform):
     def get_package_version(self, name: str) -> str:
         raise RuntimeError("manifest parse error")
 
 
+class _UnresolvedPlatform(_FakePlatform):
+    """KeyError from a package that IS installed: unresolved identity."""
+
+    def get_package_version(self, name: str) -> str:
+        raise KeyError(name)
+
+    def get_package(self, name: str) -> object:
+        return object()
+
+
 class _FakeSConsEnv(dict):
     """Just enough of a SCons construction environment for pch.py."""
 
@@ -66,11 +79,17 @@ class _FakeSConsEnv(dict):
         self.prepended = CXXFLAGS
 
 
-def _fake_cxx(tmp_path: Path, fail: bool = False, reject_pch: bool = False) -> Path:
+def _fake_cxx(
+    tmp_path: Path,
+    fail: bool = False,
+    reject_pch: bool = False,
+    probe_exit: int = 0,
+) -> Path:
     """A compiler stand-in that records its argv and writes the -o target.
 
     With reject_pch it builds the .gch fine but, like GCC 10 on macOS arm64,
-    warns on any consuming compile that the .gch cannot be loaded.
+    warns on any consuming compile that the .gch cannot be loaded; probe_exit
+    sets the exit code of non-header compiles (the load probe).
     """
     cxx = tmp_path / "fake-gxx"
     body = (
@@ -84,7 +103,7 @@ def _fake_cxx(tmp_path: Path, fail: bool = False, reject_pch: bool = False) -> P
         body += '[ -n "$out" ] && echo gch > "$out"\n'
         if reject_pch:
             body += 'case " $* " in *c++-header*) ;; *) echo "warning: esphome_pch.h.gch: had text segment at different address" >&2;; esac\n'
-        body += "exit 0\n"
+        body += f'case " $* " in *c++-header*) exit 0;; *) exit {probe_exit};; esac\n'
     cxx.write_text("#!/bin/sh\n" + body)
     cxx.chmod(cxx.stat().st_mode | stat.S_IEXEC)
     return cxx
@@ -95,6 +114,7 @@ def _run_script(
     flags: list[str] | None = None,
     fail: bool = False,
     reject_pch: bool = False,
+    probe_exit: int = 0,
     env_vars: dict[str, str] | None = None,
     name: str = "dev",
     platform_cls: type[_FakePlatform] = _FakePlatform,
@@ -103,7 +123,7 @@ def _run_script(
     src = proj / "src"
     (src / "esphome" / "core").mkdir(parents=True, exist_ok=True)
     (src / "esphome" / "core" / "defines.h").write_text("#define USE_X\n")
-    cxx = _fake_cxx(tmp_path, fail=fail, reject_pch=reject_pch)
+    cxx = _fake_cxx(tmp_path, fail=fail, reject_pch=reject_pch, probe_exit=probe_exit)
     args = (proj, src, str(cxx), flags or ["-DX=1"], platform_cls)
     # Distinct objects: the script must scope ccache/flags to projenv only
     global_env = _FakeSConsEnv(*args)
@@ -199,6 +219,22 @@ def test_pch_script_probe_rejection_falls_back(
     assert "toolchain cannot load the pch" in capsys.readouterr().out
 
 
+def test_pch_script_probe_nonzero_exit_falls_back(tmp_path: Path) -> None:
+    """A probe failure whose stderr never mentions .gch must still count."""
+    scons_env = _run_script(tmp_path, probe_exit=1)
+    proj = tmp_path / "dev"
+    assert not (proj / "esphome_pch.h.gch").exists()
+    assert (proj / "esphome_pch.h.gch.failed").is_file()
+    assert scons_env.prepended == []
+
+
+def test_pch_script_unresolved_package_version_skips_pch(tmp_path: Path) -> None:
+    """A KeyError for an installed package is unresolved identity, not absence."""
+    scons_env = _run_script(tmp_path, platform_cls=_UnresolvedPlatform)
+    assert not (tmp_path / "dev" / "esphome_pch.h.gch").exists()
+    assert scons_env.prepended == []
+
+
 def test_pch_script_package_version_error_skips_pch(tmp_path: Path) -> None:
     """Without trustworthy package identity a stale .gch could survive an
     upgrade, so the script must not build one at all."""
@@ -238,3 +274,24 @@ def test_pch_script_hashes_project_local_include_dirs(tmp_path: Path) -> None:
     (tmp_path / "fake-gxx.argv").unlink(missing_ok=True)
     _run_script(tmp_path, flags=flags)
     assert (proj / "esphome_pch.h.gch.sum").read_text() != first
+
+
+@pytest.mark.skipif(os.geteuid() == 0, reason="root ignores file modes")
+def test_pch_script_unreadable_local_header_warns_and_varies(
+    tmp_path: Path, capsys: pytest.CaptureFixture[str]
+) -> None:
+    """An unreadable generated header still shifts the digest via mtime/size."""
+    proj = tmp_path / "dev"
+    override = proj / "lwip_override"
+    override.mkdir(parents=True)
+    secret = override / "lwipopts.h"
+    secret.write_text("#define TCP_MSS 1460\n")
+    secret.chmod(0)
+    flags = ["-DX=1", "-I", str(override)]
+    _run_script(tmp_path, flags=flags)
+    first = (proj / "esphome_pch.h.gch.sum").read_text()
+    assert "could not read" in capsys.readouterr().out
+    os.utime(secret, (1, 1))
+    (tmp_path / "fake-gxx.argv").unlink(missing_ok=True)
+    _run_script(tmp_path, flags=flags)
+    assert (proj / "esphome_pch.h.gch.sum").read_text() != first