From b6c1552bcb17f9444fc089baae777f8c6f6369b2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 25 Aug 2026 17:56:23 -0500 Subject: [PATCH 1/2] Simplify: shared artifact names, one cxx override param, extracted gch compile, transient spawn semantics in the script --- esphome/build_gen/arduino8266.py | 27 +++---- esphome/build_helpers/ccache.py | 3 +- esphome/build_helpers/idedata.py | 10 +-- esphome/build_helpers/pch.py | 8 ++ esphome/platformio/pch.py.script | 80 ++++++++++--------- esphome/writer.py | 13 +-- .../unit_tests/test_platformio_pch_script.py | 19 ++++- 7 files changed, 91 insertions(+), 69 deletions(-) diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 7870fd7d68..1517f29f79 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -882,13 +882,12 @@ def _ninja_compile_edges( root: Path, group: str, flags: str = "", - cxx_flags: str = "", - cxx_implicit: str = "", + cxx_override: tuple[str, str] | None = None, ) -> list[str]: """Emit compile edges for ``sources``; return the object paths. - ``cxx_flags``/``cxx_implicit`` override ``flags`` and add an implicit - dependency on C++ edges only (used for the precompiled header). + ``cxx_override`` is a (flags, implicit-dep) pair applied to C++ edges + only, replacing ``flags`` (used for the precompiled header). """ objects = [] for src in sources: @@ -896,10 +895,10 @@ def _ninja_compile_edges( obj = f"obj/{group}/{rel}.o" escaped_obj = _e(obj) kind = SOURCE_KIND_FOR_SUFFIX[src.suffix] - is_cxx = kind == "cxx" - implicit = f" | {cxx_implicit}" if is_cxx and cxx_implicit else "" + override = cxx_override if kind == "cxx" and cxx_override else None + implicit = f" | {override[1]}" if override else "" lines.append(f"build {escaped_obj}: {kind} {_e(src)}{implicit}") - edge_flags = cxx_flags if is_cxx and cxx_flags else flags + edge_flags = override[0] if override else flags if edge_flags: lines.append(f" flags = {edge_flags}") # Escaped once here: the returned paths only ever appear in build @@ -1217,8 +1216,7 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: # One shared variable instead of repeating the flags line on every src # edge (hundreds of edges in a real project) lines.append(f"srcflags = {' '.join(src_other + include_flags)}") - src_cxx_flags = "" - src_cxx_implicit = "" + src_cxx_override = None if pch_enabled(): # C++ src edges swap the force-includes for one precompiled prefix # header holding the same content plus defines.h; C and assembly @@ -1230,7 +1228,8 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: ) pch_header = build_dir / PCH_HEADER_NAME pch_includes = (*src_includes, PCH_CORE_HEADER) - write_file_if_changed(pch_header, pch_header_text(pch_includes)) + pch_text = pch_header_text(pch_includes) + write_file_if_changed(pch_header, pch_text) if ccache: # The .sum sidecar only exists for CCACHE_PCH_EXTSUM; ninja's # depfile handles staleness. Mirror CCACHE_BASEDIR: strip the @@ -1243,7 +1242,7 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: src_dir, pch_includes, ( - pch_header_text(pch_includes), + pch_text, str(paths.framework), str(paths.toolchain), flags_id, @@ -1261,16 +1260,14 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: # every compile command and defeat cross-device ccache sharing cxx_parts = src_other + [f"-Winvalid-pch -include {PCH_HEADER_NAME}"] lines.append(f"srccxxflags = {' '.join(cxx_parts)}") - src_cxx_flags = "$srccxxflags" - src_cxx_implicit = gch + src_cxx_override = ("$srccxxflags", gch) src_objs = _ninja_compile_edges( lines, _collect_sources(src_dir), src_dir, "src", flags="$srcflags", - cxx_flags=src_cxx_flags, - cxx_implicit=src_cxx_implicit, + cxx_override=src_cxx_override, ) ld_deps = [f"ld/{_COMMON_LD_NAME}"] diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index cee0774561..7daad458dd 100644 --- a/esphome/build_helpers/ccache.py +++ b/esphome/build_helpers/ccache.py @@ -87,7 +87,8 @@ def ccache_defaults_env(cache_dir: Path) -> dict[str, str]: "CCACHE_DIR": str(cache_dir), "CCACHE_NOHASHDIR": "true", "CCACHE_DEPEND": "1", - "CCACHE_BASEDIR": str(Path(CORE.build_path).resolve()), + # A user value wins via the filter below + "CCACHE_BASEDIR": effective_ccache_basedir(), } return {k: v for k, v in defaults.items() if k not in os.environ} diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 29f70b835c..76319bd681 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -59,11 +59,11 @@ def warn_if_idedata_missing(get_idedata: Callable[[], dict | None]) -> None: _LOGGER.warning("Idedata failure detail", exc_info=True) -# C++ translation-unit suffixes used to identify ESPHome source files. -_CXX_SUFFIXES = (".cpp", ".cc") +# C++ translation-unit suffixes, shared with the pch backends. +CXX_SOURCE_SUFFIXES = (".cpp", ".cc", ".cxx") # Suffixes of input/output files that appear bare on the command line (and so # must not be mistaken for compiler flags). -_INPUT_FILE_SUFFIXES = (*_CXX_SUFFIXES, ".c", ".o", ".S", ".s") +_INPUT_FILE_SUFFIXES = (*CXX_SOURCE_SUFFIXES, ".c", ".o", ".S", ".s") # Path marker identifying an ESPHome source translation unit. _ESPHOME_SRC_MARKER = "/src/esphome/" @@ -72,7 +72,7 @@ def _is_esphome_src(file: str) -> bool: """Whether ``file`` is an ESPHome C++ translation unit; normalized to ``/`` first since Windows compile DBs use backslashes.""" return _ESPHOME_SRC_MARKER in file.replace("\\", "/") and file.endswith( - _CXX_SUFFIXES + CXX_SOURCE_SUFFIXES ) @@ -147,7 +147,7 @@ def _pick_entry(entries: list[dict]) -> dict: if _is_esphome_src(entry["file"]): return entry for entry in entries: - if entry["file"].endswith(_CXX_SUFFIXES): + if entry["file"].endswith(CXX_SOURCE_SUFFIXES): return entry raise ValueError("no C++ translation unit found in compile_commands.json") diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index 0b691c06a4..4c8d0936b9 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -24,6 +24,14 @@ _LOGGER = logging.getLogger(__name__) # The header and its .gch/.sum sidecars live in the build directory. PCH_HEADER_NAME = "esphome_pch.h" +# Every artifact the pch machinery can leave behind, for cleanup. +PCH_ARTIFACT_NAMES = ( + PCH_HEADER_NAME, + f"{PCH_HEADER_NAME}.gch", + f"{PCH_HEADER_NAME}.gch.sum", + f"{PCH_HEADER_NAME}.gch.failed", +) + # The core defines header every backend anchors its prefix on. PCH_CORE_HEADER = "esphome/core/defines.h" diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 45bb9ddb05..310a64c25c 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -60,6 +60,43 @@ def _shell_arg(element) -> str: return shlex.split(arg)[0] if arg.strip() else arg +def _compile_gch(cxx, flags, header: Path, gch: Path, proj_dir: Path): + """Compile the .gch, then probe that the toolchain can load it back + (GCC 10 on macOS arm64 builds one it then rejects per-process: "had + text segment at different address"). Returns a deterministic error + string or None; OSError propagates for transient handling.""" + result = subprocess.run( # noqa: PLW1510 + [cxx, "-x", "c++-header", *flags, "-c", str(header), "-o", str(gch)], + cwd=proj_dir, + capture_output=True, + text=True, + ) + if result.returncode != 0: + return result.stderr + probe = subprocess.run( # noqa: PLW1510 + [ + cxx, + *flags, + "-MF", + os.devnull, + "-Winvalid-pch", + "-include", + str(header), + "-fsyntax-only", + "-x", + "c++", + "-", + ], + cwd=proj_dir, + input="", + capture_output=True, + text=True, + ) + if probe.returncode != 0 or ".gch" in probe.stderr: + return f"toolchain cannot load the pch: {probe.stderr.strip()}" + return None + + def _setup_pch() -> None: # Project root, not $BUILD_DIR: SCons compiles run with the project dir # as cwd, so "-include esphome_pch.h" resolves here as a relative path. @@ -180,44 +217,13 @@ def _setup_pch() -> None: return header.write_text(content, encoding="utf-8") try: - result = subprocess.run( # noqa: PLW1510 - [cxx, "-x", "c++-header", *flags, "-c", str(header), "-o", str(gch)], - cwd=proj_dir, - capture_output=True, - text=True, - ) - error = result.stderr if result.returncode != 0 else None + error = _compile_gch(cxx, flags, header, gch, proj_dir) except OSError as err: - error = str(err) - if error is None: - # Some toolchains build a .gch they cannot load back (GCC 10 on - # macOS arm64 rejects it per-process: "had text segment at - # different address"); probe once so consumers never pay for a - # pch that every compile would silently reject - try: - probe = subprocess.run( # noqa: PLW1510 - [ - cxx, - *flags, - "-MF", - os.devnull, - "-Winvalid-pch", - "-include", - str(header), - "-fsyntax-only", - "-x", - "c++", - "-", - ], - cwd=proj_dir, - input="", - capture_output=True, - text=True, - ) - if probe.returncode != 0 or ".gch" in probe.stderr: - error = f"toolchain cannot load the pch: {probe.stderr.strip()}" - except OSError as err: - error = str(err) + # Transient spawn/IO failure: no marker, retry next build + print(f"ESPHome: precompiled header compile did not run: {err}") + gch.unlink(missing_ok=True) + sum_path.unlink(missing_ok=True) + return if error is not None: print("ESPHome: precompiled header failed; compiling without it") print(error) diff --git a/esphome/writer.py b/esphome/writer.py index d2b16ffcc0..a6b89c4108 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -7,7 +7,7 @@ import re import time from esphome import loader -from esphome.build_helpers.pch import PCH_HEADER_NAME +from esphome.build_helpers.pch import PCH_ARTIFACT_NAMES from esphome.compiled_config import save_compiled_config from esphome.config import iter_component_configs, iter_components from esphome.const import ( @@ -612,15 +612,8 @@ def clean_build(clear_pio_cache: bool = True, *, full: bool = False): rmtree(idf_path) # The PlatformIO pch artifacts live at the project root so the # relative -include resolves; a partial clean must drop them too - for name in ( - PCH_HEADER_NAME, - f"{PCH_HEADER_NAME}.gch", - f"{PCH_HEADER_NAME}.gch.sum", - f"{PCH_HEADER_NAME}.gch.failed", - ): - pch_path = CORE.relative_build_path(name) - if pch_path.is_file(): - pch_path.unlink() + for name in PCH_ARTIFACT_NAMES: + CORE.relative_build_path(name).unlink(missing_ok=True) # The idedata caches are derived from the build but live under the data # dir, not the build path, so they must be removed separately in both diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py index 0f879eef47..9747dddb39 100644 --- a/tests/unit_tests/test_platformio_pch_script.py +++ b/tests/unit_tests/test_platformio_pch_script.py @@ -115,6 +115,7 @@ def _run_script( fail: bool = False, reject_pch: bool = False, probe_exit: int = 0, + missing_cxx: bool = False, env_vars: dict[str, str] | None = None, name: str = "dev", platform_cls: type[_FakePlatform] = _FakePlatform, @@ -124,6 +125,8 @@ def _run_script( (src / "esphome" / "core").mkdir(parents=True, exist_ok=True) (src / "esphome" / "core" / "defines.h").write_text("#define USE_X\n") cxx = _fake_cxx(tmp_path, fail=fail, reject_pch=reject_pch, probe_exit=probe_exit) + if missing_cxx: + cxx = tmp_path / "no-such-gxx" args = (proj, src, str(cxx), flags or ["-DX=1"], platform_cls) # Distinct objects: the script must scope ccache/flags to projenv only global_env = _FakeSConsEnv(*args) @@ -219,6 +222,18 @@ def test_pch_script_probe_rejection_falls_back( assert "toolchain cannot load the pch" in capsys.readouterr().out +def test_pch_script_spawn_failure_is_transient( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A spawn failure must not latch a .failed marker (matches espidf).""" + scons_env = _run_script(tmp_path, missing_cxx=True) + proj = tmp_path / "dev" + assert not (proj / "esphome_pch.h.gch.failed").exists() + assert not (proj / "esphome_pch.h.gch.sum").exists() + assert scons_env.prepended == [] + 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.""" scons_env = _run_script(tmp_path, probe_exit=1) @@ -276,7 +291,9 @@ def test_pch_script_hashes_project_local_include_dirs(tmp_path: Path) -> None: assert (proj / "esphome_pch.h.gch.sum").read_text() != first -@pytest.mark.skipif(os.geteuid() == 0, reason="root ignores file modes") +@pytest.mark.skipif( + getattr(os, "geteuid", lambda: -1)() == 0, reason="root ignores file modes" +) def test_pch_script_unreadable_local_header_warns_and_varies( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: From 2a41892559a39c78c1d8fb70afb9165a355c1fa9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 25 Aug 2026 17:58:05 -0500 Subject: [PATCH 2/2] Simplify: reuse discard_pch, effective_ccache_basedir, and idedata's C++ suffixes --- esphome/build_gen/espidf.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 92f227731b..639148b46b 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -6,7 +6,9 @@ import os from pathlib import Path import subprocess +from esphome.build_helpers.ccache import effective_ccache_basedir from esphome.build_helpers.idedata import ( + CXX_SOURCE_SUFFIXES, expand_response_files, is_launcher, split_command, @@ -61,7 +63,6 @@ _PCH_BUILD_HEADER = f"build/{PCH_HEADER_NAME}" # argument-less depfile flags (the pch compile must not touch depfiles) _PCH_STRIP_FLAGS_WITH_ARG = frozenset({"-o", "-c", "-MT", "-MF", "-MQ"}) _PCH_STRIP_FLAGS = frozenset({"-MD", "-MMD", "-MP", "-MM", "-M"}) -_CXX_SOURCE_SUFFIXES = (".cpp", ".cc", ".cxx") # Replaces the IDF default C++ standard (-std=gnu++2b appended to # CXX_COMPILE_OPTIONS by project.cmake's __build_init) with the one set via @@ -369,7 +370,7 @@ def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str] for e in entries if isinstance(e, dict) and e.get("file", "").replace("\\", "/").startswith(src_prefix) - and e.get("file", "").endswith(_CXX_SOURCE_SUFFIXES) + and e.get("file", "").endswith(CXX_SOURCE_SUFFIXES) ), None, ) @@ -453,10 +454,12 @@ def prepare_pch() -> None: "Could not read %s for the pch checksum: %s", sdkconfig_path, err ) sdkconfig = f"unreadable:{type(err).__name__}:{err.errno}" - # Build-path stripped so identical configs hash identically across devices + # Stripped like ccache's own rewriting (a user CCACHE_BASEDIR wins) so + # identical configs hash identically across devices; the raw build path + # covers unresolved spellings in the compile DB cmd_id = ( " ".join(cmd) - .replace(str(Path(CORE.build_path).resolve()), "") + .replace(effective_ccache_basedir(), "") .replace(str(CORE.build_path), "") ) checksum = pch_checksum( @@ -507,8 +510,7 @@ def prepare_pch() -> None: _LOGGER.warning( "Precompiled header failed; compiling without it: %s", error[:400] ) - gch.unlink(missing_ok=True) - sum_path.unlink(missing_ok=True) + discard_pch() # Skip retries until a header/flag/sdkconfig/command change failed_marker.write_text(checksum + "\n", encoding="utf-8") os.utime(header)