From c0aa121c3d0a9ae8ba515e81d313fe62756a3afb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 27 Jul 2026 08:39:03 -1000 Subject: [PATCH] [espidf] Install only the toolchains for the variants being built (#17688) --- .github/actions/cache-esp-idf/action.yml | 6 +- esphome/components/esp32/const.py | 6 + esphome/espidf/component.py | 3 +- esphome/espidf/framework.py | 160 +++++++++++-- esphome/espidf/toolchain.py | 33 ++- tests/unit_tests/test_espidf_framework.py | 275 +++++++++++++++++++++- tests/unit_tests/test_espidf_toolchain.py | 26 +- 7 files changed, 476 insertions(+), 33 deletions(-) diff --git a/.github/actions/cache-esp-idf/action.yml b/.github/actions/cache-esp-idf/action.yml index f566ba4c434..b884e1e4c67 100644 --- a/.github/actions/cache-esp-idf/action.yml +++ b/.github/actions/cache-esp-idf/action.yml @@ -3,8 +3,10 @@ description: > Resolve the pinned ESP-IDF version and cache the native ESP-IDF install (toolchains + source) at ~/.esphome-idf. Every job that installs ESP-IDF natively (clang-tidy for IDF/Arduino and the component test batches) shares - one cache, since the install is identical (ESPHOME_IDF_DEFAULT_TARGETS - defaults to "all", so all toolchains are present regardless of the chip). + one cache, since the install is identical: ESPHOME_IDF_DEFAULT_TARGETS + defaults to "all", and _get_configured_targets() in espidf/toolchain.py + skips per-variant narrowing whenever CI is set, so all toolchains are + present regardless of the chip a job builds. Callers must set env ESPHOME_ESP_IDF_PREFIX: ~/.esphome-idf and have the Python venv already restored. inputs: diff --git a/esphome/components/esp32/const.py b/esphome/components/esp32/const.py index 83fcfd233e7..248f84c6bca 100644 --- a/esphome/components/esp32/const.py +++ b/esphome/components/esp32/const.py @@ -63,4 +63,10 @@ VARIANT_FRIENDLY = { VARIANT_ESP32S31: "ESP32-S31", } + +def variant_to_idf_target(variant: str) -> str: + """Map an esp32 variant name (e.g. "ESP32S3") to its ESP-IDF target name.""" + return variant.lower().replace("-", "") + + esp32_ns = cg.esphome_ns.namespace("esp32") diff --git a/esphome/espidf/component.py b/esphome/espidf/component.py index 51d023099ed..182f29c92d6 100644 --- a/esphome/espidf/component.py +++ b/esphome/espidf/component.py @@ -53,9 +53,10 @@ def _apply_extra_script(component: IDFComponent) -> None: if not script_path.is_relative_to(library_root) or not script_path.is_file(): return from esphome.components.esp32 import get_esp32_variant + from esphome.components.esp32.const import variant_to_idf_target from esphome.espidf.extra_script import captured_as_build_flags, run_extra_script - idf_target = get_esp32_variant().lower().replace("-", "") + idf_target = variant_to_idf_target(get_esp32_variant()) result = run_extra_script( script_path, library_dir=component.path, idf_target=idf_target ) diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 3a594c738cc..0ca7a9d14bd 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -9,7 +9,7 @@ from pathlib import Path import platform import re import shutil -from typing import NoReturn +from typing import Any, NoReturn import platformdirs @@ -49,6 +49,10 @@ STAMP_SCHEMA_VERSION = "0" ESPHOME_IDF_DEFAULT_TARGETS = str_to_lst_of_str( os.environ.get("ESPHOME_IDF_DEFAULT_TARGETS", "all") ) +# An explicitly set ESPHOME_IDF_DEFAULT_TARGETS overrides the per-variant +# targets a caller requests, so a builder image can still pre-warm every +# target with one env var. +_IDF_DEFAULT_TARGETS_EXPLICIT = bool(os.environ.get("ESPHOME_IDF_DEFAULT_TARGETS")) ESPHOME_IDF_DEFAULT_TOOLS = str_to_lst_of_str( os.environ.get("ESPHOME_IDF_DEFAULT_TOOLS", "cmake;ninja") @@ -199,7 +203,35 @@ def _get_python_env_path(version: str) -> Path: return get_idf_tools_path() / "penvs" / f"{version}" -def _check_stamp(file: PathType, data: dict[str, str]) -> bool: +def _read_stamp(file: PathType) -> dict | None: + """Return a stamp file's dict contents, or None if missing or invalid. + + A missing stamp is the normal first-install case and stays silent; the + other branches indicate a real fault that forces a full reinstall on + every build, so they warn. + """ + try: + with Path(file).open(encoding="utf-8") as f: + data = json.load(f) + except FileNotFoundError: + return None + except json.JSONDecodeError as e: + _LOGGER.warning("Ignoring corrupt stamp file %s: %s", file, e) + return None + except OSError as e: + _LOGGER.warning("Could not read stamp file %s: %s", file, e) + return None + if not isinstance(data, dict): + _LOGGER.warning( + "Ignoring stamp file %s with unexpected type %s", + file, + type(data).__name__, + ) + return None + return data + + +def _check_stamp(file: PathType, data: dict[str, Any]) -> bool: """ Check if a stamp file contains the expected data. @@ -210,17 +242,43 @@ def _check_stamp(file: PathType, data: dict[str, str]) -> bool: Returns: True if file exists and contains expected data, False otherwise """ - if not Path(file).is_file(): + return _read_stamp(file) == data + + +def _stamps_match_except_targets(stored: dict, requested: dict) -> bool: + """Whether two stamps agree on every field other than ``targets``. + + Compares whole dicts (minus ``targets``) rather than named keys so any + stamp field added later participates in invalidation by default instead + of being silently ignored. + """ + + def _strip(stamp: dict) -> dict: + return {k: v for k, v in stamp.items() if k != "targets"} + + return _strip(stored) == _strip(requested) + + +def _stamp_covers(stored: dict | None, requested: dict) -> bool: + """Return True if a stored framework stamp already covers this request. + + Every field except ``targets`` must match exactly. ``targets`` may be a + superset of the requested ones: ``idf_tools.py install`` accumulates + targets in idf-env.json across runs, so a framework installed for more + targets than this build needs is still valid. A stored ``all`` covers + every target. + """ + if stored is None: return False - - try: - with Path(file).open(encoding="utf-8") as f: - return json.load(f) == data - except (json.JSONDecodeError, OSError): + if not _stamps_match_except_targets(stored, requested): return False + stored_targets = stored.get("targets") + if not isinstance(stored_targets, list): + return False + return "all" in stored_targets or set(requested["targets"]) <= set(stored_targets) -def _write_stamp(file: PathType, data: dict[str, str]): +def _write_stamp(file: PathType, data: dict[str, Any]): """ Write data to a stamp file in JSON format. @@ -557,6 +615,19 @@ _UNUSED_IDF_TOOLS: tuple[str, ...] = ( "xtensa-esp-elf-gdb", ) +# tools.json also lists riscv32-esp-elf as supported on the xtensa chips +# because the S2/S3 ULP coprocessor is a RISC-V core, so installing for an +# S2/S3 target pulls in the whole riscv compiler (~290MB download, 2GB disk) +# just for ULP programs — which ESPHome never builds (the IDF ``ulp`` +# component is excluded by default; a user who re-enables it via +# ``include_builtin_idf_components: [ulp]`` on an S2/S3 and hits a missing +# riscv compiler can set ESPHOME_IDF_DEFAULT_TARGETS=all to install it). +# Removing the xtensa chips from its supported targets keeps it out of +# xtensa-only installs; building a RISC-V variant still installs it. Add any +# future Xtensa chip here; a missing entry only costs the download, while a +# wrongly listed RISC-V chip would strip its own compiler. +_XTENSA_TARGETS: tuple[str, ...] = ("esp32", "esp32s2", "esp32s3") + def _patch_tools_json_demote_unused_tools(framework_path: Path) -> None: """Demote tools ESPHome never runs from ``install: always`` to ``on_request``. @@ -572,6 +643,10 @@ def _patch_tools_json_demote_unused_tools(framework_path: Path) -> None: its stamp file) heals on the next build without a clean. A user who wants one of these tools can still name it explicitly in ESPHOME_IDF_DEFAULT_TOOLS; explicit names bypass install-type filtering. + + Also removes the xtensa chips from riscv32-esp-elf's supported targets + (see ``_XTENSA_TARGETS``) so xtensa-only installs don't pull in the + RISC-V compiler for ULP programs ESPHome never builds. """ def apply_patch(data: dict) -> bool: @@ -583,13 +658,31 @@ def _patch_tools_json_demote_unused_tools(framework_path: Path) -> None: ): tool["install"] = "on_request" changed = True + if tool.get("name") == "riscv32-esp-elf": + targets = tool.get("supported_targets") + # Guard the type so unexpected JSON here cannot abort the + # other demotions; this patch is best-effort. Log it so a + # silently resumed riscv download is diagnosable. + if not isinstance(targets, list): + _LOGGER.warning( + "Unexpected supported_targets for riscv32-esp-elf " + "in tools.json (%s); not excluding it from xtensa " + "installs", + type(targets).__name__, + ) + continue + if any(t in targets for t in _XTENSA_TARGETS): + tool["supported_targets"] = [ + t for t in targets if t not in _XTENSA_TARGETS + ] + changed = True return changed _patch_tools_json( framework_path, apply_patch, "Patched %s to skip installing tools ESPHome does not use " - "(openocd, gdb, ULP toolchain).", + "(openocd, gdb, ULP toolchains).", ) @@ -685,7 +778,9 @@ def _check_esphome_idf_framework_install( the URL. Returns: - tuple of (framework_path, install_flag) + tuple of (framework_path, fresh_extract_flag). The flag is True only + when the framework tree was downloaded and extracted this run, not + when tools were installed into an existing tree. """ # Sanitize inputs @@ -718,8 +813,8 @@ def _check_esphome_idf_framework_install( # avoids post-extraction renames that race with antivirus on Windows. # Tool install state is tracked separately by the stamp file in step 3, # so we only re-extract when extraction itself is missing or incomplete. - install = force or not extracted_marker.is_file() - if install: + fresh_extract = force or not extracted_marker.is_file() + if fresh_extract: rmdir(framework_path, msg=f"Clean up ESP-IDF {version} framework") git_source = _parse_git_source(source_url) if source_url else None @@ -790,9 +885,11 @@ def _check_esphome_idf_framework_install( _patch_tools_json_demote_unused_tools(framework_path) # 3. Check if the framework tools are the same and correctly installed + stored_stamp = None if fresh_extract else _read_stamp(env_stamp_file) + install = fresh_extract if not install: install = True - if _check_stamp(env_stamp_file, stamp_info): + if _stamp_covers(stored_stamp, stamp_info): _LOGGER.info("Checking ESP-IDF %s framework installation ...", version) # Validate via the managed tool-path resolution, not ``idf_tools.py check``: # ``check`` probes tools on the system PATH and aborts if any fail to run (e.g. a @@ -843,9 +940,25 @@ def _check_esphome_idf_framework_install( except RuntimeError as err: _LOGGER.debug("Could not remove ESP-IDF tool download cache: %s", err) + # Record the union of every target installed so far, not just this + # build's. idf_tools.py accumulates targets in idf-env.json and the + # ``required`` metapackage installs tools for all of them, so the + # union is what is actually on disk — and it keeps two variants + # alternating between builds from re-running the installer each time. + # Merge only when everything except targets matches: a reinstall + # triggered by a schema or tools change ran the installer for this + # build's targets alone, so carrying the old targets forward would + # let later builds of those variants skip the reinstall they need. + if ( + stored_stamp + and isinstance(stored_stamp.get("targets"), list) + and _stamps_match_except_targets(stored_stamp, stamp_info) + ): + merged = set(stamp_info["targets"]) | set(stored_stamp["targets"]) + stamp_info["targets"] = ["all"] if "all" in merged else sorted(merged) _write_stamp(env_stamp_file, stamp_info) - return framework_path, install + return framework_path, fresh_extract def _check_esp_idf_python_env_install( @@ -991,7 +1104,11 @@ def check_esp_idf_install( env["IDF_TOOLS_PATH"] = str(get_idf_tools_path()) env["IDF_PATH"] = "" - targets = targets or ESPHOME_IDF_DEFAULT_TARGETS + # An explicit ESPHOME_IDF_DEFAULT_TARGETS wins over the caller's + # per-variant request (builder-image pre-warm); otherwise the caller's + # targets are used, falling back to the default when none were given. + if _IDF_DEFAULT_TARGETS_EXPLICIT or not targets: + targets = ESPHOME_IDF_DEFAULT_TARGETS # Determine which tools need to be installed if not provided if tools is None: @@ -1004,15 +1121,18 @@ def check_esp_idf_install( tools.append(tool) # 1) Framework - framework_path, installed = _check_esphome_idf_framework_install( + framework_path, fresh_extract = _check_esphome_idf_framework_install( version, targets, tools, force=force, env=env, source_url=source_url ) features = features or ESPHOME_IDF_DEFAULT_FEATURES - # 2) Python env - python_env_path, installed = _check_esp_idf_python_env_install( - version, features, force=force or installed, env=env + # 2) Python env. Only a freshly extracted framework forces a rebuild — + # the venv depends on the framework version and features, not on which + # toolchains are installed, so adding a target to an existing tree must + # not wipe it. It still self-validates against its own stamp. + python_env_path, _ = _check_esp_idf_python_env_install( + version, features, force=force or fresh_extract, env=env ) return framework_path, python_env_path diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 9dd34749107..fd95805c6c2 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -9,7 +9,13 @@ import re import shutil import subprocess -from esphome.components.esp32.const import KEY_ESP32, KEY_FLASH_SIZE, KEY_IDF_VERSION +from esphome.components.esp32.const import ( + KEY_ESP32, + KEY_FLASH_SIZE, + KEY_IDF_VERSION, + KEY_VARIANT, + variant_to_idf_target, +) from esphome.const import ( CONF_COMPILE_PROCESS_LIMIT, CONF_ESPHOME, @@ -56,6 +62,27 @@ def _get_framework_source_override() -> str | None: return CORE.config.get(KEY_ESP32, {}).get(CONF_FRAMEWORK, {}).get(CONF_SOURCE) +def _get_configured_targets() -> list[str] | None: + """Return the IDF install target for the configured variant, if known. + + Limiting the toolchain install to the variant being built skips the other + architecture's compiler entirely (several hundred MB of download and 1-2GB + of disk). idf_tools.py accumulates targets across runs, so building a + second variant later installs just its toolchain incrementally. None (no + variant stored, e.g. tooling outside a build) falls back to the default + inside check_esp_idf_install. + + CI always installs every target (None falls through to the "all" + default): runners share one toolchain cache across jobs that build + different variants, so a full install keeps the cached tree identical + everywhere instead of per-variant supersets invalidating each other. + """ + if os.environ.get("CI"): + return None + variant = CORE.data.get(KEY_ESP32, {}).get(KEY_VARIANT) + return [variant_to_idf_target(variant)] if variant else None + + def _get_esphome_esp_idf_paths( version: str | None = None, ) -> tuple[os.PathLike, os.PathLike]: @@ -63,7 +90,9 @@ def _get_esphome_esp_idf_paths( paths = _cache().paths if version not in paths: paths[version] = check_esp_idf_install( - version, source_url=_get_framework_source_override() + version, + targets=_get_configured_targets(), + source_url=_get_framework_source_override(), ) return paths[version] diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index 59872bf2338..5912facbb30 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -19,7 +19,10 @@ from unittest.mock import patch import pytest from esphome.espidf.framework import ( + ESPHOME_STAMP_FILE, + STAMP_SCHEMA_VERSION, _ccache_env, + _check_esphome_idf_framework_install, _check_stamp, _check_windows_path_length, _clone_idf_with_submodules, @@ -32,6 +35,8 @@ from esphome.espidf.framework import ( _patch_tools_json_demote_unused_tools, _patch_tools_json_for_linux_arm64, _prefetch_idf_tool_archives, + _read_stamp, + _stamp_covers, _windows_long_paths_enabled, _write_idf_version_txt, _write_stamp, @@ -385,6 +390,7 @@ def espidf_mocks(setup_core: Path): patch("esphome.espidf.framework._prefetch_idf_tool_archives"), patch("esphome.espidf.framework._write_stamp"), patch("esphome.espidf.framework._check_stamp", return_value=True), + patch("esphome.espidf.framework._stamp_covers", return_value=True), patch("esphome.espidf.framework._get_idf_version", return_value=_IDF_VERSION), patch("esphome.espidf.framework._get_python_version", return_value="3.11.0"), patch("esphome.espidf.framework.get_system_python_path", return_value="python"), @@ -514,13 +520,17 @@ def _mark_installed() -> None: def test_check_esp_idf_install_stamp_mismatch_reinstalls( espidf_mocks: SimpleNamespace, ) -> None: - """A stamp mismatch reinstalls tools (marker present, so no re-extract).""" + """A stamp mismatch reinstalls tools (marker present, so no re-extract). + + The python env is left alone: it depends on the framework version and + features, not on which toolchains are installed. + """ _mark_installed() - with patch("esphome.espidf.framework._check_stamp", return_value=False): + with patch("esphome.espidf.framework._stamp_covers", return_value=False): check_esp_idf_install(_IDF_VERSION) espidf_mocks.extract.assert_not_called() # marker present -> no re-extract - espidf_mocks.venv.assert_called_once() # tools reinstall -> venv rebuilt + espidf_mocks.venv.assert_not_called() # tools-only install -> venv kept def test_check_esp_idf_install_check_command_failure_reinstalls( @@ -533,7 +543,7 @@ def test_check_esp_idf_install_check_command_failure_reinstalls( check_esp_idf_install(_IDF_VERSION, features=["fb"]) espidf_mocks.extract.assert_not_called() - espidf_mocks.venv.assert_called_once() + espidf_mocks.venv.assert_not_called() # tools-only install -> venv kept def test_check_esp_idf_install_unknown_python_version_reinstalls( @@ -553,8 +563,8 @@ def test_check_esp_idf_install_python_stamp_mismatch_rebuilds_venv( ) -> None: """Framework stamp matches but the python-env stamp does not -> venv rebuilt.""" - # _check_stamp passes for the framework (no python_version key) and fails - # for the python env (carries python_version), so only the venv rebuilds. + # _check_stamp only guards the python env now (the framework uses + # _stamp_covers, patched True by the fixture); failing it rebuilds the venv. def stamp_ok(_stamp_file, info: dict) -> bool: return "python_version" not in info @@ -566,6 +576,146 @@ def test_check_esp_idf_install_python_stamp_mismatch_rebuilds_venv( espidf_mocks.venv.assert_called_once() +def _requested_stamp(targets: list[str], tools: list[str] | None = None) -> dict: + return { + "schema_version": STAMP_SCHEMA_VERSION, + "targets": targets, + "tools": tools or ["required"], + } + + +@pytest.mark.parametrize( + ("stored", "targets", "expected"), + [ + # a stored "all" covers any target + (_requested_stamp(["all"]), ["esp32"], True), + # exact match and superset both cover + (_requested_stamp(["esp32"]), ["esp32"], True), + (_requested_stamp(["esp32", "esp32c3"]), ["esp32"], True), + # a new target is not covered + (_requested_stamp(["esp32"]), ["esp32c3"], False), + # tools and schema_version must match exactly + (_requested_stamp(["all"], tools=["cmake", "required"]), ["esp32"], False), + (_requested_stamp(["all"]) | {"schema_version": "no"}, ["esp32"], False), + # an unknown extra field participates in invalidation by default + (_requested_stamp(["all"]) | {"module_version": 1}, ["esp32"], False), + # missing/corrupt stamps never cover + (None, ["esp32"], False), + ( + {"schema_version": STAMP_SCHEMA_VERSION, "tools": ["required"]}, + ["esp32"], + False, + ), + ], +) +def test_stamp_covers(stored: dict | None, targets: list[str], expected: bool) -> None: + assert _stamp_covers(stored, _requested_stamp(targets)) is expected + + +@contextmanager +def _framework_install_patches(): + """Patches for calling _check_esphome_idf_framework_install directly with + real stamp files (unlike espidf_mocks, which stubs the stamp layer).""" + with ( + patch("esphome.espidf.framework.run_command_ok", return_value=True) as run_ok, + patch("esphome.espidf.framework._get_idf_tool_paths", return_value=([], {})), + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch("esphome.espidf.framework.rmdir"), + ): + yield run_ok + + +def _extracted_framework_with_stamp(stamp: dict) -> Path: + framework_path = _get_framework_path(_IDF_VERSION) + framework_path.mkdir(parents=True, exist_ok=True) + (framework_path / ".esphome_extracted").touch() + _write_stamp(framework_path / ESPHOME_STAMP_FILE, stamp) + return framework_path + + +def test_framework_install_target_subset_skips_install() -> None: + """A stamp holding a superset of the requested targets skips the installer.""" + framework_path = _extracted_framework_with_stamp(_requested_stamp(["all"])) + + with _framework_install_patches() as run_ok: + _, fresh_extract = _check_esphome_idf_framework_install( + _IDF_VERSION, ["esp32"], ["required"] + ) + + run_ok.assert_not_called() + assert fresh_extract is False + # the stamp is untouched + stamp = json.loads((framework_path / ESPHOME_STAMP_FILE).read_text()) + assert stamp["targets"] == ["all"] + + +def test_framework_install_new_target_installs_and_merges_stamp() -> None: + """A new target runs the installer for just that target and the stamp + records the union of everything installed so far.""" + framework_path = _extracted_framework_with_stamp(_requested_stamp(["esp32"])) + + with _framework_install_patches() as run_ok: + _, fresh_extract = _check_esphome_idf_framework_install( + _IDF_VERSION, ["esp32c3"], ["required"] + ) + + assert fresh_extract is False + assert "--targets=esp32c3" in run_ok.call_args[0][0] + stamp = json.loads((framework_path / ESPHOME_STAMP_FILE).read_text()) + assert stamp["targets"] == ["esp32", "esp32c3"] + + +def test_check_esp_idf_install_env_targets_override_wins( + espidf_mocks: SimpleNamespace, +) -> None: + """An explicitly set ESPHOME_IDF_DEFAULT_TARGETS overrides per-variant targets.""" + with patch("esphome.espidf.framework._IDF_DEFAULT_TARGETS_EXPLICIT", True): + check_esp_idf_install(_IDF_VERSION, force=True, targets=["esp32"]) + + install_cmd = espidf_mocks.run_ok.call_args_list[0][0][0] + assert "--targets=all" in install_cmd + + +def test_check_esp_idf_install_uses_requested_targets( + espidf_mocks: SimpleNamespace, +) -> None: + """Without the env override, the caller's per-variant targets are installed.""" + check_esp_idf_install(_IDF_VERSION, force=True, targets=["esp32"]) + + install_cmd = espidf_mocks.run_ok.call_args_list[0][0][0] + assert "--targets=esp32" in install_cmd + + +def test_framework_install_all_request_collapses_merged_stamp_to_all() -> None: + """Requesting "all" over a per-variant stamp merges and collapses to + ["all"], not ["all", "esp32"], so the stamp shape stays canonical.""" + framework_path = _extracted_framework_with_stamp(_requested_stamp(["esp32"])) + + with _framework_install_patches() as run_ok: + _check_esphome_idf_framework_install(_IDF_VERSION, ["all"], ["required"]) + + run_ok.assert_called_once() + stamp = json.loads((framework_path / ESPHOME_STAMP_FILE).read_text()) + assert stamp["targets"] == ["all"] + + +def test_framework_install_tools_change_resets_stamp_targets() -> None: + """A reinstall triggered by a tools change must not carry the old stamp's + targets forward: the installer only ran for this build's targets, so a + merged stamp would let other variants skip the reinstall they need.""" + framework_path = _extracted_framework_with_stamp( + _requested_stamp(["all"], tools=["cmake", "required"]) + ) + + with _framework_install_patches() as run_ok: + _check_esphome_idf_framework_install(_IDF_VERSION, ["esp32"], ["required"]) + + run_ok.assert_called_once() + stamp = json.loads((framework_path / ESPHOME_STAMP_FILE).read_text()) + assert stamp["targets"] == ["esp32"] + assert stamp["tools"] == ["required"] + + @pytest.mark.parametrize( ("lib", "expect_hint"), [ @@ -1014,6 +1164,66 @@ def test_demote_unused_tools_patches_install_type(tmp_path: Path) -> None: } +def test_demote_unused_tools_drops_xtensa_from_riscv_targets(tmp_path: Path) -> None: + """riscv32-esp-elf loses the xtensa chips (ULP-RISC-V only, which ESPHome + never builds) but keeps its RISC-V targets; other tools are untouched.""" + tools_json = _write_tools_json( + tmp_path, + { + "tools": [ + { + "name": "riscv32-esp-elf", + "install": "always", + "supported_targets": ["esp32s2", "esp32s3", "esp32c3", "esp32p4"], + }, + { + "name": "xtensa-esp-elf", + "install": "always", + "supported_targets": ["esp32", "esp32s2", "esp32s3"], + }, + ] + }, + ) + _patch_tools_json_demote_unused_tools(tmp_path) + + data = json.loads(tools_json.read_text(encoding="utf-8")) + riscv = next(t for t in data["tools"] if t["name"] == "riscv32-esp-elf") + xtensa = next(t for t in data["tools"] if t["name"] == "xtensa-esp-elf") + assert riscv["supported_targets"] == ["esp32c3", "esp32p4"] + assert riscv["install"] == "always" + assert xtensa["supported_targets"] == ["esp32", "esp32s2", "esp32s3"] + + +def test_demote_unused_tools_bad_supported_targets_type_still_demotes( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A non-list supported_targets on riscv32-esp-elf must not abort the + other demotions; the targets patch is best-effort and logs the skip so a + silently resumed riscv download is diagnosable.""" + tools_json = _write_tools_json( + tmp_path, + { + "tools": [ + { + "name": "riscv32-esp-elf", + "install": "always", + "supported_targets": None, + }, + {"name": "openocd-esp32", "install": "always"}, + ] + }, + ) + with caplog.at_level(logging.WARNING, logger="esphome.espidf.framework"): + _patch_tools_json_demote_unused_tools(tmp_path) + + data = json.loads(tools_json.read_text(encoding="utf-8")) + openocd = next(t for t in data["tools"] if t["name"] == "openocd-esp32") + riscv = next(t for t in data["tools"] if t["name"] == "riscv32-esp-elf") + assert openocd["install"] == "on_request" + assert riscv["supported_targets"] is None + assert "Unexpected supported_targets" in caplog.text + + def test_patch_tools_json_unexpected_structure_warns_and_skips( tmp_path: Path, ) -> None: @@ -1036,6 +1246,11 @@ def test_demote_unused_tools_already_patched_is_noop(tmp_path: Path) -> None: {"name": "xtensa-esp-elf-gdb", "install": "on_request"}, {"name": "riscv32-esp-elf-gdb", "install": "on_request"}, {"name": "esp32ulp-elf", "install": "on_request"}, + { + "name": "riscv32-esp-elf", + "install": "always", + "supported_targets": ["esp32c3", "esp32p4"], + }, ] }, ) @@ -1270,6 +1485,54 @@ def test_check_stamp_corrupt_file(tmp_path: Path) -> None: assert _check_stamp(f, {"a": "1"}) is False +def test_read_stamp_corrupt_file_warns( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + # A corrupt stamp forces a full reinstall on every build, so it warns + # where the normal missing-file case stays silent. + f = tmp_path / "s.json" + f.write_text("{ not json", encoding="utf-8") + with caplog.at_level(logging.WARNING, logger="esphome.espidf.framework"): + assert _read_stamp(f) is None + assert "Ignoring corrupt stamp file" in caplog.text + + +def test_read_stamp_unreadable_file_warns( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + # An I/O fault (permissions, disk error) is distinguished from a simply + # missing stamp with a warning before falling back to reinstall. + f = tmp_path / "s.json" + f.write_text(json.dumps({"a": "1"}), encoding="utf-8") + with ( + patch.object(Path, "open", side_effect=PermissionError("denied")), + caplog.at_level(logging.WARNING, logger="esphome.espidf.framework"), + ): + assert _read_stamp(f) is None + assert "Could not read stamp file" in caplog.text + + +def test_read_stamp_non_dict_warns( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + # Well-formed JSON that is not an object is a fault, not a first install; + # it must leave a trace before forcing reinstalls. + f = tmp_path / "s.json" + f.write_text("null", encoding="utf-8") + with caplog.at_level(logging.WARNING, logger="esphome.espidf.framework"): + assert _read_stamp(f) is None + assert "unexpected type NoneType" in caplog.text + + +def test_read_stamp_missing_file_is_silent( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + # Missing stamps are the normal first-install case and must not log. + with caplog.at_level(logging.DEBUG, logger="esphome.espidf.framework"): + assert _read_stamp(tmp_path / "nope.json") is None + assert "stamp file" not in caplog.text + + def test_write_idf_version_txt_writes_when_missing(tmp_path: Path) -> None: _write_idf_version_txt(tmp_path, "5.1.2") assert (tmp_path / "version.txt").read_text(encoding="utf-8") == "v5.1.2\n" diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index 2735746264f..56f358a24c5 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -10,6 +10,7 @@ from unittest.mock import patch import pytest +from esphome.components.esp32.const import KEY_ESP32, KEY_VARIANT from esphome.const import ( CONF_COMPILE_PROCESS_LIMIT, CONF_ESPHOME, @@ -55,7 +56,7 @@ def test_get_esphome_esp_idf_paths_forwards_source_override(): toolchain, "check_esp_idf_install", return_value=("/fw", "/penv") ) as mock_install: toolchain._get_esphome_esp_idf_paths("5.5.4") - mock_install.assert_called_once_with("5.5.4", source_url=url) + mock_install.assert_called_once_with("5.5.4", targets=None, source_url=url) def test_get_esphome_esp_idf_paths_no_override(): @@ -66,7 +67,28 @@ def test_get_esphome_esp_idf_paths_no_override(): toolchain, "check_esp_idf_install", return_value=("/fw", "/penv") ) as mock_install: toolchain._get_esphome_esp_idf_paths("5.5.4") - mock_install.assert_called_once_with("5.5.4", source_url=None) + mock_install.assert_called_once_with("5.5.4", targets=None, source_url=None) + + +def test_get_configured_targets_from_variant(monkeypatch: pytest.MonkeyPatch): + """The configured variant restricts the toolchain install to its target.""" + monkeypatch.delenv("CI", raising=False) + CORE.data[KEY_ESP32] = {KEY_VARIANT: "ESP32S3"} + assert toolchain._get_configured_targets() == ["esp32s3"] + + +def test_get_configured_targets_without_variant(monkeypatch: pytest.MonkeyPatch): + """No stored variant (e.g. tooling outside a build) keeps the default.""" + monkeypatch.delenv("CI", raising=False) + CORE.data.pop(KEY_ESP32, None) + assert toolchain._get_configured_targets() is None + + +def test_get_configured_targets_ci_installs_all(monkeypatch: pytest.MonkeyPatch): + """CI installs every target so the shared cache covers all variants.""" + monkeypatch.setenv("CI", "true") + CORE.data[KEY_ESP32] = {KEY_VARIANT: "ESP32S3"} + assert toolchain._get_configured_targets() is None def _setup_build(setup_core: Path) -> tuple[Path, Path]: