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

esphome/build_gen/arduino8266.py

@@ -39,6 +39,7 @@ from esphome.build_helpers.pch import (
     PCH_HEADER_NAME,
     mark_pch_emitted,
     pch_checksum,
+    pch_consumer_escalation,
     pch_degraded,
     pch_disabled_degraded,
     pch_enabled,
@@ -1292,9 +1293,7 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool:
             # strict inverts it so any consumer rejection reds the build
             # (rejection is per-process, so the probe alone cannot prove
             # the consumers)
-            escalation = (
-                "-Werror=invalid-pch" if pch_strict() else "-Wno-error=invalid-pch"
-            )
+            escalation = pch_consumer_escalation()
             cxx_parts = src_other + [
                 f"-Winvalid-pch {escalation} -include {PCH_HEADER_NAME}"
             ]
@@ -1303,9 +1302,7 @@ 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, source=str(Path(os.devnull)))
-                )
+                probe = " ".join(pch_probe_args(PCH_HEADER_NAME, source=os.devnull))
                 lines.append("rule pchprobe")
                 # $out only expands in rule text, hence the inline stamp
                 lines.append(

esphome/build_gen/espidf.py

@@ -301,7 +301,7 @@ def _pch_cmake() -> str:
     # 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"
+    escalation = pch.pch_consumer_escalation()
     return f"""
 # ESPHome precompiled header (see esphome/build_helpers/pch.py).
 # OBJECT_DEPENDS is on the header, not the .gch: pch-baked headers drop

esphome/build_helpers/ccache.py

@@ -21,12 +21,13 @@ def _ccache_runs(ccache: str) -> bool:
     )
 
 
-def parse_enable_env(name: str) -> bool | None:
+def parse_enable_env(name: str, strict: bool = False) -> bool | None:
     """Strictly parse an on/off environment knob; None when unset or invalid.
 
     ``bool(str)`` truthiness would flip ``no``/``off`` to enabled, so only
     1/true/yes/on and 0/false/no/off count; anything else warns and reads
-    as unset so the caller's default policy applies.
+    as unset so the caller's default policy applies — or raises when
+    ``strict`` (a typo must not silently disable a CI gate).
     """
     raw = os.environ.get(name)
     if raw is None:
@@ -39,6 +40,10 @@ def parse_enable_env(name: str) -> bool | None:
         return True
     if lowered in FALSY_ENV_STRINGS:
         return False
+    if strict:
+        from esphome.core import EsphomeError
+
+        raise EsphomeError(f"Unrecognized {name}={raw!r}; use 1 or 0")
     _LOGGER.warning("Ignoring unrecognized %s=%r; use 1 or 0", name, raw)
     return None
 

esphome/build_helpers/pch.py

@@ -108,15 +108,7 @@ def pch_strict() -> bool:
     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
+    return parse_enable_env("ESPHOME_PCH_STRICT", strict=True) is True
 
 
 def pch_degraded(reason: str) -> None:
@@ -132,6 +124,11 @@ def pch_disabled_degraded() -> None:
     pch_degraded("pch disabled by ESPHOME_PCH_ENABLE")
 
 
+def pch_probe_tail(source: str = "-") -> list[str]:
+    """The syntax-only compile shared by the probe and its baseline."""
+    return ["-fsyntax-only", "-x", "c++", source]
+
+
 def pch_probe_args(header: str, source: str = "-") -> list[str]:
     """Flags that load-check a built .gch via a syntax-only compile.
 
@@ -144,13 +141,16 @@ def pch_probe_args(header: str, source: str = "-") -> list[str]:
         "-Werror=invalid-pch",
         "-include",
         header,
-        "-fsyntax-only",
-        "-x",
-        "c++",
-        source,
+        *pch_probe_tail(source),
     ]
 
 
+def pch_consumer_escalation() -> str:
+    """Consumer-side invalid-pch flag: strict reds the build on rejection
+    (per-process, so the probe alone cannot prove the consumers)."""
+    return "-Werror=invalid-pch" if pch_strict() else "-Wno-error=invalid-pch"
+
+
 def ccache_pch_env() -> dict[str, str]:
     """Settings ccache needs to cache compiles that consume the .gch;
     empty unless this build actually emitted one. User-set values win.
@@ -354,13 +354,24 @@ def discard_pch(build_dir: Path) -> None:
 
     Bumps the header only when a .gch was actually removed: TUs compiled
     against it have incomplete depfiles, while a repeat failure with no
-    .gch must not force a full rebuild every build.
+    .gch must not force a full rebuild every build. A .gch that survives
+    an unlink failure would be consumed silently (wrong output, not a
+    slow build), so that raises.
     """
     header = build_dir / PCH_HEADER_NAME
     gch = Path(f"{header}.gch")
     had_gch = gch.is_file()
-    gch.unlink(missing_ok=True)
-    Path(f"{gch}.sum").unlink(missing_ok=True)
+    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
+
+            raise EsphomeError(
+                f"Could not discard the stale precompiled header: {err}"
+            ) from err
+        _LOGGER.warning("Could not discard the pch sidecars: %s", err)
     if had_gch and header.is_file():
         os.utime(header)
 
@@ -451,21 +462,18 @@ def prepare_pch(
             return None
         return proc
 
-    def _fail(error: str, reason: str) -> None:
-        """Discard and degrade; deterministic failures also latch."""
+    def _fail(error: str, reason: str, latch: bool) -> None:
+        """Discard and degrade; deterministic failures latch when asked."""
         _LOGGER.warning(
             "Precompiled header failed; compiling without it: %s", error[:400]
         )
         # Latching paths keep the full compiler output recoverable
         _LOGGER.debug("Full pch output: %s", error)
         discard_pch(build_dir)
-        if any(m in error for m in _TRANSIENT_ERRORS):
-            # Resource exhaustion clears on its own; retry next build
-            pch_degraded(f"transient {reason}: {error[:200]}")
-            return
-        # Skip retries until a header/flag/backend-identity/command change
-        failed_marker.write_text(checksum + "\n", encoding="utf-8")
-        os.utime(header)
+        if latch and not any(m in error for m in _TRANSIENT_ERRORS):
+            # Skip retries until a header/flag/backend-identity/command change
+            failed_marker.write_text(checksum + "\n", encoding="utf-8")
+            os.utime(header)
         pch_degraded(f"{reason}: {error[:200]}")
 
     def _probe(latch: bool = True) -> None:
@@ -486,23 +494,16 @@ def prepare_pch(
             return
         if probe.returncode != 0:
             error = probe.stderr.strip() or f"exit code {probe.returncode}"
-            # 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=""
-            )
+            # Disambiguate: only blame (and latch on) the pch when the same
+            # compile passes without it; anything else is environmental
+            baseline = _run([*base, *pch_probe_tail()], "probe baseline", stdin="")
             if baseline is None:
                 return
-            if latch and baseline.returncode == 0:
-                _fail(error, "toolchain cannot load the pch")
-            else:
-                _LOGGER.warning(
-                    "Precompiled header failed; compiling without it: %s",
-                    error[:400],
-                )
-                discard_pch(build_dir)
-                pch_degraded(f"toolchain cannot load the pch: {error[:200]}")
+            _fail(
+                error,
+                "toolchain cannot load the pch",
+                latch=latch and baseline.returncode == 0,
+            )
 
     if gch.is_file() and _read_stamp(sum_path) == checksum:
         _log_pch_in_use()
@@ -529,7 +530,7 @@ def prepare_pch(
     elif not gch.is_file():
         error = "compiler produced no .gch"
     if error is not None:
-        _fail(error, "compile failed")
+        _fail(error, "compile failed", latch=True)
         return
     _probe()
     if not gch.is_file():

esphome/espidf/toolchain.py

@@ -535,21 +535,10 @@ def run_compile(config, verbose: bool) -> int:
     try:
         prepare_pch()
     except Exception:  # noqa: BLE001  # pylint: disable=broad-exception-caught
-        # Discard so a stale .gch can never be consumed
-        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
 
+        # Raises itself if a stale .gch survives (silently wrong output)
+        discard_pch()
         if pch_strict():
             raise
         _LOGGER.warning(

esphome/platformio/pch.py.script

@@ -27,14 +27,12 @@ except Exception as err:  # noqa: BLE001 -- not exported under -t nobuild
 _TRANSIENT_ERRORS = ("No space left", "Cannot allocate", "Resource temporarily")
 
 # Keep in sync with helpers.TRUTHY_ENV_STRINGS / FALSY_ENV_STRINGS
+_TRUTHY = ("1", "true", "yes", "on", "enable")
+_FALSY = ("", "0", "false", "no", "off", "disable")
 _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")
-):
+_STRICT = _STRICT_VALUE in _TRUTHY
+if _STRICT_RAW is not None and _STRICT_VALUE not in _TRUTHY + _FALSY:
     # 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")
 
@@ -186,9 +184,9 @@ def _setup_pch() -> bool | None:
         try:
             from SCons.Script import COMMAND_LINE_TARGETS
         except ImportError:
-            # No SCons is an anomaly under PlatformIO: strict must not
-            # read the unknown state as success
-            return not _STRICT
+            # No SCons is an anomaly under PlatformIO: the unknown state
+            # must not read as success (strict decides fatality at the gate)
+            return False
         # Expected under -t nobuild (nothing compiles); a missing
         # projenv on a real compile must not pass strict
         return "nobuild" in [str(t) for t in COMMAND_LINE_TARGETS]

tests/unit_tests/build_helpers/test_pch.py

@@ -264,3 +264,48 @@ def test_pch_strict_rejects_unrecognized_values(
     monkeypatch.setenv("ESPHOME_PCH_STRICT", "yolo")
     with pytest.raises(EsphomeError, match="Unrecognized ESPHOME_PCH_STRICT"):
         pch.pch_strict()
+
+
+def test_discard_pch_raises_when_gch_survives(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """A .gch an unlink failure leaves behind would be consumed silently."""
+    from pathlib import Path as _P
+
+    from esphome.core import EsphomeError
+
+    (tmp_path / "esphome_pch.h").write_text("")
+    gch = tmp_path / "esphome_pch.h.gch"
+    gch.write_bytes(b"gch")
+    real_unlink = _P.unlink
+
+    def failing_unlink(self, missing_ok=False):
+        if self.name.endswith(".gch"):
+            raise OSError("readonly")
+        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_only_sidecar_unlink_fails(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    from pathlib import Path as _P
+
+    (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")
+    real_unlink = _P.unlink
+
+    def failing_unlink(self, missing_ok=False):
+        if self.name.endswith(".sum"):
+            raise OSError("readonly")
+        return real_unlink(self, missing_ok=missing_ok)
+
+    monkeypatch.setattr(_P, "unlink", failing_unlink)
+    pch.discard_pch(tmp_path)
+    assert "Could not discard the pch sidecars" in caplog.text

tests/unit_tests/test_espidf_toolchain.py

@@ -669,12 +669,12 @@ def test_get_core_framework_version_from_core_data():
     assert toolchain._get_core_framework_version() == "5.5.4"
 
 
-def test_run_compile_logs_failed_pch_discard(
-    setup_core: Path,
-    monkeypatch: pytest.MonkeyPatch,
-    caplog: pytest.LogCaptureFixture,
+def test_run_compile_aborts_when_stale_pch_survives_discard(
+    setup_core: Path, monkeypatch: pytest.MonkeyPatch
 ) -> None:
-    """A discard failure in the catch-all must be visible, not silent."""
+    """An undiscardable stale .gch means silently wrong output: abort."""
+    from esphome.core import EsphomeError
+
     monkeypatch.setenv("ESPHOME_PCH_ENABLE", "1")
     _setup_build(setup_core)
 
@@ -683,10 +683,13 @@ def test_run_compile_logs_failed_pch_discard(
         patch.object(toolchain, "run_idf_py", return_value=0),
         patch.object(toolchain, "print_summary"),
         patch("esphome.build_gen.espidf.prepare_pch", side_effect=RuntimeError("boom")),
-        patch("esphome.build_gen.espidf.discard_pch", side_effect=OSError("readonly")),
+        patch(
+            "esphome.build_gen.espidf.discard_pch",
+            side_effect=EsphomeError("Could not discard the stale precompiled header"),
+        ),
+        pytest.raises(EsphomeError, match="Could not discard"),
     ):
-        assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) == 0
-    assert "Could not discard the stale pch" in caplog.text
+        toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False)
 
 
 def test_run_compile_strict_reraises_pch_failure(