diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 1f59241281..200ee23184 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -201,9 +201,17 @@ def parse_entry( if tok in ("-c", "-o"): next(it, None) # drop the flag and its argument (input/output) elif tok == "-include": - # Resolve like -I so cached idedata works from any cwd (the pch - # include is emitted relative to the build dir) - cxx_flags.extend(("-include", _include(next(it, "")))) + # -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/platformio/pch.py.script b/esphome/platformio/pch.py.script index 6e0ba70ddc..45bb9ddb05 100644 --- a/esphome/platformio/pch.py.script +++ b/esphome/platformio/pch.py.script @@ -5,6 +5,7 @@ import posixpath import re import shlex import subprocess +import traceback # pylint: disable=E0602 Import("env", "projenv") # noqa: F821 @@ -107,6 +108,11 @@ def _setup_pch() -> None: try: version = platform.get_package_version(package) 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 @@ -141,8 +147,15 @@ def _setup_pch() -> None: for local in sorted(p for p in inc_dir.rglob("*") if p.is_file()): try: data = local.read_bytes() - except OSError: - data = b"" + 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(data) digest.update(b"\0") @@ -181,27 +194,30 @@ def _setup_pch() -> None: # 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 - 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 ".gch" in probe.stderr: - error = f"toolchain cannot load the pch: {probe.stderr.strip()}" + 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) if error is not None: print("ESPHome: precompiled header failed; compiling without it") print(error) @@ -230,5 +246,6 @@ def _setup_pch() -> None: try: _setup_pch() -except Exception as err: # noqa: BLE001 -- a speedup must never break the build - print(f"ESPHome: precompiled header setup failed; compiling without it: {err}") +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/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index 9ad2bbd2b9..6461b3ad75 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -85,13 +85,13 @@ 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() -> None: +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.""" - directory = f"{ABS}build/proj" + (tmp_path / "esphome_pch.h").write_text("") entry = _entry( - directory, - f"{directory}/src/esphome/x.cpp", + str(tmp_path), + f"{tmp_path}/src/esphome/x.cpp", "g++ -include esphome_pch.h -c x.cpp", ) @@ -100,7 +100,34 @@ def test_parse_entry_resolves_force_include_path() -> None: idx = cxx_flags.index("-include") resolved = cxx_flags[idx + 1] assert Path(resolved).is_absolute() - assert resolved.endswith("build/proj/esphome_pch.h") + 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: diff --git a/tests/unit_tests/test_platformio_pch_script.py b/tests/unit_tests/test_platformio_pch_script.py index 4e4e5ca8e5..0f879eef47 100644 --- a/tests/unit_tests/test_platformio_pch_script.py +++ b/tests/unit_tests/test_platformio_pch_script.py @@ -26,12 +26,25 @@ 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.""" @@ -66,11 +79,17 @@ class _FakeSConsEnv(dict): self.prepended = CXXFLAGS -def _fake_cxx(tmp_path: Path, fail: bool = False, reject_pch: bool = False) -> Path: +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. + 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 = ( @@ -84,7 +103,7 @@ def _fake_cxx(tmp_path: Path, fail: bool = False, reject_pch: bool = False) -> P 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 += "exit 0\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 @@ -95,6 +114,7 @@ def _run_script( flags: list[str] | None = None, fail: bool = False, reject_pch: bool = False, + probe_exit: int = 0, env_vars: dict[str, str] | None = None, name: str = "dev", platform_cls: type[_FakePlatform] = _FakePlatform, @@ -103,7 +123,7 @@ def _run_script( 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, reject_pch=reject_pch) + cxx = _fake_cxx(tmp_path, fail=fail, reject_pch=reject_pch, probe_exit=probe_exit) 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) @@ -199,6 +219,22 @@ def test_pch_script_probe_rejection_falls_back( assert "toolchain cannot load the pch" 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.""" @@ -238,3 +274,24 @@ 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(os.geteuid() == 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