diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 1517f29f79..1dd5eb9aba 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -1223,44 +1223,54 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool: # 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) 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 - # per-device build path so identically-configured devices - # produce identical .sum files and share cache entries - flags_id = " ".join(cxxflags).replace(effective_ccache_basedir(), "") - # The header text covers include order, which the sorted - # closure alone does not - checksum = pch_checksum( - src_dir, - pch_includes, - ( - pch_text, - str(paths.framework), - str(paths.toolchain), - flags_id, - ), + checksum = None + try: + if ccache: + # The .sum sidecar only exists for CCACHE_PCH_EXTSUM; ninja's + # depfile handles staleness. Mirror CCACHE_BASEDIR: strip the + # per-device build path so identically-configured devices + # produce identical .sum files and share cache entries + flags_id = " ".join(cxxflags).replace(effective_ccache_basedir(), "") + # The header text covers include order, which the sorted + # closure alone does not + checksum = pch_checksum( + src_dir, + pch_includes, + ( + pch_text, + str(paths.framework), + str(paths.toolchain), + flags_id, + ), + ) + except OSError as err: + # Identity unknown: a stale cache entry must never be served + _LOGGER.warning( + "Could not establish the pch identity; compiling without it: %s", err ) - write_file_if_changed( - build_dir / f"{PCH_HEADER_NAME}.gch.sum", checksum + "\n" + else: + _LOGGER.info( + "Compiling with a precompiled header " + "(set ESPHOME_PCH_ENABLE=0 to disable)" ) - gch = _e(f"{PCH_HEADER_NAME}.gch") - lines.append(f"build {gch}: pch {_e(pch_header)}") - if src_other: - lines.append(f" flags = {' '.join(src_other)}") - # 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"-Winvalid-pch -include {PCH_HEADER_NAME}"] - lines.append(f"srccxxflags = {' '.join(cxx_parts)}") - src_cxx_override = ("$srccxxflags", gch) + write_file_if_changed(pch_header, pch_text) + if checksum is not None: + write_file_if_changed( + build_dir / f"{PCH_HEADER_NAME}.gch.sum", checksum + "\n" + ) + gch = _e(f"{PCH_HEADER_NAME}.gch") + lines.append(f"build {gch}: pch {_e(pch_header)}") + if src_other: + lines.append(f" flags = {' '.join(src_other)}") + # 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"-Winvalid-pch -include {PCH_HEADER_NAME}"] + lines.append(f"srccxxflags = {' '.join(cxx_parts)}") + src_cxx_override = ("$srccxxflags", gch) src_objs = _ninja_compile_edges( lines, _collect_sources(src_dir), diff --git a/esphome/build_helpers/pch.py b/esphome/build_helpers/pch.py index 3dc32a71d4..b3ce7a759f 100644 --- a/esphome/build_helpers/pch.py +++ b/esphome/build_helpers/pch.py @@ -57,15 +57,22 @@ def ccache_pch_env() -> dict[str, str]: export these process-wide; only time_macros affects non-pch TUs.""" if not pch_enabled(): return {} + env = {k: v for k, v in _CCACHE_PCH_ENV.items() if k not in os.environ} user_sloppiness = os.environ.get("CCACHE_SLOPPINESS") - if user_sloppiness is not None and "pch_defines" not in user_sloppiness: - # EXTSUM without pch_defines makes ccache silently decline every - # pch-consuming compile + if user_sloppiness is not None and ( + missing := [ + t for t in ("pch_defines", "time_macros") if t not in user_sloppiness + ] + ): + # Without these ccache declines every pch-consuming compile; union + # rather than override so the user's own tokens survive + env["CCACHE_SLOPPINESS"] = ",".join((user_sloppiness, *missing)) _LOGGER.warning( - "CCACHE_SLOPPINESS lacks pch_defines; ccache will not cache " - "compiles that use the precompiled header" + "Adding %s to CCACHE_SLOPPINESS so ccache can cache compiles " + "that use the precompiled header", + ",".join(missing), ) - return {k: v for k, v in _CCACHE_PCH_ENV.items() if k not in os.environ} + return env def pch_extra_scripts() -> list[str]: @@ -105,13 +112,12 @@ def _include_closure(src_dir: Path, roots: Iterable[str]) -> dict[str, bytes]: data = (src_dir / rel).read_bytes() except OSError as err: # mtime/size keep a changed-but-unreadable header shifting the - # digest without device paths in it + # digest without device paths in it; if stat also fails the + # header's identity is unknown and the OSError propagates so + # callers compile without a pch _LOGGER.warning("Could not read %s for the pch checksum: %s", rel, err) - try: - st = (src_dir / rel).stat() - data = f"".encode() - except OSError: - data = b"" + st = (src_dir / rel).stat() + data = f"".encode() seen[rel] = data parent = posixpath.dirname(rel) stack.extend((inc.decode(), parent) for inc in _INCLUDE_RE.findall(data)) @@ -123,7 +129,8 @@ def pch_checksum( ) -> str: """Digest standing in for the .gch in ccache's hash: the include closure of the prefix header plus caller-supplied identity strings (versioned - install paths, flags).""" + install paths, flags). Raises OSError when a header's identity cannot + be established at all; callers must then compile without a pch.""" digest = hashlib.sha256() closure = _include_closure(src_dir, include_headers) for name in sorted(closure): diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 479c19588a..f0126c950d 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -9,10 +9,12 @@ import traceback # pylint: disable=E0602 Import("env") # noqa: F821 +_projenv_error = None try: Import("projenv") # noqa: F821 -except Exception: # noqa: BLE001 -- not exported under -t nobuild +except Exception as err: # noqa: BLE001 -- not exported under -t nobuild projenv = None + _projenv_error = err # Precompile the src force-includes plus defines.h (which pulls in # Arduino.h on Arduino platforms) and force-include the result into C++ src @@ -45,12 +47,11 @@ def _include_closure(src_dir: Path, roots: list) -> dict: try: data = (src_dir / rel).read_bytes() except OSError as err: + # If stat also fails the identity is unknown: the OSError + # propagates to the outer handler, which skips the pch print(f"ESPHome: could not read {rel} for the pch checksum: {err}") - try: - st = (src_dir / rel).stat() - data = f"".encode() - except OSError: - data = b"" + st = (src_dir / rel).stat() + data = f"".encode() seen[rel] = data parent = posixpath.dirname(rel) stack.extend((inc.decode(), parent) for inc in _INCLUDE_RE.findall(data)) @@ -107,6 +108,8 @@ def _compile_gch(cxx, flags, header: Path, gch: Path, proj_dir: Path): def _setup_pch() -> None: if projenv is None: + # Expected under -t nobuild; anything else must leave a trail + print(f"ESPHome: projenv unavailable ({_projenv_error}); skipping pch") 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. @@ -189,8 +192,9 @@ 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 + # lib_deps trees are not part of the prefix closure today (the + # roots resolve under src/ only); walking them would read every + # library file on every build for nothing and not inc_dir.is_relative_to(proj_dir / ".piolibdeps") and not inc_dir.is_relative_to(proj_dir / ".pioenvs") ): @@ -204,14 +208,11 @@ def _setup_pch() -> None: try: data = local.read_bytes() except OSError as err: + # mtime/size keep a changed-but-unreadable header shifting + # the digest; a stat failure propagates and skips the pch 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"" + st = local.stat() + data = f"".encode() digest.update(str(local.relative_to(proj_dir)).encode()) digest.update(data) digest.update(b"\0") diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index aeec4e916c..866ee8747c 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -398,6 +398,24 @@ def test_write_project_pch_sum_only_with_ccache(tmp_path: Path) -> None: assert not (build_dir / "esphome_pch.h.gch.sum").exists() +def test_write_project_pch_identity_unknown_skips_pch( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """An OSError from the checksum means the .sum cannot vouch for the + .gch: the build must fall back to plain srcflags.""" + paths = _make_framework(tmp_path) + _set_flags("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH") + with patch( + "esphome.build_gen.arduino8266.pch_checksum", + side_effect=OSError("stat failed"), + ): + content = _write_ninja(paths, ccache="/usr/bin/ccache") + assert "esphome_pch" not in content + assert "srccxxflags" not in content + assert " flags = $srcflags" in content + assert "Could not establish the pch identity" in caplog.text + + def test_write_project_pch_disabled( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/unit_tests/build_helpers/test_pch.py b/tests/unit_tests/build_helpers/test_pch.py index 693ce80669..c75d9e7b4d 100644 --- a/tests/unit_tests/build_helpers/test_pch.py +++ b/tests/unit_tests/build_helpers/test_pch.py @@ -45,22 +45,23 @@ def test_ccache_pch_env_disabled() -> None: assert pch.ccache_pch_env() == {} -def test_ccache_pch_env_respects_user_values( +def test_ccache_pch_env_unions_user_sloppiness( caplog: pytest.LogCaptureFixture, ) -> None: - """A user CCACHE_SLOPPINESS wins, but one without pch_defines silently - stops ccache from caching pch consumers, so it must warn.""" + """Without pch_defines/time_macros ccache declines every pch-consuming + compile, so missing tokens are unioned onto the user's value.""" with patch.dict(os.environ, {"CCACHE_SLOPPINESS": "locale"}, clear=True): env = pch.ccache_pch_env() - assert "CCACHE_SLOPPINESS" not in env + assert env["CCACHE_SLOPPINESS"] == "locale,pch_defines,time_macros" assert env["CCACHE_PCH_EXTSUM"] == "true" - assert "lacks pch_defines" in caplog.text + assert "Adding pch_defines,time_macros" in caplog.text caplog.clear() with patch.dict( - os.environ, {"CCACHE_SLOPPINESS": "pch_defines,locale"}, clear=True + os.environ, {"CCACHE_SLOPPINESS": "pch_defines,time_macros"}, clear=True ): - pch.ccache_pch_env() - assert "lacks pch_defines" not in caplog.text + env = pch.ccache_pch_env() + assert "CCACHE_SLOPPINESS" not in env + assert not caplog.records def test_pch_header_text_preserves_order() -> None: @@ -140,3 +141,28 @@ def test_pch_extra_scripts_gated(monkeypatch: pytest.MonkeyPatch) -> None: assert pch.pch_extra_scripts() == ["post:pch.py"] monkeypatch.setenv("ESPHOME_PCH_ENABLE", "0") assert pch.pch_extra_scripts() == [] + + +def test_include_closure_raises_when_identity_unknown( + caplog: pytest.LogCaptureFixture, +) -> None: + """Read AND stat failing means no marker can vouch for the header, so + the OSError propagates and callers compile without a pch.""" + + class _BadFile: + def is_file(self) -> bool: + return True + + def read_bytes(self) -> bytes: + raise OSError("read failed") + + def stat(self) -> None: + raise OSError("stat failed") + + class _FakeSrcDir: + def __truediv__(self, rel: str) -> _BadFile: + return _BadFile() + + with pytest.raises(OSError, match="stat failed"): + pch._include_closure(_FakeSrcDir(), ["a.h"]) + assert "Could not read a.h" in caplog.text diff --git a/tests/unit_tests/test_arduino8266_toolchain.py b/tests/unit_tests/test_arduino8266_toolchain.py index 7b229f54ef..7f41687909 100644 --- a/tests/unit_tests/test_arduino8266_toolchain.py +++ b/tests/unit_tests/test_arduino8266_toolchain.py @@ -649,5 +649,6 @@ def test_ccache_env_pch_disabled() -> None: def test_ccache_env_respects_user_sloppiness() -> None: with patch.dict(os.environ, {"CCACHE_SLOPPINESS": "locale"}, clear=True): env = framework.ccache_env("/usr/bin/ccache") - assert "CCACHE_SLOPPINESS" not in env + # The user's tokens survive; the ones the pch needs are unioned on + assert env["CCACHE_SLOPPINESS"] == "locale,pch_defines,time_macros" assert env["CCACHE_PCH_EXTSUM"] == "true"