From 6530bb62f8a2296984fac3a470b67fa5160c2aba Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 26 Aug 2026 13:14:03 -0500 Subject: [PATCH] Stat-aware local walk, atomic flag+env application, guard degenerate basedir and -include- flags --- esphome/build_gen/arduino8266.py | 10 +- esphome/build_helpers/ccache.py | 7 +- esphome/build_helpers/idedata.py | 4 +- esphome/build_helpers/pch.py | 4 + esphome/platformio/pch.py.script | 124 +++++++++++------- tests/unit_tests/build_helpers/test_ccache.py | 4 + .../unit_tests/test_platformio_pch_script.py | 33 +++++ 7 files changed, 128 insertions(+), 58 deletions(-) diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 1f2fbb651f..d8ebf92a53 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -1211,7 +1211,7 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: "build_src_flags has a trailing '-include' with no header" ) src_includes.append(header) - elif tok.startswith("-include"): + elif tok.startswith("-include") and not tok.startswith("-include-"): # Joined spelling; left in src_other it would precede the pch # include and silently defeat the .gch src_includes.append(tok[len("-include") :]) @@ -1268,12 +1268,14 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: "(set ESPHOME_PCH_ENABLE=0 to disable)" ) write_file_if_changed(pch_header, pch_text) + sum_path = build_dir / f"{PCH_HEADER_NAME}.gch.sum" if checksum is not None: # Generate-time stamp: a hand-run ninja can rebuild the .gch # while this .sum lags - write_file_if_changed( - build_dir / f"{PCH_HEADER_NAME}.gch.sum", checksum + "\n" - ) + write_file_if_changed(sum_path, checksum + "\n") + else: + # A stale .sum from an earlier ccache run must not survive + sum_path.unlink(missing_ok=True) gch = _e(f"{PCH_HEADER_NAME}.gch") lines.append(f"build {gch}: pch {_e(pch_header)}") if src_other: diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index 7daad458dd..2fbddd7008 100644 --- a/esphome/build_helpers/ccache.py +++ b/esphome/build_helpers/ccache.py @@ -100,6 +100,9 @@ def effective_ccache_basedir() -> str: raw = os.environ.get("CCACHE_BASEDIR") if raw is not None: - # An explicitly empty value disables ccache's rewriting; mirror it - return raw + # A degenerate value ("", "/", relative) must not be used for + # substring stripping; fall back to the resolved build path + if len(Path(raw).parts) > 1: + return raw + return str(Path(CORE.build_path).resolve()) return str(Path(CORE.build_path).resolve()) diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 01444db7fa..402ab0fd2e 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -201,7 +201,9 @@ 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" or tok.startswith("-include"): + elif tok == "-include" or ( + tok.startswith("-include") and not 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, "") if tok == "-include" else tok[len("-include") :] diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index 0addb83807..aa9bff74cf 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -81,6 +81,10 @@ def ccache_pch_env() -> dict[str, str]: non-pch TUs.""" if not (pch_enabled() and _pch_data().emitted): return {} + extsum = os.environ.get("CCACHE_PCH_EXTSUM") + if extsum is not None and extsum.strip().lower() not in ("1", "true", "yes", "on"): + # ccache then hashes the non-reproducible .gch bytes: permanent misses + _LOGGER.warning("CCACHE_PCH_EXTSUM=%s disables pch caching", extsum) env = {k: v for k, v in _CCACHE_PCH_ENV.items() if k not in os.environ} user_sloppiness = os.environ.get("CCACHE_SLOPPINESS") if user_sloppiness is not None and ( diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 4c15854c8a..2e0cdaa926 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -29,6 +29,18 @@ _INCLUDE_RE = re.compile(rb'^\s*#\s*include\s+["<]([^">]+)[">]', re.MULTILINE) _CORE_HEADER = "esphome/core/defines.h" +def _raise_walk_error(err: OSError) -> None: + raise err + + +def _resolves_dir(path: Path) -> bool: + """False when missing; other stat failures propagate.""" + try: + return stat.S_ISDIR(path.stat().st_mode) + except (FileNotFoundError, NotADirectoryError): + return False + + def _resolves(path: Path) -> bool: """False when missing; other stat failures propagate (identity unknown).""" try: @@ -67,7 +79,7 @@ def _include_closure(src_dir: Path, roots: list) -> dict: return seen -def _shell_arg(element) -> str: +def _shell_arg(element) -> str | None: """One compiler argv from one SCons element, matching the real spawn: spaced elements pass whole, the rest get one shell unquote (skipped on Windows, where shlex would eat path backslashes).""" @@ -169,7 +181,7 @@ def _setup_pch() -> None: for tok in flag_it: if tok == "-include": include_headers.append(next(flag_it, "")) - elif tok.startswith("-include"): + elif tok.startswith("-include") and not tok.startswith("-include-"): include_headers.append(tok[len("-include") :]) else: flags.append(tok) @@ -231,38 +243,40 @@ def _setup_pch() -> None: # Project-local -I dirs (e.g. rp2's lwip_override) hold generated # headers the src closure cannot see; hash them too prev = "" - for tok in flags: - inc = tok[2:] if tok.startswith("-I") and len(tok) > 2 else "" - if prev == "-I": - inc = tok - prev = tok - if not inc: - continue - inc_dir = Path(inc) - if not ( - inc_dir.is_dir() - and inc_dir.is_relative_to(proj_dir) - and not inc_dir.is_relative_to(src_dir) - # Library trees never enter the prefix closure; walking them - # would read every library file each build - and not inc_dir.is_relative_to(proj_dir / ".piolibdeps") - and not inc_dir.is_relative_to(proj_dir / ".pioenvs") - ): - continue - headers = ( - p - for p in inc_dir.rglob("*") - if p.is_file() and p.suffix in (".h", ".hpp", ".hh", ".inc") - ) - for local in sorted(headers): - try: - data = local.read_bytes() - except OSError as err: - 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") + try: + for tok in flags: + inc = tok[2:] if tok.startswith("-I") and len(tok) > 2 else "" + if prev == "-I": + inc = tok + prev = tok + if not inc: + continue + inc_dir = Path(inc) + if not ( + _resolves_dir(inc_dir) + and inc_dir.is_relative_to(proj_dir) + and not inc_dir.is_relative_to(src_dir) + # Library trees never enter the prefix closure; walking them + # would read every library file each build + and not inc_dir.is_relative_to(proj_dir / ".piolibdeps") + and not inc_dir.is_relative_to(proj_dir / ".pioenvs") + ): + continue + local_headers = [] + # os.walk with onerror: rglob would swallow unlistable subtrees + for root, _dirs, files in os.walk(inc_dir, onerror=_raise_walk_error): + local_headers.extend( + Path(root) / f + for f in files + if f.endswith((".h", ".hpp", ".hh", ".inc")) + ) + for local in sorted(local_headers): + digest.update(str(local.relative_to(proj_dir)).encode()) + digest.update(local.read_bytes()) + digest.update(b"\0") + except OSError as err: + print(f"ESPHome: skipping precompiled header: {err}") + return checksum = digest.hexdigest() # The ccache .sum sidecar doubles as the freshness stamp @@ -302,6 +316,30 @@ def _setup_pch() -> None: failed_marker.unlink(missing_ok=True) sum_path.write_text(checksum + "\n", encoding="utf-8") + # Computed first so the flags and env land together: a raise between + # them would leave a pch-consuming build without its ccache settings. + # projenv["ENV"] aliases os.environ, so these reach all TUs; only + # time_macros affects non-pch TUs. User values win. + ccache_updates = { + key: value + for key, value in ( + ("CCACHE_SLOPPINESS", "pch_defines,time_macros"), + ("CCACHE_PCH_EXTSUM", "true"), + ) + if key not in os.environ + } + sloppiness = os.environ.get("CCACHE_SLOPPINESS") + if sloppiness is not None: + 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 + ccache_updates["CCACHE_SLOPPINESS"] = ",".join((sloppiness, *missing)) + print(f"ESPHome: adding {','.join(missing)} to CCACHE_SLOPPINESS for the pch") + extsum = os.environ.get("CCACHE_PCH_EXTSUM") + if extsum is not None and extsum.strip().lower() not in ("1", "true", "yes", "on"): + # ccache then hashes the non-reproducible .gch bytes: permanent misses + print(f"ESPHome: CCACHE_PCH_EXTSUM={extsum} disables pch caching") # Prepended: GCC only uses a .gch while no other tokens precede it. # The relative name also reaches "pio run -t idedata" output. # -Wno-error: the per-process probe can pass while a later cc1plus @@ -309,23 +347,7 @@ def _setup_pch() -> None: projenv.Prepend( # noqa: F821 CXXFLAGS=["-Winvalid-pch", "-Wno-error=invalid-pch", "-include", header.name] ) - # Exported last so a raise above cannot leave the relaxed settings with - # no pch in the build. projenv["ENV"] aliases os.environ, so these - # reach all TUs; only time_macros affects non-pch TUs. User values win. - for key, value in ( - ("CCACHE_SLOPPINESS", "pch_defines,time_macros"), - ("CCACHE_PCH_EXTSUM", "true"), - ): - if key not in os.environ: - projenv["ENV"][key] = value # noqa: F821 - sloppiness = os.environ.get("CCACHE_SLOPPINESS") - if sloppiness is not None: - 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 - projenv["ENV"]["CCACHE_SLOPPINESS"] = ",".join((sloppiness, *missing)) # noqa: F821 - print(f"ESPHome: adding {','.join(missing)} to CCACHE_SLOPPINESS for the pch") + projenv["ENV"].update(ccache_updates) # noqa: F821 print("ESPHome: Compiling with precompiled header") diff --git a/tests/unit_tests/build_helpers/test_ccache.py b/tests/unit_tests/build_helpers/test_ccache.py index d86589d378..23f38019aa 100644 --- a/tests/unit_tests/build_helpers/test_ccache.py +++ b/tests/unit_tests/build_helpers/test_ccache.py @@ -129,3 +129,7 @@ def test_effective_ccache_basedir_prefers_user_value(tmp_path: Path) -> None: assert ccache.effective_ccache_basedir() == "/custom/base" with patch.dict(os.environ, {}, clear=True): assert ccache.effective_ccache_basedir() == str(tmp_path.resolve()) + # Degenerate values would strip substrings ccache never rewrites + for bad in ("", "/"): + with patch.dict(os.environ, {"CCACHE_BASEDIR": bad}, clear=True): + assert ccache.effective_ccache_basedir() == str(tmp_path.resolve()) diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py index cf857c45b9..4469eecc41 100644 --- a/tests/unit_tests/test_platformio_pch_script.py +++ b/tests/unit_tests/test_platformio_pch_script.py @@ -352,6 +352,39 @@ def test_pch_script_unions_user_sloppiness( assert "adding pch_defines,time_macros" in capsys.readouterr().out +def test_pch_script_unmodelable_flag_skips_pch( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """Unbalanced quotes and multi-token elements cannot be reproduced as + one argv; the pch is skipped rather than built with diverging flags.""" + for bad in ("-DFOO='bar", "-DA=1\t-DB=2"): + scons_env = _run_script(tmp_path, flags=["-DX=1", bad]) + assert scons_env.prepended == [] + assert not (tmp_path / "dev" / "esphome_pch.h.gch").exists() + assert "unmodelable flag" in capsys.readouterr().out + + +@pytest.mark.skipif( + getattr(os, "geteuid", lambda: -1)() == 0, reason="root ignores file modes" +) +def test_pch_script_unlistable_include_dir_skips_pch( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """An unlistable subtree must not silently drop out of the digest.""" + proj = tmp_path / "dev" + override = proj / "lwip_override" + hidden = override / "hidden" + hidden.mkdir(parents=True) + (hidden / "gen.h").write_text("") + hidden.chmod(0) + try: + scons_env = _run_script(tmp_path, flags=["-DX=1", "-I", str(override)]) + finally: + hidden.chmod(0o755) + assert scons_env.prepended == [] + assert "skipping precompiled header" in capsys.readouterr().out + + def test_pch_script_nobuild_without_projenv_is_noop(tmp_path: Path) -> None: """-t nobuild never exports projenv; the script must not abort.""" proj = tmp_path / "dev"