diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index ac9a22b18a2..42dda5c4599 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -90,15 +90,11 @@ def _cmake_quote(value: str) -> str: # esphome.espidf.bootloader compares the live macro against this copy and # falls back to the in-tree bootloader build if an IDF update changes it. IDF_BOOTLOADER_OVERRIDE = """\ -# ESPHome: with ESPHOME_USE_CACHED_BOOTLOADER the bootloader comes from a -# machine-global cache instead of the in-tree ExternalProject. This is IDF's -# own __build_process_project_includes macro with one added set(); see -# esphome/espidf/bootloader.py. +# ESPHome cached bootloader switch; see esphome/espidf/bootloader.py. if(ESPHOME_USE_CACHED_BOOTLOADER) macro(__build_process_project_includes) idf_build_get_property(sdkconfig_cmake SDKCONFIG_CMAKE) include(${sdkconfig_cmake}) - # ESPHome: the bootloader comes from the machine-global cache set(CONFIG_APP_BUILD_BOOTLOADER "") idf_build_get_property(build_properties __BUILD_PROPERTIES) foreach(build_property ${build_properties}) diff --git a/esphome/espidf/bootloader.py b/esphome/espidf/bootloader.py index b893d18b602..a9e11f7b931 100644 --- a/esphome/espidf/bootloader.py +++ b/esphome/espidf/bootloader.py @@ -1,16 +1,17 @@ """Machine-global bootloader cache for the native ESP-IDF toolchain. -The bootloader is its own CMake project that depends only on the IDF version, -target, compiler and the config subset its reduced Kconfig universe defines, -so one build serves every device that shares those. With -``CONFIG_APP_REPRODUCIBLE_BUILD`` (always on) the result is byte identical to -the in-tree ExternalProject build. Builds that configure signing, secure boot -or flash encryption never use the cache: their bootloader can depend on key -file contents, and they keep the stock in-tree path. +One build serves every device that shares the IDF version, target, compiler +and bootloader config; CONFIG_APP_REPRODUCIBLE_BUILD (always on) makes it +byte identical to the in-tree build. Signing/secure/encryption builds keep +the stock in-tree path: their bootloader can depend on key file contents. """ +# pylint: disable=protected-access +# This module is part of the espidf toolchain and shares its private helpers. + from __future__ import annotations +import contextlib import hashlib import json import logging @@ -25,7 +26,8 @@ from esphome.build_helpers.tool_runner import run_build_tool from esphome.const import KEY_ESP32, KEY_VARIANT from esphome.core import CORE, EsphomeError from esphome.espidf import toolchain, variant_to_idf_target -from esphome.helpers import write_file +from esphome.framework_helpers import _rename_with_retry +from esphome.helpers import rmtree, write_file _LOGGER = logging.getLogger(__name__) @@ -34,6 +36,7 @@ BOOTLOADER_CACHE_ENV = "ESPHOME_BOOTLOADER_CACHE" # entries then miss instead of being served stale. _CACHE_SCHEMA = "1" _SECURE_OPTION = re.compile(r"CONFIG_(SECURE_|FLASH_ENCRYPTION)") +_DISABLED_VALUES = frozenset(("n", "0", "")) _MACRO = re.compile( r"macro\(__build_process_project_includes\)(.*?)endmacro\(\)", re.DOTALL ) @@ -44,10 +47,7 @@ _OUTPUTS = ("bootloader.bin", "bootloader.elf", "bootloader.map") def bootloader_cache_enabled() -> bool: - """Whether this build may take its bootloader from the cache. - - False keeps IDF's in-tree bootloader build, exactly as before. - """ + """Whether this build may take its bootloader from the cache.""" cache = toolchain._cache() if cache.bootloader_enabled is None: cache.bootloader_enabled = _compute_enabled() @@ -76,16 +76,13 @@ def _compute_enabled() -> bool: def _has_secure_options(sdkconfig_path: Path) -> bool: """Whether any signing, secure boot or encryption option is enabled. - Reads ESPHome's sdkconfig snapshot, so raw ``sdkconfig_options`` count. - A disabled value (``n``/``0``/empty) is the compiled-out state ESPHome - writes by default and is safe to cache; anything else bypasses. + Disabled values (n/0/empty) are the compiled-out default and safe to cache. """ for line in sdkconfig_path.read_text(encoding="utf-8").splitlines(): name, _, value = line.partition("=") - if _SECURE_OPTION.match(name.strip()) and value.strip().strip('"') not in ( - "n", - "0", - "", + if ( + _SECURE_OPTION.match(name.strip()) + and value.strip().strip('"') not in _DISABLED_VALUES ): return True return False @@ -105,8 +102,7 @@ def _normalized_macro(text: str) -> list[str] | None: def idf_macro_matches() -> bool: """Whether IDF's macro still matches the copy the override replays. - A mismatch after an IDF update silently falls back to the in-tree - bootloader build; script/check_idf_py_equivalence.py reports it loudly. + A mismatch falls back to the in-tree build; the CI guard reports it loudly. """ from esphome.build_gen.espidf import ( BOOTLOADER_OVERRIDE_ADDED_LINE, @@ -169,34 +165,23 @@ def _load_build_config(build_dir: Path) -> dict | None: def _compiler_id() -> str | None: - """The app build's C compiler, resolved; the toolchain part of the key. - - IDF's toolchain file sets the compiler, so CMakeCache.txt has no plain - CMAKE_C_COMPILER entry; the compile database names it instead. None - means the cache cannot be trusted for this run and the caller falls - back to the in-tree build. - """ + """The app build's resolved C compiler; None falls back to in-tree.""" try: - commands = json.loads( - (toolchain._build_dir() / "compile_commands.json").read_text( + description = json.loads( + (toolchain._build_dir() / "project_description.json").read_text( encoding="utf-8" ) ) - compiler = commands[0]["command"].split()[0] + compiler = description["c_compiler"] except (OSError, ValueError, LookupError): - cache_path = toolchain._build_dir() / "CMakeCache.txt" - if not cache_path.is_file(): - return None - # Same install dir and version as the compiler itself. - compiler = toolchain._parse_cmakecache(cache_path).get("CMAKE_C_COMPILER_AR") + return None return os.path.realpath(compiler) if compiler else None def _subproject_cmake_args( cmake: str, python: str, sdkconfig: str, idf_path: str, project_dir: str ) -> list[str]: - """The configure argv, mirroring the ExternalProject in IDF's - components/bootloader/project_include.cmake for the pinned version.""" + """The configure argv, mirroring IDF's bootloader ExternalProject args.""" from esphome.components.esp32 import idf_version import esphome.config_validation as cv @@ -236,26 +221,27 @@ def _version_stamp() -> str: return "" -def _compute_key(names: list[str], app_config: dict, compiler: str) -> str: +def _key_payload(names: list[str], app_config: dict, compiler: str) -> dict: + """Everything that can influence the built bootloader.""" + return { + "schema": _CACHE_SCHEMA, + "idf": toolchain._get_core_framework_version(), + "source": toolchain._get_framework_source_override() or "", + "stamp": _version_stamp(), + "target": _idf_target(), + "compiler": compiler, + # Placeholder paths: any change to the invocation shape misses. + "args": _subproject_cmake_args( + "cmake", "@PYTHON@", "@SDKCONFIG@", "@IDF@", "@PROJECT@" + ), + "config": {n: app_config.get(n, _ABSENT) for n in sorted(names)}, + } + + +def _compute_key(payload: dict) -> str: """The cache entry name for this build's bootloader inputs.""" - payload = json.dumps( - { - "schema": _CACHE_SCHEMA, - "idf": toolchain._get_core_framework_version(), - "source": toolchain._get_framework_source_override() or "", - "stamp": _version_stamp(), - "target": _idf_target(), - "compiler": compiler, - # Placeholder paths: any change to the invocation shape misses. - "args": _subproject_cmake_args( - "cmake", "@PYTHON@", "@SDKCONFIG@", "@IDF@", "@PROJECT@" - ), - "config": {n: app_config.get(n, _ABSENT) for n in sorted(names)}, - }, - sort_keys=True, - separators=(",", ":"), - ) - return hashlib.sha256(payload.encode()).hexdigest()[:16] + text = json.dumps(payload, sort_keys=True, separators=(",", ":")) + return hashlib.sha256(text.encode()).hexdigest()[:16] def _build_standalone(build_dir: Path, verbose: bool) -> int: @@ -285,38 +271,55 @@ def _build_standalone(build_dir: Path, verbose: bool) -> int: return 0 -def _publish(build_dir: Path, key: str, meta: dict) -> Path: +def _publish(build_dir: Path, key: str, payload: dict) -> Path: """Move the distilled outputs into the cache; losing a race is fine.""" entry = _cache_root() / key - if (entry / "bootloader.bin").is_file(): - return entry stage = _cache_root() / f".stage-{key}-{os.getpid()}" stage.mkdir(parents=True, exist_ok=True) for name in _OUTPUTS: shutil.copy2(build_dir / name, stage / name) shutil.copy2(build_dir / "config" / "sdkconfig.json", stage / "sdkconfig.json") - write_file(stage / "meta.json", json.dumps(meta, indent=2, sort_keys=True)) + write_file(stage / "meta.json", json.dumps(payload, indent=2, sort_keys=True)) try: - stage.rename(entry) + _rename_with_retry(stage, entry) except OSError: # Another process published the same key first. - shutil.rmtree(stage, ignore_errors=True) + _remove_dir(stage) if not (entry / "bootloader.bin").is_file(): raise return entry -def _install_into_build(entry: Path) -> None: - """Copy the cached outputs to build/bootloader, the path every consumer - (factory image, OTA --bootloader, the CI guard) already reads.""" +def _remove_dir(path: Path) -> None: + """Best-effort rmtree with the repo's read-only and retry hardening.""" + with contextlib.suppress(OSError): + rmtree(path) + + +def _install_into_build(entry: Path) -> Path: + """Copy the cached outputs to build/bootloader, where every consumer reads.""" dest = toolchain._build_dir() / "bootloader" if (dest / "CMakeCache.txt").is_file(): # A leftover in-tree sub-build; its ninja state must not linger. - shutil.rmtree(dest) + rmtree(dest) dest.mkdir(parents=True, exist_ok=True) for name in _OUTPUTS: - if (src := entry / name).is_file(): - shutil.copy2(src, dest / name) + if not (src := entry / name).is_file(): + continue + target = dest / name + src_stat = src.stat() + try: + dest_stat = target.stat() + except OSError: + dest_stat = None + # copy2 keeps the source mtime, so a matching copy is already current. + if ( + dest_stat is None + or dest_stat.st_size != src_stat.st_size + or dest_stat.st_mtime != src_stat.st_mtime + ): + shutil.copy2(src, target) + return dest def _check_bootloader_size(bin_path: Path, app_config: dict) -> None: @@ -345,64 +348,63 @@ def _prune_stale_dirs() -> None: try: for path in _cache_root().glob(".*"): if path.is_dir() and path.stat().st_mtime < cutoff: - shutil.rmtree(path, ignore_errors=True) + _remove_dir(path) except OSError: pass +def _install_and_check(entry: Path, app_config: dict) -> int: + """Copy an entry into the build tree and verify it fits before the table.""" + dest = _install_into_build(entry) + _check_bootloader_size(dest / "bootloader.bin", app_config) + return 0 + + def ensure_cached_bootloader(verbose: bool = False) -> int: """Put the cached bootloader into build/bootloader, building on a miss. - Returns 0 on success. A nonzero tool exit code tells the caller to fall - back to the in-tree build; a too-large bootloader raises instead, since - the in-tree build would fail the same way. + Nonzero tells the caller to fall back to the in-tree build; a too-large + bootloader raises instead, since in-tree would fail the same way. """ + try: + return _ensure_cached_bootloader(verbose) + except OSError as err: + _LOGGER.warning("Bootloader cache failed: %s", err) + return 1 + + +def _ensure_cached_bootloader(verbose: bool) -> int: app_config = _load_build_config(toolchain._build_dir()) compiler = _compiler_id() if app_config is None or compiler is None: _LOGGER.debug("Bootloader cache unusable: app configure outputs missing") return 1 - entry = None if (names := _load_config_names()) is not None: - candidate = _cache_root() / _compute_key(names, app_config, compiler) - if (candidate / "bootloader.bin").is_file(): - entry = candidate - if entry is None: - tmp = _cache_root() / f".build-{os.getpid()}" - try: - if (rc := _build_standalone(tmp, verbose)) != 0: - return rc - if (built_config := _load_build_config(tmp)) is None: - _LOGGER.debug("Bootloader build produced no sdkconfig.json") - return 1 - names = _merge_config_names(built_config) - key = _compute_key(names, app_config, compiler) - entry = _publish( - tmp, - key, - { - "schema": _CACHE_SCHEMA, - "idf": toolchain._get_core_framework_version(), - "target": _idf_target(), - "compiler": compiler, - }, - ) - _LOGGER.info("Cached bootloader %s for later builds", key) - finally: - shutil.rmtree(tmp, ignore_errors=True) - _prune_stale_dirs() - _install_into_build(entry) - _check_bootloader_size( - toolchain._build_dir() / "bootloader" / "bootloader.bin", app_config - ) - return 0 + entry = _cache_root() / _compute_key(_key_payload(names, app_config, compiler)) + if (entry / "bootloader.bin").is_file(): + return _install_and_check(entry, app_config) + tmp = _cache_root() / f".build-{os.getpid()}" + try: + if (rc := _build_standalone(tmp, verbose)) != 0: + return rc + if (built_config := _load_build_config(tmp)) is None: + _LOGGER.debug("Bootloader build produced no sdkconfig.json") + return 1 + names = _merge_config_names(built_config) + payload = _key_payload(names, app_config, compiler) + key = _compute_key(payload) + entry = _publish(tmp, key, payload) + _LOGGER.info("Cached bootloader %s for later builds", key) + finally: + _remove_dir(tmp) + _prune_stale_dirs() + return _install_and_check(entry, app_config) def inject_bootloader_flash_file(flash_data: dict, build_dir: Path) -> None: """Add the cached bootloader to a flasher_args ``flash_files`` map. - In cached mode IDF writes no bootloader entry; stock and bypass trees - already carry one, which makes this a no-op there. + Cached mode has no IDF-written entry; stock/bypass trees do (no-op there). """ flash_files = flash_data.setdefault("flash_files", {}) if any("bootloader/" in name for name in flash_files.values()): diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 28d78abbaa7..162e5895932 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -38,7 +38,6 @@ class _CacheData: env: dict[str, dict[str, str]] = field(default_factory=dict) cmake_output: dict[Path, str] = field(default_factory=dict) bootloader_enabled: bool | None = None - bootloader_fallback: bool = False cmake_tools: dict[Path, dict[str, Path]] = field(default_factory=dict) @@ -286,8 +285,6 @@ def _cache_entries_changed() -> bool: def _use_cached_bootloader() -> bool: """Whether this run takes the bootloader from the machine-global cache.""" - if _cache().bootloader_fallback: - return False from esphome.espidf import bootloader return bootloader.bootloader_cache_enabled() @@ -300,9 +297,8 @@ def _configure_defines() -> dict[str, str]: "PYTHON": _get_idf_tool("python"), "ESP_PLATFORM": "1", **_cache_entries(), - # ESPHome's own switch, appended after idf.py's entries. idf.py - # never passes it and cmake keeps the cached value, so an idf.py - # run against the tree stays in the same mode. + # ESPHome's own switch; idf.py never passes it and cmake keeps the + # cached value, so idf.py runs against the tree stay in the same mode. "ESPHOME_USE_CACHED_BOOTLOADER": "1" if _use_cached_bootloader() else "0", } @@ -696,7 +692,7 @@ def run_compile(config, verbose: bool) -> int: if bootloader.ensure_cached_bootloader(verbose) != 0: # The stock in-tree build still works; flip this run over to it. _LOGGER.warning("Cached bootloader unavailable; building it in-tree") - _cache().bootloader_fallback = True + _cache().bootloader_enabled = False if (rc := run_reconfigure(verbose)) != 0: return rc diff --git a/script/check_idf_py_equivalence.py b/script/check_idf_py_equivalence.py index fcd97798890..40c755a7d47 100755 --- a/script/check_idf_py_equivalence.py +++ b/script/check_idf_py_equivalence.py @@ -39,12 +39,22 @@ TOP_NINJA_LOG = "build/.ninja_log" def _ninja_logs(build_path: Path) -> list[str]: + """The mode comes from the configured tree, so a missing sub-build log + stays an error in the mode that requires one.""" logs = [TOP_NINJA_LOG] - if (build_path / "build" / "bootloader" / "build.ninja").is_file(): + if not _uses_cached_bootloader(build_path): logs.append("build/bootloader/.ninja_log") return logs +def _uses_cached_bootloader(build_path: Path) -> bool: + cache = build_path / "build" / "CMakeCache.txt" + for line in cache.read_text(encoding="utf-8").splitlines(): + if line.startswith("ESPHOME_USE_CACHED_BOOTLOADER:"): + return line.partition("=")[2].strip() == "1" + return False + + BOOTLOADER_BYPRODUCT = re.compile(r"(^|/build/)bootloader/") WORK_SUFFIXES = (".obj", ".o", ".a", ".elf", ".map", ".bin", ".ld") DEFAULT_GLOB = "tests/test_build_components/build/.esphome/build/*" @@ -119,8 +129,10 @@ def check(build_path: Path) -> list[str]: if not bootloader.idf_macro_matches(): return [ - "IDF changed __build_process_project_includes; update " - "IDF_BOOTLOADER_OVERRIDE in esphome/build_gen/espidf.py" + ( + "IDF changed __build_process_project_includes; update " + "IDF_BOOTLOADER_OVERRIDE in esphome/build_gen/espidf.py" + ) ] env = toolchain._get_idf_env(version) python = toolchain._get_idf_tool("python") diff --git a/tests/script/test_check_idf_py_equivalence.py b/tests/script/test_check_idf_py_equivalence.py index d0bc5ca73ca..f0914958920 100644 --- a/tests/script/test_check_idf_py_equivalence.py +++ b/tests/script/test_check_idf_py_equivalence.py @@ -28,17 +28,19 @@ BOOTLOADER_LOG = "build/bootloader/.ninja_log" ALL_LOGS = (guard.TOP_NINJA_LOG, BOOTLOADER_LOG) -def _make_tree(tmp_path: Path, bootloader_subbuild: bool = True) -> Path: - """A fake build tree; the sub-build shape mirrors a bypass/stock build, - without it the cached-bootloader shape (bin present, no sub-build).""" +def _make_tree(tmp_path: Path, cached_bootloader: bool = False) -> Path: + """A fake build tree; stock/bypass shape by default, or the cached + shape (define set to 1, bin present, no sub-build).""" tree = tmp_path / "config" / ".esphome" / "build" / "dev" build = tree / "build" - logs = ALL_LOGS if bootloader_subbuild else (guard.TOP_NINJA_LOG,) + logs = (guard.TOP_NINJA_LOG,) if cached_bootloader else ALL_LOGS for name in (*guard.watched("dev"), *logs): (tree / name).parent.mkdir(parents=True, exist_ok=True) (tree / name).write_bytes(b"x") - if bootloader_subbuild: - (build / "bootloader" / "build.ninja").write_bytes(b"x") + define = "1" if cached_bootloader else "0" + (build / "CMakeCache.txt").write_text( + f"ESPHOME_USE_CACHED_BOOTLOADER:UNINITIALIZED={define}\n" + ) (build / "project_description.json").write_text( json.dumps( { @@ -64,6 +66,7 @@ def _run_check( side_effect: Callable[[list[str]], None] = lambda cmd: None, rc: int = 0, esphome_rcs: tuple[int, int] = (0, 0), + macro_matches: bool = True, ) -> tuple[list[str], list[list[str]]]: """Run check() with idf.py replaced by ``side_effect``; return problems, calls.""" calls: list[list[str]] = [] @@ -79,7 +82,7 @@ def _run_check( patch.object(toolchain, "_get_idf_path", return_value=Path("/idf")), patch.object(toolchain, "run_reconfigure", return_value=esphome_rcs[0]), patch.object(toolchain, "_run_ninja", return_value=esphome_rcs[1]), - patch.object(bootloader, "idf_macro_matches", return_value=True), + patch.object(bootloader, "idf_macro_matches", return_value=macro_matches), patch.object(guard.subprocess, "run", side_effect=run), ): return guard.check(tree), calls @@ -215,28 +218,32 @@ def test_main_rejects_a_path_that_is_not_a_tree( def test_check_accepts_a_cached_bootloader_tree(tmp_path: Path) -> None: """No bootloader sub-build is the cached shape, not a missing input.""" - tree = _make_tree(tmp_path, bootloader_subbuild=False) + tree = _make_tree(tmp_path, cached_bootloader=True) problems, calls = _run_check(tree) assert problems == [] assert len(calls) == 2 +def test_check_requires_the_sub_log_on_a_stock_tree(tmp_path: Path) -> None: + """The mode comes from the define, so a vanished sub-build stays an error.""" + tree = _make_tree(tmp_path) + (tree / BOOTLOADER_LOG).unlink() + problems, calls = _run_check(tree) + assert problems == [f"missing {BOOTLOADER_LOG}"] + assert calls == [] + + def test_check_fails_loudly_when_the_idf_macro_changed(tmp_path: Path) -> None: """An IDF bump that rewrites the overridden macro must fail CI.""" tree = _make_tree(tmp_path) - with ( - patch.object(toolchain, "_get_idf_env", return_value={}), - patch.object(toolchain, "_get_idf_tool", return_value="/py"), - patch.object(toolchain, "_get_idf_path", return_value=Path("/idf")), - patch.object(bootloader, "idf_macro_matches", return_value=False), - patch.object(guard.subprocess, "run") as mock_run, - ): - problems = guard.check(tree) + problems, calls = _run_check(tree, macro_matches=False) assert problems == [ - "IDF changed __build_process_project_includes; update " - "IDF_BOOTLOADER_OVERRIDE in esphome/build_gen/espidf.py" + ( + "IDF changed __build_process_project_includes; update " + "IDF_BOOTLOADER_OVERRIDE in esphome/build_gen/espidf.py" + ) ] - mock_run.assert_not_called() + assert calls == [] def test_main_without_build_trees( diff --git a/tests/unit_tests/test_espidf_bootloader.py b/tests/unit_tests/test_espidf_bootloader.py index cd172c19443..03f784f6cee 100644 --- a/tests/unit_tests/test_espidf_bootloader.py +++ b/tests/unit_tests/test_espidf_bootloader.py @@ -2,6 +2,8 @@ # pylint: disable=protected-access +from collections.abc import Iterator +from contextlib import contextmanager import json import os from pathlib import Path @@ -191,13 +193,18 @@ def test_idf_macro_mismatch_detected(tmp_path: Path) -> None: # ------------------------------------------------------------------ cache key -def _key(names: list[str], config: dict, compiler: str = "/tc/gcc") -> str: +def _key( + names: list[str], + config: dict, + compiler: str = "/tc/gcc", + version: str = "5.5.5", +) -> str: with ( - patch.object(toolchain, "_get_core_framework_version", return_value="5.5.5"), + patch.object(toolchain, "_get_core_framework_version", return_value=version), patch.object(toolchain, "_get_framework_source_override", return_value=None), patch.object(bootloader, "_version_stamp", return_value="v5.5.5"), ): - return bootloader._compute_key(names, config, compiler) + return bootloader._compute_key(bootloader._key_payload(names, config, compiler)) def test_key_is_stable_and_name_order_independent() -> None: @@ -216,13 +223,8 @@ def test_key_changes_with_each_input() -> None: base = _key(["A"], {"A": "1"}) assert _key(["A"], {"A": "2"}) != base assert _key(["A"], {"A": "1"}, compiler="/other/gcc") != base - with ( - patch.object(toolchain, "_get_core_framework_version", return_value="6.1.0"), - patch.object(toolchain, "_get_framework_source_override", return_value=None), - patch.object(bootloader, "_version_stamp", return_value="v5.5.5"), - ): - CORE.data[KEY_ESP32][KEY_IDF_VERSION] = cv.Version(6, 1, 0) - assert bootloader._compute_key(["A"], {"A": "1"}, "/tc/gcc") != base + CORE.data[KEY_ESP32][KEY_IDF_VERSION] = cv.Version(6, 1, 0) + assert _key(["A"], {"A": "1"}, version="6.1.0") != base def test_key_ignores_unlisted_config_values() -> None: @@ -288,25 +290,27 @@ def test_load_build_config(tmp_path: Path) -> None: # ------------------------------------------------------------- compiler id -def test_compiler_id_from_compile_commands(tmp_path: Path) -> None: +def test_compiler_id_from_project_description(tmp_path: Path) -> None: build = CORE.relative_build_path("build") build.mkdir(parents=True) - (build / "compile_commands.json").write_text( - json.dumps([{"command": "/tools/xtensa-esp32-elf-gcc -c a.c"}]) + (build / "project_description.json").write_text( + json.dumps({"c_compiler": "/tools/xtensa-esp32-elf-gcc"}) ) - assert bootloader._compiler_id() == "/tools/xtensa-esp32-elf-gcc" + assert bootloader._compiler_id() == os.path.realpath("/tools/xtensa-esp32-elf-gcc") -def test_compiler_id_falls_back_to_cmakecache(tmp_path: Path) -> None: - build = CORE.relative_build_path("build") - build.mkdir(parents=True) - (build / "CMakeCache.txt").write_text( - "CMAKE_C_COMPILER_AR:FILEPATH=/tools/xtensa-esp32-elf-gcc-ar\n" - ) - assert bootloader._compiler_id() == "/tools/xtensa-esp32-elf-gcc-ar" - - -def test_compiler_id_none_without_configure_outputs() -> None: +@pytest.mark.parametrize( + "description", + [None, "not json", "{}", '{"c_compiler": ""}'], + ids=["missing", "corrupt", "no-key", "empty"], +) +def test_compiler_id_none_without_a_usable_description( + tmp_path: Path, description: str | None +) -> None: + if description is not None: + build = CORE.relative_build_path("build") + build.mkdir(parents=True) + (build / "project_description.json").write_text(description) assert bootloader._compiler_id() is None @@ -344,18 +348,25 @@ def test_subproject_args_for_idf_6() -> None: # -------------------------------------------------------- standalone build -def test_build_standalone_runs_cmake_then_ninja(tmp_path: Path) -> None: - build_dir = tmp_path / "work" +@contextmanager +def _standalone_tools(run_rc: int) -> Iterator: + """Stub the tool lookup and runner; yield the run_build_tool mock.""" with ( patch.object(toolchain, "_get_idf_tool", side_effect=lambda n: f"/tools/{n}"), patch.object(toolchain, "_get_idf_path", return_value=Path("/idf")), patch.object(toolchain, "_tool_env", return_value={"A": "1"}), - patch.object(bootloader, "run_build_tool", return_value=0) as mock_run, + patch.object(bootloader, "run_build_tool", return_value=run_rc) as mock_run, ): + yield mock_run + + +def test_build_standalone_runs_cmake_then_ninja(tmp_path: Path) -> None: + build_dir = tmp_path / "work" + with _standalone_tools(0) as mock_run: assert bootloader._build_standalone(build_dir, verbose=True) == 0 cmake_call, ninja_call = mock_run.call_args_list assert cmake_call.args[0][0] == "/tools/cmake" - assert cmake_call.args[0][-1] == "/idf/components/bootloader/subproject" + assert cmake_call.args[0][-1] == f"{Path('/idf')}/components/bootloader/subproject" assert cmake_call.kwargs["cwd"] == build_dir assert cmake_call.kwargs["filter_lines"] is None # verbose assert cmake_call.kwargs["log_path"] == build_dir / "log" / "cmake_output.log" @@ -365,10 +376,7 @@ def test_build_standalone_runs_cmake_then_ninja(tmp_path: Path) -> None: def test_build_standalone_failure_prints_hints(tmp_path: Path) -> None: build_dir = tmp_path / "work" with ( - patch.object(toolchain, "_get_idf_tool", side_effect=lambda n: f"/tools/{n}"), - patch.object(toolchain, "_get_idf_path", return_value=Path("/idf")), - patch.object(toolchain, "_tool_env", return_value={}), - patch.object(bootloader, "run_build_tool", return_value=2) as mock_run, + _standalone_tools(2) as mock_run, patch.object(toolchain, "_print_hints") as mock_hints, ): assert bootloader._build_standalone(build_dir, verbose=False) == 2 @@ -389,7 +397,7 @@ def _make_built_tree(build_dir: Path) -> None: (build_dir / "config" / "sdkconfig.json").write_text('{"A": "1"}') -def test_publish_creates_entry(tmp_path: Path) -> None: +def test_publish_creates_entry_with_the_key_payload(tmp_path: Path) -> None: root = tmp_path / "cache" build = tmp_path / "work" _make_built_tree(build) @@ -397,20 +405,11 @@ def test_publish_creates_entry(tmp_path: Path) -> None: entry = bootloader._publish(build, "k" * 16, {"idf": "5.5.5"}) assert entry == root / ("k" * 16) assert (entry / "bootloader.bin").read_bytes() == b"bootloader.bin" + # meta.json is the full key payload, for debugging cache misses. assert json.loads((entry / "meta.json").read_text()) == {"idf": "5.5.5"} assert not list(root.glob(".stage-*")) -def test_publish_existing_entry_short_circuits(tmp_path: Path) -> None: - root = tmp_path / "cache" - entry = root / ("k" * 16) - entry.mkdir(parents=True) - (entry / "bootloader.bin").write_bytes(b"winner") - with patch.object(bootloader, "_cache_root", return_value=root): - assert bootloader._publish(tmp_path / "work", "k" * 16, {}) == entry - assert (entry / "bootloader.bin").read_bytes() == b"winner" - - def test_publish_lost_race_reuses_winner(tmp_path: Path) -> None: """A rename beaten by another process keeps that process's entry.""" root = tmp_path / "cache" @@ -418,14 +417,14 @@ def test_publish_lost_race_reuses_winner(tmp_path: Path) -> None: _make_built_tree(build) entry = root / ("k" * 16) - def rename_raises(self: Path, target: Path) -> None: + def rename_raises(src: Path, dst: Path) -> None: entry.mkdir(parents=True) (entry / "bootloader.bin").write_bytes(b"winner") raise OSError("directory not empty") with ( patch.object(bootloader, "_cache_root", return_value=root), - patch.object(Path, "rename", rename_raises), + patch.object(bootloader, "_rename_with_retry", rename_raises), ): assert bootloader._publish(build, "k" * 16, {}) == entry assert (entry / "bootloader.bin").read_bytes() == b"winner" @@ -438,7 +437,7 @@ def test_publish_rename_failure_without_winner_raises(tmp_path: Path) -> None: _make_built_tree(build) with ( patch.object(bootloader, "_cache_root", return_value=root), - patch.object(Path, "rename", side_effect=OSError("denied")), + patch.object(bootloader, "_rename_with_retry", side_effect=OSError("denied")), pytest.raises(OSError), ): bootloader._publish(build, "k" * 16, {}) @@ -457,13 +456,26 @@ def test_install_into_build_replaces_stale_subbuild(tmp_path: Path) -> None: dest.mkdir(parents=True) (dest / "CMakeCache.txt").write_text("stale in-tree sub-build") (dest / "old.obj").write_bytes(b"x") - bootloader._install_into_build(entry) + assert bootloader._install_into_build(entry) == dest assert (dest / "bootloader.bin").read_bytes() == b"bin" assert not (dest / "CMakeCache.txt").exists() assert not (dest / "old.obj").exists() assert not (dest / "bootloader.map").exists() +def test_install_into_build_skips_current_copies(tmp_path: Path) -> None: + """copy2 keeps the source mtime, so a matching copy is not rewritten.""" + entry = tmp_path / "entry" + entry.mkdir() + (entry / "bootloader.bin").write_bytes(b"bin") + dest = bootloader._install_into_build(entry) + first = (dest / "bootloader.bin").stat() + with patch.object(bootloader.shutil, "copy2") as mock_copy: + bootloader._install_into_build(entry) + mock_copy.assert_not_called() + assert (dest / "bootloader.bin").stat().st_mtime == first.st_mtime + + # -------------------------------------------------------------- size check @@ -494,8 +506,10 @@ def test_ensure_fails_soft_without_configure_outputs() -> None: assert bootloader.ensure_cached_bootloader() == 1 -def _orchestration_env(tmp_path: Path): - """Patches shared by the ensure_cached_bootloader tests.""" +@contextmanager +def _orchestration_env(tmp_path: Path) -> Iterator[Path]: + """A configured app tree plus the patches every ensure_* test shares; + yields the cache root.""" root = tmp_path / "cache-root" build = CORE.relative_build_path("build") build.mkdir(parents=True, exist_ok=True) @@ -509,14 +523,16 @@ def _orchestration_env(tmp_path: Path): } ) ) - return root - - -def test_ensure_cache_hit_installs_without_building(tmp_path: Path) -> None: - root = _orchestration_env(tmp_path) with ( patch.object(bootloader, "_cache_root", return_value=root), patch.object(bootloader, "_compiler_id", return_value="/tc/gcc"), + ): + yield root + + +def test_ensure_cache_hit_installs_without_building(tmp_path: Path) -> None: + with ( + _orchestration_env(tmp_path) as root, patch.object(bootloader, "_compute_key", return_value="deadbeefdeadbeef"), patch.object(bootloader, "_build_standalone") as mock_build, ): @@ -532,35 +548,30 @@ def test_ensure_cache_hit_installs_without_building(tmp_path: Path) -> None: def test_ensure_cache_miss_builds_and_publishes(tmp_path: Path) -> None: """Known names but no matching entry: build, harvest, publish.""" - root = _orchestration_env(tmp_path) - root.mkdir(parents=True) - (root / "config_names.json").write_text('["A"]') def fake_build(build_dir: Path, verbose: bool) -> int: _make_built_tree(build_dir) return 0 with ( - patch.object(bootloader, "_cache_root", return_value=root), - patch.object(bootloader, "_compiler_id", return_value="/tc/gcc"), + _orchestration_env(tmp_path) as root, patch.object(bootloader, "_compute_key", return_value="deadbeefdeadbeef"), patch.object(bootloader, "_build_standalone", side_effect=fake_build), ): + root.mkdir(parents=True) + (root / "config_names.json").write_text('["A"]') assert bootloader.ensure_cached_bootloader() == 0 # Entry published under the key, names harvested, work dir removed. assert (root / "deadbeefdeadbeef" / "bootloader.bin").is_file() - with patch.object(bootloader, "_cache_root", return_value=root): - assert "A" in bootloader._load_config_names() + assert "A" in json.loads((root / "config_names.json").read_text()) assert not list(root.glob(".build-*")) installed = CORE.relative_build_path("build", "bootloader", "bootloader.bin") assert installed.is_file() def test_ensure_returns_build_failure(tmp_path: Path) -> None: - root = _orchestration_env(tmp_path) with ( - patch.object(bootloader, "_cache_root", return_value=root), - patch.object(bootloader, "_compiler_id", return_value="/tc/gcc"), + _orchestration_env(tmp_path) as root, patch.object(bootloader, "_build_standalone", return_value=2), ): assert bootloader.ensure_cached_bootloader() == 2 @@ -568,20 +579,26 @@ def test_ensure_returns_build_failure(tmp_path: Path) -> None: def test_ensure_soft_fails_when_build_yields_no_config(tmp_path: Path) -> None: - root = _orchestration_env(tmp_path) - def build_without_config(build_dir: Path, verbose: bool) -> int: build_dir.mkdir(parents=True, exist_ok=True) return 0 with ( - patch.object(bootloader, "_cache_root", return_value=root), - patch.object(bootloader, "_compiler_id", return_value="/tc/gcc"), + _orchestration_env(tmp_path), patch.object(bootloader, "_build_standalone", side_effect=build_without_config), ): assert bootloader.ensure_cached_bootloader() == 1 +def test_ensure_turns_oserror_into_a_soft_failure(tmp_path: Path) -> None: + """Any filesystem surprise means fall back, never crash the build.""" + with ( + _orchestration_env(tmp_path), + patch.object(bootloader, "_build_standalone", side_effect=OSError("disk")), + ): + assert bootloader.ensure_cached_bootloader() == 1 + + def test_prune_removes_only_old_work_dirs(tmp_path: Path) -> None: """A concurrent build's fresh work dir must survive the prune.""" old = tmp_path / ".build-1" diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index 54ef2a05f76..f223a4a6de4 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -1117,12 +1117,6 @@ def test_get_cmake_cache_value_reads_the_configured_cache(setup_core: Path) -> N assert toolchain.get_cmake_cache_value("ESPHOME_MISSING") is None -def test_use_cached_bootloader_fallback_wins(setup_core: Path) -> None: - """A mid-run fallback pins the rest of the run to the in-tree build.""" - toolchain._cache().bootloader_fallback = True - assert toolchain._use_cached_bootloader() is False - - def test_use_cached_bootloader_delegates_to_predicate(setup_core: Path) -> None: from esphome.espidf import bootloader @@ -1185,7 +1179,8 @@ def test_run_compile_falls_back_when_bootloader_cache_fails( ): rc = toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) assert rc == reconfigure_rc - assert toolchain._cache().bootloader_fallback is True + # The fallback pins the memoized predicate for the rest of the run. + assert toolchain._cache().bootloader_enabled is False mock_reconfigure.assert_called_once_with(False) assert mock_ninja.called is (reconfigure_rc == 0)