From 84028e787a3018d66242e3c8e6bb702e2191dbf6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 25 Aug 2026 19:23:49 -0500 Subject: [PATCH] Log the pch line only when true, fail closed on unreadable sdkconfig, treat signal-killed compiles as transient --- esphome/build_gen/espidf.py | 10 +++-- esphome/build_helpers/pch.py | 22 +++++++++-- tests/unit_tests/build_gen/test_espidf.py | 48 ++++++++++++++++++++++- 3 files changed, 72 insertions(+), 8 deletions(-) diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index ba6daed1ec..3a8e6ac6b0 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -329,12 +329,14 @@ def prepare_pch() -> None: try: sdkconfig = sdkconfig_path.read_text(encoding="utf-8") except OSError as err: - # Path-independent marker: str(err) embeds the per-device path and - # would defeat cross-device .sum sharing + # Fail closed: the sdkconfig is the only config-awareness the .sum + # has for options that surface via sdkconfig.h, and any stand-in + # marker would collide across devices _LOGGER.warning( - "Could not read %s for the pch checksum: %s", sdkconfig_path, err + "Could not read %s; compiling without the pch: %s", sdkconfig_path, err ) - sdkconfig = f"unreadable:{type(err).__name__}:{err.errno}" + pch.discard_pch(CORE.relative_build_path("build")) + return pch.prepare_pch( CORE.relative_build_path("build"), PCH_DEFAULT_HEADERS, diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index e397ad7a3c..12539ad2d1 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -233,6 +233,14 @@ def pch_compile_command( return [*args, "-x", "c++-header", "-c", str(header), "-o", str(gch)], cmd_dir +def _log_pch_in_use() -> None: + # The only place a user can discover the knob; emitted only once a + # .gch is actually fresh or being built + _LOGGER.info( + "Compiling with a precompiled header (set ESPHOME_PCH_ENABLE=0 to disable)" + ) + + def discard_pch(build_dir: Path) -> None: """Remove the pch sidecars so a stale .gch is never consumed. @@ -262,9 +270,6 @@ def prepare_pch( """ from esphome.core import CORE - _LOGGER.info( - "Compiling with a precompiled header (set ESPHOME_PCH_ENABLE=0 to disable)" - ) header = build_dir / PCH_HEADER_NAME gch = Path(f"{header}.gch") sum_path = Path(f"{gch}.sum") @@ -297,6 +302,7 @@ def prepare_pch( and sum_path.is_file() and sum_path.read_text(encoding="utf-8").strip() == checksum ): + _log_pch_in_use() return failed_marker = Path(f"{gch}.failed") if ( @@ -308,11 +314,21 @@ def prepare_pch( failed_marker, ) return + _log_pch_in_use() try: result = subprocess.run( cmd, cwd=cmd_dir, capture_output=True, text=True, check=False, timeout=300 ) error = None + if result.returncode < 0: + # Killed by a signal (OOM, ^C): environmental, do not latch + _LOGGER.warning( + "Precompiled header compile was killed (signal %d); retrying " + "next build", + -result.returncode, + ) + discard_pch(build_dir) + return if result.returncode != 0: error = result.stderr.strip() or f"exit code {result.returncode}" elif not gch.is_file(): diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index d8d3725804..62fee5f8ad 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/tests/unit_tests/build_gen/test_espidf.py @@ -513,7 +513,9 @@ def _make_pch_device(tmp_path: Path, name: str) -> Path: '#include "esphome/core/macros.h"\n' ) (dev / "src" / "esphome" / "core" / "macros.h").write_text("#define M 1\n") + # Both spellings: tests patch CORE.name to "test" or to the device name (dev / f"sdkconfig.{name}").write_text("CONFIG_X=y\n") + (dev / "sdkconfig.test").write_text("CONFIG_X=y\n") build = dev / "build" build.mkdir(exist_ok=True) from esphome.build_helpers.pch import pch_header_text @@ -782,16 +784,60 @@ def test_prepare_pch_transient_with_stale_gch_bumps_header(tmp_path: Path) -> No assert header.stat().st_mtime_ns > 1_000_000_000 -def test_prepare_pch_disabled_is_noop( +def test_prepare_pch_disabled_discards_and_skips_compile( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + """The escape hatch is self-cleaning: a leftover .gch is removed.""" from esphome.build_gen.espidf import prepare_pch monkeypatch.setenv("ESPHOME_PCH_ENABLE", "0") dev = _make_pch_device(tmp_path, "dev_d") CORE.build_path = dev + stale = dev / "build" / "esphome_pch.h.gch" + stale.write_bytes(b"stale") with patch("esphome.build_helpers.pch.subprocess.run", side_effect=AssertionError): prepare_pch() + assert not stale.exists() + + +def test_prepare_pch_missing_sdkconfig_fails_closed(tmp_path: Path) -> None: + """No sdkconfig means no config identity for the .sum: no pch at all.""" + from esphome.build_gen.espidf import prepare_pch + + dev = _make_pch_device(tmp_path, "dev_m") + (dev / "sdkconfig.test").unlink() + CORE.build_path = dev + stale = dev / "build" / "esphome_pch.h.gch" + stale.write_bytes(b"stale") + with ( + patch.object(CORE, "name", "test"), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=AssertionError), + ): + prepare_pch() + assert not stale.exists() + assert not (dev / "build" / "esphome_pch.h.gch.sum").exists() + + +def test_prepare_pch_signal_kill_is_transient(tmp_path: Path) -> None: + """A signal-killed compile (OOM) must not latch the .failed marker.""" + from esphome.build_gen.espidf import prepare_pch + + dev = _make_pch_device(tmp_path, "dev_k") + CORE.build_path = dev + calls = [] + + def killed(cmd, **kwargs): + calls.append(cmd) + return subprocess.CompletedProcess(cmd, -9, "", "") + + with ( + patch.object(CORE, "name", "test"), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=killed), + ): + prepare_pch() + prepare_pch() + assert not (dev / "build" / "esphome_pch.h.gch.failed").exists() + assert len(calls) == 2 def test_prepare_pch_without_compile_commands(tmp_path: Path) -> None: