Blame the pch only when a baseline passes, validate the strict knob, abort on an undiscardable stale header

esphome/build_gen/arduino8266.py

@@ -1303,7 +1303,9 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool:
             if pch_strict():
                 # Consumers wait on the probe stamp, so an unloadable .gch
                 # reds the build here instead of warning ~100 times
-                probe = " ".join(pch_probe_args(PCH_HEADER_NAME))
+                probe = " ".join(
+                    pch_probe_args(PCH_HEADER_NAME, source=str(Path(os.devnull)))
+                )
                 lines.append("rule pchprobe")
                 # $out only expands in rule text, hence the inline stamp
                 lines.append(

esphome/build_gen/espidf.py

@@ -298,7 +298,9 @@ def _pch_cmake() -> str:
     """
     if not pch_enabled():
         return ""
-    # Strict inverts: a per-process consumer rejection reds the build
+    # Strict inverts: a per-process consumer rejection reds the build.
+    # Baked at generation: a knob flip takes effect when the CMakeLists is
+    # rewritten (every esphome compile); a hand-run idf.py keeps the old one
     escalation = "-Werror=invalid-pch" if pch.pch_strict() else "-Wno-error=invalid-pch"
     return f"""
 # ESPHome precompiled header (see esphome/build_helpers/pch.py).

esphome/build_helpers/pch.py

@@ -103,8 +103,20 @@ def pch_enabled() -> bool:
 
 
 def pch_strict() -> bool:
-    """CI knob: ``ESPHOME_PCH_STRICT=1`` turns pch degrade paths fatal."""
-    return parse_enable_env("ESPHOME_PCH_STRICT") is True
+    """CI knob: ``ESPHOME_PCH_STRICT=1`` turns pch degrade paths fatal.
+
+    A set-but-unrecognized value raises: a typo must not silently turn
+    the gate into a no-op that proves nothing.
+    """
+    parsed = parse_enable_env("ESPHOME_PCH_STRICT")
+    if parsed is None and os.environ.get("ESPHOME_PCH_STRICT") is not None:
+        from esphome.core import EsphomeError
+
+        raise EsphomeError(
+            f"Unrecognized ESPHOME_PCH_STRICT="
+            f"{os.environ['ESPHOME_PCH_STRICT']!r}; use 1 or 0"
+        )
+    return parsed is True
 
 
 def pch_degraded(reason: str) -> None:
@@ -120,11 +132,12 @@ def pch_disabled_degraded() -> None:
     pch_degraded("pch disabled by ESPHOME_PCH_ENABLE")
 
 
-def pch_probe_args(header: str) -> list[str]:
+def pch_probe_args(header: str, source: str = "-") -> list[str]:
     """Flags that load-check a built .gch via a syntax-only compile.
 
     Rejection must be a nonzero exit (never just a wording match), so the
-    invalid-pch class is always escalated.
+    invalid-pch class is always escalated. ``source`` defaults to stdin
+    (host independent); the ninja probe edge passes a real file.
     """
     return [
         "-Winvalid-pch",
@@ -134,7 +147,7 @@ def pch_probe_args(header: str) -> list[str]:
         "-fsyntax-only",
         "-x",
         "c++",
-        os.devnull,
+        source,
     ]
 
 
@@ -403,7 +416,9 @@ def prepare_pch(
         return
     failed_marker = Path(f"{gch}.failed")
 
-    def _run(run_cmd: list[str], what: str) -> subprocess.CompletedProcess | None:
+    def _run(
+        run_cmd: list[str], what: str, stdin: str | None = None
+    ) -> subprocess.CompletedProcess | None:
         """Spawn one pch tool step; environmental failures discard and
         degrade (None): spawn/IO/timeout errors and signal kills never
         latch the marker."""
@@ -413,6 +428,7 @@ def prepare_pch(
                 cwd=cmd_dir,
                 # C locale keeps diagnostics matchable by _TRANSIENT_ERRORS
                 env={**os.environ, "LC_ALL": "C"},
+                input=stdin,
                 capture_output=True,
                 text=True,
                 check=False,
@@ -458,15 +474,27 @@ def prepare_pch(
         from cmd, so no -MF is needed; cmd ends with the fixed
         "-x c++-header -c -o" tail. A cached-header rejection may not
         reproduce (per-process), so that caller passes latch=False."""
-        # The fixed tail pch_compile_command appends; the slice below
-        # depends on it
-        assert cmd[-6:-4] == ["-x", "c++-header"], cmd[-6:]
-        probe = _run([*cmd[:-6], *pch_probe_args(str(header))], "probe")
+        if cmd[-6:-4] != ["-x", "c++-header"]:
+            # The slice below depends on pch_compile_command's fixed tail
+            _LOGGER.warning("Unexpected pch command shape: %s", cmd[-6:])
+            discard_pch(build_dir)
+            pch_degraded("unexpected pch command shape")
+            return
+        base = cmd[:-6]
+        probe = _run([*base, *pch_probe_args(str(header))], "probe", stdin="")
         if probe is None:
             return
-        if probe.returncode != 0 or ".gch" in probe.stderr:
+        if probe.returncode != 0:
             error = probe.stderr.strip() or f"exit code {probe.returncode}"
-            if latch:
+            # Disambiguate: only blame the pch when the same compile passes
+            # without it; anything else is environmental and must not latch
+            # pch_probe_args minus the warning flags and the -include pair
+            baseline = _run(
+                [*base, *pch_probe_args(str(header))[4:]], "probe baseline", stdin=""
+            )
+            if baseline is None:
+                return
+            if latch and baseline.returncode == 0:
                 _fail(error, "toolchain cannot load the pch")
             else:
                 _LOGGER.warning(

esphome/espidf/toolchain.py

@@ -539,6 +539,14 @@ def run_compile(config, verbose: bool) -> int:
         try:
             discard_pch()
         except OSError as discard_err:
+            # A stale .gch that survives would be consumed silently: that
+            # is wrong output, not a slow build, so it must abort
+            from esphome.build_helpers.pch import PCH_HEADER_NAME
+
+            if CORE.relative_build_path("build", f"{PCH_HEADER_NAME}.gch").is_file():
+                raise EsphomeError(
+                    f"Could not discard the stale precompiled header: {discard_err}"
+                ) from discard_err
             _LOGGER.warning("Could not discard the stale pch: %s", discard_err)
         from esphome.build_helpers.pch import pch_strict
 

esphome/platformio/pch.py.script

@@ -26,14 +26,17 @@ except Exception as err:  # noqa: BLE001 -- not exported under -t nobuild
 # Compiler failures that clear on their own must not latch the .failed marker
 _TRANSIENT_ERRORS = ("No space left", "Cannot allocate", "Resource temporarily")
 
-# Keep in sync with helpers.TRUTHY_ENV_STRINGS
-_STRICT = os.environ.get("ESPHOME_PCH_STRICT", "").strip().lower() in (
-    "1",
-    "true",
-    "yes",
-    "on",
-    "enable",
-)
+# Keep in sync with helpers.TRUTHY_ENV_STRINGS / FALSY_ENV_STRINGS
+_STRICT_RAW = os.environ.get("ESPHOME_PCH_STRICT")
+_STRICT_VALUE = (_STRICT_RAW or "").strip().lower()
+_STRICT = _STRICT_VALUE in ("1", "true", "yes", "on", "enable")
+if (
+    _STRICT_RAW is not None
+    and not _STRICT
+    and _STRICT_VALUE not in ("", "0", "false", "no", "off", "disable")
+):
+    # A typo must not silently turn the gate into a no-op
+    raise RuntimeError(f"Unrecognized ESPHOME_PCH_STRICT={_STRICT_RAW!r}; use 1 or 0")
 
 _INCLUDE_RE = re.compile(rb'^\s*#\s*include\s+["<]([^">]+)[">]', re.MULTILINE)
 _CORE_HEADER = "esphome/core/defines.h"
@@ -130,28 +133,9 @@ def _compile_gch(cxx, flags, header: Path, gch: Path, proj_dir: Path):
     return _probe_gch(cxx, flags, header, proj_dir)
 
 
-def _probe_gch(cxx, flags, header: Path, proj_dir: Path):
-    """Load-check an existing .gch; error string or None."""
-    # -MF is only legal alongside a dependency flag; pass it solely to
-    # redirect a depfile that -MD/-MMD in the flags would otherwise write
-    dep_redirect = (
-        ["-MF", os.devnull]
-        if any(f in ("-MD", "-MMD", "-M", "-MM") for f in flags)
-        else []
-    )
+def _probe_run(cxx, flags, extra, proj_dir: Path):
     probe = subprocess.run(  # noqa: PLW1510
-        [
-            cxx,
-            *flags,
-            *dep_redirect,
-            "-Winvalid-pch",
-            "-include",
-            str(header),
-            "-fsyntax-only",
-            "-x",
-            "c++",
-            "-",
-        ],
+        [cxx, *flags, *extra, "-fsyntax-only", "-x", "c++", "-"],
         cwd=proj_dir,
         env={**os.environ, "LC_ALL": "C"},
         input="",
@@ -160,9 +144,32 @@ def _probe_gch(cxx, flags, header: Path, proj_dir: Path):
     )
     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
+    return probe
+
+
+def _probe_gch(cxx, flags, header: Path, proj_dir: Path):
+    """Load-check an existing .gch; error string or None. Rejection must
+    be a nonzero exit (keep in sync with pch_probe_args); a baseline run
+    without the pch keeps environmental failures from being blamed on it."""
+    # -MF is only legal alongside a dependency flag; pass it solely to
+    # redirect a depfile that -MD/-MMD in the flags would otherwise write
+    dep_redirect = (
+        ["-MF", os.devnull]
+        if any(f in ("-MD", "-MMD", "-M", "-MM") for f in flags)
+        else []
+    )
+    probe = _probe_run(
+        cxx,
+        flags,
+        [*dep_redirect, "-Winvalid-pch", "-Werror=invalid-pch", "-include", str(header)],
+        proj_dir,
+    )
+    if probe.returncode == 0:
+        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]}")
+    return f"toolchain cannot load the pch: {probe.stderr.strip()}"
 
 
 def _read_stamp(path: Path) -> str:

tests/unit_tests/build_gen/test_espidf.py

@@ -1240,8 +1240,11 @@ def test_prepare_pch_probe_rejection_latches_and_degrades(
 
     def rejecting(cmd, **kwargs):
         if "-fsyntax-only" in cmd:
+            if "-include" not in cmd:
+                # Baseline without the pch passes: the pch is to blame
+                return subprocess.CompletedProcess(cmd, 0, "", "")
             return subprocess.CompletedProcess(
-                cmd, 0, "", "warning: esphome_pch.h.gch: had text segment "
+                cmd, 1, "", "error: esphome_pch.h.gch: had text segment "
             )
         gch.write_bytes(b"gch")
         return subprocess.CompletedProcess(cmd, 0, "", "")
@@ -1289,8 +1292,10 @@ def test_prepare_pch_strict_reprobes_cached_gch(
 
     def reject(cmd, **kwargs):
         assert "-fsyntax-only" in cmd, "cached path must not recompile"
+        if "-include" not in cmd:
+            return subprocess.CompletedProcess(cmd, 0, "", "")
         return subprocess.CompletedProcess(
-            cmd, 0, "", "warning: esphome_pch.h.gch: had text segment "
+            cmd, 1, "", "error: esphome_pch.h.gch: had text segment "
         )
 
     monkeypatch.setenv("ESPHOME_PCH_STRICT", "1")
@@ -1304,3 +1309,78 @@ def test_prepare_pch_strict_reprobes_cached_gch(
     # Per-process rejection may not reproduce: the cached path must not
     # latch the pch off for later non-strict builds
     assert not (dev / "build" / "esphome_pch.h.gch.failed").exists()
+
+
+def test_prepare_pch_unexpected_command_shape_degrades(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """A tail the probe slice cannot trust discards and degrades."""
+    from esphome.build_gen.espidf import prepare_pch
+    from esphome.core import EsphomeError
+
+    monkeypatch.setenv("ESPHOME_PCH_STRICT", "1")
+    dev = _make_pch_device(tmp_path, "dev_sh")
+    CORE.build_path = dev
+    gch = dev / "build" / "esphome_pch.h.gch"
+
+    def ok(cmd, **kwargs):
+        gch.write_bytes(b"gch")
+        return subprocess.CompletedProcess(cmd, 0, "", "")
+
+    with (
+        patch.object(CORE, "name", "test"),
+        patch("esphome.build_helpers.pch.subprocess.run", side_effect=ok),
+        patch(
+            "esphome.build_helpers.pch.pch_compile_command",
+            return_value=(
+                ["g++", "-DX=1", "-c", "x", "-o", "y", "extra"],
+                dev / "build",
+            ),
+        ),
+        pytest.raises(EsphomeError, match="command shape"),
+    ):
+        prepare_pch()
+    assert not gch.exists()
+
+    # Non-strict: same shape problem degrades without raising
+    monkeypatch.delenv("ESPHOME_PCH_STRICT")
+    with (
+        patch.object(CORE, "name", "test"),
+        patch("esphome.build_helpers.pch.subprocess.run", side_effect=ok),
+        patch(
+            "esphome.build_helpers.pch.pch_compile_command",
+            return_value=(
+                ["g++", "-DX=1", "-c", "x", "-o", "y", "extra"],
+                dev / "build",
+            ),
+        ),
+    ):
+        prepare_pch()
+    assert not gch.exists()
+
+
+def test_prepare_pch_probe_baseline_spawn_failure_is_transient(
+    tmp_path: Path,
+) -> None:
+    """A baseline that cannot spawn is environmental: no marker."""
+    from esphome.build_gen.espidf import prepare_pch
+
+    dev = _make_pch_device(tmp_path, "dev_bs")
+    CORE.build_path = dev
+    gch = dev / "build" / "esphome_pch.h.gch"
+
+    def flaky(cmd, **kwargs):
+        if "-fsyntax-only" in cmd:
+            if "-include" in cmd:
+                return subprocess.CompletedProcess(cmd, 1, "", "boom")
+            raise OSError("baseline spawn failed")
+        gch.write_bytes(b"gch")
+        return subprocess.CompletedProcess(cmd, 0, "", "")
+
+    with (
+        patch.object(CORE, "name", "test"),
+        patch("esphome.build_helpers.pch.subprocess.run", side_effect=flaky),
+    ):
+        prepare_pch()
+    assert not gch.exists()
+    assert not (dev / "build" / "esphome_pch.h.gch.failed").exists()

tests/unit_tests/build_helpers/test_pch.py

@@ -253,3 +253,14 @@ def test_pch_extra_scripts_strict_raises_when_disabled(
     monkeypatch.setenv("ESPHOME_PCH_STRICT", "1")
     with pytest.raises(EsphomeError, match="disabled"):
         pch.pch_extra_scripts()
+
+
+def test_pch_strict_rejects_unrecognized_values(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """A typo must not silently disable the gate."""
+    from esphome.core import EsphomeError
+
+    monkeypatch.setenv("ESPHOME_PCH_STRICT", "yolo")
+    with pytest.raises(EsphomeError, match="Unrecognized ESPHOME_PCH_STRICT"):
+        pch.pch_strict()

tests/unit_tests/test_platformio_pch_script.py

@@ -100,12 +100,14 @@ def _fake_cxx(
         body += f"echo {fail_msg or 'boom'} >&2\nexit 1\n"
     else:
         # Only the c++-header compile has a -o; the load probe has none
-        body += 'out=""; prev=""; mf=0; dep=0\nfor a in "$@"; do [ "$prev" = "-o" ] && out="$a"; prev="$a"; [ "$a" = "-MF" ] && mf=1; case "$a" in -M|-MM|-MD|-MMD) dep=1;; esac; done\n'
+        body += 'out=""; prev=""; mf=0; dep=0; inc=0\nfor a in "$@"; do [ "$prev" = "-o" ] && out="$a"; prev="$a"; [ "$a" = "-MF" ] && mf=1; [ "$a" = "-include" ] && inc=1; case "$a" in -M|-MM|-MD|-MMD) dep=1;; esac; done\n'
         # Real cc1plus rejects -MF without a dependency flag
         body += 'if [ "$mf" = 1 ] && [ "$dep" = 0 ]; then echo "cc1plus: error: to generate dependencies you must specify either \x27-M\x27 or \x27-MM\x27" >&2; exit 1; fi\n'
         body += '[ -n "$out" ] && echo gch > "$out"\n'
         if reject_pch:
-            body += 'case " $* " in *c++-header*) ;; *) echo "warning: esphome_pch.h.gch: had text segment at different address" >&2;; esac\n'
+            # -Werror=invalid-pch makes rejection a nonzero exit; the
+            # baseline (no -include) still passes
+            body += 'case " $* " in *c++-header*) ;; *) if [ "$inc" = 1 ]; then echo "error: esphome_pch.h.gch: had text segment at different address" >&2; exit 1; fi;; esac\n'
         body += f'case " $* " in *c++-header*) exit 0;; *) exit {probe_exit};; esac\n'
     cxx.write_text("#!/bin/sh\n" + body)
     cxx.chmod(cxx.stat().st_mode | stat.S_IEXEC)
@@ -293,12 +295,14 @@ def test_pch_script_spawn_failure_is_transient(
     assert "did not run" in capsys.readouterr().out
 
 
-def test_pch_script_probe_nonzero_exit_falls_back(tmp_path: Path) -> None:
-    """A probe failure whose stderr never mentions .gch must still count."""
+def test_pch_script_probe_environment_failure_does_not_latch(
+    tmp_path: Path,
+) -> None:
+    """Probe AND baseline failing is environmental: no marker, retry."""
     scons_env = _run_script(tmp_path, probe_exit=1)
     proj = tmp_path / "dev"
     assert not (proj / "esphome_pch.h.gch").exists()
-    assert (proj / "esphome_pch.h.gch.failed").is_file()
+    assert not (proj / "esphome_pch.h.gch.failed").exists()
     assert scons_env.prepended == []