From 3730f7bbae4533d345fb54b5ce64c6169ef51391 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 26 Aug 2026 11:58:20 -0500 Subject: [PATCH] Distinguish stat failure from absence, fail closed on unmodelable flags and unreadable local headers --- esphome/build_helpers/idedata.py | 4 +- esphome/build_helpers/pch.py | 12 ++++- esphome/platformio/pch.py.script | 53 +++++++++++++------ .../unit_tests/build_helpers/test_idedata.py | 15 ++++++ tests/unit_tests/build_helpers/test_pch.py | 7 ++- .../unit_tests/test_platformio_pch_script.py | 25 +++++---- 6 files changed, 84 insertions(+), 32 deletions(-) diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index c5acab40d6..01444db7fa 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -201,10 +201,10 @@ def parse_entry( for tok in it: if tok in ("-c", "-o"): next(it, None) # drop the flag and its argument (input/output) - elif tok == "-include": + elif tok == "-include" or tok.startswith("-include"): # Re-anchor only names next to the compile (the pch); a name # meant for the -I chain must stay untouched - raw = next(it, "") + raw = next(it, "") if tok == "-include" else tok[len("-include") :] if not raw: _LOGGER.warning("Dropping -include with no argument") elif Path(resolved := _include(raw)).is_file(): diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index 54566e127a..0addb83807 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -15,6 +15,7 @@ import os from pathlib import Path import posixpath import re +import stat from esphome.build_helpers.ccache import parse_enable_env @@ -110,6 +111,15 @@ def pch_header_text(include_headers: Iterable[str]) -> str: return "".join(f'#include "{name}"\n' for name in include_headers) +def _resolves(path: Path) -> bool: + """False when missing; other stat failures propagate (identity unknown, + unlike is_file(), which would silently drop the header).""" + try: + return stat.S_ISREG(path.stat().st_mode) + except (FileNotFoundError, NotADirectoryError): + return False + + def _include_closure(src_dir: Path, roots: Iterable[str]) -> dict[str, bytes]: """Include closure of ``roots``: src-relative name -> contents. @@ -123,7 +133,7 @@ def _include_closure(src_dir: Path, roots: Iterable[str]) -> dict[str, bytes]: name, from_dir = stack.pop() for candidate in (f"{from_dir}/{name}" if from_dir else name, name): rel = posixpath.normpath(candidate) - if not rel.startswith("..") and (src_dir / rel).is_file(): + if not rel.startswith("..") and _resolves(src_dir / rel): break else: continue diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 8b04cb6137..4c15854c8a 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -4,6 +4,7 @@ from pathlib import Path import posixpath import re import shlex +import stat import subprocess import traceback @@ -28,6 +29,14 @@ _INCLUDE_RE = re.compile(rb'^\s*#\s*include\s+["<]([^">]+)[">]', re.MULTILINE) _CORE_HEADER = "esphome/core/defines.h" +def _resolves(path: Path) -> bool: + """False when missing; other stat failures propagate (identity unknown).""" + try: + return stat.S_ISREG(path.stat().st_mode) + except (FileNotFoundError, NotADirectoryError): + return False + + def _include_closure(src_dir: Path, roots: list) -> dict: """Quoted-include closure: src-relative name -> contents (mirror of build_helpers/pch.py).""" @@ -37,7 +46,7 @@ def _include_closure(src_dir: Path, roots: list) -> dict: name, from_dir = stack.pop() for candidate in (f"{from_dir}/{name}" if from_dir else name, name): rel = posixpath.normpath(candidate) - if not rel.startswith("..") and (src_dir / rel).is_file(): + if not rel.startswith("..") and _resolves(src_dir / rel): break else: continue @@ -71,11 +80,11 @@ def _shell_arg(element) -> str: tokens = shlex.split(arg) except ValueError as err: print(f"ESPHome: could not lex flag {arg!r} for the pch: {err}") - return arg + return None if len(tokens) != 1: - # The quoting model is wrong for this element; leave a trail - print(f"ESPHome: passing flag {arg!r} through unlexed for the pch") - return arg + # A flag the model cannot reproduce would diverge the .gch's flags + print(f"ESPHome: cannot model flag {arg!r} for the pch") + return None return tokens[0] @@ -149,10 +158,14 @@ def _setup_pch() -> None: # not see them; consumers keep theirs, which the .gch then satisfies. flags = [] include_headers = [] - flag_it = iter( + raw_args = [ _shell_arg(element) for element in projenv.subst_list("$CXXFLAGS $CCFLAGS $_CCCOMCOM")[0] # noqa: F821 - ) + ] + if any(arg is None for arg in raw_args): + print("ESPHome: skipping precompiled header: unmodelable flag") + return + flag_it = iter(raw_args) for tok in flag_it: if tok == "-include": include_headers.append(next(flag_it, "")) @@ -166,11 +179,15 @@ def _setup_pch() -> None: # Fold only relative names resolving under src/: consumers keep their # own -include entries, so folding an unguarded user header would # include it twice; unfolded ones stay consumer-only. - folded = [ - name - for name in include_headers - if not Path(name).is_absolute() and (src_dir / name).is_file() - ] + try: + folded = [ + name + for name in include_headers + if not Path(name).is_absolute() and _resolves(src_dir / name) + ] + except OSError as err: + print(f"ESPHome: skipping precompiled header: {err}") + return 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)) @@ -202,7 +219,11 @@ def _setup_pch() -> None: return digest.update(f"{package}={version}".encode()) digest.update(b"\0") - closure = _include_closure(src_dir, [*folded, _CORE_HEADER]) + try: + closure = _include_closure(src_dir, [*folded, _CORE_HEADER]) + except OSError as err: + print(f"ESPHome: skipping precompiled header: {err}") + return for rel in sorted(closure): digest.update(rel.encode(errors="surrogateescape")) digest.update(closure[rel]) @@ -237,10 +258,8 @@ def _setup_pch() -> None: try: data = local.read_bytes() except OSError as err: - # mtime/size still shift the digest; stat failure skips the pch - print(f"ESPHome: could not read {local} for the pch checksum: {err}") - st = local.stat() - data = f"".encode() + print(f"ESPHome: skipping precompiled header: {local}: {err}") + return digest.update(str(local.relative_to(proj_dir)).encode()) digest.update(data) digest.update(b"\0") diff --git a/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index b0dc6de2ce..c4512ee718 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -103,6 +103,21 @@ def test_parse_entry_resolves_force_include_path(tmp_path: Path) -> None: assert resolved == str(tmp_path / "esphome_pch.h").replace("\\", "/") +def test_parse_entry_resolves_joined_force_include(tmp_path: Path) -> None: + """The joined -includefoo.h spelling takes the same resolve path.""" + (tmp_path / "esphome_pch.h").write_text("") + entry = _entry( + str(tmp_path), + f"{tmp_path}/src/esphome/x.cpp", + "g++ -includeesphome_pch.h -c x.cpp", + ) + + _, _, _, cxx_flags = idedata.parse_entry(entry) + + resolved = cxx_flags[cxx_flags.index("-include") + 1] + assert resolved == str(tmp_path / "esphome_pch.h").replace("\\", "/") + + def test_parse_entry_keeps_search_chain_force_include(tmp_path: Path) -> None: """-include names resolved via the -I chain (libretiny's Arduino.h) must not be re-anchored to a nonexistent build-dir path.""" diff --git a/tests/unit_tests/build_helpers/test_pch.py b/tests/unit_tests/build_helpers/test_pch.py index 967eb0d890..0fc2378288 100644 --- a/tests/unit_tests/build_helpers/test_pch.py +++ b/tests/unit_tests/build_helpers/test_pch.py @@ -165,8 +165,11 @@ def test_include_closure_raises_when_identity_unknown( """An unreadable header propagates; callers compile without a pch.""" class _BadFile: - def is_file(self) -> bool: - return True + def stat(self): # noqa: ANN202 -- regular-file mode only + import os + import stat as stat_mod + + return os.stat_result((stat_mod.S_IFREG | 0o644,) + (0,) * 9) def read_bytes(self) -> bytes: raise OSError("read failed") diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py index 54c0ee5972..cf857c45b9 100644 --- a/tests/unit_tests/test_platformio_pch_script.py +++ b/tests/unit_tests/test_platformio_pch_script.py @@ -188,6 +188,15 @@ def test_pch_script_preserves_spaced_flag_elements(tmp_path: Path) -> None: assert pch.splitlines()[0] == '#include "other.h"' +def test_pch_script_folds_joined_force_include_spelling(tmp_path: Path) -> None: + """-includefoo.h folds like the separated form, matching the native path.""" + (tmp_path / "dev" / "src").mkdir(parents=True, exist_ok=True) + (tmp_path / "dev" / "src" / "other.h").write_text("") + _run_script(tmp_path, flags=["-DX=1", "-includeother.h"]) + pch = (tmp_path / "dev" / "esphome_pch.h").read_text() + assert pch.splitlines()[0] == '#include "other.h"' + + def test_pch_script_leaves_absolute_force_includes_unfolded( tmp_path: Path, ) -> None: @@ -400,21 +409,17 @@ def test_pch_script_hashes_project_local_include_dirs(tmp_path: Path) -> None: @pytest.mark.skipif( getattr(os, "geteuid", lambda: -1)() == 0, reason="root ignores file modes" ) -def test_pch_script_unreadable_local_header_warns_and_varies( +def test_pch_script_unreadable_local_header_skips_pch( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: - """An unreadable generated header still shifts the digest via mtime/size.""" + """An unreadable generated header means unknown identity: no pch.""" proj = tmp_path / "dev" override = proj / "lwip_override" override.mkdir(parents=True) secret = override / "lwipopts.h" secret.write_text("#define TCP_MSS 1460\n") secret.chmod(0) - flags = ["-DX=1", "-I", str(override)] - _run_script(tmp_path, flags=flags) - first = (proj / "esphome_pch.h.gch.sum").read_text() - assert "could not read" in capsys.readouterr().out - os.utime(secret, (1, 1)) - (tmp_path / "fake-gxx.argv").unlink(missing_ok=True) - _run_script(tmp_path, flags=flags) - assert (proj / "esphome_pch.h.gch.sum").read_text() != first + scons_env = _run_script(tmp_path, flags=["-DX=1", "-I", str(override)]) + assert not (proj / "esphome_pch.h.gch.sum").exists() + assert scons_env.prepended == [] + assert "skipping precompiled header" in capsys.readouterr().out