From d0263b0bef61deadd9218056842e7e7c5264f6b4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 26 Aug 2026 10:44:17 -0500 Subject: [PATCH] Mirror the joined -include fold, harden the probe, fail closed on unreadable closure headers --- esphome/build_helpers/pch.py | 6 ++--- esphome/platformio/pch.py.script | 10 +++++--- tests/unit_tests/build_helpers/test_pch.py | 24 +++++++------------ .../unit_tests/test_arduino8266_toolchain.py | 2 +- 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index ebf2c506d7..54566e127a 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -132,11 +132,9 @@ def _include_closure(src_dir: Path, roots: Iterable[str]) -> dict[str, bytes]: try: data = (src_dir / rel).read_bytes() except OSError as err: - # mtime/size still shift the digest; a stat failure propagates - # so callers compile without a pch + # A marker would truncate the transitive walk; fail closed _LOGGER.warning("Could not read %s for the pch checksum: %s", rel, err) - st = (src_dir / rel).stat() - data = f"".encode() + raise seen[rel] = data parent = posixpath.dirname(rel) stack.extend( diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index b631897e29..8b04cb6137 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -46,10 +46,9 @@ def _include_closure(src_dir: Path, roots: list) -> dict: try: data = (src_dir / rel).read_bytes() except OSError as err: - # A stat failure propagates and the outer handler skips the pch + # A marker would truncate the transitive walk; fail closed print(f"ESPHome: could not read {rel} for the pch checksum: {err}") - st = (src_dir / rel).stat() - data = f"".encode() + raise seen[rel] = data parent = posixpath.dirname(rel) stack.extend( @@ -112,10 +111,13 @@ def _compile_gch(cxx, flags, header: Path, gch: Path, proj_dir: Path): "-", ], cwd=proj_dir, + env={**os.environ, "LC_ALL": "C"}, input="", capture_output=True, text=True, ) + if probe.returncode < 0: + raise OSError(f"probe killed by signal {-probe.returncode}") if probe.returncode != 0 or ".gch" in probe.stderr: return f"toolchain cannot load the pch: {probe.stderr.strip()}" return None @@ -154,6 +156,8 @@ def _setup_pch() -> None: for tok in flag_it: if tok == "-include": include_headers.append(next(flag_it, "")) + elif tok.startswith("-include"): + include_headers.append(tok[len("-include") :]) else: flags.append(tok) if any(not name for name in include_headers): diff --git a/tests/unit_tests/build_helpers/test_pch.py b/tests/unit_tests/build_helpers/test_pch.py index bb6ee81a7f..967eb0d890 100644 --- a/tests/unit_tests/build_helpers/test_pch.py +++ b/tests/unit_tests/build_helpers/test_pch.py @@ -55,8 +55,8 @@ def test_ccache_pch_env_disabled() -> None: def test_ccache_pch_env_token_check_is_membership_not_substring( caplog: pytest.LogCaptureFixture, ) -> None: - pch.mark_pch_emitted() """A token merely containing ours must not suppress the union.""" + pch.mark_pch_emitted() 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" @@ -65,9 +65,9 @@ def test_ccache_pch_env_token_check_is_membership_not_substring( def test_ccache_pch_env_unions_user_sloppiness( caplog: pytest.LogCaptureFixture, ) -> None: - pch.mark_pch_emitted() """Without pch_defines/time_macros ccache declines every pch-consuming compile, so missing tokens are unioned onto the user's value.""" + pch.mark_pch_emitted() with patch.dict(os.environ, {"CCACHE_SLOPPINESS": "locale"}, clear=True): env = pch.ccache_pch_env() assert env["CCACHE_SLOPPINESS"] == "locale,pch_defines,time_macros" @@ -135,22 +135,20 @@ 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_marks_unreadable( +def test_include_closure_fails_closed_on_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.""" + """A marker would truncate the transitive walk; the OSError propagates + so callers compile without a pch.""" _write(tmp_path, "a.h", '#include "locked.h"\n') locked = tmp_path / "locked.h" locked.write_text("") locked.chmod(0) try: - closure = pch._include_closure(tmp_path, ["a.h"]) + with pytest.raises(OSError): + pch._include_closure(tmp_path, ["a.h"]) finally: locked.chmod(0o644) - # stat still works, so the marker varies with mtime/size and a later - # edit to the unreadable file still shifts the digest - assert closure["locked.h"].startswith(b" None: def test_include_closure_raises_when_identity_unknown( caplog: pytest.LogCaptureFixture, ) -> None: - """Read AND stat failing means no marker can vouch for the header, so - the OSError propagates and callers compile without a pch.""" + """An unreadable header propagates; callers compile without a pch.""" class _BadFile: def is_file(self) -> bool: @@ -174,14 +171,11 @@ def test_include_closure_raises_when_identity_unknown( def read_bytes(self) -> bytes: raise OSError("read failed") - def stat(self) -> None: - raise OSError("stat failed") - class _FakeSrcDir: def __truediv__(self, rel: str) -> _BadFile: return _BadFile() - with pytest.raises(OSError, match="stat failed"): + with pytest.raises(OSError, match="read failed"): pch._include_closure(_FakeSrcDir(), ["a.h"]) assert "Could not read a.h" in caplog.text diff --git a/tests/unit_tests/test_arduino8266_toolchain.py b/tests/unit_tests/test_arduino8266_toolchain.py index a17dec4397..c58ca534c1 100644 --- a/tests/unit_tests/test_arduino8266_toolchain.py +++ b/tests/unit_tests/test_arduino8266_toolchain.py @@ -633,8 +633,8 @@ def test_get_idedata_accepts_preresolved_ccache() -> None: def test_ccache_env_includes_pch_settings() -> None: - mark_pch_emitted() """The native build exports the ccache settings the pch needs.""" + mark_pch_emitted() with patch.dict(os.environ, {}, clear=True): env = framework.ccache_env("/usr/bin/ccache") assert env["CCACHE_SLOPPINESS"] == "pch_defines,time_macros"