From 5a8e6761cb5f89ff50c110507180fa0fd7d89d99 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 25 Aug 2026 21:44:52 -0500 Subject: [PATCH] Fold only src-resolvable force-includes, surrogateescape the digest names, harden flag lexing and failure reporting --- esphome/build_helpers/pch.py | 3 +- esphome/platformio/pch.py.script | 33 +++++++++++++++---- .../unit_tests/test_platformio_pch_script.py | 17 +++++++++- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index 0f65055c35..72aa097597 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -140,7 +140,8 @@ def pch_checksum( digest = hashlib.sha256() closure = _include_closure(src_dir, include_headers) for name in sorted(closure): - digest.update(name.encode()) + # surrogateescape round-trips names from non-UTF-8 filesystems + digest.update(name.encode(errors="surrogateescape")) digest.update(closure[name]) digest.update(b"\0") for item in extra: diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 9fe84b26f3..a09d1ebc64 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -69,7 +69,16 @@ def _shell_arg(element) -> str: arg = str(element) if " " in arg or os.name == "nt": return arg.replace('\\"', '"') - return shlex.split(arg)[0] if arg.strip() else arg + if not arg.strip(): + return arg + try: + tokens = shlex.split(arg) + except ValueError as err: + print(f"ESPHome: could not lex flag {arg!r} for the pch: {err}") + return arg + # Anything but exactly one token means the model above is wrong for + # this element; pass it through untouched rather than dropping flags + return tokens[0] if len(tokens) == 1 else arg def _compile_gch(cxx, flags, header: Path, gch: Path, proj_dir: Path): @@ -83,6 +92,10 @@ def _compile_gch(cxx, flags, header: Path, gch: Path, proj_dir: Path): capture_output=True, text=True, ) + if result.returncode < 0: + # Signal-killed (OOM, ^C) is environmental; raising OSError routes + # it to the transient no-marker path + raise OSError(f"compiler killed by signal {-result.returncode}") if result.returncode != 0: return result.stderr probe = subprocess.run( # noqa: PLW1510 @@ -141,9 +154,14 @@ def _setup_pch() -> None: 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) - ) + # Fold only names that resolve under src/: consumers keep their own + # -include entries, so an unguarded user header folded here would be + # included twice. An unfolded header simply stays consumer-only and + # ccache hashes it directly off the command line. + folded = [name for name in include_headers if (src_dir / name).is_file()] + if unfolded := [n for n in include_headers if n not in folded]: + print(f"ESPHome: not precompiling non-src force-includes: {unfolded}") + content = "".join(f'#include "{name}"\n' for name in (*folded, _CORE_HEADER)) digest = hashlib.sha256() digest.update(content.encode()) @@ -174,9 +192,9 @@ def _setup_pch() -> None: return digest.update(f"{package}={version}".encode()) digest.update(b"\0") - closure = _include_closure(src_dir, [*include_headers, _CORE_HEADER]) + closure = _include_closure(src_dir, [*folded, _CORE_HEADER]) for rel in sorted(closure): - digest.update(rel.encode()) + digest.update(rel.encode(errors="surrogateescape")) digest.update(closure[rel]) digest.update(b"\0") # Project-local include dirs outside src (e.g. rp2's lwip_override) @@ -287,5 +305,6 @@ def _setup_pch() -> None: try: _setup_pch() except Exception: # noqa: BLE001 -- a speedup must never break the build - print("ESPHome: precompiled header setup failed; compiling without it") + # Stable marker: an unexpected error, unlike the expected skip prints + print("ESPHome: pch internal error; compiling without it") traceback.print_exc() diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py index 7ce2d0f644..e9aafd3951 100644 --- a/tests/unit_tests/test_platformio_pch_script.py +++ b/tests/unit_tests/test_platformio_pch_script.py @@ -161,6 +161,8 @@ def test_pch_script_preserves_spaced_flag_elements(tmp_path: Path) -> None: stripped from the .gch compile.""" spaced = tmp_path / "My Configs" spaced.mkdir() + (tmp_path / "dev" / "src").mkdir(parents=True, exist_ok=True) + (tmp_path / "dev" / "src" / "other.h").write_text("") flags = ['-DUSB_PRODUCT=\\"Pico 2W\\"', "-I", str(spaced), "-include", "other.h"] _run_script(tmp_path, flags=flags) calls = (tmp_path / "fake-gxx.argv").read_text().split("---call---\n") @@ -168,11 +170,24 @@ def test_pch_script_preserves_spaced_flag_elements(tmp_path: Path) -> None: assert '-DUSB_PRODUCT="Pico 2W"' in gch_call assert str(spaced) in gch_call assert "-include" not in gch_call - # The stripped -include header is folded into the prefix header instead + # The stripped src-resolvable -include is folded into the prefix header pch = (tmp_path / "dev" / "esphome_pch.h").read_text() assert pch.splitlines()[0] == '#include "other.h"' +def test_pch_script_leaves_non_src_force_includes_unfolded( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A user -include outside src/ must not enter the prefix header: + consumers keep their own copy, so folding an unguarded header would + include it twice.""" + _run_script(tmp_path, flags=["-DX=1", "-include", "user_extra.h"]) + pch = (tmp_path / "dev" / "esphome_pch.h").read_text() + assert "user_extra.h" not in pch + assert pch.splitlines()[-1] == '#include "esphome/core/defines.h"' + assert "not precompiling non-src force-includes" in capsys.readouterr().out + + def test_pch_script_sum_is_device_independent(tmp_path: Path) -> None: """Regression: identical configs in different dirs share cache keys.""" sums = []