From 8defc48611622425bceb358baf50de2b1a1b6242 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 27 Aug 2026 10:34:17 -0500 Subject: [PATCH] Resolve the sysbuild app domain dir for the pch --- esphome/components/nrf52/__init__.py | 50 +++++++++++++------ tests/unit_tests/components/nrf52/test_pch.py | 50 +++++++++++++++++-- 2 files changed, 81 insertions(+), 19 deletions(-) diff --git a/esphome/components/nrf52/__init__.py b/esphome/components/nrf52/__init__.py index 0b68e7b02d..df86c1693e 100644 --- a/esphome/components/nrf52/__init__.py +++ b/esphome/components/nrf52/__init__.py @@ -847,17 +847,35 @@ def _generate_cmake_lists() -> bool: ) -def _prepare_pch(build_dir: Path) -> None: +def _app_build_dir(build_dir: Path) -> Path: + """The CMake binary dir of the application image. + + Sysbuild (SDK >= 2.9.2) nests the app in a domain dir named after the + app source dir ("zephyr"); older SDKs configure it at the top level. + In the non-sysbuild layout build_dir/zephyr is the Zephyr output dir, + which has no CMakeCache.txt, so the probe cannot misfire.""" + sysbuild_app = build_dir / "zephyr" + if (sysbuild_app / "CMakeCache.txt").is_file(): + return sysbuild_app + return build_dir + + +def _prepare_pch(app_dir: Path) -> None: """Build the .gch between the cmake and compile phases of west.""" if not pch_enabled(): - pch.discard_pch(build_dir) + pch.discard_pch(app_dir) pch.pch_disabled_degraded() return - autoconf = next(build_dir.glob("zephyr/include/generated/**/autoconf.h"), None) + # First, so OBJECT_DEPENDS is satisfied even when the pch degrades + app_dir.mkdir(parents=True, exist_ok=True) + write_file_if_changed( + app_dir / PCH_HEADER_NAME, pch_header_text(PCH_DEFAULT_HEADERS) + ) + autoconf = next(app_dir.glob("zephyr/include/generated/**/autoconf.h"), None) if autoconf is None: # Fail closed: autoconf.h is the .sum's Kconfig identity _LOGGER.warning("No autoconf.h found; compiling without the pch") - pch.discard_pch(build_dir) + pch.discard_pch(app_dir) pch.pch_degraded("autoconf.h missing") return try: @@ -866,11 +884,11 @@ def _prepare_pch(build_dir: Path) -> None: _LOGGER.warning( "Could not read %s; compiling without the pch: %s", autoconf, err ) - pch.discard_pch(build_dir) + pch.discard_pch(app_dir) pch.pch_degraded(f"autoconf unreadable: {err}") return pch.prepare_pch( - build_dir, + app_dir, PCH_DEFAULT_HEADERS, ( str(CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION]), @@ -934,11 +952,6 @@ def run_compile(args, config: ConfigType) -> bool: ] if pch_enabled(): - # Before the cmake phase: OBJECT_DEPENDS names the header - build_dir.mkdir(parents=True, exist_ok=True) - write_file_if_changed( - build_dir / PCH_HEADER_NAME, pch_header_text(PCH_DEFAULT_HEADERS) - ) # Consumers carry the -include; gate the ccache relaxation on it # (Zephyr auto-enables ccache as the compiler launcher when found) mark_pch_emitted() @@ -946,9 +959,13 @@ def run_compile(args, config: ConfigType) -> bool: # Split west into configure + build so the .gch is compiled from the # settled compile_commands.json flags between the two phases. Only - # when the DB is missing: any input change wipes the build dir, so - # an existing DB is settled, and --cmake-only always reconfigures - if not (build_dir / "compile_commands.json").is_file() and not run_command_ok( + # when the app DB is missing: any input change wipes the build dir, + # so an existing DB is settled, and --cmake-only reconfigures. + # Sysbuild configures the app image during its own configure, so the + # app's flags and autoconf.h are settled after this phase too. + if not ( + _app_build_dir(build_dir) / "compile_commands.json" + ).is_file() and not run_command_ok( west_cmd + ["--cmake-only", "--", "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON"], env=env, stream_output=True, @@ -957,13 +974,14 @@ def run_compile(args, config: ConfigType) -> bool: raise EsphomeError("nRF52 native build failed") # An optional speedup must never abort the build + app_dir = _app_build_dir(build_dir) try: - _prepare_pch(build_dir) + _prepare_pch(app_dir) except Exception: # noqa: BLE001 # pylint: disable=broad-exception-caught # Strict first: its own knob error must not mask the real failure strict = pch.pch_strict() # Raises itself if a stale .gch survives (silently wrong output) - pch.discard_pch(build_dir) + pch.discard_pch(app_dir) if strict: raise _LOGGER.warning( diff --git a/tests/unit_tests/components/nrf52/test_pch.py b/tests/unit_tests/components/nrf52/test_pch.py index 36539d91a6..370a5c43ba 100644 --- a/tests/unit_tests/components/nrf52/test_pch.py +++ b/tests/unit_tests/components/nrf52/test_pch.py @@ -80,6 +80,26 @@ def test_prepare_pch_unreadable_autoconf_fails_closed( assert "Could not read" in caplog.text +def test_app_build_dir_sysbuild_layout(build_dir: Path) -> None: + app = build_dir / "zephyr" + app.mkdir() + (app / "CMakeCache.txt").write_text("") + assert nrf52._app_build_dir(build_dir) == app + + +def test_app_build_dir_top_level_layout(build_dir: Path) -> None: + # Non-sysbuild: build_dir/zephyr is the Zephyr output dir, no cache + (build_dir / "zephyr").mkdir() + assert nrf52._app_build_dir(build_dir) == build_dir + + +def test_prepare_pch_writes_header_before_degrading(build_dir: Path) -> None: + # OBJECT_DEPENDS must be satisfied even when the pch degrades + with patch.object(nrf52.pch, "prepare_pch"): + nrf52._prepare_pch(build_dir) + assert (build_dir / "esphome_pch.h").is_file() + + def test_prepare_pch_extras_carry_build_identity(build_dir: Path) -> None: _write_autoconf(build_dir, "#define CONFIG_GPIO 1\n") with ( @@ -91,6 +111,7 @@ def test_prepare_pch_extras_carry_build_identity(build_dir: Path) -> None: patch.object(nrf52.pch, "prepare_pch") as prepare, ): nrf52._prepare_pch(build_dir) + assert (build_dir / "esphome_pch.h").is_file() (passed_dir, headers, extras) = prepare.call_args.args assert passed_dir == build_dir assert headers == nrf52.PCH_DEFAULT_HEADERS @@ -174,13 +195,22 @@ class TestRunCompilePhases: def test_missing_db_runs_cmake_phase(self, compile_ctx) -> None: run_cmd, prepare, build_dir = compile_ctx - run_cmd.side_effect = [True, False] # cmake-only ok, final build fails + results = iter([True, False]) # cmake-only ok, final build fails + + def west(cmd, **kwargs): + # Phase 1 configures the sysbuild app domain + app = build_dir / "zephyr" + app.mkdir(parents=True, exist_ok=True) + (app / "CMakeCache.txt").write_text("") + return next(results) + + run_cmd.side_effect = west with pytest.raises(EsphomeError, match="nRF52 native build failed"): self._run() assert "--cmake-only" in run_cmd.call_args_list[0].args[0] assert "--cmake-only" not in run_cmd.call_args_list[1].args[0] - assert prepare.called - assert (build_dir / "esphome_pch.h").is_file() + # The pch is prepared in the app domain dir, not the sysbuild root + assert prepare.call_args.args[0] == build_dir / "zephyr" def test_cmake_phase_failure_raises(self, compile_ctx) -> None: run_cmd, prepare, _ = compile_ctx @@ -202,6 +232,20 @@ class TestRunCompilePhases: assert "--cmake-only" not in run_cmd.call_args.args[0] assert prepare.called + def test_settled_sysbuild_db_skips_cmake_phase(self, compile_ctx) -> None: + run_cmd, prepare, build_dir = compile_ctx + app = build_dir / "zephyr" + app.mkdir(parents=True) + (build_dir / "CMakeCache.txt").write_text("") + (app / "CMakeCache.txt").write_text("") + (app / "compile_commands.json").write_text("[]") + run_cmd.side_effect = [False] + with pytest.raises(EsphomeError, match="nRF52 native build failed"): + self._run() + assert run_cmd.call_count == 1 + assert "--cmake-only" not in run_cmd.call_args.args[0] + assert prepare.call_args.args[0] == app + def test_disabled_skips_header_and_cmake_phase( self, monkeypatch: pytest.MonkeyPatch, compile_ctx ) -> None: