diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 1f20ec7b06..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,15 +1216,20 @@ 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 # edges keep srcflags (a .gch is a C++ artifact) + # The opt-out hint matters when a toolchain rejects its own .gch: + # the build stays correct but every TU warns via -Winvalid-pch + _LOGGER.info( + "Compiling with a precompiled header (set ESPHOME_PCH_ENABLE=0 to disable)" + ) 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 @@ -1238,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, @@ -1254,18 +1258,16 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: # Relative -include (resolved from the ninja cwd, where the header # lives): an absolute path would put the per-device build path on # every compile command and defeat cross-device ccache sharing - cxx_parts = src_other + [f"-include {PCH_HEADER_NAME}"] + 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_gen/espidf.py b/esphome/build_gen/espidf.py index b8f5e95db5..f5965c0a27 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -1,21 +1,13 @@ """ESP-IDF direct build generator for ESPHome.""" -import hashlib import json import logging -import os from pathlib import Path -import subprocess -from esphome.build_helpers.idedata import ( - expand_response_files, - is_launcher, - split_command, -) +from esphome.build_helpers import pch from esphome.build_helpers.pch import ( - PCH_CORE_HEADER, + PCH_DEFAULT_HEADERS, PCH_HEADER_NAME, - pch_checksum, pch_enabled, pch_header_text, ) @@ -37,31 +29,6 @@ from esphome.helpers import mkdir_p, write_file_if_changed _LOGGER = logging.getLogger(__name__) -# Prefix-header contents, defines.h first so USE_* macros exist for the -# rest. Deliberately hard-coded: frequency-derived sets measured no better -# and kept selecting headers that cannot compile standalone (X-macro, -# platform-variant). Every entry must be safe to include first in an -# empty TU. -_PCH_HEADERS = ( - PCH_CORE_HEADER, - "esphome/core/component.h", - "esphome/core/helpers.h", - "esphome/core/log.h", - "esphome/core/application.h", - "esphome/core/automation.h", -) - -# Header and .gch/.sum sidecars, relative to the device dir; see -# _pch_cmake() and prepare_pch() for the layout rationale -_PCH_BUILD_HEADER = f"build/{PCH_HEADER_NAME}" - -# Compile-command tokens dropped when retargeting a TU's flags at the -# prefix header: source/output/depfile flags with an argument, and the -# argument-less depfile flags (the pch compile must not touch depfiles) -_PCH_STRIP_FLAGS_WITH_ARG = frozenset({"-include", "-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 # cg.set_cpp_standard(). Emitted between include(project.cmake) and project(), @@ -337,6 +304,7 @@ def _pch_cmake() -> str: # a .gch drop out of the TU depfiles, and prepare_pch() touches the # header whenever it rebuilds the .gch so consumers recompile. target_compile_options(${{COMPONENT_LIB}} PRIVATE + "$<$:-Winvalid-pch>" "$<$:-include>" "$<$:{PCH_HEADER_NAME}>" ) @@ -345,72 +313,29 @@ set_source_files_properties(${{app_sources}} PROPERTIES """ -def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str] | None: - """The exact src C++ flags from compile_commands.json, retargeted at - the header; None (logged) when no configured C++ TU is available yet.""" - try: - entries = json.loads( - (build_dir / "compile_commands.json").read_text(encoding="utf-8") - ) - except (OSError, json.JSONDecodeError) as err: - _LOGGER.debug("No usable compile database, skipping pch: %s", err) - return None - # Windows compile DBs use backslashes; normalize both sides - src_prefix = str(CORE.relative_src_path()).replace("\\", "/") - entry = next( - ( - e - for e in entries - if e.get("file", "").replace("\\", "/").startswith(src_prefix) - and e.get("file", "").endswith(_CXX_SOURCE_SUFFIXES) - ), - None, - ) - if entry is None: - _LOGGER.debug("No src C++ entry in the compile database, skipping pch") - return None - tokens = expand_response_files( - split_command(entry["command"]), Path(entry.get("directory", build_dir)) - ) - # A DB recorded with ccache enabled prefixes the compiler with the - # launcher; the .gch must be compiled directly - if tokens and is_launcher(tokens[0]): - tokens = tokens[1:] - args: list[str] = [] - arg_it = iter(tokens) - for tok in arg_it: - if tok in _PCH_STRIP_FLAGS_WITH_ARG: - next(arg_it, None) - continue - if tok in _PCH_STRIP_FLAGS: - continue - args.append(tok) - return [*args, "-x", "c++-header", "-c", str(header), "-o", str(gch)] +def discard_pch() -> None: + """Drop the pch sidecars in the IDF build dir.""" + pch.discard_pch(CORE.relative_build_path("build")) def prepare_pch() -> None: - """Compile the prefix header's .gch and write its ccache .sum. - - Runs right before ninja, after every reconfigure, so the flags in - compile_commands.json and the sdkconfig are the settled ones. The .sum - doubles as the freshness stamp; a failed .gch compile falls back to - the plain header include. - """ + """Build the .gch right before ninja, after every reconfigure, so the + compile_commands.json flags and the sdkconfig are the settled ones.""" if not pch_enabled(): return - build_dir = CORE.relative_build_path("build") - header = CORE.relative_build_path(_PCH_BUILD_HEADER) - gch = Path(f"{header}.gch") - sum_path = Path(f"{gch}.sum") + sdkconfig_path = CORE.relative_build_path(f"sdkconfig.{CORE.name}") try: - sdkconfig = CORE.relative_build_path(f"sdkconfig.{CORE.name}").read_text( - encoding="utf-8" + 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 + _LOGGER.warning( + "Could not read %s for the pch checksum: %s", sdkconfig_path, err ) - except OSError: - sdkconfig = "no-sdkconfig" - checksum = pch_checksum( - CORE.relative_src_path(), - _PCH_HEADERS, + sdkconfig = f"unreadable:{type(err).__name__}:{err.errno}" + pch.prepare_pch( + CORE.relative_build_path("build"), + PCH_DEFAULT_HEADERS, ( str(idf_version()), CORE.cpp_standard or "", @@ -419,54 +344,6 @@ def prepare_pch() -> None: *get_project_cxx_compile_flags(), ), ) - if ( - gch.is_file() - and sum_path.is_file() - and sum_path.read_text(encoding="utf-8").strip() == checksum - ): - return - cmd = _pch_compile_command(build_dir, header, gch) - if cmd is None: - # The checksum is stale; a leftover .gch must not be consumed - gch.unlink(missing_ok=True) - sum_path.unlink(missing_ok=True) - return - # Keyed on the checksum and the compile command: a failure caused by - # the command alone must retry when the command changes - marker_key = f"{checksum} {hashlib.sha256(' '.join(cmd).encode()).hexdigest()}" - failed_marker = Path(f"{gch}.failed") - if ( - failed_marker.is_file() - and failed_marker.read_text(encoding="utf-8").strip() == marker_key - ): - _LOGGER.debug("Pch previously failed for these inputs; skipping") - return - try: - result = subprocess.run( - cmd, cwd=build_dir, capture_output=True, text=True, check=False, timeout=300 - ) - error = None - if result.returncode != 0: - error = result.stderr.strip() or f"exit code {result.returncode}" - elif not gch.is_file(): - error = "compiler produced no .gch" - except (OSError, subprocess.SubprocessError) as err: - error = str(err) - if error is not None: - _LOGGER.warning( - "Precompiled header failed; compiling without it: %s", error[:400] - ) - gch.unlink(missing_ok=True) - sum_path.unlink(missing_ok=True) - # Skip retries until a header/flag/sdkconfig/command change - failed_marker.write_text(marker_key + "\n", encoding="utf-8") - os.utime(header) - return - failed_marker.unlink(missing_ok=True) - sum_path.write_text(checksum + "\n", encoding="utf-8") - # The OBJECT_DEPENDS edge watches the header; bump it so consumers of - # the previous .gch recompile - os.utime(header) def write_project( @@ -490,7 +367,8 @@ def write_project( if pch_enabled(): write_file_if_changed( - CORE.relative_build_path(_PCH_BUILD_HEADER), pch_header_text(_PCH_HEADERS) + CORE.relative_build_path("build", PCH_HEADER_NAME), + pch_header_text(PCH_DEFAULT_HEADERS), ) # Snapshot the exclusion set so has_outdated_files() can trigger a diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index 99d1bbc111..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} @@ -97,4 +98,8 @@ def effective_ccache_basedir() -> str: wins, else the resolved build path (matching ccache_defaults_env).""" from esphome.core import CORE - return os.environ.get("CCACHE_BASEDIR") or str(Path(CORE.build_path).resolve()) + raw = os.environ.get("CCACHE_BASEDIR") + if raw is not None: + # An explicitly empty value disables ccache's rewriting; mirror it + return raw + return str(Path(CORE.build_path).resolve()) diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 8a46203890..d627e3ce4a 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. +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") @@ -198,10 +198,20 @@ def parse_entry( it = iter(tokens[1:]) for tok in it: - if tok in ("-c", "-o", "-include"): - # Drop the flag and its argument; the injected relative - # -include esphome_pch.h does not resolve outside the build dir - next(it, None) + if tok in ("-c", "-o"): + next(it, None) # drop the flag and its argument (input/output) + elif tok == "-include": + # -include searches the compile cwd first, then the -I chain, so + # only re-anchor paths that really live next to the compile (the + # pch); a name meant for the -I chain must stay untouched + raw = next(it, "") + if not raw: + _LOGGER.warning("Dropping -include with no argument") + else: + resolved = _include(raw) + cxx_flags.extend( + ("-include", resolved if Path(resolved).is_file() else raw) + ) elif tok.startswith("-D"): # ``.strip()`` handles tokens like ``-D CONFIGURED=1`` (a single # quoted arg with a space after -D) that some flags arrive as. diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index 10761cdded..aa3a65cd3b 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -2,29 +2,64 @@ Safe by construction when the prefix header mirrors what the TUs already include first (ESP8266); a backend may instead inject a curated set of -self-contained core headers (ESP-IDF). +self-contained core headers (ESP-IDF). User sources from ``esphome: +includes:`` also receive the prefix, so they now see defines.h (and +Arduino.h on Arduino platforms) even when they did not include it. """ from __future__ import annotations from collections.abc import Iterable import hashlib +import json import logging import os from pathlib import Path import posixpath import re +import subprocess -from esphome.build_helpers.ccache import parse_enable_env +from esphome.build_helpers.ccache import effective_ccache_basedir, parse_enable_env +from esphome.build_helpers.idedata import ( + CXX_SOURCE_SUFFIXES, + expand_response_files, + is_launcher, + split_command, +) _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" +# Prefix-header contents for backends that inject a curated set (rather +# than mirroring the TUs' own force-includes), defines.h first so USE_* +# macros exist for the rest. Deliberately hard-coded: frequency-derived +# sets measured no better and kept selecting headers that cannot compile +# standalone (X-macro, platform-variant). Every entry must be safe to +# include first in an empty TU. Caveat: application.h/automation.h become +# ambiently visible, so a TU missing those #includes still builds on such +# backends; ESPHOME_PCH_ENABLE=0 restores the strict view. +PCH_DEFAULT_HEADERS = ( + PCH_CORE_HEADER, + "esphome/core/component.h", + "esphome/core/helpers.h", + "esphome/core/log.h", + "esphome/core/application.h", + "esphome/core/automation.h", +) + # ccache cannot hash through a .gch; CCACHE_PCH_EXTSUM makes it hash the # .sum sidecar instead of the .gch bytes, which are not reproducible. # Keep in sync with the literals in platformio/pch.py.script. @@ -112,3 +147,173 @@ def pch_checksum( digest.update(item.encode()) digest.update(b"\0") return digest.hexdigest() + + +# Compile-command tokens dropped when retargeting a TU's flags at the +# prefix header: source/output/depfile flags with an argument, and the +# 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"}) + + +def pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str] | None: + """The exact src C++ flags from compile_commands.json, retargeted at + the header; None (logged) when no configured C++ TU is available yet.""" + from esphome.core import CORE + + try: + entries = json.loads( + (build_dir / "compile_commands.json").read_text(encoding="utf-8") + ) + except (OSError, json.JSONDecodeError) as err: + # Configure already succeeded, so an unusable DB is a real anomaly + _LOGGER.warning("No usable compile database, skipping pch: %s", err) + return None + if not isinstance(entries, list): + _LOGGER.warning("Malformed compile database, skipping pch") + return None + # Windows compile DBs use backslashes; normalize both sides + src_prefix = str(CORE.relative_src_path()).replace("\\", "/") + entry = next( + ( + e + for e in entries + if isinstance(e, dict) + and e.get("file", "").replace("\\", "/").startswith(src_prefix) + and e.get("file", "").endswith(CXX_SOURCE_SUFFIXES) + ), + None, + ) + if entry is None: + _LOGGER.warning("No src C++ entry in the compile database, skipping pch") + return None + tokens = expand_response_files( + split_command(entry.get("command", "")), Path(entry.get("directory", build_dir)) + ) + # A DB recorded with ccache enabled prefixes the compiler with the + # launcher; the .gch must be compiled directly + if tokens and is_launcher(tokens[0]): + tokens = tokens[1:] + if not tokens: + # An "arguments"-style or empty entry must skip cleanly, not spawn + # a compiler-less argv that warns on every build + _LOGGER.warning("Compile database entry has no usable command, skipping pch") + return None + args: list[str] = [] + arg_it = iter(tokens) + for tok in arg_it: + if tok in _PCH_STRIP_FLAGS_WITH_ARG: + next(arg_it, None) + continue + if tok in _PCH_STRIP_FLAGS: + continue + if tok == "-include": + # Drop only the injected prefix; user force-includes must reach + # the .gch compile or GCC rejects it over the macro mismatch + inc = next(arg_it, "") + if not inc.endswith(PCH_HEADER_NAME): + args.extend(("-include", inc)) + continue + args.append(tok) + return [*args, "-x", "c++-header", "-c", str(header), "-o", str(gch)] + + +def discard_pch(build_dir: Path) -> None: + """Remove the pch sidecars so a stale .gch is never consumed. + + 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. + """ + 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) + if had_gch and header.is_file(): + os.utime(header) + + +def prepare_pch( + build_dir: Path, include_headers: tuple[str, ...], extra: Iterable[str] +) -> None: + """Compile ``build_dir``'s .gch from compile_commands.json flags and + write its ccache .sum. + + The .sum doubles as the freshness stamp and folds in the compile + command, so a flag-only change rebuilds the .gch; ``extra`` carries + backend identity (framework version, sdkconfig, ...). A failed + compile falls back to the plain header include. + """ + from esphome.core import CORE + + header = build_dir / PCH_HEADER_NAME + gch = Path(f"{header}.gch") + sum_path = Path(f"{gch}.sum") + cmd = pch_compile_command(build_dir, header, gch) + if cmd is None: + # Freshness cannot be validated; a leftover .gch must not be consumed + discard_pch(build_dir) + return + # 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(effective_ccache_basedir(), "") + .replace(str(CORE.build_path), "") + ) + checksum = pch_checksum( + CORE.relative_src_path(), + include_headers, + ( + # The closure is sorted, so root order only enters via the text + pch_header_text(include_headers), + *extra, + cmd_id, + ), + ) + if ( + gch.is_file() + and sum_path.is_file() + and sum_path.read_text(encoding="utf-8").strip() == checksum + ): + return + failed_marker = Path(f"{gch}.failed") + if ( + failed_marker.is_file() + and failed_marker.read_text(encoding="utf-8").strip() == checksum + ): + _LOGGER.info( + "Precompiled header disabled after an earlier failure; delete %s to retry", + failed_marker, + ) + return + try: + result = subprocess.run( + cmd, cwd=build_dir, capture_output=True, text=True, check=False, timeout=300 + ) + error = None + if result.returncode != 0: + error = result.stderr.strip() or f"exit code {result.returncode}" + elif not gch.is_file(): + error = "compiler produced no .gch" + except (OSError, subprocess.SubprocessError) as err: + # Transient (timeout, spawn/IO): warn and retry next build, no marker + _LOGGER.warning("Precompiled header compile did not run: %s", err) + discard_pch(build_dir) + return + if error is not None: + _LOGGER.warning( + "Precompiled header failed; compiling without it: %s", error[:400] + ) + discard_pch(build_dir) + # Skip retries until a header/flag/backend-identity/command change + failed_marker.write_text(checksum + "\n", encoding="utf-8") + os.utime(header) + return + failed_marker.unlink(missing_ok=True) + sum_path.write_text(checksum + "\n", encoding="utf-8") + # Consumers depend on the header (depfiles cannot see through a .gch); + # bump it so users of the previous .gch recompile + os.utime(header) diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index a5d145e3c1..0eccf5af79 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -1,5 +1,6 @@ """ESP-IDF direct build API for ESPHome.""" +from contextlib import suppress from dataclasses import dataclass, field import hashlib import json @@ -528,10 +529,20 @@ def run_compile(config, verbose: bool) -> int: return result.returncode _patch_memory_segments() - # After every reconfigure so compile_commands and sdkconfig are settled - from esphome.build_gen.espidf import prepare_pch + # After every reconfigure so compile_commands and sdkconfig are settled. + # An optional speedup must never abort the build + from esphome.build_gen.espidf import discard_pch, prepare_pch - prepare_pch() + try: + prepare_pch() + except Exception: # noqa: BLE001 # pylint: disable=broad-exception-caught + # Discard so an unexpected error can never leave a stale .gch that + # GCC would silently consume; exc_info keeps the failure diagnosable + with suppress(OSError): + discard_pch() + _LOGGER.warning( + "Precompiled header setup failed; compiling without it", exc_info=True + ) # Build args = [] diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 03f6567285..eb2a3362f4 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -5,9 +5,14 @@ import posixpath import re import shlex import subprocess +import traceback # pylint: disable=E0602 -Import("env", "projenv") # noqa: F821 +Import("env") # noqa: F821 +try: + Import("projenv") # noqa: F821 +except Exception: # noqa: BLE001 -- not exported under -t nobuild + projenv = None # Precompile the src force-includes plus defines.h (which pulls in # Arduino.h on Arduino platforms) and force-include the result into C++ src @@ -50,14 +55,55 @@ def _include_closure(src_dir: Path, roots: list) -> dict: def _shell_arg(element) -> str: """One compiler argv from one SCons element, matching the real spawn: - SCons whole-quotes spaced elements, the shell unquotes the rest.""" + SCons whole-quotes spaced elements, the shell unquotes the rest. On + Windows there is no POSIX shell pass and shlex would eat path + backslashes.""" arg = str(element) - if " " in arg: + if " " in arg or os.name == "nt": return arg.replace('\\"', '"') - return shlex.split(arg)[0] if arg else arg + 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: + if projenv is None: + return # 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. # An absolute path would put the per-device build path on every compile @@ -104,10 +150,18 @@ def _setup_pch() -> None: for package in sorted(platform.packages): try: version = platform.get_package_version(package) - except Exception as err: # noqa: BLE001 -- absent optional package - # Folded into the digest so an unexpected lookup failure still - # invalidates instead of hashing like a fixed absence - version = f"error:{type(err).__name__}" + except KeyError: + # Only trust KeyError as "absent" when the package really is not + # installed; an unresolved manifest must not hash as a constant + if platform.get_package(package) is not None: + print(f"ESPHome: skipping precompiled header: no version for {package}") + return + version = None # absent optional package + except Exception as err: # noqa: BLE001 + # Without trustworthy package identity a stale .gch could be + # reused across upgrades; skip the pch instead + print(f"ESPHome: skipping precompiled header: {err}") + return digest.update(f"{package}={version}".encode()) digest.update(b"\0") closure = _include_closure(src_dir, [*include_headers, _CORE_HEADER]) @@ -131,11 +185,31 @@ def _setup_pch() -> None: inc_dir.is_dir() and inc_dir.is_relative_to(proj_dir) and not inc_dir.is_relative_to(src_dir) + # Library/build trees are versioned via the package digest above; + # walking them would read every library file on every build + and not inc_dir.is_relative_to(proj_dir / ".piolibdeps") + and not inc_dir.is_relative_to(proj_dir / ".pioenvs") ): continue - for local in sorted(inc_dir.rglob("*.h")): + headers = ( + p + for p in inc_dir.rglob("*") + if p.is_file() and p.suffix in (".h", ".hpp", ".hh", ".inc") + ) + for local in sorted(headers): + try: + data = local.read_bytes() + except OSError as err: + print(f"ESPHome: could not read {local} for the pch checksum: {err}") + try: + # mtime/size keep a changed-but-unreadable header shifting + # the digest without putting device paths in it + st = local.stat() + data = f"".encode() + except OSError: + data = b"" digest.update(str(local.relative_to(proj_dir)).encode()) - digest.update(local.read_bytes()) + digest.update(data) digest.update(b"\0") checksum = digest.hexdigest() @@ -158,15 +232,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) + # 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) @@ -178,8 +250,9 @@ def _setup_pch() -> None: failed_marker.unlink(missing_ok=True) sum_path.write_text(checksum + "\n", encoding="utf-8") - # Scoped to src compiles: framework/library TUs never consume the .gch - # and keep strict ccache hashing. User-set values win. + # projenv["ENV"] aliases os.environ under PlatformIO, so these reach + # framework/library TUs too; only time_macros affects non-pch TUs (the + # trade-off ccache_pch_env documents). User-set values win. for key, value in ( ("CCACHE_SLOPPINESS", "pch_defines,time_macros"), ("CCACHE_PCH_EXTSUM", "true"), @@ -189,8 +262,12 @@ def _setup_pch() -> None: # Prepended so it is processed before the build_src_flags -include # entries: GCC only uses a .gch while no other tokens have been seen. - projenv.Prepend(CXXFLAGS=["-include", header.name]) # noqa: F821 + projenv.Prepend(CXXFLAGS=["-Winvalid-pch", "-include", header.name]) # noqa: F821 print("ESPHome: Compiling with precompiled header") -_setup_pch() +try: + _setup_pch() +except Exception: # noqa: BLE001 -- a speedup must never break the build + print("ESPHome: precompiled header setup failed; compiling without it") + traceback.print_exc() diff --git a/esphome/writer.py b/esphome/writer.py index 435c4804f1..a6b89c4108 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -7,6 +7,7 @@ import re import time from esphome import loader +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 ( @@ -609,6 +610,10 @@ def clean_build(clear_pio_cache: bool = True, *, full: bool = False): if idf_path.is_dir(): _LOGGER.info("Deleting %s", idf_path) 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_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/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index de6abba6dc..aeec4e916c 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -1766,7 +1766,7 @@ def test_write_project_pch_no_device_path_poison(tmp_path: Path) -> None: CORE.build_path = tmp_path / name _set_flags("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH") content = _write_ninja(paths, ccache="/usr/bin/ccache") - assert "srccxxflags = -include esphome_pch.h" in content + assert "srccxxflags = -Winvalid-pch -include esphome_pch.h" in content sums.append( (CORE.relative_pioenvs_path(name) / "esphome_pch.h.gch.sum").read_text() ) diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index 22c3f7ef8d..ad520b9190 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/tests/unit_tests/build_gen/test_espidf.py @@ -4,6 +4,7 @@ from __future__ import annotations import json import logging +import os from pathlib import Path import subprocess from unittest.mock import patch @@ -493,10 +494,10 @@ def test_get_component_cmakelists_no_compile_features() -> None: def _make_pch_device(tmp_path: Path, name: str) -> Path: """A device dir with the pch source headers and a stub compile_commands.""" - from esphome.build_gen.espidf import _PCH_HEADERS + from esphome.build_helpers.pch import PCH_DEFAULT_HEADERS dev = tmp_path / name - for header in _PCH_HEADERS: + for header in PCH_DEFAULT_HEADERS: path = dev / "src" / header path.parent.mkdir(parents=True, exist_ok=True) path.write_text("") @@ -511,7 +512,7 @@ def _make_pch_device(tmp_path: Path, name: str) -> Path: build.mkdir(exist_ok=True) from esphome.build_helpers.pch import pch_header_text - (build / "esphome_pch.h").write_text(pch_header_text(_PCH_HEADERS)) + (build / "esphome_pch.h").write_text(pch_header_text(PCH_DEFAULT_HEADERS)) # Native separators: mixed f-string paths break the src-prefix match # on Windows src_file = str(dev / "src" / "a.cpp") @@ -548,7 +549,7 @@ def test_prepare_pch_writes_header_and_sum(tmp_path: Path) -> None: with ( patch.object(CORE, "name", "test"), - patch("esphome.build_gen.espidf.subprocess.run", side_effect=fake_compile), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=fake_compile), ): prepare_pch() checksum = (dev / "build" / "esphome_pch.h.gch.sum").read_text().strip() @@ -556,7 +557,7 @@ def test_prepare_pch_writes_header_and_sum(tmp_path: Path) -> None: # Unchanged inputs: the second call must not recompile with ( patch.object(CORE, "name", "test"), - patch("esphome.build_gen.espidf.subprocess.run", side_effect=AssertionError), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=AssertionError), ): prepare_pch() @@ -578,7 +579,7 @@ def test_pch_no_device_path_poison(tmp_path: Path) -> None: with ( patch.object(CORE, "name", name), - patch("esphome.build_gen.espidf.subprocess.run", side_effect=fake_compile), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=fake_compile), ): prepare_pch() content = get_component_cmakelists() @@ -599,13 +600,13 @@ def test_component_cmakelists_pch_block(monkeypatch: pytest.MonkeyPatch) -> None def test_pch_compile_command_variants(tmp_path: Path) -> None: """Missing DB, no matching entry, and launcher-prefixed commands.""" - from esphome.build_gen.espidf import _pch_compile_command + from esphome.build_helpers.pch import pch_compile_command build = tmp_path / "build" build.mkdir() header = build / "esphome_pch.h" gch = build / "esphome_pch.h.gch" - assert _pch_compile_command(build, header, gch) is None + assert pch_compile_command(build, header, gch) is None (build / "compile_commands.json").write_text( json.dumps( @@ -614,7 +615,7 @@ def test_pch_compile_command_variants(tmp_path: Path) -> None: ] ) ) - assert _pch_compile_command(build, header, gch) is None + assert pch_compile_command(build, header, gch) is None src_file = str(tmp_path / "src" / "esphome" / "a.cpp") (build / "compile_commands.json").write_text( @@ -633,7 +634,7 @@ def test_pch_compile_command_variants(tmp_path: Path) -> None: ) ) # Launcher stripped; -include/-o/-c and depfile flags removed - assert _pch_compile_command(build, header, gch) == [ + assert pch_compile_command(build, header, gch) == [ "g++", "-DX=1", "-x", @@ -645,6 +646,61 @@ def test_pch_compile_command_variants(tmp_path: Path) -> None: ] +def test_pch_compile_command_rejects_unusable_entries(tmp_path: Path) -> None: + """Malformed DB shapes and command-less entries skip cleanly instead of + producing a compiler-less argv retried every build.""" + from esphome.build_helpers.pch import pch_compile_command + + build = tmp_path / "build" + build.mkdir() + header = build / "esphome_pch.h" + gch = build / "esphome_pch.h.gch" + db = build / "compile_commands.json" + src_file = str(tmp_path / "src" / "esphome" / "a.cpp") + + db.write_text(json.dumps({"not": "a list"})) + assert pch_compile_command(build, header, gch) is None + + db.write_text(json.dumps(["just a string"])) + assert pch_compile_command(build, header, gch) is None + + # arguments-style entry (allowed by the spec, unused by CMake) + db.write_text( + json.dumps([{"arguments": ["g++", "-c", src_file], "file": src_file}]) + ) + assert pch_compile_command(build, header, gch) is None + + +def test_pch_header_list_order_is_in_checksum( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Reordering PCH_DEFAULT_HEADERS keeps the include closure identical, but the + generated header text differs, so the .gch must rebuild.""" + import esphome.build_gen.espidf as espidf_mod + + dev = _make_pch_device(tmp_path, "dev_r") + CORE.build_path = dev + gch = dev / "build" / "esphome_pch.h.gch" + + def fake_compile(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=fake_compile), + ): + espidf_mod.prepare_pch() + first = (dev / "build" / "esphome_pch.h.gch.sum").read_text() + monkeypatch.setattr( + espidf_mod, + "PCH_DEFAULT_HEADERS", + tuple(reversed(espidf_mod.PCH_DEFAULT_HEADERS)), + ) + espidf_mod.prepare_pch() + assert (dev / "build" / "esphome_pch.h.gch.sum").read_text() != first + + def test_prepare_pch_failure_writes_marker_and_skips_retry(tmp_path: Path) -> None: from esphome.build_gen.espidf import prepare_pch @@ -658,7 +714,7 @@ def test_prepare_pch_failure_writes_marker_and_skips_retry(tmp_path: Path) -> No with ( patch.object(CORE, "name", "test"), - patch("esphome.build_gen.espidf.subprocess.run", side_effect=failing_compile), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=failing_compile), ): prepare_pch() prepare_pch() @@ -667,20 +723,54 @@ def test_prepare_pch_failure_writes_marker_and_skips_retry(tmp_path: Path) -> No assert (dev / "build" / "esphome_pch.h.gch.failed").exists() -def test_prepare_pch_spawn_oserror_degrades(tmp_path: Path) -> None: +def test_prepare_pch_spawn_oserror_is_transient(tmp_path: Path) -> None: + """Spawn/IO failures retry on the next build instead of latching.""" from esphome.build_gen.espidf import prepare_pch dev = _make_pch_device(tmp_path, "dev_o") CORE.build_path = dev + calls = [] + + def raising(cmd, **kwargs): + calls.append(cmd) + raise OSError("no such compiler") + + header = dev / "build" / "esphome_pch.h" + before = header.stat().st_mtime_ns + with ( + patch.object(CORE, "name", "test"), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=raising), + ): + prepare_pch() + prepare_pch() + assert not (dev / "build" / "esphome_pch.h.gch.failed").exists() + assert not (dev / "build" / "esphome_pch.h.gch.sum").exists() + assert len(calls) == 2 + # No .gch was ever in play, so the header must not be re-touched into + # forcing a full rebuild on every failing build + assert header.stat().st_mtime_ns == before + + +def test_prepare_pch_transient_with_stale_gch_bumps_header(tmp_path: Path) -> None: + """A stale .gch removed on a transient failure must dirty its consumers.""" + from esphome.build_gen.espidf import prepare_pch + + dev = _make_pch_device(tmp_path, "dev_s") + CORE.build_path = dev + gch = dev / "build" / "esphome_pch.h.gch" + gch.write_bytes(b"stale") + header = dev / "build" / "esphome_pch.h" + os.utime(header, (1, 1)) with ( patch.object(CORE, "name", "test"), patch( - "esphome.build_gen.espidf.subprocess.run", + "esphome.build_helpers.pch.subprocess.run", side_effect=OSError("no such compiler"), ), ): prepare_pch() - assert (dev / "build" / "esphome_pch.h.gch.failed").exists() + assert not gch.exists() + assert header.stat().st_mtime_ns > 1_000_000_000 def test_prepare_pch_disabled_is_noop( @@ -691,7 +781,7 @@ def test_prepare_pch_disabled_is_noop( monkeypatch.setenv("ESPHOME_PCH_ENABLE", "0") dev = _make_pch_device(tmp_path, "dev_d") CORE.build_path = dev - with patch("esphome.build_gen.espidf.subprocess.run", side_effect=AssertionError): + with patch("esphome.build_helpers.pch.subprocess.run", side_effect=AssertionError): prepare_pch() @@ -704,7 +794,7 @@ def test_prepare_pch_without_compile_commands(tmp_path: Path) -> None: CORE.build_path = dev with ( patch.object(CORE, "name", "test"), - patch("esphome.build_gen.espidf.subprocess.run", side_effect=AssertionError), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=AssertionError), ): prepare_pch() assert not (dev / "build" / "esphome_pch.h.gch.sum").exists() @@ -730,8 +820,8 @@ def test_write_project_pch_disabled_writes_no_header( def test_write_project_writes_pch_header(tmp_path: Path) -> None: """The header write_project emits is what _pch_cmake() force-includes; this pairing is the one non-fail-safe path in the design.""" - from esphome.build_gen.espidf import _PCH_HEADERS, write_project - from esphome.build_helpers.pch import pch_header_text + from esphome.build_gen.espidf import write_project + from esphome.build_helpers.pch import PCH_DEFAULT_HEADERS, pch_header_text _write_project_description(tmp_path, {}) CORE.build_path = tmp_path @@ -741,7 +831,7 @@ def test_write_project_writes_pch_header(tmp_path: Path) -> None: ): write_project() assert (tmp_path / "build" / "esphome_pch.h").read_text() == pch_header_text( - _PCH_HEADERS + PCH_DEFAULT_HEADERS ) @@ -772,7 +862,7 @@ def test_prepare_pch_zero_exit_without_gch_is_failure(tmp_path: Path) -> None: with ( patch.object(CORE, "name", "test"), - patch("esphome.build_gen.espidf.subprocess.run", side_effect=no_output), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=no_output), ): prepare_pch() assert not (dev / "build" / "esphome_pch.h.gch.sum").exists() @@ -799,7 +889,7 @@ def test_prepare_pch_bumps_header_for_object_depends(tmp_path: Path) -> None: with ( patch.object(CORE, "name", "test"), - patch("esphome.build_gen.espidf.subprocess.run", side_effect=fake_compile), + patch("esphome.build_helpers.pch.subprocess.run", side_effect=fake_compile), ): prepare_pch() assert header.stat().st_mtime > before @@ -810,3 +900,53 @@ def test_component_cmakelists_pch_object_depends() -> None: content = get_component_cmakelists() assert 'OBJECT_DEPENDS "${CMAKE_BINARY_DIR}/esphome_pch.h"' in content + + +def test_prepare_pch_command_change_invalidates_sum(tmp_path: Path) -> None: + """A flag-only change in the compile DB must rebuild the .gch.""" + from esphome.build_gen.espidf import prepare_pch + + dev = _make_pch_device(tmp_path, "dev_c") + CORE.build_path = dev + gch = dev / "build" / "esphome_pch.h.gch" + + def fake_compile(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=fake_compile), + ): + prepare_pch() + first = (dev / "build" / "esphome_pch.h.gch.sum").read_text() + db = dev / "build" / "compile_commands.json" + db.write_text(db.read_text().replace("-DX=1", "-DX=2")) + prepare_pch() + assert (dev / "build" / "esphome_pch.h.gch.sum").read_text() != first + + +def test_prepare_pch_keeps_user_force_includes(tmp_path: Path) -> None: + from esphome.build_helpers.pch import pch_compile_command + + dev = _make_pch_device(tmp_path, "dev_u") + CORE.build_path = dev + build = dev / "build" + src_file = str(dev / "src" / "esphome" / "a.cpp") + build.joinpath("compile_commands.json").write_text( + json.dumps( + [ + { + "directory": str(build), + "command": ( + "g++ -include user.h -include esphome_pch.h " + f"-o a.obj -c {src_file}" + ), + "file": src_file, + } + ] + ) + ) + cmd = pch_compile_command(build, build / "esphome_pch.h", build / "x.gch") + assert "user.h" in cmd + assert "esphome_pch.h" not in " ".join(cmd[:-3]) diff --git a/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index f21e28cd13..6461b3ad75 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -85,6 +85,51 @@ def test_parse_entry_resolves_relative_includes() -> None: assert all(Path(inc).is_absolute() for inc in includes) +def test_parse_entry_resolves_force_include_path(tmp_path: Path) -> None: + """The pch -include is emitted relative to the build dir; idedata must + resolve it so cached flags work from any cwd.""" + (tmp_path / "esphome_pch.h").write_text("") + entry = _entry( + str(tmp_path), + f"{tmp_path}/src/esphome/x.cpp", + "g++ -include esphome_pch.h -c x.cpp", + ) + + _, _, _, cxx_flags = idedata.parse_entry(entry) + + idx = cxx_flags.index("-include") + resolved = cxx_flags[idx + 1] + assert Path(resolved).is_absolute() + assert resolved == str(tmp_path / "esphome_pch.h").replace("\\", "/") + + +def test_parse_entry_keeps_search_chain_force_include(tmp_path: Path) -> None: + """-include names resolved via the -I chain (libretiny's Arduino.h) must + not be re-anchored to a nonexistent build-dir path.""" + entry = _entry( + str(tmp_path), + f"{tmp_path}/src/esphome/x.cpp", + "g++ -include Arduino.h -c x.cpp", + ) + + _, _, _, cxx_flags = idedata.parse_entry(entry) + + assert cxx_flags[cxx_flags.index("-include") + 1] == "Arduino.h" + + +def test_parse_entry_drops_trailing_force_include( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + entry = _entry( + str(tmp_path), f"{tmp_path}/src/esphome/x.cpp", "g++ -c x.cpp -include" + ) + + _, _, _, cxx_flags = idedata.parse_entry(entry) + + assert "-include" not in cxx_flags + assert "no argument" in caplog.text + + def test_parse_entry_skips_dependency_flags() -> None: """Dependency-generation flags (and their args) are dropped.""" entry = _entry( diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index a49c292e2a..7e48593ad3 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -1991,3 +1991,12 @@ def test_ccache_env_opt_in_with_usable_binary( env = _ccache_env() assert env["IDF_CCACHE_ENABLE"] == "1" assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +def test_ccache_env_exports_pch_settings(tmp_path: Path) -> None: + # The pch cannot cache under ccache without these + p1, p2, p3 = _ccache_patches(tmp_path, "/usr/bin/ccache", tmp_path / "build") + with patch.dict("os.environ", {}, clear=True), p1, p2, p3: + env = _ccache_env() + assert env["CCACHE_SLOPPINESS"] == "pch_defines,time_macros" + assert env["CCACHE_PCH_EXTSUM"] == "true" diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index 2cd5d4e1cb..7458d44764 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -667,3 +667,29 @@ def test_get_core_framework_version_from_core_data(): CORE.data = {KEY_ESP32: {KEY_IDF_VERSION: cv.Version(5, 5, 4)}} assert toolchain._get_core_framework_version() == "5.5.4" + + +def test_run_compile_invokes_prepare_pch_and_survives_failure( + setup_core: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The pch hook runs before the build and a failure never aborts it.""" + monkeypatch.setenv("ESPHOME_PCH_ENABLE", "1") + _setup_build(setup_core) + # A stale .gch must be discarded on the failure path, never consumed + build = setup_core / "build" / "test" / "build" + build.mkdir(parents=True, exist_ok=True) + (build / "esphome_pch.h").write_text("") + stale_gch = build / "esphome_pch.h.gch" + stale_gch.write_bytes(b"stale") + + with ( + patch.object(toolchain, "need_reconfigure", return_value=False), + 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") + ) as prepare, + ): + assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) == 0 + prepare.assert_called_once() + assert not stale_gch.exists() diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py index 2ff162b565..77302a7196 100644 --- a/tests/unit_tests/test_platformio_pch_script.py +++ b/tests/unit_tests/test_platformio_pch_script.py @@ -26,11 +26,36 @@ class _FakePlatform: raise KeyError(name) return "1.2.3" + def get_package(self, name: str) -> object | None: + return None + + +class _BrokenPlatform(_FakePlatform): + def get_package_version(self, name: str) -> str: + raise RuntimeError("manifest parse error") + + +class _UnresolvedPlatform(_FakePlatform): + """KeyError from a package that IS installed: unresolved identity.""" + + def get_package_version(self, name: str) -> str: + raise KeyError(name) + + def get_package(self, name: str) -> object: + return object() + class _FakeSConsEnv(dict): """Just enough of a SCons construction environment for pch.py.""" - def __init__(self, proj_dir: Path, src_dir: Path, cxx: str, flags: list[str]): + def __init__( + self, + proj_dir: Path, + src_dir: Path, + cxx: str, + flags: list[str], + platform_cls: type[_FakePlatform] = _FakePlatform, + ): super().__init__(ENV={}) self._subst = { "$PROJECT_DIR": str(proj_dir), @@ -38,6 +63,7 @@ class _FakeSConsEnv(dict): "$CXX": cxx, } self._flags = flags + self._platform_cls = platform_cls self.prepended: list[str] = [] def subst(self, expr: str) -> str: # noqa: N802 @@ -47,20 +73,37 @@ class _FakeSConsEnv(dict): return [self._flags] def PioPlatform(self) -> _FakePlatform: # noqa: N802 - return _FakePlatform() + return self._platform_cls() def Prepend(self, CXXFLAGS: list[str]) -> None: # noqa: N802, N803 self.prepended = CXXFLAGS -def _fake_cxx(tmp_path: Path, fail: bool = False) -> Path: - """A compiler stand-in that records its argv and writes the -o target.""" +def _fake_cxx( + tmp_path: Path, + fail: bool = False, + reject_pch: bool = False, + probe_exit: int = 0, +) -> Path: + """A compiler stand-in that records its argv and writes the -o target. + + With reject_pch it builds the .gch fine but, like GCC 10 on macOS arm64, + warns on any consuming compile that the .gch cannot be loaded; probe_exit + sets the exit code of non-header compiles (the load probe). + """ cxx = tmp_path / "fake-gxx" - body = 'printf \'%s\\n\' "$@" >> "$0.argv"\n' + body = ( + 'printf -- ---call---\\\\n >> "$0.argv"; printf \'%s\\n\' "$@" >> "$0.argv"\n' + ) if fail: body += "echo boom >&2\nexit 1\n" else: - body += 'out=""; prev=""\nfor a in "$@"; do [ "$prev" = "-o" ] && out="$a"; prev="$a"; done\necho gch > "$out"\n' + # Only the c++-header compile has a -o; the load probe has none + body += 'out=""; prev=""\nfor a in "$@"; do [ "$prev" = "-o" ] && out="$a"; prev="$a"; done\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' + 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) return cxx @@ -70,22 +113,32 @@ def _run_script( tmp_path: Path, flags: list[str] | None = None, 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, ) -> _FakeSConsEnv: proj = tmp_path / name src = proj / "src" (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) - scons_env = _FakeSConsEnv(proj, src, str(cxx), flags or ["-DX=1"]) + 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 -include flags must land on projenv only + global_env = _FakeSConsEnv(*args) + projenv = _FakeSConsEnv(*args) + projenv.global_env = global_env source = _SCRIPT.read_text() with patch.dict(os.environ, env_vars or {}, clear=True): exec( # noqa: S102 compile(source, "pch.py", "exec"), - {"Import": lambda *_names: None, "env": scons_env, "projenv": scons_env}, + {"Import": lambda *_names: None, "env": global_env, "projenv": projenv}, ) - return scons_env + return projenv def test_pch_script_builds_and_prepends_relative_include(tmp_path: Path) -> None: @@ -95,11 +148,12 @@ def test_pch_script_builds_and_prepends_relative_include(tmp_path: Path) -> None assert (proj / "esphome_pch.h.gch").is_file() assert len((proj / "esphome_pch.h.gch.sum").read_text().strip()) == 64 # Relative include: an absolute path would poison ccache keys - assert scons_env.prepended == ["-include", "esphome_pch.h"] - # ccache settings land on the SCons ENV only, never os.environ + assert scons_env.prepended == ["-Winvalid-pch", "-include", "esphome_pch.h"] + # In production projenv["ENV"] aliases os.environ; only the -include + # flags are genuinely scoped to projenv (src compiles) assert scons_env["ENV"]["CCACHE_SLOPPINESS"] == "pch_defines,time_macros" assert scons_env["ENV"]["CCACHE_PCH_EXTSUM"] == "true" - assert "CCACHE_SLOPPINESS" not in os.environ + assert scons_env.global_env.prepended == [] def test_pch_script_preserves_spaced_flag_elements(tmp_path: Path) -> None: @@ -109,10 +163,11 @@ def test_pch_script_preserves_spaced_flag_elements(tmp_path: Path) -> None: spaced.mkdir() flags = ['-DUSB_PRODUCT=\\"Pico 2W\\"', "-I", str(spaced), "-include", "other.h"] _run_script(tmp_path, flags=flags) - argv = (tmp_path / "fake-gxx.argv").read_text().splitlines() - assert '-DUSB_PRODUCT="Pico 2W"' in argv - assert str(spaced) in argv - assert "-include" not in argv + calls = (tmp_path / "fake-gxx.argv").read_text().split("---call---\n") + gch_call = next(c for c in calls if "c++-header" in c).splitlines() + assert '-DUSB_PRODUCT="Pico 2W"' in gch_call + assert str(spaced) in gch_call + assert "-include" not in gch_call # The stripped -include header is folded into the prefix header instead pch = (tmp_path / "dev" / "esphome_pch.h").read_text() assert pch.splitlines()[0] == '#include "other.h"' @@ -151,6 +206,57 @@ def test_pch_script_failure_marker_suppresses_retry( assert "delete esphome_pch.h.gch.failed to retry" in out +def test_pch_script_probe_rejection_falls_back( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A toolchain that cannot load its own .gch (GCC 10 on macOS arm64) + must not leave consumers paying for a pch every compile rejects.""" + scons_env = _run_script(tmp_path, reject_pch=True) + proj = tmp_path / "dev" + assert not (proj / "esphome_pch.h.gch").exists() + assert not (proj / "esphome_pch.h.gch.sum").exists() + assert (proj / "esphome_pch.h.gch.failed").is_file() + assert scons_env.prepended == [] + 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) + proj = tmp_path / "dev" + assert not (proj / "esphome_pch.h.gch").exists() + assert (proj / "esphome_pch.h.gch.failed").is_file() + assert scons_env.prepended == [] + + +def test_pch_script_unresolved_package_version_skips_pch(tmp_path: Path) -> None: + """A KeyError for an installed package is unresolved identity, not absence.""" + scons_env = _run_script(tmp_path, platform_cls=_UnresolvedPlatform) + assert not (tmp_path / "dev" / "esphome_pch.h.gch").exists() + assert scons_env.prepended == [] + + +def test_pch_script_package_version_error_skips_pch(tmp_path: Path) -> None: + """Without trustworthy package identity a stale .gch could survive an + upgrade, so the script must not build one at all.""" + scons_env = _run_script(tmp_path, platform_cls=_BrokenPlatform) + proj = tmp_path / "dev" + assert not (proj / "esphome_pch.h.gch").exists() + assert scons_env.prepended == [] + + def test_pch_script_rebuilds_when_header_missing(tmp_path: Path) -> None: _run_script(tmp_path) proj = tmp_path / "dev" @@ -167,6 +273,44 @@ def test_copy_pch_script(tmp_path: Path) -> None: assert (tmp_path / "pch.py").read_text() == _SCRIPT.read_text() +def test_pch_script_nobuild_without_projenv_is_noop(tmp_path: Path) -> None: + """-t nobuild never exports projenv; the script must not abort.""" + proj = tmp_path / "dev" + (proj / "src").mkdir(parents=True) + + def strict_import(*names: str) -> None: + if "projenv" in names: + raise RuntimeError("Import of non-existent variable 'projenv'") + + env = _FakeSConsEnv(proj, proj / "src", "g++", ["-DX=1"]) + exec( # noqa: S102 + compile(_SCRIPT.read_text(), "pch.py", "exec"), + {"Import": strict_import, "env": env}, + ) + assert not (proj / "esphome_pch.h").exists() + + +def test_pch_script_ignores_library_trees_and_non_headers(tmp_path: Path) -> None: + """.piolibdeps and non-header files must not enter the digest (or be + read at all); package versions already cover library identity.""" + proj = tmp_path / "dev" + libdeps = proj / ".piolibdeps" / "lib" / "src" + libdeps.mkdir(parents=True) + (libdeps / "lib.h").write_text("#define A 1\n") + override = proj / "lwip_override" + override.mkdir(parents=True) + (override / "lwipopts.h").write_text("#define TCP_MSS 1460\n") + (override / "notes.txt").write_text("v1\n") + flags = ["-DX=1", "-I", str(libdeps), "-I", str(override)] + _run_script(tmp_path, flags=flags) + first = (proj / "esphome_pch.h.gch.sum").read_text() + (libdeps / "lib.h").write_text("#define A 2\n") + (override / "notes.txt").write_text("v2\n") + (tmp_path / "fake-gxx.argv").unlink(missing_ok=True) + _run_script(tmp_path, flags=flags) + assert (proj / "esphome_pch.h.gch.sum").read_text() == first + + def test_pch_script_hashes_project_local_include_dirs(tmp_path: Path) -> None: """Generated headers in project-local -I dirs (e.g. rp2's lwip_override) must invalidate the checksum when they change.""" @@ -181,3 +325,26 @@ def test_pch_script_hashes_project_local_include_dirs(tmp_path: Path) -> None: (tmp_path / "fake-gxx.argv").unlink(missing_ok=True) _run_script(tmp_path, flags=flags) assert (proj / "esphome_pch.h.gch.sum").read_text() != first + + +@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: + """An unreadable generated header still shifts the digest via mtime/size.""" + proj = tmp_path / "dev" + override = proj / "lwip_override" + override.mkdir(parents=True) + secret = override / "lwipopts.h" + secret.write_text("#define TCP_MSS 1460\n") + secret.chmod(0) + flags = ["-DX=1", "-I", str(override)] + _run_script(tmp_path, flags=flags) + first = (proj / "esphome_pch.h.gch.sum").read_text() + assert "could not read" in capsys.readouterr().out + os.utime(secret, (1, 1)) + (tmp_path / "fake-gxx.argv").unlink(missing_ok=True) + _run_script(tmp_path, flags=flags) + assert (proj / "esphome_pch.h.gch.sum").read_text() != first diff --git a/tests/unit_tests/test_writer.py b/tests/unit_tests/test_writer.py index 3dcc4b12b8..3760496510 100644 --- a/tests/unit_tests/test_writer.py +++ b/tests/unit_tests/test_writer.py @@ -677,6 +677,32 @@ def test_clean_build_partial_exists( assert "dependencies.lock" not in caplog.text +@patch("esphome.writer.CORE") +def test_clean_build_partial_removes_pch_artifacts( + mock_core: MagicMock, + tmp_path: Path, +) -> None: + """The PlatformIO pch sidecars live at the project root and must go in + a partial clean, like the native backend's under .pioenvs.""" + names = ( + "esphome_pch.h", + "esphome_pch.h.gch", + "esphome_pch.h.gch.sum", + "esphome_pch.h.gch.failed", + ) + for name in names: + (tmp_path / name).write_text("x") + mock_core.relative_pioenvs_path.return_value = tmp_path / ".pioenvs" + mock_core.relative_piolibdeps_path.return_value = tmp_path / ".piolibdeps" + mock_core.relative_build_path.side_effect = lambda name: tmp_path / name + mock_core.relative_internal_path.side_effect = tmp_path.joinpath + + clean_build() + + for name in names: + assert not (tmp_path / name).exists() + + @patch("esphome.writer.CORE") def test_clean_build_nothing_exists( mock_core: MagicMock,