Merge branch 'pch-strict-ci' into host-pch

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()

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

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()}"
 
 

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

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"),
     ):