Merge remote-tracking branch 'origin/esp8266-native-pch' into esp8266-native-pch

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()
         )

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(

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

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,