Merge branch 'platformio-pch-rp2' into platformio-pch-libretiny

esphome/build_gen/arduino8266.py

@@ -1232,10 +1232,17 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool:
             # per-device build path so identically-configured devices
             # produce identical .sum files and share cache entries
             flags_id = " ".join(cxxflags).replace(effective_ccache_basedir(), "")
+            # The header text covers include order, which the sorted
+            # closure alone does not
             checksum = pch_checksum(
                 src_dir,
                 pch_includes,
-                (str(paths.framework), str(paths.toolchain), flags_id),
+                (
+                    pch_header_text(pch_includes),
+                    str(paths.framework),
+                    str(paths.toolchain),
+                    flags_id,
+                ),
             )
             write_file_if_changed(
                 build_dir / f"{PCH_HEADER_NAME}.gch.sum", checksum + "\n"

esphome/build_helpers/pch.py

@@ -9,6 +9,7 @@ from __future__ import annotations
 
 from collections.abc import Iterable
 import hashlib
+import logging
 import os
 from pathlib import Path
 import posixpath
@@ -16,6 +17,8 @@ import re
 
 from esphome.build_helpers.ccache import parse_enable_env
 
+_LOGGER = logging.getLogger(__name__)
+
 # The header and its .gch/.sum sidecars live in the build directory.
 PCH_HEADER_NAME = "esphome_pch.h"
 
@@ -76,8 +79,11 @@ def _include_closure(src_dir: Path, roots: Iterable[str]) -> dict[str, bytes]:
             continue
         try:
             data = (src_dir / rel).read_bytes()
-        except OSError:
-            continue
+        except OSError as err:
+            # Hash a marker so an unreadable header invalidates instead of
+            # silently vanishing from the digest
+            _LOGGER.warning("Could not read %s for the pch checksum: %s", rel, err)
+            data = b"<unreadable>"
         seen[rel] = data
         parent = posixpath.dirname(rel)
         stack.extend((inc.decode(), parent) for inc in _INCLUDE_RE.findall(data))

esphome/platformio/pch.py.script

@@ -39,8 +39,9 @@ def _include_closure(src_dir: Path, roots: list) -> dict:
             continue
         try:
             data = (src_dir / rel).read_bytes()
-        except OSError:
-            continue
+        except OSError as err:
+            print(f"ESPHome: could not read {rel} for the pch checksum: {err}")
+            data = b"<unreadable>"
         seen[rel] = data
         parent = posixpath.dirname(rel)
         stack.extend((inc.decode(), parent) for inc in _INCLUDE_RE.findall(data))
@@ -81,6 +82,9 @@ def _setup_pch() -> None:
             include_headers.append(next(flag_it, ""))
         else:
             flags.append(tok)
+    if any(not name for name in include_headers):
+        print("ESPHome: build_src_flags has a trailing -include; skipping pch")
+        return
     content = "".join(
         f'#include "{name}"\n' for name in (*include_headers, _CORE_HEADER)
     )
@@ -100,8 +104,10 @@ def _setup_pch() -> None:
     for package in sorted(platform.packages):
         try:
             version = platform.get_package_version(package)
-        except Exception:  # noqa: BLE001 -- absent optional package
-            version = None
+        except Exception as err:  # noqa: BLE001 -- absent optional package
+            # Folded into the digest so an unexpected lookup failure still
+            # invalidates instead of hashing like a fixed absence
+            version = f"error:{type(err).__name__}"
         digest.update(f"{package}={version}".encode())
         digest.update(b"\0")
     closure = _include_closure(src_dir, [*include_headers, _CORE_HEADER])
@@ -113,7 +119,8 @@ def _setup_pch() -> None:
 
     # The ccache .sum sidecar doubles as the freshness stamp
     if (
-        not gch.is_file()
+        not header.is_file()
+        or not gch.is_file()
         or not sum_path.is_file()
         or (sum_path.read_text(encoding="utf-8").strip() != checksum)
     ):
@@ -122,11 +129,16 @@ def _setup_pch() -> None:
             failed_marker.is_file()
             and failed_marker.read_text(encoding="utf-8").strip() == checksum
         ):
+            print(
+                "ESPHome: skipping precompiled header (previous attempt "
+                f"failed); delete {failed_marker.name} to retry"
+            )
             return
         header.write_text(content, encoding="utf-8")
         try:
             result = subprocess.run(  # noqa: PLW1510
                 [cxx, "-x", "c++-header", *flags, "-c", str(header), "-o", str(gch)],
+                cwd=proj_dir,
                 capture_output=True,
                 text=True,
             )

tests/unit_tests/build_helpers/test_pch.py

@@ -105,12 +105,18 @@ def test_pch_checksum_tracks_closure_content(tmp_path: Path) -> None:
 @pytest.mark.skipif(
     os.name == "nt" or os.geteuid() == 0, reason="chmod is ineffective here"
 )
-def test_include_closure_skips_unreadable(tmp_path: Path) -> None:
+def test_include_closure_marks_unreadable(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """An unreadable header warns and hashes as a marker, so it still
+    invalidates instead of silently vanishing from the digest."""
     _write(tmp_path, "a.h", '#include "locked.h"\n')
     locked = tmp_path / "locked.h"
     locked.write_text("")
     locked.chmod(0)
     try:
-        assert sorted(pch._include_closure(tmp_path, ["a.h"])) == ["a.h"]
+        closure = pch._include_closure(tmp_path, ["a.h"])
     finally:
         locked.chmod(0o644)
+    assert closure["locked.h"] == b"<unreadable>"
+    assert "Could not read locked.h" in caplog.text

tests/unit_tests/test_platformio_pch_script.py

@@ -0,0 +1,167 @@
+"""Tests for esphome/platformio/pch.py.script against a fake SCons env."""
+
+from __future__ import annotations
+
+import os
+from pathlib import Path
+import stat
+from unittest.mock import patch
+
+import pytest
+
+from esphome.platformio import toolchain
+
+pytestmark = pytest.mark.skipif(
+    os.name == "nt", reason="the fake compiler is a POSIX shell script"
+)
+
+_SCRIPT = Path(toolchain.__file__).parent / "pch.py.script"
+
+
+class _FakePlatform:
+    packages = {"framework-x": {}, "toolchain-y": {}}
+
+    def get_package_version(self, name: str) -> str:
+        if name == "toolchain-y":
+            raise KeyError(name)
+        return "1.2.3"
+
+
+class _FakeSConsEnv(dict):
+    """Just enough of a SCons construction environment for pch.py."""
+
+    def __init__(self, proj_dir: Path, src_dir: Path, cxx: str, flags: list[str]):
+        super().__init__(ENV={})
+        self._subst = {
+            "$PROJECT_DIR": str(proj_dir),
+            "$PROJECT_SRC_DIR": str(src_dir),
+            "$CXX": cxx,
+        }
+        self._flags = flags
+        self.prepended: list[str] = []
+
+    def subst(self, expr: str) -> str:  # noqa: N802
+        return self._subst[expr]
+
+    def subst_list(self, expr: str) -> list[list[str]]:  # noqa: N802
+        return [self._flags]
+
+    def PioPlatform(self) -> _FakePlatform:  # noqa: N802
+        return _FakePlatform()
+
+    def Prepend(self, CXXFLAGS: list[str]) -> None:  # noqa: N802, N803
+        self.prepended = CXXFLAGS
+
+
+def _fake_cxx(tmp_path: Path, fail: bool = False) -> Path:
+    """A compiler stand-in that records its argv and writes the -o target."""
+    cxx = tmp_path / "fake-gxx"
+    body = 'printf \'%s\\n\' "$@" >> "$0.argv"\n'
+    if fail:
+        body += "echo boom >&2\nexit 1\n"
+    else:
+        body += 'out=""; prev=""\nfor a in "$@"; do [ "$prev" = "-o" ] && out="$a"; prev="$a"; done\necho gch > "$out"\n'
+    cxx.write_text("#!/bin/sh\n" + body)
+    cxx.chmod(cxx.stat().st_mode | stat.S_IEXEC)
+    return cxx
+
+
+def _run_script(
+    tmp_path: Path,
+    flags: list[str] | None = None,
+    fail: bool = False,
+    env_vars: dict[str, str] | None = None,
+    name: str = "dev",
+) -> _FakeSConsEnv:
+    proj = tmp_path / name
+    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)
+    scons_env = _FakeSConsEnv(proj, src, str(cxx), flags or ["-DX=1"])
+    source = _SCRIPT.read_text()
+    with patch.dict(os.environ, env_vars or {}, clear=True):
+        exec(  # noqa: S102
+            compile(source, "pch.py", "exec"),
+            {"Import": lambda *_names: None, "env": scons_env, "projenv": scons_env},
+        )
+    return scons_env
+
+
+def test_pch_script_builds_and_prepends_relative_include(tmp_path: Path) -> None:
+    scons_env = _run_script(tmp_path)
+    proj = tmp_path / "dev"
+    assert (proj / "esphome_pch.h").read_text().endswith('"esphome/core/defines.h"\n')
+    assert (proj / "esphome_pch.h.gch").is_file()
+    assert len((proj / "esphome_pch.h.gch.sum").read_text().strip()) == 64
+    # Relative include: an absolute path would poison ccache keys
+    assert scons_env.prepended == ["-include", "esphome_pch.h"]
+    # ccache settings land on the SCons ENV only, never os.environ
+    assert scons_env["ENV"]["CCACHE_SLOPPINESS"] == "pch_defines,time_macros"
+    assert scons_env["ENV"]["CCACHE_PCH_EXTSUM"] == "true"
+    assert "CCACHE_SLOPPINESS" not in os.environ
+
+
+def test_pch_script_preserves_spaced_flag_elements(tmp_path: Path) -> None:
+    """One SCons element stays one compiler argv; -include pairs are
+    stripped from the .gch compile."""
+    spaced = tmp_path / "My Configs"
+    spaced.mkdir()
+    flags = ['-DUSB_PRODUCT=\\"Pico 2W\\"', "-I", str(spaced), "-include", "other.h"]
+    _run_script(tmp_path, flags=flags)
+    argv = (tmp_path / "fake-gxx.argv").read_text().splitlines()
+    assert '-DUSB_PRODUCT="Pico 2W"' in argv
+    assert str(spaced) in argv
+    assert "-include" not in argv
+    # The stripped -include header is folded into the prefix header instead
+    pch = (tmp_path / "dev" / "esphome_pch.h").read_text()
+    assert pch.splitlines()[0] == '#include "other.h"'
+
+
+def test_pch_script_sum_is_device_independent(tmp_path: Path) -> None:
+    """Regression: identical configs in different dirs share cache keys."""
+    sums = []
+    for name in ("dev_a", "dev_b"):
+        proj = tmp_path / name
+        _run_script(
+            tmp_path,
+            flags=["-DX=1", "-I", str(proj / "include")],
+            env_vars={"CCACHE_BASEDIR": str(proj)},
+            name=name,
+        )
+        sums.append((proj / "esphome_pch.h.gch.sum").read_text())
+        (tmp_path / "fake-gxx").unlink()
+        (tmp_path / "fake-gxx.argv").unlink(missing_ok=True)
+    assert sums[0] == sums[1]
+
+
+def test_pch_script_failure_marker_suppresses_retry(
+    tmp_path: Path, capsys: pytest.CaptureFixture[str]
+) -> None:
+    scons_env = _run_script(tmp_path, fail=True)
+    proj = tmp_path / "dev"
+    assert (proj / "esphome_pch.h.gch.failed").is_file()
+    assert not (proj / "esphome_pch.h.gch.sum").exists()
+    assert scons_env.prepended == []
+    # Second run: same checksum, no compile attempt, but says so
+    attempts = (tmp_path / "fake-gxx.argv").read_text().count("c++-header")
+    _run_script(tmp_path, fail=True)
+    out = capsys.readouterr().out
+    assert (tmp_path / "fake-gxx.argv").read_text().count("c++-header") == attempts
+    assert "delete esphome_pch.h.gch.failed to retry" in out
+
+
+def test_pch_script_rebuilds_when_header_missing(tmp_path: Path) -> None:
+    _run_script(tmp_path)
+    proj = tmp_path / "dev"
+    (proj / "esphome_pch.h").unlink()
+    _run_script(tmp_path)
+    assert (proj / "esphome_pch.h").is_file()
+
+
+def test_copy_pch_script(tmp_path: Path) -> None:
+    from esphome.core import CORE
+
+    CORE.build_path = tmp_path
+    toolchain.copy_pch_script()
+    assert (tmp_path / "pch.py").read_text() == _SCRIPT.read_text()