[nrf52] Share the ccache between devices (#20028)

esphome/components/nrf52/__init__.py

@@ -27,6 +27,7 @@ from esphome.components.zephyr.const import (
     CONF_CDC_ACM,
     KEY_BOARD,
     KEY_BOOTLOADER,
+    KEY_SYSBUILD,
     KEY_ZEPHYR,
     CdcAcm,
 )
@@ -77,6 +78,7 @@ from .framework import (
     get_build_paths,
     setup_platformio_python_env,
     toolchain_tool,
+    wanted_west_projects,
 )
 
 # force import gpio to register pin schema
@@ -822,19 +824,22 @@ _PCH_SUM_PATH = "CMakeFiles/app.dir/cmake_pch.hxx.gch.sum"
 
 
 def _write_pch_checksum(build_dir: Path, source_dir: Path) -> None:
-    """Write the checksum ccache reads in place of the .gch. The app binary
-    dir only exists after the first configure; sysbuild nests it."""
-    app_dir = build_dir / "zephyr"
-    if not (app_dir / "CMakeCache.txt").is_file():
-        app_dir = build_dir
-    if not (app_dir / "CMakeCache.txt").is_file():
-        return
+    """Write the checksum ccache reads in place of the .gch; before the
+    first build too, or its compiles hash the path laden .gch instead.
+    The app image dir follows the SDK version, like get_elf_path;
+    2.9.2+ always wraps the build in sysbuild."""
+    app_dir = build_dir
+    if CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION] >= cv.Version(2, 9, 2):
+        app_dir = build_dir / "zephyr"
     checksum = pch.pch_checksum(
         CORE.relative_src_path(),
         pch.PCH_DEFAULT_HEADERS,
         (
             str(CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION]),
             zephyr_data()[KEY_BOARD],
+            # Kconfig inputs that reach autoconf.h without a .conf line
+            ",".join(sorted(wanted_west_projects())),
+            str(zephyr_data().get(KEY_SYSBUILD)),
             # What the Zephyr configuration is generated from
             *(
                 path.read_text(encoding="utf-8")

esphome/components/nrf52/framework.py

@@ -225,6 +225,19 @@ def get_build_env(ccache: str | None) -> dict:
         env.setdefault("CCACHE_DISABLE", "1")
     else:
         env.update(ccache_env(ccache, SDK_NRF_TOOLS_CACHE))
+        # Drop only the per build map entry (posix, CMake's spelling);
+        # its from side covers no compiled sources. A spaced path cannot
+        # survive ccache's space split list, so it stays hashed.
+        source_dir = CORE.relative_build_path("zephyr").as_posix()
+        if any(ch.isspace() for ch in source_dir):
+            _LOGGER.debug(
+                "Whitespace in %s; the per build map stays hashed", source_dir
+            )
+        else:
+            device_map = f"-fmacro-prefix-map={source_dir}=CMAKE_SOURCE_DIR"
+            env["CCACHE_IGNOREOPTIONS"] = (
+                f"{env.get('CCACHE_IGNOREOPTIONS', '')} {device_map}".strip()
+            )
     return env
 
 
@@ -301,21 +314,62 @@ def setup_platformio_python_env() -> None:
     _prepend_env_path("PATH", str(env_python_path.parent))
 
 
+def _patch_framework_file(path: Path, old: str, new: str) -> bool:
+    """Replace ``old`` with ``new`` in a framework script, atomically and
+    keeping the file mode (helpers.write_file would flatten it to 0o644).
+    Returns False when nothing matched."""
+    import tempfile
+
+    content = path.read_text(encoding="utf-8")
+    patched = content.replace(old, new)
+    if patched == content:
+        return False
+    # Unique sibling tmp: the install lock is best effort, so two builds
+    # may patch at once and a shared tmp name could rename a half
+    # written file into place.
+    fd, tmp_name = tempfile.mkstemp(dir=path.parent, suffix=".tmp")
+    tmp = Path(tmp_name)
+    try:
+        with os.fdopen(fd, "w", encoding="utf-8") as f:
+            f.write(patched)
+        shutil.copymode(path, tmp)
+        tmp.replace(path)
+    except BaseException:
+        tmp.unlink(missing_ok=True)
+        raise
+    return True
+
+
 def _patch_uf2conv_escape_sequences(framework_path: Path) -> None:
     # SDK v2.6.1 ships uf2conv.py with '\s+' — an unrecognised escape that
     # Python 3.12+ flags with SyntaxWarning (a future version will reject it).
     uf2conv = framework_path / "zephyr" / "scripts" / "build" / "uf2conv.py"
-    if not uf2conv.exists():
+    if uf2conv.exists():
+        _patch_framework_file(
+            uf2conv, "re.split('\\s+', line)", "re.split('\\\\s+', line)"
+        )
+
+
+def _patch_gen_defines_dts_path(framework_path: Path) -> None:
+    # The absolute zephyr.dts.pre path in the header's top comment is
+    # its only per device byte and blocks ccache sharing; emit the
+    # basename. Upstream candidate.
+    gen_defines = framework_path / "zephyr" / "scripts" / "dts" / "gen_defines.py"
+    if not gen_defines.exists():
         return
-    content = uf2conv.read_text(encoding="utf-8")
-    patched = content.replace("re.split('\\s+', line)", "re.split('\\\\s+', line)")
-    if patched == content:
+    if _patch_framework_file(
+        gen_defines, "  {edt.dts_path}", "  {os.path.basename(edt.dts_path)}"
+    ):
         return
-    # Write atomically so a concurrent build never sees a truncated file
-    tmp = uf2conv.with_suffix(".py.tmp")
-    tmp.write_text(patched, encoding="utf-8")
-    shutil.copymode(uf2conv, tmp)
-    tmp.replace(uf2conv)
+    if "{os.path.basename(edt.dts_path)}" not in gen_defines.read_text(
+        encoding="utf-8"
+    ):
+        # Upstream reformatted the comment; sharing silently degrading
+        # would be invisible, so say it out loud.
+        _LOGGER.warning(
+            "gen_defines.py no longer matches; the devicetree header "
+            "stays per device and ccache sharing between devices degrades"
+        )
 
 
 # West projects every build needs; components add others with include_west_project()
@@ -349,7 +403,7 @@ def bluetooth_west_projects() -> tuple[str, ...]:
     return ("tinycrypt",)
 
 
-def _wanted_west_projects() -> set[str]:
+def wanted_west_projects() -> set[str]:
     projects = set(_get_data().west_projects)
     # Zephyr 4.1 moved the Cortex-M core headers to cmsis_6
     if CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION] >= cv.Version(3, 1, 0):
@@ -587,7 +641,7 @@ def _check_and_install(version: str) -> None:
     framework_path = _get_framework_path(version)
     sentinel = framework_path / ".ready"
     zephyr_reqs = framework_path / "zephyr" / "scripts" / "requirements.txt"
-    projects = _wanted_west_projects()
+    projects = wanted_west_projects()
     if not sentinel.exists() or not zephyr_reqs.exists():
         _install_framework(env_python_path, framework_path, version, projects)
         framework_ver = CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION]
@@ -596,6 +650,8 @@ def _check_and_install(version: str) -> None:
         sentinel.touch()
     else:
         _fetch_missing_west_projects(env_python_path, framework_path, version, projects)
+    # Every run: existing installs need it too, and it is a no-op once applied
+    _patch_gen_defines_dts_path(framework_path)
 
     zephyr_sentinel = python_env_path / ".zephyr_reqs_ready"
     if (

tests/unit_tests/components/nrf52/test_pch.py

@@ -5,9 +5,11 @@ from unittest.mock import Mock, patch
 
 import pytest
 
+from esphome.build_helpers import pch
 from esphome.components import nrf52
 from esphome.components.nrf52 import framework
-from esphome.components.zephyr.const import KEY_BOARD
+from esphome.components.nrf52.toolchain import get_elf_path
+from esphome.components.zephyr.const import KEY_BOARD, KEY_SYSBUILD
 import esphome.config_validation as cv
 from esphome.const import KEY_CORE, KEY_FRAMEWORK_VERSION, Toolchain
 from esphome.core import CORE, EsphomeError
@@ -55,8 +57,6 @@ def test_cmake_lists_pch_block_disabled(tmp_path: Path) -> None:
 def test_the_zephyr_compiler_decides_on_windows(
     windows_gcc_rule: None, version: tuple[int, ...], on: bool
 ) -> None:
-    from esphome.build_helpers import pch
-
     # platformdirs would pick its Windows backend from the patched sys.platform
     with (
         patch.object(nrf52, "toolchain_tool", lambda name: Path(f"/sdk/{name}.exe")),
@@ -66,8 +66,16 @@ def test_the_zephyr_compiler_decides_on_windows(
     assert asked.call_args.args[0] == (Path("/sdk/g++.exe"),)
 
 
-def _write_checksum(tmp_path: Path, app: str, conf: str = "CONFIG_X=y\n") -> Path:
+def _write_checksum(
+    tmp_path: Path,
+    app: str,
+    conf: str = "CONFIG_X=y\n",
+    configured: bool = True,
+    sysbuild: bool = False,
+    version: cv.Version | None = None,
+) -> Path:
     """Write the checksum for a build dir whose app image sits in ``app``."""
+    version = version or cv.Version(2, 9, 2)
     CORE.build_path = tmp_path
     header = tmp_path / "src" / "esphome" / "core" / "pch_prefix.h"
     header.parent.mkdir(parents=True, exist_ok=True)
@@ -77,33 +85,65 @@ def _write_checksum(tmp_path: Path, app: str, conf: str = "CONFIG_X=y\n") -> Pat
     (source_dir / "prj.conf").write_text(conf)
     (source_dir / "CMakeLists.txt").write_text("not part of the checksum\n")
     build_dir = tmp_path / ".pioenvs" / "livingroom"
-    (build_dir / app).mkdir(parents=True, exist_ok=True)
-    (build_dir / app / "CMakeCache.txt").write_text("")
+    if configured:
+        (build_dir / app).mkdir(parents=True, exist_ok=True)
+        (build_dir / app / "CMakeCache.txt").write_text("")
     with (
-        patch.dict(CORE.data, {KEY_CORE: {KEY_FRAMEWORK_VERSION: "2.9.2"}}),
-        patch.object(nrf52, "zephyr_data", return_value={KEY_BOARD: "board"}),
+        patch.dict(CORE.data, {KEY_CORE: {KEY_FRAMEWORK_VERSION: version}}),
+        patch.object(
+            nrf52,
+            "zephyr_data",
+            return_value={KEY_BOARD: "board", KEY_SYSBUILD: sysbuild},
+        ),
     ):
         nrf52._write_pch_checksum(build_dir, source_dir)
     return build_dir / app / SUM
 
 
-@pytest.mark.parametrize("app", ["zephyr", "."])
-def test_pch_checksum_is_written_next_to_the_gch(tmp_path: Path, app: str) -> None:
-    """Sysbuild nests the app image; without it the build dir is the app."""
-    sum_path = _write_checksum(tmp_path, app)
+@pytest.mark.parametrize(
+    ("app", "version"), [("zephyr", cv.Version(2, 9, 2)), (".", cv.Version(2, 9, 1))]
+)
+def test_pch_checksum_is_written_next_to_the_gch(
+    tmp_path: Path, app: str, version: cv.Version
+) -> None:
+    """The SDK version decides the layout, like get_elf_path."""
+    sum_path = _write_checksum(tmp_path, app, version=version)
     assert len(sum_path.read_text().strip()) == 64
 
 
+def test_pch_checksum_tracks_the_kconfig_side_inputs(tmp_path: Path) -> None:
+    """West projects and the sysbuild flag reach autoconf without a .conf
+    line; the sum must move with them or stale objects get served."""
+    first = _write_checksum(tmp_path, "zephyr").read_text()
+    with patch.object(nrf52, "wanted_west_projects", return_value={"extra"}):
+        second = _write_checksum(tmp_path, "zephyr").read_text()
+    assert first != second
+    third = _write_checksum(tmp_path, "zephyr", sysbuild=True).read_text()
+    assert first != third
+
+
 def test_pch_checksum_tracks_the_zephyr_configuration(tmp_path: Path) -> None:
     first = _write_checksum(tmp_path, "zephyr").read_text()
     assert _write_checksum(tmp_path, "zephyr", "CONFIG_X=n\n").read_text() != first
 
 
-def test_pch_checksum_waits_for_the_first_configure(tmp_path: Path) -> None:
-    CORE.build_path = tmp_path
-    build_dir = tmp_path / ".pioenvs" / "livingroom"
-    nrf52._write_pch_checksum(build_dir, tmp_path / "zephyr")
-    assert not build_dir.exists()
+@pytest.mark.parametrize("version", [cv.Version(2, 9, 2), cv.Version(2, 9, 1)])
+def test_pch_checksum_lands_where_the_build_writes_the_image(
+    tmp_path: Path, version: cv.Version
+) -> None:
+    """One layout rule: the sum must sit in get_elf_path's app dir, or a
+    layout drift silently costs the first build's sharing."""
+    app = "zephyr" if version >= cv.Version(2, 9, 2) else "."
+    sum_path = _write_checksum(tmp_path, app, version=version)
+    CORE.name = "livingroom"
+    with patch.dict(CORE.data, {KEY_CORE: {KEY_FRAMEWORK_VERSION: version}}):
+        expected = get_elf_path().parent.parent / SUM
+    assert sum_path.resolve() == expected.resolve()
+
+
+def test_pch_checksum_written_before_the_first_configure(tmp_path: Path) -> None:
+    """The first build's compiles hash the sum in place of the .gch."""
+    assert _write_checksum(tmp_path, "zephyr", configured=False).is_file()
 
 
 def _fake_build_env(ccache: str | None) -> dict[str, str]:
@@ -130,6 +170,7 @@ def run_cmd(tmp_path: Path) -> Mock:
     CORE.name = "livingroom"
     CORE.toolchain = Toolchain.SDK_NRF
     CORE.data[KEY_CORE] = {KEY_FRAMEWORK_VERSION: cv.Version(3, 2, 0)}
+    (tmp_path / "build" / "zephyr").mkdir(parents=True)
     with (
         patch.dict("os.environ", {}, clear=True),
         patch.object(nrf52, "check_and_install"),

tests/unit_tests/test_nrf52_framework.py

@@ -11,7 +11,7 @@ from unittest.mock import ANY, call, patch
 import platformdirs
 import pytest
 
-from esphome.components.nrf52 import _resolve_toolchain
+from esphome.components.nrf52 import _resolve_toolchain, framework
 from esphome.components.nrf52.framework import (
     _PLATFORMIO_PENV_REQUIREMENTS,
     _REQUIREMENTS,
@@ -22,12 +22,12 @@ from esphome.components.nrf52.framework import (
     _get_toolchain_platform_info,
     _install_toolchain,
     _needs_venv_rebuild,
-    _wanted_west_projects,
     check_and_install,
     get_build_env,
     get_sdk_nrf_tools_path,
     include_west_project,
     setup_platformio_python_env,
+    wanted_west_projects,
 )
 from esphome.components.zephyr.const import KEY_SYSBUILD, KEY_ZEPHYR
 import esphome.config_validation as cv
@@ -539,7 +539,7 @@ class TestCheckAndInstall:
         """Zephyr 4.1 moved the Cortex-M core headers to the cmsis_6 module."""
         CORE.data[KEY_CORE] = {KEY_FRAMEWORK_VERSION: Version.parse(sdk_version)}
 
-        assert ("cmsis_6" in _wanted_west_projects()) is has_cmsis_6
+        assert ("cmsis_6" in wanted_west_projects()) is has_cmsis_6
 
     def test_default_projects_never_read_the_stamp(
         self,
@@ -1121,6 +1121,22 @@ def test_get_build_env_with_ccache(
     assert env["CCACHE_DEPEND"] == "1"
     assert env["CCACHE_BASEDIR"] == str((tmp_path / "build").resolve())
     assert "CCACHE_DISABLE" not in env
+    # Only the per build map entry leaves the hash; user maps stay in
+    assert env["CCACHE_IGNOREOPTIONS"] == (
+        f"-fmacro-prefix-map={(tmp_path / 'build' / 'zephyr').as_posix()}"
+        "=CMAKE_SOURCE_DIR"
+    )
+
+
+def test_get_build_env_skips_the_map_entry_on_whitespace(
+    nrf52_dirs: SimpleNamespace, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """The ignore list splits on spaces; a spaced path cannot be
+    expressed, so the entry is left hashed rather than emitted broken."""
+    monkeypatch.delenv("CCACHE_IGNOREOPTIONS", raising=False)
+    CORE.build_path = tmp_path / "with space" / "build"
+    env = get_build_env("/usr/bin/ccache")
+    assert "CCACHE_IGNOREOPTIONS" not in env
 
 
 def test_get_build_env_sdk_3_4_0_uses_toolchain_root(
@@ -1250,3 +1266,30 @@ def test_resolve_toolchain_rejects_unsupported() -> None:
     CORE.toolchain = Toolchain.ARDUINO
     with pytest.raises(cv.Invalid, match="Unsupported toolchain 'arduino'"):
         _resolve_toolchain({})
+
+
+def test_patch_gen_defines_relativizes_the_dts_path(tmp_path: Path) -> None:
+    """The absolute dts.pre path is the only per device byte in the
+    devicetree header; the patch makes gen_defines emit the basename."""
+    gen = tmp_path / "zephyr" / "scripts" / "dts" / "gen_defines.py"
+    gen.parent.mkdir(parents=True)
+    gen.write_text("s = f'DTS input file:\\n  {edt.dts_path}\\n'\n")
+    framework._patch_gen_defines_dts_path(tmp_path)
+    assert "{os.path.basename(edt.dts_path)}" in gen.read_text()
+    assert not list(gen.parent.glob("*.tmp"))  # no leftovers
+    before = gen.read_text()
+    framework._patch_gen_defines_dts_path(tmp_path)  # idempotent
+    assert gen.read_text() == before
+    framework._patch_gen_defines_dts_path(tmp_path / "absent")  # tolerant
+
+
+def test_patch_gen_defines_warns_when_the_anchor_is_gone(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """A reformatted upstream must not silently cost the sharing."""
+    gen = tmp_path / "zephyr" / "scripts" / "dts" / "gen_defines.py"
+    gen.parent.mkdir(parents=True)
+    gen.write_text("s = 'something else entirely'\n")
+    with caplog.at_level("WARNING"):
+        framework._patch_gen_defines_dts_path(tmp_path)
+    assert "gen_defines.py no longer matches" in caplog.text