From 3044af34e31db4bec7181522f6efbcc25202abfb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 29 Sep 2026 19:58:59 +0200 Subject: [PATCH] [espidf] Move generic helpers out of the bootloader cache and gate the factory injection on the cache mode --- esphome/build_gen/espidf.py | 28 +++ esphome/espidf/bootloader.py | 131 +++++------ esphome/espidf/framework.py | 8 + esphome/espidf/toolchain.py | 24 ++- esphome/helpers.py | 10 + script/check_idf_py_equivalence.py | 5 +- tests/script/test_check_idf_py_equivalence.py | 5 +- tests/unit_tests/build_gen/test_espidf.py | 78 +++++++ tests/unit_tests/test_espidf_bootloader.py | 204 ++++++------------ tests/unit_tests/test_espidf_framework.py | 8 + tests/unit_tests/test_espidf_toolchain.py | 49 ++++- tests/unit_tests/test_helpers.py | 12 ++ 12 files changed, 336 insertions(+), 226 deletions(-) diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 001ba915625..8d04cb274b6 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -3,6 +3,7 @@ import json import logging from pathlib import Path +import re import textwrap from esphome.build_helpers import pch @@ -115,6 +116,33 @@ endif() # esphome.espidf.bootloader strips it before comparing with the live macro. BOOTLOADER_OVERRIDE_ADDED_LINE = 'set(CONFIG_APP_BUILD_BOOTLOADER "")' +_MACRO = re.compile( + r"macro\(__build_process_project_includes\)(.*?)endmacro\(\)", re.DOTALL +) + + +def _normalized_macro(text: str) -> list[str] | None: + """The macro body as comment-free, whitespace-collapsed lines.""" + if (match := _MACRO.search(text)) is None: + return None + return [ + re.sub(r"\s+", " ", line) + for raw in match.group(1).splitlines() + if (line := raw.split("#", 1)[0].strip()) + ] + + +def idf_macro_matches() -> bool: + """Whether IDF's macro still matches the copy the override replays.""" + from esphome.espidf import toolchain + + expected = _normalized_macro(IDF_BOOTLOADER_OVERRIDE) + expected.remove(BOOTLOADER_OVERRIDE_ADDED_LINE) + # pylint: disable-next=protected-access + build_cmake = toolchain._get_idf_path() / "tools" / "cmake" / "build.cmake" + live = _normalized_macro(build_cmake.read_text(encoding="utf-8")) + return live == expected + def get_project_cmakelists( minimal: bool = False, builtin_components: list[str] | None = None diff --git a/esphome/espidf/bootloader.py b/esphome/espidf/bootloader.py index f53e1c9acaf..03ba180e8f9 100644 --- a/esphome/espidf/bootloader.py +++ b/esphome/espidf/bootloader.py @@ -23,11 +23,10 @@ import time from esphome.build_helpers.ccache import parse_enable_env 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.espidf import toolchain from esphome.framework_helpers import _rename_with_retry -from esphome.helpers import rmtree, write_file +from esphome.helpers import read_json_file, rmtree, write_file _LOGGER = logging.getLogger(__name__) @@ -36,9 +35,6 @@ BOOTLOADER_CACHE_ENV = "ESPHOME_BOOTLOADER_CACHE" _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 -) # Marks a config name the app's sdkconfig does not define. _ABSENT = "\x00absent" _OUTPUTS = ("bootloader.bin", "bootloader.elf", "bootloader.map") @@ -70,7 +66,15 @@ def _compute_enabled() -> bool: if not os.access(get_idf_tools_path(), os.W_OK): # A read-only shared prefix would fail the cache on every build. return False - return idf_macro_matches() + if read_json_file(_failure_stamp()) == _stamp_inputs(): + # A previous run failed with the same inputs; stay in-tree. + return False + from esphome.build_gen.espidf import idf_macro_matches + + if not idf_macro_matches(): + _LOGGER.info("IDF changed its bootloader macro; building in-tree") + return False + return True except (OSError, KeyError, ValueError, EsphomeError) as err: _LOGGER.debug("Bootloader cache disabled: %s", err) return False @@ -88,33 +92,21 @@ def _has_secure_options(sdkconfig_path: Path) -> bool: return False -def _normalized_macro(text: str) -> list[str] | None: - """The macro body as comment-free, whitespace-collapsed lines.""" - if (match := _MACRO.search(text)) is None: - return None - return [ - re.sub(r"\s+", " ", line) - for raw in match.group(1).splitlines() - if (line := raw.split("#", 1)[0].strip()) - ] +def _failure_stamp() -> Path: + return toolchain._build_dir() / ".bootloader_cache_failed" -def idf_macro_matches() -> bool: - """Whether IDF's macro still matches the copy the override replays.""" - from esphome.build_gen.espidf import ( - BOOTLOADER_OVERRIDE_ADDED_LINE, - IDF_BOOTLOADER_OVERRIDE, - ) - - expected = _normalized_macro(IDF_BOOTLOADER_OVERRIDE) - expected.remove(BOOTLOADER_OVERRIDE_ADDED_LINE) - build_cmake = toolchain._get_idf_path() / "tools" / "cmake" / "build.cmake" - live = _normalized_macro(build_cmake.read_text(encoding="utf-8")) - return live == expected +def _stamp_inputs() -> dict: + return { + "idf": toolchain._get_core_framework_version(), + "target": toolchain._idf_target(), + } -def _idf_target() -> str: - return variant_to_idf_target(CORE.data[KEY_ESP32][KEY_VARIANT]) +def record_failure() -> None: + """Pin later runs to in-tree until the IDF version or target changes.""" + with contextlib.suppress(OSError, EsphomeError): + write_file(_failure_stamp(), json.dumps(_stamp_inputs())) def _cache_root() -> Path: @@ -124,21 +116,13 @@ def _cache_root() -> Path: get_idf_tools_path() / "bootloaders" / toolchain._get_core_framework_version() - / _idf_target() + / toolchain._idf_target() ) -def _load_json(path: Path): - """The parsed JSON file, or None when missing or unreadable.""" - try: - return json.loads(path.read_text(encoding="utf-8")) - except (OSError, ValueError): - return None - - def _load_config_names() -> list[str] | None: """The known union of config option names the bootloader consumes.""" - names = _load_json(_cache_root() / "config_names.json") + names = read_json_file(_cache_root() / "config_names.json") if isinstance(names, list) and names and all(isinstance(n, str) for n in names): return names return None @@ -154,21 +138,6 @@ def _merge_config_names(names) -> list[str]: return merged -def _load_build_config(build_dir: Path) -> dict | None: - """A build's generated config/sdkconfig.json, or None.""" - config = _load_json(build_dir / "config" / "sdkconfig.json") - return config if isinstance(config, dict) else None - - -def _compiler_id() -> str | None: - """The app build's resolved C compiler; None falls back to in-tree.""" - description = _load_json(toolchain._build_dir() / "project_description.json") - if not isinstance(description, dict): - return None - compiler = description.get("c_compiler") - return os.path.realpath(compiler) if compiler else None - - @dataclass(frozen=True, kw_only=True) class _SubprojectContract: """How the pinned IDF's bootloader ExternalProject configures the subproject.""" @@ -210,7 +179,7 @@ def _subproject_cmake_args( "Ninja", f"-DSDKCONFIG={sdkconfig}", f"-DIDF_PATH={idf_path}", - f"-DIDF_TARGET={_idf_target()}", + f"-DIDF_TARGET={toolchain._idf_target()}", "-DPYTHON_DEPS_CHECKED=1", f"-DPYTHON={python}", f"-DEXTRA_COMPONENT_DIRS={extra_dirs}", @@ -225,26 +194,16 @@ def _subproject_cmake_args( return args -def _version_stamp() -> str: - """The installed framework's version.txt, part of the key.""" - try: - return ( - (toolchain._get_idf_path() / "version.txt") - .read_text(encoding="utf-8") - .strip() - ) - except OSError: - return "" - - def _key_payload(names: list[str], app_config: dict, compiler: str) -> dict: """Everything that can influence the built bootloader.""" + from esphome.espidf.framework import read_idf_version_txt + return { "schema": _CACHE_SCHEMA, "idf": toolchain._get_core_framework_version(), "source": toolchain._get_framework_source_override() or "", - "stamp": _version_stamp(), - "target": _idf_target(), + "stamp": read_idf_version_txt(toolchain._get_idf_path()), + "target": toolchain._idf_target(), "compiler": compiler, # Placeholder paths: any change to the invocation shape misses. "args": _subproject_cmake_args( @@ -372,8 +331,8 @@ def ensure_cached_bootloader(verbose: bool = False) -> int: 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. """ - app_config = _load_build_config(toolchain._build_dir()) - compiler = _compiler_id() + app_config = toolchain._load_sdkconfig_json(toolchain._build_dir()) + compiler = toolchain._resolved_c_compiler() if app_config is None or compiler is None: _LOGGER.debug("Bootloader cache unusable: app configure outputs missing") return 1 @@ -384,6 +343,8 @@ def ensure_cached_bootloader(verbose: bool = False) -> int: return 1 if dest is None: return 1 + with contextlib.suppress(OSError): + _failure_stamp().unlink(missing_ok=True) # Outside the fail-safe net: the in-tree build would overflow the same way. _check_bootloader_size(dest / "bootloader.bin", app_config) return 0 @@ -400,7 +361,7 @@ def _install_cached(app_config: dict, compiler: str, verbose: bool) -> Path | No try: if _build_standalone(tmp, verbose) != 0: return None - if (built_config := _load_build_config(tmp)) is None: + if (built_config := toolchain._load_sdkconfig_json(tmp)) is None: _LOGGER.debug("Bootloader build produced no sdkconfig.json") return None names = _merge_config_names(built_config) @@ -417,13 +378,23 @@ def _install_cached(app_config: dict, compiler: str, verbose: bool) -> Path | No def inject_bootloader_flash_file(flash_data: dict, build_dir: Path) -> None: """Add the cached bootloader to a flasher_args ``flash_files`` map. - Cached mode has no IDF-written entry; stock/bypass trees do (no-op there). + Gated on the tree's mode: IDF also omits the entry on purpose for some + secure-boot builds, and those must stay exactly as IDF wrote them. """ + cmakecache = build_dir / "CMakeCache.txt" + if not cmakecache.is_file(): + return + if ( + toolchain._parse_cmakecache(cmakecache).get("ESPHOME_USE_CACHED_BOOTLOADER") + != "1" + ): + return + app_config = toolchain._load_sdkconfig_json(build_dir) + if ( + app_config is None + or not (build_dir / "bootloader" / "bootloader.bin").is_file() + ): + _LOGGER.error("Cached bootloader missing; factory image has no bootloader") + return flash_files = flash_data.setdefault("flash_files", {}) - if any("bootloader/" in name for name in flash_files.values()): - return - if not (build_dir / "bootloader" / "bootloader.bin").is_file(): - return - if (app_config := _load_build_config(build_dir)) is None: - return flash_files[hex(_bootloader_offset(app_config))] = "bootloader/bootloader.bin" diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 1a4b17efdb3..4137b0c15b9 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -481,6 +481,14 @@ def _clone_idf_with_submodules( ) +def read_idf_version_txt(framework_path: Path) -> str: + """The installed framework's version.txt stamp, or empty.""" + try: + return (framework_path / "version.txt").read_text(encoding="utf-8").strip() + except OSError: + return "" + + def _write_idf_version_txt(framework_path: Path, version: str) -> None: """Write /version.txt if missing. diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 162e5895932..acb40589359 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -25,7 +25,7 @@ from esphome.core import CORE, EsphomeError from esphome.espidf import variant_to_idf_target from esphome.espidf.framework import check_esp_idf_install, get_framework_env from esphome.espidf.size_summary import print_summary -from esphome.helpers import add_git_ceiling_directory, write_file +from esphome.helpers import add_git_ceiling_directory, read_json_file, write_file _LOGGER = logging.getLogger(__name__) @@ -248,6 +248,25 @@ def _build_dir() -> Path: return Path(os.path.realpath(CORE.build_path)) / "build" +def _idf_target() -> str: + return variant_to_idf_target(CORE.data[KEY_ESP32][KEY_VARIANT]) + + +def _load_sdkconfig_json(build_dir: Path) -> dict | None: + """A build's generated config/sdkconfig.json, or None.""" + config = read_json_file(build_dir / "config" / "sdkconfig.json") + return config if isinstance(config, dict) else None + + +def _resolved_c_compiler() -> str | None: + """The configured build's resolved C compiler, or None.""" + description = read_json_file(_build_dir() / "project_description.json") + if not isinstance(description, dict): + return None + compiler = description.get("c_compiler") + return os.path.realpath(compiler) if compiler else None + + def _cache_entries() -> dict[str, str]: """The ``-D`` entries idf.py passes to cmake, in idf.py's order.""" entries = {} @@ -444,7 +463,7 @@ def _builtin_component_cache_path() -> Path | None: """ if "IDF_PATH" in os.environ: return None - target = variant_to_idf_target(CORE.data[KEY_ESP32][KEY_VARIANT]) + target = _idf_target() excluded = CORE.cmake_args.get("EXCLUDE_COMPONENTS", "") excluded_key = hashlib.sha256(excluded.encode()).hexdigest()[:12] return ( @@ -693,6 +712,7 @@ def run_compile(config, verbose: bool) -> int: # The stock in-tree build still works; flip this run over to it. _LOGGER.warning("Cached bootloader unavailable; building it in-tree") _cache().bootloader_enabled = False + bootloader.record_failure() if (rc := run_reconfigure(verbose)) != 0: return rc diff --git a/esphome/helpers.py b/esphome/helpers.py index 3ccf0fe65ae..10b6f04a353 100644 --- a/esphome/helpers.py +++ b/esphome/helpers.py @@ -513,6 +513,16 @@ def walk_files(path: Path): yield Path(root) / name +def read_json_file(path: Path): + """The parsed JSON file, or None when missing or unreadable.""" + import json + + try: + return json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return None + + def read_file(path: Path) -> str: try: return path.read_text(encoding="utf-8") diff --git a/script/check_idf_py_equivalence.py b/script/check_idf_py_equivalence.py index f3b71cd5163..75aaaad0e5f 100755 --- a/script/check_idf_py_equivalence.py +++ b/script/check_idf_py_equivalence.py @@ -116,13 +116,14 @@ def _setup_core(build_path: Path, description: dict) -> tuple[str, str]: def check(build_path: Path) -> list[str]: """Return the problems found in one build tree.""" # pylint: disable=protected-access - from esphome.espidf import bootloader, toolchain + from esphome.build_gen.espidf import idf_macro_matches + from esphome.espidf import toolchain description = json.loads( (build_path / "build" / "project_description.json").read_text(encoding="utf-8") ) name, version = _setup_core(build_path, description) - if not bootloader.idf_macro_matches(): + if not idf_macro_matches(): return [ ( "IDF changed __build_process_project_includes; update " diff --git a/tests/script/test_check_idf_py_equivalence.py b/tests/script/test_check_idf_py_equivalence.py index f0914958920..7afee7521b1 100644 --- a/tests/script/test_check_idf_py_equivalence.py +++ b/tests/script/test_check_idf_py_equivalence.py @@ -13,8 +13,9 @@ sys.path.insert(0, str(Path(__file__).parent.parent.parent / "script")) import check_idf_py_equivalence as guard # noqa: E402 +from esphome.build_gen import espidf as build_gen # noqa: E402 from esphome.core import CORE # noqa: E402 -from esphome.espidf import bootloader, toolchain # noqa: E402 +from esphome.espidf import toolchain # noqa: E402 @pytest.fixture(autouse=True) @@ -82,7 +83,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=macro_matches), + patch.object(build_gen, "idf_macro_matches", return_value=macro_matches), patch.object(guard.subprocess, "run", side_effect=run), ): return guard.check(tree), calls diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index 4a842847962..28f3dc3d873 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/tests/unit_tests/build_gen/test_espidf.py @@ -623,3 +623,81 @@ def test_pch_checksum_disabled_writes_nothing( _make_pch_project(tmp_path) write_pch_checksum() assert not CORE.relative_build_path(_PCH_SUM_PATH).exists() + + +# The macro body as shipped in tools/cmake/build.cmake; byte identical in +# IDF 5.5.5 and 6.1.0, so one fixture covers both supported versions. +IDF_BUILD_CMAKE = """\ +some_other_cmake() + +macro(__build_process_project_includes) + # Include the sdkconfig cmake file, since the following operations require + # knowledge of config values. + idf_build_get_property(sdkconfig_cmake SDKCONFIG_CMAKE) + include(${sdkconfig_cmake}) + + # Make each build property available as a read-only variable + idf_build_get_property(build_properties __BUILD_PROPERTIES) + foreach(build_property ${build_properties}) + idf_build_get_property(val ${build_property}) + set(${build_property} "${val}") + endforeach() + + idf_build_get_property(build_component_targets __BUILD_COMPONENT_TARGETS) + + # Include each component's project_include.cmake + foreach(component_target ${build_component_targets}) + __component_get_property(dir ${component_target} COMPONENT_DIR) + __component_get_property(_name ${component_target} COMPONENT_NAME) + set(COMPONENT_NAME ${_name}) + set(COMPONENT_DIR ${dir}) + set(COMPONENT_PATH ${dir}) # this is deprecated, users are encouraged to use COMPONENT_DIR; + # retained for compatibility + if(EXISTS ${COMPONENT_DIR}/project_include.cmake) + include(${COMPONENT_DIR}/project_include.cmake) + endif() + endforeach() +endmacro() +""" + + +def _write_idf_build_cmake(tmp_path: Path, text: str = IDF_BUILD_CMAKE) -> Path: + idf = tmp_path / "idf" + (idf / "tools" / "cmake").mkdir(parents=True) + (idf / "tools" / "cmake" / "build.cmake").write_text(text) + return idf + + +def test_normalized_macro_strips_comments_and_whitespace() -> None: + from esphome.build_gen.espidf import _normalized_macro + + text = "macro(__build_process_project_includes)\n a( b ) # tail\n\n # only\n c(d)\nendmacro()" + assert _normalized_macro(text) == ["a( b )", "c(d)"] + + +def test_normalized_macro_none_without_macro() -> None: + from esphome.build_gen.espidf import _normalized_macro + + assert _normalized_macro("nothing here") is None + + +def test_idf_macro_matches_the_shipped_body(tmp_path: Path) -> None: + """The override's embedded copy must equal what build.cmake ships.""" + from esphome.build_gen.espidf import idf_macro_matches + from esphome.espidf import toolchain + + idf = _write_idf_build_cmake(tmp_path) + with patch.object(toolchain, "_get_idf_path", return_value=idf): + assert idf_macro_matches() is True + + +def test_idf_macro_mismatch_detected(tmp_path: Path) -> None: + from esphome.build_gen.espidf import idf_macro_matches + from esphome.espidf import toolchain + + changed = IDF_BUILD_CMAKE.replace( + "include(${sdkconfig_cmake})", "include(${sdkconfig_cmake} NEW_ARG)" + ) + idf = _write_idf_build_cmake(tmp_path, changed) + with patch.object(toolchain, "_get_idf_path", return_value=idf): + assert idf_macro_matches() is False diff --git a/tests/unit_tests/test_espidf_bootloader.py b/tests/unit_tests/test_espidf_bootloader.py index b6550817f58..a0b9a4e350d 100644 --- a/tests/unit_tests/test_espidf_bootloader.py +++ b/tests/unit_tests/test_espidf_bootloader.py @@ -11,46 +11,12 @@ from unittest.mock import patch import pytest +from esphome.build_gen import espidf as build_gen from esphome.components.esp32.const import KEY_ESP32, KEY_IDF_VERSION, KEY_VARIANT import esphome.config_validation as cv from esphome.core import CORE, EsphomeError from esphome.espidf import bootloader, toolchain -# The macro body as shipped in build.cmake; byte identical in IDF 5.5.5 and -# 6.1.0, so one fixture covers both supported versions. -IDF_BUILD_CMAKE = """\ -some_other_cmake() - -macro(__build_process_project_includes) - # Include the sdkconfig cmake file, since the following operations require - # knowledge of config values. - idf_build_get_property(sdkconfig_cmake SDKCONFIG_CMAKE) - include(${sdkconfig_cmake}) - - # Make each build property available as a read-only variable - idf_build_get_property(build_properties __BUILD_PROPERTIES) - foreach(build_property ${build_properties}) - idf_build_get_property(val ${build_property}) - set(${build_property} "${val}") - endforeach() - - idf_build_get_property(build_component_targets __BUILD_COMPONENT_TARGETS) - - # Include each component's project_include.cmake - foreach(component_target ${build_component_targets}) - __component_get_property(dir ${component_target} COMPONENT_DIR) - __component_get_property(_name ${component_target} COMPONENT_NAME) - set(COMPONENT_NAME ${_name}) - set(COMPONENT_DIR ${dir}) - set(COMPONENT_PATH ${dir}) # this is deprecated, users are encouraged to use COMPONENT_DIR; - # retained for compatibility - if(EXISTS ${COMPONENT_DIR}/project_include.cmake) - include(${COMPONENT_DIR}/project_include.cmake) - endif() - endforeach() -endmacro() -""" - @pytest.fixture(autouse=True) def _fresh_cache(setup_core: Path, monkeypatch: pytest.MonkeyPatch) -> None: @@ -73,13 +39,6 @@ def _write_snapshot(text: str = "") -> Path: return path -def _write_idf_build_cmake(tmp_path: Path, text: str = IDF_BUILD_CMAKE) -> Path: - idf = tmp_path / "idf" - (idf / "tools" / "cmake").mkdir(parents=True) - (idf / "tools" / "cmake" / "build.cmake").write_text(text) - return idf - - # ---------------------------------------------------------------- predicate @@ -112,14 +71,39 @@ def _tools_prefix(tmp_path: Path): def test_enabled_clean_snapshot_checks_macro(tmp_path: Path) -> None: _write_snapshot("CONFIG_FOO=y\n") - idf = _write_idf_build_cmake(tmp_path) with ( _tools_prefix(tmp_path), - patch.object(toolchain, "_get_idf_path", return_value=idf), + patch.object(build_gen, "idf_macro_matches", return_value=True), ): assert bootloader._compute_enabled() is True +def test_enabled_macro_mismatch_disables_with_a_log( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + _write_snapshot("CONFIG_FOO=y\n") + with ( + _tools_prefix(tmp_path), + patch.object(build_gen, "idf_macro_matches", return_value=False), + caplog.at_level("INFO"), + ): + assert bootloader._compute_enabled() is False + assert "changed its bootloader macro" in caplog.text + + +def test_enabled_after_recorded_failure(tmp_path: Path) -> None: + """A recorded failure pins in-tree until the IDF version or target changes.""" + _write_snapshot("CONFIG_FOO=y\n") + with ( + _tools_prefix(tmp_path), + patch.object(build_gen, "idf_macro_matches", return_value=True), + ): + bootloader.record_failure() + assert bootloader._compute_enabled() is False + CORE.data[KEY_ESP32][KEY_IDF_VERSION] = cv.Version(6, 1, 0) + assert bootloader._compute_enabled() is True + + def test_enabled_readonly_tools_prefix_disables(tmp_path: Path) -> None: """A shared read-only prefix would fail the cache on every build.""" _write_snapshot("CONFIG_FOO=y\n") @@ -184,34 +168,6 @@ def test_has_secure_options(tmp_path: Path, text: str, secure: bool) -> None: assert bootloader._has_secure_options(path) is secure -# ------------------------------------------------------------ macro tripwire - - -def test_normalized_macro_strips_comments_and_whitespace() -> None: - text = "macro(__build_process_project_includes)\n a( b ) # tail\n\n # only\n c(d)\nendmacro()" - assert bootloader._normalized_macro(text) == ["a( b )", "c(d)"] - - -def test_normalized_macro_none_without_macro() -> None: - assert bootloader._normalized_macro("nothing here") is None - - -def test_idf_macro_matches_the_shipped_body(tmp_path: Path) -> None: - """The override's embedded copy must equal what build.cmake ships.""" - idf = _write_idf_build_cmake(tmp_path) - with patch.object(toolchain, "_get_idf_path", return_value=idf): - assert bootloader.idf_macro_matches() is True - - -def test_idf_macro_mismatch_detected(tmp_path: Path) -> None: - changed = IDF_BUILD_CMAKE.replace( - "include(${sdkconfig_cmake})", "include(${sdkconfig_cmake} NEW_ARG)" - ) - idf = _write_idf_build_cmake(tmp_path, changed) - with patch.object(toolchain, "_get_idf_path", return_value=idf): - assert bootloader.idf_macro_matches() is False - - # ------------------------------------------------------------------ cache key @@ -221,10 +177,12 @@ def _key( compiler: str = "/tc/gcc", version: str = "5.5.5", ) -> str: + from esphome.espidf import framework + with ( 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"), + patch.object(framework, "read_idf_version_txt", return_value="v5.5.5"), ): return bootloader._compute_key(bootloader._key_payload(names, config, compiler)) @@ -254,17 +212,6 @@ def test_key_ignores_unlisted_config_values() -> None: assert _key(["A"], {"A": "1", "Z": "app-only"}) == _key(["A"], {"A": "1"}) -def test_version_stamp_reads_version_txt(tmp_path: Path) -> None: - (tmp_path / "version.txt").write_text("v5.5.5\n") - with patch.object(toolchain, "_get_idf_path", return_value=tmp_path): - assert bootloader._version_stamp() == "v5.5.5" - - -def test_version_stamp_missing_file(tmp_path: Path) -> None: - with patch.object(toolchain, "_get_idf_path", return_value=tmp_path): - assert bootloader._version_stamp() == "" - - def test_cache_root_layout(tmp_path: Path) -> None: """Entries live under the registered IDF tools cache, per version and target, so clean-all removes them with the toolchains.""" @@ -302,40 +249,6 @@ def test_load_config_names_rejects_bad_files( assert bootloader._load_config_names() is None -def test_load_build_config(tmp_path: Path) -> None: - assert bootloader._load_build_config(tmp_path) is None - (tmp_path / "config").mkdir() - (tmp_path / "config" / "sdkconfig.json").write_text('{"A": 1}') - assert bootloader._load_build_config(tmp_path) == {"A": 1} - - -# ------------------------------------------------------------- compiler id - - -def test_compiler_id_from_project_description(tmp_path: Path) -> None: - build = CORE.relative_build_path("build") - build.mkdir(parents=True) - (build / "project_description.json").write_text( - json.dumps({"c_compiler": "/tools/xtensa-esp32-elf-gcc"}) - ) - assert bootloader._compiler_id() == os.path.realpath("/tools/xtensa-esp32-elf-gcc") - - -@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 - - # ------------------------------------------------------------- cmake args @@ -540,7 +453,7 @@ def _orchestration_env(tmp_path: Path) -> Iterator[Path]: ) with ( patch.object(bootloader, "_cache_root", return_value=root), - patch.object(bootloader, "_compiler_id", return_value="/tc/gcc"), + patch.object(toolchain, "_resolved_c_compiler", return_value="/tc/gcc"), patch.object(bootloader, "_key_payload", return_value={}), patch.object(bootloader, "_compute_key", return_value="deadbeefdeadbeef"), ): @@ -556,8 +469,11 @@ def test_ensure_cache_hit_installs_without_building(tmp_path: Path) -> None: for name in bootloader._OUTPUTS: (root / "deadbeefdeadbeef" / name).write_bytes(b"\xe9" * 64) (root / "config_names.json").write_text('["A"]') + stamp = bootloader._failure_stamp() + stamp.write_text("{}") assert bootloader.ensure_cached_bootloader() == 0 mock_build.assert_not_called() + assert not stamp.exists() # success clears a stale failure stamp installed = CORE.relative_build_path("build", "bootloader", "bootloader.bin") assert installed.read_bytes() == b"\xe9" * 64 @@ -638,15 +554,17 @@ def test_prune_swallows_cache_root_errors() -> None: # --------------------------------------------------------- flash injection -def _flash_build(tmp_path: Path, with_bin: bool = True, offset: int = 0x1000) -> Path: +def _flash_build(tmp_path: Path, cached: bool = True, offset: int = 0x1000) -> Path: build = tmp_path / "build" (build / "config").mkdir(parents=True) (build / "config" / "sdkconfig.json").write_text( json.dumps({"BOOTLOADER_OFFSET_IN_FLASH": offset}) ) - if with_bin: - (build / "bootloader").mkdir() - (build / "bootloader" / "bootloader.bin").write_bytes(b"\xe9") + (build / "CMakeCache.txt").write_text( + f"ESPHOME_USE_CACHED_BOOTLOADER:UNINITIALIZED={int(cached)}\n" + ) + (build / "bootloader").mkdir() + (build / "bootloader" / "bootloader.bin").write_bytes(b"\xe9") return build @@ -664,21 +582,29 @@ def test_inject_uses_configured_offset(tmp_path: Path) -> None: assert flash_data["flash_files"] == {"0x0": "bootloader/bootloader.bin"} -def test_inject_noop_when_idf_wrote_the_entry(tmp_path: Path) -> None: - """Stock and bypass trees already carry a bootloader entry.""" - build = _flash_build(tmp_path) - flash_data = {"flash_files": {"0x1000": "bootloader/bootloader.bin"}} - bootloader.inject_bootloader_flash_file(flash_data, build) - assert flash_data["flash_files"] == {"0x1000": "bootloader/bootloader.bin"} - - -def test_inject_noop_without_bin_or_config(tmp_path: Path) -> None: +def test_inject_noop_on_a_stock_tree(tmp_path: Path) -> None: + """IDF omits the entry on purpose for some secure-boot builds; a tree + not in cached mode must stay exactly as IDF wrote it.""" + build = _flash_build(tmp_path, cached=False) flash_data: dict = {"flash_files": {}} - bootloader.inject_bootloader_flash_file( - flash_data, _flash_build(tmp_path / "a", with_bin=False) - ) - no_config = tmp_path / "b" / "build" - (no_config / "bootloader").mkdir(parents=True) - (no_config / "bootloader" / "bootloader.bin").write_bytes(b"\xe9") - bootloader.inject_bootloader_flash_file(flash_data, no_config) + bootloader.inject_bootloader_flash_file(flash_data, build) assert flash_data == {"flash_files": {}} + + +def test_inject_noop_without_cmakecache(tmp_path: Path) -> None: + flash_data: dict = {"flash_files": {}} + bootloader.inject_bootloader_flash_file(flash_data, tmp_path / "missing") + assert flash_data == {"flash_files": {}} + + +def test_inject_missing_pieces_log_an_error( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """Cached mode without a bin or config is an incomplete factory image.""" + build = _flash_build(tmp_path) + (build / "config" / "sdkconfig.json").unlink() + flash_data: dict = {"flash_files": {}} + with caplog.at_level("ERROR"): + bootloader.inject_bootloader_flash_file(flash_data, build) + assert flash_data == {"flash_files": {}} + assert "factory image has no bootloader" in caplog.text diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index c785d56fa41..fb82ceebf64 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -2003,3 +2003,11 @@ def test_check_windows_path_length_long_path_warns( assert "long path support" in message # The install is global now; the remedy is the prefix env, not moving the project. assert "ESPHOME_ESP_IDF_PREFIX" in message + + +def test_read_idf_version_txt(tmp_path: Path) -> None: + from esphome.espidf.framework import read_idf_version_txt + + assert read_idf_version_txt(tmp_path) == "" + (tmp_path / "version.txt").write_text("v5.5.5\n") + assert read_idf_version_txt(tmp_path) == "v5.5.5" diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index f223a4a6de4..9c9e95aadbe 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -1171,6 +1171,7 @@ def test_run_compile_falls_back_when_bootloader_cache_fails( patch.object(toolchain, "_cache_entries_changed", return_value=False), patch.object(toolchain, "_use_cached_bootloader", return_value=True), patch.object(bootloader, "ensure_cached_bootloader", return_value=1), + patch.object(bootloader, "record_failure") as mock_record, patch.object( toolchain, "run_reconfigure", return_value=reconfigure_rc ) as mock_reconfigure, @@ -1179,8 +1180,10 @@ def test_run_compile_falls_back_when_bootloader_cache_fails( ): rc = toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) assert rc == reconfigure_rc - # The fallback pins the memoized predicate for the rest of the run. + # The fallback pins the memoized predicate for the rest of the run and + # stamps the tree so later runs skip the retry until the inputs change. assert toolchain._cache().bootloader_enabled is False + mock_record.assert_called_once_with() mock_reconfigure.assert_called_once_with(False) assert mock_ninja.called is (reconfigure_rc == 0) @@ -1192,6 +1195,9 @@ def test_create_factory_bin_merges_cached_bootloader(setup_core: Path) -> None: build = CORE.relative_build_path("build") (build / "bootloader").mkdir(parents=True) (build / "bootloader" / "bootloader.bin").write_bytes(b"\xe9") + (build / "CMakeCache.txt").write_text( + "ESPHOME_USE_CACHED_BOOTLOADER:UNINITIALIZED=1\n" + ) (build / "config").mkdir() (build / "config" / "sdkconfig.json").write_text( json.dumps({"BOOTLOADER_OFFSET_IN_FLASH": 0x1000}) @@ -1221,3 +1227,44 @@ def test_create_factory_bin_merges_cached_bootloader(setup_core: Path) -> None: assert cmd[offset_index + 1] == str(build / "bootloader" / "bootloader.bin") # Sections stay sorted by address; the bootloader comes first. assert offset_index < cmd.index("0x10000") + + +def test_idf_target_from_variant(setup_core: Path) -> None: + CORE.data[KEY_ESP32] = {KEY_VARIANT: "ESP32C6"} + assert toolchain._idf_target() == "esp32c6" + + +def test_load_sdkconfig_json(setup_core: Path, tmp_path: Path) -> None: + assert toolchain._load_sdkconfig_json(tmp_path) is None + (tmp_path / "config").mkdir() + (tmp_path / "config" / "sdkconfig.json").write_text('{"A": 1}') + assert toolchain._load_sdkconfig_json(tmp_path) == {"A": 1} + (tmp_path / "config" / "sdkconfig.json").write_text("[]") + assert toolchain._load_sdkconfig_json(tmp_path) is None + + +def test_resolved_c_compiler_from_project_description(setup_core: Path) -> None: + _setup_build(setup_core) + build = CORE.relative_build_path("build") + build.mkdir(parents=True) + (build / "project_description.json").write_text( + json.dumps({"c_compiler": "/tools/xtensa-esp32-elf-gcc"}) + ) + expected = os.path.realpath("/tools/xtensa-esp32-elf-gcc") + assert toolchain._resolved_c_compiler() == expected + + +@pytest.mark.parametrize( + "description", + [None, "not json", "{}", '{"c_compiler": ""}', "[]"], + ids=["missing", "corrupt", "no-key", "empty", "not-a-dict"], +) +def test_resolved_c_compiler_none_without_a_usable_description( + setup_core: Path, description: str | None +) -> None: + _setup_build(setup_core) + if description is not None: + build = CORE.relative_build_path("build") + build.mkdir(parents=True) + (build / "project_description.json").write_text(description) + assert toolchain._resolved_c_compiler() is None diff --git a/tests/unit_tests/test_helpers.py b/tests/unit_tests/test_helpers.py index ff82fa3c80f..155d40bbf27 100644 --- a/tests/unit_tests/test_helpers.py +++ b/tests/unit_tests/test_helpers.py @@ -1250,3 +1250,15 @@ def test_get_usable_cpu_count_sources() -> None: mock_os_unknown = types.SimpleNamespace(cpu_count=lambda: None) with patch("esphome.helpers.os", mock_os_unknown): assert helpers.get_usable_cpu_count() == 1 + + +@pytest.mark.parametrize( + ("content", "expected"), + [('{"a": 1}', {"a": 1}), ("[1, 2]", [1, 2]), ("not json", None), (None, None)], + ids=["object", "list", "corrupt", "missing"], +) +def test_read_json_file(tmp_path: Path, content: str | None, expected) -> None: + path = tmp_path / "data.json" + if content is not None: + path.write_text(content) + assert helpers.read_json_file(path) == expected