From f5593deb380bcf86f66841d93127b5ad105d11a7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 26 Aug 2026 00:46:14 -0500 Subject: [PATCH] Warn and skip the pch when a user -include precedes it, walk angle includes, read sidecars defensively --- esphome/build_gen/arduino8266.py | 17 ++++++++++++++-- esphome/build_helpers/pch.py | 10 ++++++++-- esphome/platformio/pch.py.script | 20 ++++++++++++------- .../unit_tests/build_gen/test_arduino8266.py | 15 ++++++++++++++ tests/unit_tests/build_helpers/test_pch.py | 18 +++++++++++++++++ .../unit_tests/test_platformio_pch_script.py | 10 ++++++++++ 6 files changed, 79 insertions(+), 11 deletions(-) diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 8dc1a6c19e..bc40825c83 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -1217,7 +1217,15 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: # edge (hundreds of edges in a real project) lines.append(f"srcflags = {' '.join(src_other + include_flags)}") src_cxx_override = None - if pch_enabled(): + if pch_enabled() and "-include" in cxxflags: + # GCC only loads a .gch while no tokens precede it, and the cxx rule + # expands $cxxflags before $flags: a user -include in build_flags + # means every TU would silently skip the .gch + _LOGGER.warning( + "A -include in build_flags prevents the precompiled header from " + "loading; compiling without it" + ) + elif pch_enabled(): # C++ src edges swap the force-includes for one precompiled prefix # header holding the same content plus defines.h; C and assembly # edges keep srcflags (a .gch is a C++ artifact) @@ -1233,7 +1241,12 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: # depfile handles staleness. Mirror CCACHE_BASEDIR: strip the # per-device build path so identically-configured devices # produce identical .sum files and share cache entries - flags_id = " ".join(cxxflags).replace(effective_ccache_basedir(), "") + # Raw path too: a symlinked build dir resolves differently + flags_id = ( + " ".join(cxxflags) + .replace(effective_ccache_basedir(), "") + .replace(str(CORE.build_path), "") + ) # The header text covers include order, which the sorted # closure alone does not checksum = pch_checksum( diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index 82b00b34f3..4bf16eb9ff 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -44,7 +44,9 @@ _CCACHE_PCH_ENV = { "CCACHE_PCH_EXTSUM": "true", } -_INCLUDE_RE = re.compile(rb'^\s*#\s*include\s+"([^"]+)"', re.MULTILINE) +# Both include forms: an angle include resolving under src/ must enter the +# digest too; ones that do not resolve simply end the walk +_INCLUDE_RE = re.compile(rb'^\s*#\s*include\s+["<]([^">]+)[">]', re.MULTILINE) def pch_enabled() -> bool: @@ -62,7 +64,11 @@ def ccache_pch_env() -> dict[str, str]: user_sloppiness = os.environ.get("CCACHE_SLOPPINESS") if user_sloppiness is not None and ( missing := [ - t for t in ("pch_defines", "time_macros") if t not in user_sloppiness + t + for t in ("pch_defines", "time_macros") + # Set membership: substring matching could be fooled by a token + # that merely contains one of ours + if t not in {tok.strip() for tok in user_sloppiness.split(",")} ] ): # Without these ccache declines every pch-consuming compile; union diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 38596333c4..1ae0ae697c 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -25,7 +25,7 @@ except Exception as err: # noqa: BLE001 -- not exported under -t nobuild # include-closure recipe, and the checksum/failed-marker stamp flow in sync # with build_helpers/pch.py. -_INCLUDE_RE = re.compile(rb'^\s*#\s*include\s+"([^"]+)"', re.MULTILINE) +_INCLUDE_RE = re.compile(rb'^\s*#\s*include\s+["<]([^">]+)[">]', re.MULTILINE) _CORE_HEADER = "esphome/core/defines.h" @@ -122,6 +122,14 @@ def _compile_gch(cxx, flags, header: Path, gch: Path, proj_dir: Path): return None +def _read_stamp(path: Path) -> str: + """A corrupt sidecar must read as stale, not kill the pch forever.""" + try: + return path.read_text(encoding="utf-8").strip() + except (OSError, UnicodeDecodeError): + return "" + + def _setup_pch() -> None: if projenv is None: # Expected under -t nobuild; anything else must leave a trail @@ -249,13 +257,10 @@ def _setup_pch() -> None: 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) + or (_read_stamp(sum_path) != checksum) ): failed_marker = Path(f"{gch}.failed") - if ( - failed_marker.is_file() - and failed_marker.read_text(encoding="utf-8").strip() == checksum - ): + if _read_stamp(failed_marker) == checksum: print( "ESPHome: skipping precompiled header (previous attempt " f"failed); delete {failed_marker.name} to retry" @@ -292,7 +297,8 @@ def _setup_pch() -> None: projenv["ENV"][key] = value # noqa: F821 sloppiness = os.environ.get("CCACHE_SLOPPINESS") if sloppiness is not None: - missing = [t for t in ("pch_defines", "time_macros") if t not in sloppiness] + tokens = {tok.strip() for tok in sloppiness.split(",")} + missing = [t for t in ("pch_defines", "time_macros") if t not in tokens] if missing: # Without these ccache declines every pch-consuming compile; # union rather than override so the user's own tokens survive diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index 866ee8747c..d1e6726dd5 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -416,6 +416,21 @@ def test_write_project_pch_identity_unknown_skips_pch( assert "Could not establish the pch identity" in caplog.text +def test_write_project_pch_skipped_when_user_force_include_precedes( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A -include in build_flags lands ahead of the pch include, so GCC + would never load the .gch; skip it and say so.""" + paths = _make_framework(tmp_path) + _set_flags( + "-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH", "-include foo.h" + ) + content = _write_ninja(paths, ccache="/usr/bin/ccache") + assert "esphome_pch" not in content + assert "srccxxflags" not in content + assert "prevents the precompiled header" in caplog.text + + def test_write_project_pch_disabled( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/unit_tests/build_helpers/test_pch.py b/tests/unit_tests/build_helpers/test_pch.py index ec81226e4d..e8ea0c18bb 100644 --- a/tests/unit_tests/build_helpers/test_pch.py +++ b/tests/unit_tests/build_helpers/test_pch.py @@ -45,6 +45,15 @@ def test_ccache_pch_env_disabled() -> None: assert pch.ccache_pch_env() == {} +def test_ccache_pch_env_token_check_is_membership_not_substring( + caplog: pytest.LogCaptureFixture, +) -> None: + """A token merely containing ours must not suppress the union.""" + with patch.dict(os.environ, {"CCACHE_SLOPPINESS": "pch_defines_extra"}, clear=True): + env = pch.ccache_pch_env() + assert env["CCACHE_SLOPPINESS"] == "pch_defines_extra,pch_defines,time_macros" + + def test_ccache_pch_env_unions_user_sloppiness( caplog: pytest.LogCaptureFixture, ) -> None: @@ -181,3 +190,12 @@ def test_pch_checksum_survives_surrogate_extra(tmp_path: Path) -> None: """Install paths from non-UTF-8 filesystems carry surrogates; hashing them must not raise past the caller's identity-unknown guard.""" assert pch.pch_checksum(tmp_path, [], ["/opt/bad\udcff/framework"]) + + +def test_include_closure_walks_angle_includes_under_src(tmp_path: Path) -> None: + """An angle include resolving under src/ must enter the digest; one + that does not simply ends the walk.""" + _write(tmp_path, "a.h", "#include \n#include \n") + (tmp_path / "local.h").write_text("") + closure = pch._include_closure(tmp_path, ["a.h"]) + assert set(closure) == {"a.h", "local.h"} diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py index 510fcbce97..6687fe938d 100644 --- a/tests/unit_tests/test_platformio_pch_script.py +++ b/tests/unit_tests/test_platformio_pch_script.py @@ -284,6 +284,16 @@ def test_pch_script_package_version_error_skips_pch(tmp_path: Path) -> None: assert scons_env.prepended == [] +def test_pch_script_corrupt_sidecar_reads_as_stale(tmp_path: Path) -> None: + """A truncated/corrupt .failed marker must not disable the pch forever.""" + _run_script(tmp_path, fail=True) + proj = tmp_path / "dev" + (proj / "esphome_pch.h.gch.failed").write_bytes(b"\xff\xfe corrupt") + _run_script(tmp_path) + assert (proj / "esphome_pch.h.gch").is_file() + assert (proj / "esphome_pch.h.gch.sum").is_file() + + def test_pch_script_rebuilds_when_header_missing(tmp_path: Path) -> None: _run_script(tmp_path) proj = tmp_path / "dev"