From 87cbe42caed5c27e0240975daa2704b76ed25ef1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 27 Aug 2026 09:13:51 -0500 Subject: [PATCH] Blame the baseline honestly, split sidecar unlinks, pin the script's knob and import branches --- esphome/build_helpers/pch.py | 41 +++++++++++-------- esphome/espidf/toolchain.py | 4 +- esphome/platformio/pch.py.script | 4 +- tests/unit_tests/build_helpers/test_pch.py | 33 +++++++++++++-- .../unit_tests/test_platformio_pch_script.py | 35 ++++++++++++++-- 5 files changed, 89 insertions(+), 28 deletions(-) diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index 4c6e3783ed..512995376f 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -8,6 +8,7 @@ it too; Arduino.h visibility there is intended (esphome#8693). from __future__ import annotations from collections.abc import Iterable +from contextlib import suppress from dataclasses import dataclass import hashlib import json @@ -361,19 +362,23 @@ def discard_pch(build_dir: Path) -> None: header = build_dir / PCH_HEADER_NAME gch = Path(f"{header}.gch") had_gch = gch.is_file() - try: - gch.unlink(missing_ok=True) - Path(f"{gch}.sum").unlink(missing_ok=True) - except OSError as err: - if gch.is_file(): - from esphome.core import EsphomeError + errors = [] + for sidecar in (gch, Path(f"{gch}.sum")): + try: + sidecar.unlink(missing_ok=True) + except OSError as err: + if sidecar.is_file(): + from esphome.core import EsphomeError - raise EsphomeError( - f"Could not discard the stale precompiled header: {err}" - ) from err + raise EsphomeError( + f"Could not discard the stale precompiled header: {err}" + ) from err + errors.append(err) + for err in errors: _LOGGER.warning("Could not discard the pch sidecars: %s", err) if had_gch and header.is_file(): - os.utime(header) + with suppress(OSError): + os.utime(header) def prepare_pch( @@ -493,17 +498,17 @@ def prepare_pch( if probe is None: return if probe.returncode != 0: - error = probe.stderr.strip() or f"exit code {probe.returncode}" - # Disambiguate: only blame (and latch on) the pch when the same - # compile passes without it; anything else is environmental + # Disambiguate: only blame the pch when the same compile passes + # without it; a failing baseline is its own (latchable) problem baseline = _run([*base, *pch_probe_tail()], "probe baseline", stdin="") if baseline is None: return - _fail( - error, - "toolchain cannot load the pch", - latch=latch and baseline.returncode == 0, - ) + if baseline.returncode == 0: + error = probe.stderr.strip() or f"exit code {probe.returncode}" + _fail(error, "toolchain cannot load the pch", latch=latch) + else: + error = baseline.stderr.strip() or f"exit code {baseline.returncode}" + _fail(error, "probe cannot run at all", latch=latch) if gch.is_file() and _read_stamp(sum_path) == checksum: _log_pch_in_use() diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index a1e3f055fd..5d7f96c716 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -537,9 +537,11 @@ def run_compile(config, verbose: bool) -> int: except Exception: # noqa: BLE001 # pylint: disable=broad-exception-caught from esphome.build_helpers.pch import pch_strict + # Strict first: its own knob error must not mask the real failure + strict = pch_strict() # Raises itself if a stale .gch survives (silently wrong output) discard_pch() - if pch_strict(): + if strict: raise _LOGGER.warning( "Precompiled header setup failed; compiling without it", exc_info=True diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 76c074aee2..1d0543bf4d 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -166,7 +166,9 @@ def _probe_gch(cxx, flags, header: Path, proj_dir: Path): return None baseline = _probe_run(cxx, flags, dep_redirect, proj_dir) if baseline.returncode != 0: - raise OSError(f"probe cannot run at all: {baseline.stderr.strip()[:200]}") + # Deterministic and latchable; the transient filter at the caller + # keeps resource exhaustion from latching + return f"probe cannot run at all: {baseline.stderr.strip()[:200]}" return f"toolchain cannot load the pch: {probe.stderr.strip()}" diff --git a/tests/unit_tests/build_helpers/test_pch.py b/tests/unit_tests/build_helpers/test_pch.py index e55b2cf4fc..cb931816c4 100644 --- a/tests/unit_tests/build_helpers/test_pch.py +++ b/tests/unit_tests/build_helpers/test_pch.py @@ -289,13 +289,15 @@ def test_discard_pch_raises_when_gch_survives( pch.discard_pch(tmp_path) -def test_discard_pch_warns_when_only_sidecar_unlink_fails( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, - caplog: pytest.LogCaptureFixture, +def test_discard_pch_raises_when_sum_survives( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + """The .sum is ccache's pch identity; one that survives is as unsafe + as a surviving .gch.""" from pathlib import Path as _P + from esphome.core import EsphomeError + (tmp_path / "esphome_pch.h").write_text("") (tmp_path / "esphome_pch.h.gch").write_bytes(b"gch") (tmp_path / "esphome_pch.h.gch.sum").write_text("x") @@ -307,5 +309,28 @@ def test_discard_pch_warns_when_only_sidecar_unlink_fails( return real_unlink(self, missing_ok=missing_ok) monkeypatch.setattr(_P, "unlink", failing_unlink) + with pytest.raises(EsphomeError, match="Could not discard"): + pch.discard_pch(tmp_path) + + +def test_discard_pch_warns_when_file_vanished_concurrently( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """An unlink error on a file that is nonetheless gone only warns.""" + from pathlib import Path as _P + + (tmp_path / "esphome_pch.h").write_text("") + (tmp_path / "esphome_pch.h.gch").write_bytes(b"gch") + real_unlink = _P.unlink + + def racing_unlink(self, missing_ok=False): + if self.name.endswith(".sum"): + # Racer removed it, then our unlink errored + raise OSError("stale handle") + return real_unlink(self, missing_ok=missing_ok) + + monkeypatch.setattr(_P, "unlink", racing_unlink) pch.discard_pch(tmp_path) assert "Could not discard the pch sidecars" in caplog.text diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py index 84b8a10ee9..d2eaf80161 100644 --- a/tests/unit_tests/test_platformio_pch_script.py +++ b/tests/unit_tests/test_platformio_pch_script.py @@ -295,15 +295,17 @@ def test_pch_script_spawn_failure_is_transient( assert "did not run" in capsys.readouterr().out -def test_pch_script_probe_environment_failure_does_not_latch( - tmp_path: Path, +def test_pch_script_probe_baseline_failure_latches_with_honest_label( + tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: - """Probe AND baseline failing is environmental: no marker, retry.""" + """Probe AND baseline failing is a deterministic environment problem: + latch, but blame the environment rather than the pch.""" scons_env = _run_script(tmp_path, probe_exit=1) proj = tmp_path / "dev" assert not (proj / "esphome_pch.h.gch").exists() - assert not (proj / "esphome_pch.h.gch.failed").exists() + assert (proj / "esphome_pch.h.gch.failed").is_file() assert scons_env.prepended == [] + assert "probe cannot run at all" in capsys.readouterr().out def test_pch_script_unresolved_package_version_skips_pch(tmp_path: Path) -> None: @@ -464,6 +466,27 @@ def test_pch_script_unreadable_local_header_skips_pch( assert "skipping precompiled header" in capsys.readouterr().out +def test_pch_script_rejects_unrecognized_strict_value(tmp_path: Path) -> None: + """A typo'd knob must fail the build, not silently disable the gate.""" + with pytest.raises(RuntimeError, match="Unrecognized ESPHOME_PCH_STRICT"): + _run_script(tmp_path, env_vars={"ESPHOME_PCH_STRICT": "yolo"}) + + +def test_pch_script_strict_tables_match_helpers(tmp_path: Path) -> None: + """The script's mirrored spelling tables must not drift.""" + from esphome.helpers import FALSY_ENV_STRINGS, TRUTHY_ENV_STRINGS + + proj = tmp_path / "dev" + (proj / "src").mkdir(parents=True) + env = _FakeSConsEnv(proj, proj / "src", "g++", ["-DX=1"]) + namespace = {"Import": lambda *_names: None, "env": env, "projenv": env} + with patch.dict(os.environ, {}, clear=True): + exec(compile(_SCRIPT.read_text(), "pch.py", "exec"), namespace) # noqa: S102 + assert set(namespace["_TRUTHY"]) == set(TRUTHY_ENV_STRINGS) + # parse_enable_env handles the empty string separately + assert set(namespace["_FALSY"]) - {""} == set(FALSY_ENV_STRINGS) + + def test_pch_script_strict_reprobes_cached_gch(tmp_path: Path) -> None: """Rejection is per-process: strict re-proves a cached .gch loads.""" _run_script(tmp_path) @@ -491,8 +514,12 @@ def test_pch_script_strict_fails_without_scons(tmp_path: Path) -> None: if "projenv" in names: raise RuntimeError("Import of non-existent variable 'projenv'") + import sys + env = _FakeSConsEnv(proj, proj / "src", "g++", ["-DX=1"]) with ( + # None forces ImportError even where SCons is installed + patch.dict(sys.modules, {"SCons.Script": None}), patch.dict(os.environ, {"ESPHOME_PCH_STRICT": "1"}, clear=True), pytest.raises(RuntimeError, match="not used"), ):