diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 37e149474b..1f20ec7b06 100644 --- a/esphome/build_gen/arduino8266.py +++ b/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" diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index 22c18aea89..48c70d4da8 100644 --- a/esphome/build_helpers/pch.py +++ b/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"" seen[rel] = data parent = posixpath.dirname(rel) stack.extend((inc.decode(), parent) for inc in _INCLUDE_RE.findall(data)) diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 4ad5f368ac..db7db9966d 100644 --- a/esphome/platformio/pch.py.script +++ b/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"" 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, ) diff --git a/tests/unit_tests/build_helpers/test_pch.py b/tests/unit_tests/build_helpers/test_pch.py index 39b46dbc06..e6a977b9d4 100644 --- a/tests/unit_tests/build_helpers/test_pch.py +++ b/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"" + assert "Could not read locked.h" in caplog.text diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py new file mode 100644 index 0000000000..3c4ea36860 --- /dev/null +++ b/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()