From 1b54d1b5b0ce7e051d00a2ae594fd8075e3ecccb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 25 Aug 2026 18:13:15 -0500 Subject: [PATCH] Survive -t nobuild, skip library trees in the local include digest, drop the false ENV scoping claim --- esphome/build_helpers/idedata.py | 2 +- esphome/platformio/pch.py.script | 24 ++++++++-- .../unit_tests/test_platformio_pch_script.py | 46 +++++++++++++++++-- 3 files changed, 62 insertions(+), 10 deletions(-) diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 76319bd681..a2eaea5b7a 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -59,7 +59,7 @@ def warn_if_idedata_missing(get_idedata: Callable[[], dict | None]) -> None: _LOGGER.warning("Idedata failure detail", exc_info=True) -# C++ translation-unit suffixes, shared with the pch backends. +# 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). diff --git a/esphome/platformio/pch.py.script b/esphome/platformio/pch.py.script index 310a64c25c..eb2a3362f4 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -8,7 +8,11 @@ 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 @@ -98,6 +102,8 @@ def _compile_gch(cxx, flags, header: Path, gch: Path, proj_dir: Path): 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 @@ -179,9 +185,18 @@ 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(p for p in inc_dir.rglob("*") if p.is_file()): + 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: @@ -235,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"), diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py index 9747dddb39..77302a7196 100644 --- a/tests/unit_tests/test_platformio_pch_script.py +++ b/tests/unit_tests/test_platformio_pch_script.py @@ -128,7 +128,7 @@ def _run_script( 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 + # Distinct objects: the -include flags must land on projenv only global_env = _FakeSConsEnv(*args) projenv = _FakeSConsEnv(*args) projenv.global_env = global_env @@ -149,13 +149,11 @@ def test_pch_script_builds_and_prepends_relative_include(tmp_path: Path) -> None 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 == ["-Winvalid-pch", "-include", "esphome_pch.h"] - # ccache settings land on projenv's ENV only: framework/library TUs - # compile under the global env and must keep strict hashing + # 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 scons_env.global_env["ENV"] == {} assert scons_env.global_env.prepended == [] - assert "CCACHE_SLOPPINESS" not in os.environ def test_pch_script_preserves_spaced_flag_elements(tmp_path: Path) -> None: @@ -275,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."""