diff --git a/esphome/espidf/component.py b/esphome/espidf/component.py index aa6f10c261..4eeaa30e7f 100644 --- a/esphome/espidf/component.py +++ b/esphome/espidf/component.py @@ -27,6 +27,7 @@ from esphome.platformio.library import ( collect_filtered_files, convert_libraries, ensure_list, + lex_build_flags, split_list_by_condition, ) @@ -40,37 +41,6 @@ def _idf_framework() -> str: return "arduino" if CORE.using_arduino else "espidf" -def _apply_extra_script(component: IDFComponent) -> None: - """Run a PIO ``extraScript`` and fold its captured env vars into - ``component.data["build"]["flags"]`` so the existing -L/-l/-D - extraction in ``generate_cmakelists_txt`` picks them up.""" - extra_script = component.data.get("build", {}).get("extraScript") - if not extra_script: - return - # Resolve and confine to the library's source dir so a malicious - # library.json can't escape (e.g. ``"extraScript": "../../etc/passwd"``). - source_path = component.source_dir - library_root = source_path.resolve() - script_path = (source_path / extra_script).resolve() - 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.espidf.extra_script import captured_as_build_flags, run_extra_script - - idf_target = variant_to_idf_target(get_esp32_variant()) - result = run_extra_script( - script_path, library_dir=source_path, idf_target=idf_target - ) - extra_flags = captured_as_build_flags(result, library_dir=source_path) - if not extra_flags: - return - flags = component.data.setdefault("build", {}).setdefault("flags", []) - if isinstance(flags, str): - flags = [flags] - flags.extend(extra_flags) - component.data["build"]["flags"] = flags - - def generate_cmakelists_txt(component: IDFComponent) -> str: """ Generate a CMakeLists.txt file for an ESP-IDF component. @@ -85,10 +55,6 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: Returns: str: The complete CMakeLists.txt content as a string """ - # Late import: this module loads with the esp32 platform on every - # validate/compile, but shlex is only needed when generating component - # CMakeLists. - import shlex def escape_entry(p: PathType) -> str: # In CMakeLists.txt, backslashes need to be escaped @@ -122,26 +88,12 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: build_src_filter = ensure_list( component.data.get("build", {}).get("srcFilter", DEFAULT_BUILD_SRC_FILTER) ) - build_flags = ensure_list( - component.data.get("build", {}).get("flags", DEFAULT_BUILD_FLAGS) + # PlatformIO shell-lexes each build.flags entry; bare -I/-L/-l/-D tokens + # re-glue to their argument so the prefix classifiers below route them. + build_flags = lex_build_flags( + component.data.get("build", {}).get("flags", DEFAULT_BUILD_FLAGS), + f"library {component.name}", ) - # PlatformIO shell-lexes each build.flags entry, so one entry can carry a - # flag and its argument (e.g. "-include cp_custom_alloc.h"). Split the - # same way; emitting such an entry as a single quoted compile option - # hands the compiler one argv with an embedded space. - build_flags = [token for entry in build_flags for token in shlex.split(entry)] - # Re-glue bare -I/-L/-l tokens to their argument ("-I foo" -> "-Ifoo") so - # the prefix classifiers below still route them to INCLUDE_DIRS and the - # link handling. - tokens, build_flags = build_flags, [] - i = 0 - while i < len(tokens): - if tokens[i] in ("-I", "-L", "-l") and i + 1 < len(tokens): - build_flags.append(tokens[i] + tokens[i + 1]) - i += 2 - else: - build_flags.append(tokens[i]) - i += 1 # List all sources files build_src_files = collect_filtered_files( @@ -299,7 +251,14 @@ def generate_idf_component_yml(component: IDFComponent) -> str: def _emit_idf_component(component: IDFComponent) -> None: """Write the ESP-IDF build files for a resolved library into its cache dir.""" - _apply_extra_script(component) + from esphome.components.esp32 import get_esp32_variant + from esphome.platformio.extra_script import apply_extra_script + + apply_extra_script( + component, + board_mcu=lambda: variant_to_idf_target(get_esp32_variant()), + pio_platform="espressif32", + ) write_file_if_changed( component.path / "CMakeLists.txt", generate_cmakelists_txt(component), diff --git a/esphome/espidf/extra_script.py b/esphome/espidf/extra_script.py deleted file mode 100644 index 487fef7cc1..0000000000 --- a/esphome/espidf/extra_script.py +++ /dev/null @@ -1,161 +0,0 @@ -"""Run a PlatformIO ``extraScript`` against a captured SCons-env stand-in. - -PlatformIO libraries occasionally configure per-target link/build state -via a Python ``extraScript`` declared in ``library.json``'s ``build`` -section instead of static fields. The script runs under SCons during -PIO's build and mutates the active ``Environment`` (``env.Append``, -``env.Replace``, …) — chiefly to set ``LIBPATH``/``LIBS`` per chip MCU. - -ESPHome's PIO→IDF converter doesn't run SCons, so these scripts were -previously ignored and any library -relying on them failed to link under ``toolchain: esp-idf``. This -module provides a small shim that ``exec``s an extra-script with a -fake ``env`` object, captures the common ``env.Append(...)`` calls, -and returns the captured vars so the caller can fold them back into -the library's generated CMakeLists. - -Caveats -------- -* Only the ``env.Append`` API is captured. ``env.Replace``, - ``env.Prepend``, ``env.AddPreAction``, SCons file generators, and any - arbitrary I/O are silently no-ops. Scripts that depend on those will - produce incomplete output. -* Running arbitrary Python from third-party libraries is a non-trivial - trust decision. The shim does no sandboxing — anything in the - script's process can run. Use only with libraries whose source you - trust. -""" - -from __future__ import annotations - -from dataclasses import dataclass, field -import logging -import os -from pathlib import Path - -_LOGGER = logging.getLogger(__name__) - -# Keys we know how to translate back into ESPHome's build-flag pipeline. -# Other env.Append kwargs are recorded but ignored downstream. -_CAPTURED_KEYS = frozenset({"LIBPATH", "LIBS", "CPPDEFINES", "LINKFLAGS", "CPPFLAGS"}) - - -@dataclass -class ExtraScriptResult: - """Build-var deltas captured from a PIO extra-script ``env.Append`` call.""" - - libpath: list[str] = field(default_factory=list) - libs: list[str] = field(default_factory=list) - cppdefines: list[str | tuple[str, str]] = field(default_factory=list) - linkflags: list[str] = field(default_factory=list) - cppflags: list[str] = field(default_factory=list) - - -class _FakeSConsEnv: - """Minimal stand-in for SCons ``Environment`` exposed to extra-scripts. - - Implements just enough surface area to let scripts query ``BOARD_MCU`` - / ``PIOENV`` and call ``env.Append(LIBPATH=…, LIBS=…, …)``. Every - other env method swallows silently so unrelated calls don't raise - ``AttributeError`` and abort the script. - """ - - def __init__(self, *, board_mcu: str, pio_env: str) -> None: - self._vars: dict[str, str] = { - "BOARD_MCU": board_mcu, - "PIOPLATFORM": "espressif32", - "PIOENV": pio_env, - } - self.result = ExtraScriptResult() - - # ----- SCons env API the common scripts use ----- - - def get(self, key: str, default: str | None = None) -> str | None: - return self._vars.get(key, default) - - def Append(self, **kwargs) -> None: # noqa: N802 (SCons API name) - for key, value in kwargs.items(): - if key not in _CAPTURED_KEYS: - continue - items = list(value) if isinstance(value, (list, tuple)) else [value] - bucket = getattr(self.result, key.lower()) - bucket.extend(items) - - # ----- Everything else is a no-op so unsupported scripts don't crash ----- - - def __getattr__(self, name: str): - def _noop(*args, **kwargs): - return None - - return _noop - - -def run_extra_script( - script_path: Path, *, library_dir: Path, idf_target: str -) -> ExtraScriptResult: - """Execute ``script_path`` with a fake SCons env and return captured vars. - - ``idf_target`` is the active ESP-IDF target name (e.g. ``esp32``, - ``esp32s3``); it's exposed to the script as PlatformIO's - ``BOARD_MCU`` so chip-conditional logic resolves the same way it - would under PIO. The script runs with ``library_dir`` as the - process CWD so relative-path lookups (``join``, ``realpath``, - ``open``) resolve against the library tree. - - On any exception inside the script we log at debug level and return - an empty result — extra-scripts are best-effort, and an unsupported - script shouldn't block the build. - """ - env = _FakeSConsEnv(board_mcu=idf_target, pio_env=f"esphome_{idf_target}") - code = compile(script_path.read_text(encoding="utf-8"), str(script_path), "exec") - old_cwd = Path.cwd() - try: - os.chdir(library_dir) - exec( # noqa: S102 pylint: disable=exec-used - code, - { - "Import": lambda *_args: None, # SCons-side import; harmless here - "env": env, - "__file__": str(script_path), - "__name__": "__pio_extra_script__", - }, - ) - except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught - _LOGGER.warning("PIO extra-script %s raised %s; skipping", script_path, e) - return ExtraScriptResult() - finally: - os.chdir(old_cwd) - return env.result - - -def captured_as_build_flags( - result: ExtraScriptResult, *, library_dir: Path -) -> list[str]: - """Translate captured env vars into the ``-L`` / ``-l`` / ``-D`` / - raw-flag form ``_generate_cmakelists_txt`` already knows how to consume. - - ``LIBPATH`` entries are made relative to ``library_dir`` so the - generated CMakeLists is portable; absolute paths outside the library - tree are kept as-is (CMake handles absolute paths in - ``target_link_directories`` fine). - """ - flags: list[str] = [] - library_root = library_dir.resolve() - for path in result.libpath: - # Anchor relative paths to library_dir (not the current CWD, which - # has been restored by the time we get here). Joining an absolute - # path against library_dir returns the absolute path unchanged. - resolved = (library_dir / path).resolve() - try: - flags.append(f"-L{resolved.relative_to(library_root)}") - except ValueError: - flags.append(f"-L{resolved}") - flags.extend(f"-l{lib}" for lib in result.libs) - for define in result.cppdefines: - if isinstance(define, tuple) and len(define) == 2: - flags.append(f"-D{define[0]}={define[1]}") - else: - flags.append(f"-D{define}") - flags.extend(result.linkflags) - flags.extend(result.cppflags) - return flags diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 0f6ef873b8..a31a339fee 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -15,6 +15,7 @@ import platformdirs from esphome.core import CORE, Version from esphome.framework_helpers import ( + BatchDownloadProgress, PathType, archive_extract_all, create_venv, @@ -23,6 +24,7 @@ from esphome.framework_helpers import ( get_python_env_executable_path, get_system_python_path, rmdir, + run_batch_downloads, run_command, run_command_ok, str_to_lst_of_str, @@ -702,10 +704,10 @@ def _prefetch_idf_tool_archives( which makes large archives effectively impossible to fetch on unstable connections (#17703). This asks the framework's idf_tools (via ``get_tool_downloads.py``) which archives the coming install needs, then - downloads each into ``/dist`` with - ``download_with_resume``. The installer then finds the verified archives - already in place ("file ... is already downloaded") and never touches the - network. + downloads them into ``/dist`` with + ``download_with_resume``, a few at a time under one combined progress + bar. The installer then finds the verified archives already in place + ("file ... is already downloaded") and never touches the network. Strictly best-effort: any failure here just logs and returns, leaving ``idf_tools.py install`` to download whatever is missing exactly as @@ -727,26 +729,52 @@ def _prefetch_idf_tool_archives( ) return dist_path = get_idf_tools_path() / "dist" - entries = [ - entry - for entry in json.loads(stdout) - if not (dist_path / entry["dest"]).is_file() - ] - for index, entry in enumerate(entries, start=1): - _LOGGER.info( - "Downloading %s (%d/%d) ...", entry["name"], index, len(entries) - ) - try: - download_with_resume( - entry["url"], - dist_path / entry["dest"], - sha256=entry["sha256"], - size=entry["size"], + entries = [] + for entry in json.loads(stdout): + if (dist_path / entry["dest"]).is_file(): + continue + # tools.json always carries sha256 and size; an entry missing + # either must not be downloaded unverified here, so leave it to + # the installer (which fails loudly on a bad archive). + if entry.get("sha256") and entry.get("size"): + entries.append(entry) + else: + _LOGGER.warning( + "Tool %s has no sha256/size in the download list; " + "leaving it to the installer", + entry["name"], ) - except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught - # Keep prefetching the remaining archives; the installer - # will retry this one itself (without resume). - _LOGGER.warning("Could not prefetch %s: %s", entry["name"], e) + if not entries: + return + _LOGGER.info( + "Downloading %d ESP-IDF tool archive(s): %s", + len(entries), + ", ".join(entry["name"] for entry in entries), + ) + + # Every entry carries a size (checked above), so the combined bar can + # be trusted. Unlike the library prefetch there is no sequential + # fallback: per-file bars from several threads would interleave, and + # skipping the prefetch would lose the resume workaround for #17703. + def _download(entry: dict): + return lambda tracker: download_with_resume( + entry["url"], + dist_path / entry["dest"], + sha256=entry["sha256"], + size=entry["size"], + progress=tracker, + ) + + # A failed archive is retried by the installer itself (without + # resume); keep prefetching the rest. + failures = run_batch_downloads( + BatchDownloadProgress( + "Downloading ESP-IDF tools", sum(entry["size"] for entry in entries) + ), + [(entry["name"], _download(entry)) for entry in entries], + ) + for name, e in failures: + _LOGGER.warning("Could not prefetch %s: %s", name, e) except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught # The installer downloads anything missing itself; never let the # prefetch become a new way for the install to fail. diff --git a/esphome/framework_helpers.py b/esphome/framework_helpers.py index b8a43220ff..4d0574372a 100644 --- a/esphome/framework_helpers.py +++ b/esphome/framework_helpers.py @@ -1,6 +1,7 @@ """Generic toolchain installation helpers shared across framework implementations.""" -from collections.abc import Iterable +from collections.abc import Callable, Iterable +from concurrent.futures import ThreadPoolExecutor from contextlib import ExitStack import hashlib import io @@ -10,6 +11,7 @@ import os from pathlib import Path import subprocess import sys +import threading import time from typing import IO, TYPE_CHECKING @@ -23,6 +25,7 @@ PathType = str | os.PathLike _LOGGER = logging.getLogger(__name__) + # Attempts per mirror URL before falling through to the next mirror; only # mid-stream drops retry (resuming when the server gave a validator), # connect errors move on to the next mirror immediately. @@ -697,7 +700,11 @@ def _response_validator(resp: "requests.Response") -> str | None: def _stream_response_to_file( - resp: "requests.Response", f: IO[bytes], offset: int, size: int | None = None + resp: "requests.Response", + f: IO[bytes], + offset: int, + size: int | None = None, + progress: Callable[[int], None] | None = None, ) -> None: """Stream an open ``_open_ranged`` response body into ``f`` at ``offset``. @@ -705,21 +712,112 @@ def _stream_response_to_file( (effective offset 0) discards the stale bytes. ``offset`` also seeds the progress bar so a resumed download shows overall progress. ``size`` is the known full file size; when None it is derived from the response's - content-length, and without either there is no progress bar. + content-length, and without either there is no progress bar. With + ``progress`` set, no bar is drawn here; the callback gets the absolute + byte count, seeded with ``offset`` and then after each chunk. """ f.seek(offset) f.truncate(offset) total_size = size or offset + _content_length(resp) downloaded = offset - progress = ProgressBar("Downloading") if total_size > 0 else None + own_bar: ProgressBar | None = None + if progress is None: + own_bar = ProgressBar("Downloading") if total_size > 0 else None + progress = ( + (lambda done: own_bar.update(done / total_size)) + if own_bar + else (lambda _: None) + ) + progress(downloaded) for chunk in resp.iter_content(chunk_size=256 * 1024): if chunk: f.write(chunk) downloaded += len(chunk) - if progress is not None: - progress.update(downloaded / total_size) - if progress is not None: - progress.update(1) + progress(downloaded) + if own_bar is not None: + own_bar.update(1) + + +# Concurrent downloads per batch; enough to hide latency without +# hammering the host or the mirrors. +BATCH_DOWNLOAD_WORKERS = 4 + + +def run_batch_downloads( + progress: "BatchDownloadProgress", + jobs: list[tuple[str, Callable[[Callable[[int], None]], None]]], + max_workers: int = BATCH_DOWNLOAD_WORKERS, +) -> list[tuple[str, Exception]]: + """Run download jobs concurrently, reporting into one combined bar. + + ``jobs`` holds ``(name, fetch)`` pairs where ``fetch(tracker)`` performs + one download reporting absolute byte counts to ``tracker``. Failures are + collected (list.append is atomic under the GIL) and returned after the + bar is done, so the caller's warnings never land on the bar's row; a + failed job credits its tracker 0 so the bar can still complete. Ctrl-C + drops queued jobs instead of downloading them all before the process + can exit; in-flight ones still finish. + """ + failures: list[tuple[str, Exception]] = [] + + def _run(name: str, fetch: Callable[[Callable[[int], None]], None]) -> None: + tracker = progress.tracker() + try: + fetch(tracker) + except Exception as err: # noqa: BLE001 # pylint: disable=broad-exception-caught + failures.append((name, err)) + tracker(0) + + ex = ThreadPoolExecutor(max_workers=min(max_workers, len(jobs))) + try: + for future in [ex.submit(_run, name, fetch) for name, fetch in jobs]: + future.result() + finally: + ex.shutdown(wait=True, cancel_futures=True) + progress.done() + return failures + + +class BatchDownloadProgress: + """One progress bar across several concurrent ``download_with_resume`` calls. + + Each ``tracker()`` is a ``progress`` callback for one download; it reports + that file's absolute byte count and the bar shows the sum over ``total``. + The lock also serialises the bar's stderr writes, so worker threads never + interleave frames. With an unknown ``total`` (0) nothing is drawn. Call + ``done()`` once every download has finished (or failed) so a bar that + never reached 100% still ends its line before the next log message. + """ + + def __init__(self, header: str, total: int) -> None: + self._bar = ProgressBar(header) if total > 0 else None + self._total = total + self._sum = 0 + self._lock = threading.Lock() + + def tracker(self) -> Callable[[int], None]: + last = 0 + + def update(done: int) -> None: + nonlocal last + if self._bar is None: + return + with self._lock: + self._sum += done - last + last = done + self._bar.update(min(self._sum / self._total, 1)) + + return update + + def done(self) -> None: + # Nothing to end unless a frame was drawn and it was not the final + # one (update(1) already emitted its own newline). + if ( + self._bar is not None + and self._bar.last_progress is not None + and self._bar.last_progress != 100 + ): + self._bar.done() def download_with_resume( @@ -732,6 +830,7 @@ def download_with_resume( attempts: int = 5, timeout: int = 30, retry_connect_errors: bool = True, + progress: Callable[[int], None] | None = None, ) -> None: """Download ``url`` to ``dest``, resuming partial downloads. @@ -754,6 +853,12 @@ def download_with_resume( of consuming attempts — for callers with their own fallback, like ``download_from_mirrors``. + ``progress``, when given, replaces the built-in progress bar: it is called + with the absolute number of bytes of ``dest`` obtained so far (including + a resumed prefix, and the final size once the file is verified), so a + caller running several downloads at once can draw one combined bar (see + ``BatchDownloadProgress``). + Raises EsphomeError when all attempts are exhausted. """ # Imported lazily: requests is a heavy import (~85ms) and is only needed @@ -777,6 +882,8 @@ def download_with_resume( if dest.is_file() and (sha256 is not None or size is not None): try: _verify_file(dest, sha256, size) + if progress is not None: + progress(size if size is not None else dest.stat().st_size) return except EsphomeError: dest.unlink() @@ -822,7 +929,7 @@ def download_with_resume( # Recorded so a later run can prove an If-Range # resume of this part file safe. _write_download_meta(meta, url, validator, expected_total) - _stream_response_to_file(resp, f, offset, size) + _stream_response_to_file(resp, f, offset, size, progress) # else: a previous run already wrote every byte (or more) but # was killed before the rename below. Skip the network entirely # — a Range request past EOF would draw HTTP 416 — and let @@ -831,6 +938,10 @@ def download_with_resume( expected_size = size if size is not None else expected_total _verify_file(part, sha256, expected_size or None) + if progress is not None: + # Also credits a part file an earlier run completed without + # streaming anything this time. + progress(expected_size or part.stat().st_size) if not expected_size and sha256 is None: # No sha, no size, and the server sent no usable # content-length: nothing can prove the download complete @@ -933,6 +1044,7 @@ def _try_mirrors_once( f: IO[bytes] | None, timeout: int, failures: list[tuple[str, Exception]], + progress: Callable[[int], None] | None = None, ) -> str | None: """Single pass over the resolved mirror ``urls``, one try per URL. @@ -961,6 +1073,7 @@ def _try_mirrors_once( # next mirror immediately; only mid-stream drops # retry-with-resume on the same URL. retry_connect_errors=False, + progress=progress, ) return url except (requests.RequestException, OSError, EsphomeError) as e: @@ -1002,7 +1115,7 @@ def _try_mirrors_once( if offset == 0: validator = _response_validator(resp) expected_total = _content_length(resp) - _stream_response_to_file(resp, f, offset) + _stream_response_to_file(resp, f, offset, progress=progress) if expected_total and f.tell() != expected_total: raise EsphomeError( @@ -1051,6 +1164,7 @@ def download_from_mirrors( substitutions: dict[str, str], target: io.RawIOBase | IO[bytes] | PathType, timeout: int = 30, + progress: Callable[[int], None] | None = None, ) -> str: """ Download file from multiple mirrors with substitution support. @@ -1060,6 +1174,8 @@ def download_from_mirrors( substitutions: Dictionary of substitutions to apply to URLs target: Target file path or file-like object timeout: Download timeout in seconds + progress: Passed through to the download (see ``download_with_resume``); + replaces the built-in per-file bar Returns: The source URL. @@ -1124,7 +1240,9 @@ def download_from_mirrors( for sweep in range(1, _MIRROR_SWEEP_ATTEMPTS + 1): sweep_failures: list[tuple[str, Exception]] = [] if ( - url := _try_mirrors_once(urls, path_target, f, timeout, sweep_failures) + url := _try_mirrors_once( + urls, path_target, f, timeout, sweep_failures, progress + ) ) is not None: return url failures.extend(sweep_failures) diff --git a/esphome/platformio/extra_script.py b/esphome/platformio/extra_script.py new file mode 100644 index 0000000000..2d5667a3e5 --- /dev/null +++ b/esphome/platformio/extra_script.py @@ -0,0 +1,253 @@ +"""Run a PlatformIO library ``extraScript`` against a fake SCons env. + +The shim execs the script with a stand-in ``env``, captures ``env.Append`` +calls (everything else is a logged no-op), and folds the result into the +library's build flags. No sandboxing: the script runs with full process +access, so it carries the same trust as the library's own source. +""" + +from __future__ import annotations + +from collections.abc import Callable +from dataclasses import dataclass, field +import logging +import os +from pathlib import Path +from typing import TYPE_CHECKING + +from esphome.core import EsphomeError + +if TYPE_CHECKING: + from esphome.platformio.library import ConvertedLibrary + +_LOGGER = logging.getLogger(__name__) + + +def apply_extra_script( + component: ConvertedLibrary, + board_mcu: Callable[[], str], + pio_platform: str, +) -> None: + """Run a library's ``extraScript`` and fold its captured env vars into + ``component.data["build"]["flags"]``. + + ``board_mcu`` is a callable so its lookup runs only when a script will. + """ + extra_script = component.data.get("build", {}).get("extraScript") + if not extra_script: + return + # Resolve and confine to the library's source dir so a malicious + # library.json can't escape (e.g. ``"extraScript": "../../etc/passwd"``). + source_path = component.source_dir + library_root = source_path.resolve() + script_path = (source_path / extra_script).resolve() + if not script_path.is_relative_to(library_root): + # More hostile than a missing script; must not be quieter than it + raise EsphomeError( + f"extraScript {extra_script} of library {component.name} escapes " + "the library directory" + ) + if not script_path.is_file(): + # A declared-but-absent script is a broken or half-downloaded + # package, not an unsupported script; PlatformIO fails on it too + raise EsphomeError( + f"extraScript {extra_script} of library {component.name} not found" + ) + result = run_extra_script( + script_path, + library_dir=source_path, + board_mcu=board_mcu(), + pio_platform=pio_platform, + ) + extra_flags = captured_as_build_flags(result, library_dir=source_path) + if not extra_flags: + return + flags = component.data.setdefault("build", {}).setdefault("flags", []) + if isinstance(flags, str): + flags = [flags] + elif not isinstance(flags, list): + # A null/dict value coerced through a list wrapper would inject a + # non-string into the compiler command line; fail naming the library + raise EsphomeError( + f"Library {component.name} has a malformed build.flags " + f"({type(flags).__name__}); expected a string or list" + ) + component.data["build"]["flags"] = [*flags, *extra_flags] + + +# Keys we know how to translate back into ESPHome's build-flag pipeline. +# Other env.Append kwargs are recorded but ignored downstream. +_CAPTURED_KEYS = frozenset({"LIBPATH", "LIBS", "CPPDEFINES", "LINKFLAGS", "CPPFLAGS"}) + + +@dataclass +class ExtraScriptResult: + """Build-var deltas captured from a PIO extra-script ``env.Append`` call.""" + + libpath: list[str] = field(default_factory=list) + libs: list[str] = field(default_factory=list) + cppdefines: list[str | tuple[str, str]] = field(default_factory=list) + linkflags: list[str] = field(default_factory=list) + cppflags: list[str] = field(default_factory=list) + + +class _FakeSConsEnv: + """Minimal SCons ``Environment`` stand-in: ``get`` and ``Append`` work; + every other method is a swallowed no-op so scripts don't abort.""" + + def __init__(self, *, board_mcu: str, pio_env: str, pio_platform: str) -> None: + self._vars: dict[str, str] = { + "BOARD_MCU": board_mcu, + "PIOPLATFORM": pio_platform, + "PIOENV": pio_env, + } + self.result = ExtraScriptResult() + self._warned_methods: set[str] = set() + self._warned_keys: set[str] = set() + + # ----- SCons env API the common scripts use ----- + + def get(self, key: str, default: str | None = None) -> str | None: + return self._vars.get(key, default) + + def __getitem__(self, key: str) -> str: + # Scripts also read env["BOARD_MCU"]; without this the broad + # handler would discard every flag the script captured + return self._vars[key] + + def Append(self, **kwargs) -> None: # noqa: N802 (SCons API name) + for key, value in kwargs.items(): + if key not in _CAPTURED_KEYS: + # Warn once per key so a loop of Appends cannot spam + if key not in self._warned_keys: + self._warned_keys.add(key) + _LOGGER.warning( + "PIO extra-script env.Append(%s=...) is not captured; ignoring", + key, + ) + continue + items = list(value) if isinstance(value, (list, tuple)) else [value] + bucket = getattr(self.result, key.lower()) + bucket.extend(items) + + # ----- Everything else is a no-op so unsupported scripts don't crash ----- + + def __getattr__(self, name: str): + def _noop(*args, **kwargs): + # Once per method: a script whose whole effect is env.Replace() + # must be diagnosable from a normal build log + if name not in self._warned_methods: + self._warned_methods.add(name) + _LOGGER.warning( + "PIO extra-script env.%s(...) is not supported; ignoring", name + ) + + return _noop + + +def run_extra_script( + script_path: Path, + *, + library_dir: Path, + board_mcu: str, + pio_platform: str, +) -> ExtraScriptResult: + """Execute ``script_path`` with a fake SCons env and return captured vars. + + Runs with ``library_dir`` as CWD so relative lookups resolve against + the library tree. A crashed script warns and returns an empty result, + never a partial capture. + """ + env = _FakeSConsEnv( + board_mcu=board_mcu, + pio_env=f"esphome_{board_mcu}", + pio_platform=pio_platform, + ) + try: + source = script_path.read_text(encoding="utf-8") + except OSError as err: + # An unreadable declared script is a broken package, exactly like a + # missing one; must not be quieter than that case + raise EsphomeError(f"extraScript {script_path} is unreadable: {err}") from err + except UnicodeDecodeError as e: + # A content problem, best-effort like a SyntaxError below + _LOGGER.warning( + "PIO extra-script %s (in %s) is not UTF-8 (%r); ignoring its output", + script_path, + library_dir.name, + e, + ) + return ExtraScriptResult() + old_cwd = Path.cwd() + try: + # Inside the try: a SyntaxError in a vendored script is just as + # best-effort as a runtime failure + code = compile(source, str(script_path), "exec") + os.chdir(library_dir) + exec( # noqa: S102 pylint: disable=exec-used + code, + { + "Import": lambda *_args: None, # SCons-side import; harmless here + "env": env, + "__file__": str(script_path), + "__name__": "__pio_extra_script__", + }, + ) + except SystemExit as e: + if not e.code: + # sys.exit() / sys.exit(0) is a normal PlatformIO script ending; + # the capture is complete + return env.result + _LOGGER.warning( + "PIO extra-script %s (in %s) exited with status %r; ignoring its output", + script_path, + library_dir.name, + e.code, + ) + return ExtraScriptResult() + except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught + # Discard any partial capture: half-applied flags could build wrong + # firmware that links cleanly. + _LOGGER.warning( + "PIO extra-script %s (in %s) raised %r; ignoring its output", + script_path, + library_dir.name, + e, + ) + return ExtraScriptResult() + finally: + os.chdir(old_cwd) + return env.result + + +def captured_as_build_flags( + result: ExtraScriptResult, *, library_dir: Path +) -> list[str]: + """Translate captured env vars into -L/-l/-D/raw build flags. + + ``LIBPATH`` entries are made relative to ``library_dir`` so the + generated build files stay portable. + """ + flags: list[str] = [] + library_root = library_dir.resolve() + for path in result.libpath: + # Anchor relative paths to library_dir; the script's CWD has been + # restored by now + resolved = (library_dir / path).resolve() + try: + flags.append(f"-L{resolved.relative_to(library_root)}") + except ValueError: + flags.append(f"-L{resolved}") + flags.extend(f"-l{lib}" for lib in result.libs) + for define in result.cppdefines: + # SCons also accepts dict/list CPPDEFINES; formatting those blind + # would hand the compiler garbage like -D{'FOO': '1'} + if isinstance(define, (tuple, list)) and len(define) == 2: + flags.append(f"-D{define[0]}={define[1]}") + elif isinstance(define, str): + flags.append(f"-D{define}") + else: + _LOGGER.warning("Ignoring unsupported CPPDEFINES entry %r", define) + flags.extend(result.linkflags) + flags.extend(result.cppflags) + return flags diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index ee0a758a31..24bd9db490 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -13,7 +13,8 @@ regardless of which toolchain consumes the result. """ from collections import deque -from collections.abc import Callable +from collections.abc import Callable, Iterable +from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass, field import glob import hashlib @@ -30,7 +31,14 @@ from urllib.request import url2pathname from esphome import git from esphome.core import CORE, EsphomeError, Library -from esphome.framework_helpers import archive_extract_all, download_from_mirrors, rmdir +from esphome.framework_helpers import ( + BATCH_DOWNLOAD_WORKERS, + BatchDownloadProgress, + archive_extract_all, + download_from_mirrors, + rmdir, + run_batch_downloads, +) _LOGGER = logging.getLogger(__name__) @@ -47,20 +55,25 @@ DEFAULT_BUILD_SRC_FILTER = ( DEFAULT_BUILD_SRC_DIRS = "src" DEFAULT_BUILD_INCLUDE_DIR = "include" DEFAULT_BUILD_FLAGS = [] -SRC_FILE_EXTENSIONS = [ - ".c", - ".cpp", - ".cc", - ".cxx", - ".c++", - ".S", - ".spp", - ".SPP", - ".sx", - ".s", - ".asm", - ".ASM", -] +# Suffix -> compiler kind (PlatformIO's CSUFFIXES/CXXSUFFIXES/ASSUFFIXES). +# "asm" merges SCons's AS and ASPP sets: all compile as assembler-with-cpp. +SOURCE_KIND_FOR_SUFFIX: dict[str, str] = { + ".c": "c", + ".cpp": "cxx", + ".cc": "cxx", + ".cxx": "cxx", + ".c++": "cxx", + ".C": "cxx", + ".C++": "cxx", + ".S": "asm", + ".spp": "asm", + ".SPP": "asm", + ".sx": "asm", + ".s": "asm", + ".asm": "asm", + ".ASM": "asm", +} +SRC_FILE_EXTENSIONS = list(SOURCE_KIND_FOR_SUFFIX) DOMAIN = "pio_components" @@ -70,7 +83,12 @@ ESPHOME_DATA_EXTRA_CMAKE_KEY = "EXTRA_CMAKE" class Source: def download( - self, dir_suffix: str, force: bool = False, salt: str = "", namespace: str = "" + self, + dir_suffix: str, + force: bool = False, + salt: str = "", + namespace: str = "", + progress: Callable[[int], None] | None = None, ) -> Path: raise NotImplementedError @@ -87,9 +105,7 @@ class URLSource(Source): def __init__(self, url: str): self.url = url - def download( - self, dir_suffix: str, force: bool = False, salt: str = "", namespace: str = "" - ) -> Path: + def _cache_dir(self, dir_suffix: str, salt: str, namespace: str) -> Path: # Namespace the cache per backend (e.g. pio_components/idf, .../zephyr) so # the build files each backend writes into the library dir can't collide. base_dir = Path(CORE.data_dir) / DOMAIN @@ -99,7 +115,23 @@ class URLSource(Source): h.update(self.url.encode()) if salt: h.update(salt.encode()) - path = base_dir / h.hexdigest()[:8] / dir_suffix + return base_dir / h.hexdigest()[:8] / dir_suffix + + def is_cached(self, dir_suffix: str, salt: str = "", namespace: str = "") -> bool: + """Whether a completed extraction already exists for this source.""" + return ( + self._cache_dir(dir_suffix, salt, namespace) / ".esphome_extracted" + ).is_file() + + def download( + self, + dir_suffix: str, + force: bool = False, + salt: str = "", + namespace: str = "", + progress: Callable[[int], None] | None = None, + ) -> Path: + path = self._cache_dir(dir_suffix, salt, namespace) # Marker file written last to signal a complete extraction. Using a # marker (instead of just `path.is_dir()`) means an interrupted # extraction is correctly detected and re-run on the next invocation, @@ -111,10 +143,12 @@ class URLSource(Source): # Download in temporary file with tempfile.NamedTemporaryFile() as tmp: - _LOGGER.info("Downloading %s ...", self.url) + if progress is None: + # A batch caller draws one combined bar and logs the list + _LOGGER.info("Downloading %s ...", self.url) _LOGGER.debug("Location: %s", path) - download_from_mirrors([self.url], {}, tmp.file) + download_from_mirrors([self.url], {}, tmp.file, progress=progress) _LOGGER.debug("Extracting archive to %s ...", path) archive_extract_all(tmp.file, path) @@ -131,7 +165,12 @@ class GitSource(Source): self.ref = ref def download( - self, dir_suffix: str, force: bool = False, salt: str = "", namespace: str = "" + self, + dir_suffix: str, + force: bool = False, + salt: str = "", + namespace: str = "", + progress: Callable[[int], None] | None = None, ) -> Path: domain = DOMAIN if namespace: @@ -166,7 +205,12 @@ class LocalSource(Source): self.local_path = path def download( - self, dir_suffix: str, force: bool = False, salt: str = "", namespace: str = "" + self, + dir_suffix: str, + force: bool = False, + salt: str = "", + namespace: str = "", + progress: Callable[[int], None] | None = None, ) -> Path: src = Path(self.local_path) if not src.is_dir(): @@ -203,6 +247,14 @@ class InvalidLibrary(Exception): pass +class IncompatiblePlatform(InvalidLibrary): + """The manifest's platform filter rejected the target platform. + + A distinct type so callers can treat the routine cross-platform skip + differently from other manifest problems without matching message text. + """ + + class ConvertedLibrary: """A resolved PlatformIO library plus its parsed manifest and on-disk path. @@ -251,7 +303,13 @@ class ConvertedLibrary: def get_require_name(self): return self.get_sanitized_name().replace("/", "__") - def download(self, force: bool = False, salt: str = "", namespace: str = ""): + def download( + self, + force: bool = False, + salt: str = "", + namespace: str = "", + progress: Callable[[int], None] | None = None, + ): """Fetch the library into the shared cache and record its ``path``. The cache directory is named after the sanitized library name; backends @@ -260,7 +318,11 @@ class ConvertedLibrary: ``get_require_name``). ``namespace`` keeps each backend's cache separate. """ self.path = self.source.download( - self.get_sanitized_name(), force=force, salt=salt, namespace=namespace + self.get_sanitized_name(), + force=force, + salt=salt, + namespace=namespace, + progress=progress, ) self.source_path = self.source.source_root(self.path) @@ -432,7 +494,7 @@ def check_library_data(data: dict, platform: str | None, framework: str): valid_platforms = platform is None or "*" in platforms or platform in platforms if not valid_platforms: - raise InvalidLibrary(f"Unsupported library platforms: {platforms}") + raise IncompatiblePlatform(f"Unsupported library platforms: {platforms}") frameworks = data.get("frameworks", "*") if isinstance(frameworks, str): @@ -455,7 +517,7 @@ def check_library_data(data: dict, platform: str | None, framework: str): ) -def _parse_library_json(library_json_path: PathType): +def parse_library_json(library_json_path: PathType): """ Load and parse a JSON file describing a library. @@ -469,7 +531,7 @@ def _parse_library_json(library_json_path: PathType): return json.load(fp) -def _parse_library_properties(library_properties_path: PathType): +def parse_library_properties(library_properties_path: PathType): """ Parse a key-value platformio .properties style file into a dictionary. @@ -553,19 +615,127 @@ def _resolve_registry_version( return owner, name, best["name"], pkgfile["download_url"] -def _normalize_dependencies(dependencies: Any) -> list[dict]: +def split_flag_entry(entry: Any, owner: str) -> list[str]: + """``shlex.split`` with a clean error naming the offending flags entry.""" + # Late import: shlex is only needed when actually lexing flags + import shlex + + try: + return shlex.split(entry) + except (ValueError, AttributeError, TypeError) as err: + # AttributeError/TypeError: a dict or number from a third-party + # manifest; name the entry instead of an opaque shlex traceback + raise EsphomeError(f"Malformed build flag {entry!r} in {owner}: {err}") from err + + +def lex_build_flags(entries: str | list[str], owner: str) -> list[str]: + """Shell-lex ``build.flags`` entries the way PlatformIO's ParseFlags + does; bare -I/-L/-l/-D tokens re-glue to their argument.""" + # Join per entry, as SCons's ParseFlags lexes each string independently: + # a dangling -I ending one entry must warn, not absorb the next entry's + # first token. + return [ + token + for entry in ensure_list(entries) + for token in join_flag_args(split_flag_entry(entry, owner), owner) + ] + + +# Flags whose argument may follow as a separate token; ParseFlags glues them +BARE_ARG_FLAGS = frozenset({"-I", "-L", "-l", "-D"}) + + +def join_flag_args(tokens: Iterable[str], owner: str) -> list[str]: + """Join a bare ``-I``/``-L``/``-l``/``-D`` with its following token, + the way PlatformIO's ParseFlags lexes them.""" + out: list[str] = [] + it = iter(tokens) + for tok in it: + if tok in BARE_ARG_FLAGS: + arg = next(it, None) + if arg is None: + _LOGGER.warning("Ignoring trailing '%s' in %s build flags", tok, owner) + break + tok += arg + out.append(tok) + return out + + +def warn_properties_depends(name: str, data: object) -> None: + """Warn when a manifest declares dependencies only as ``depends=``. + + The dependency walk reads the JSON ``dependencies`` key; the raw + ``library.properties`` spelling would otherwise drop silently. + """ + if isinstance(data, dict) and not data.get("dependencies") and data.get("depends"): + _LOGGER.warning( + "Library %s declares dependencies via library.properties " + "depends=, which are not resolved automatically; add them with " + "add_library() if needed", + name, + ) + + +def dependency_is_usable( + dep: dict, platform: str | None, framework: str, requester: str +) -> bool: + """Compatibility filter for a manifest dependency: platform mismatches + skip at debug, any other ``InvalidLibrary`` warns naming the requester.""" + try: + check_library_data(dep, platform, framework) + except IncompatiblePlatform as e: + _LOGGER.debug("Skip dependency %s of %s: %s", dep.get("name"), requester, e) + return False + except InvalidLibrary as e: + _LOGGER.warning( + "Skipping dependency %s of %s: %s", dep.get("name"), requester, e + ) + return False + return True + + +def _valid_dependency_entry(entry: dict, manifest_name: str) -> bool: + """Whether a normalized entry carries a usable name and version. + + The name must be a non-empty string (every consumer indexes or joins + it); a present version must be a string (a container would raise from + ``set.add()``, an int fails opaquely inside the registry resolution). + Invalid entries warn naming the manifest. + """ + name = entry.get("name") + if ( + isinstance(name, str) + and name + and ("version" not in entry or isinstance(entry["version"], str)) + ): + return True + _LOGGER.warning( + "Ignoring unrecognized dependency entry %r of %s", entry, manifest_name + ) + return False + + +def normalize_dependencies( + dependencies: Any, manifest_name: str = "manifest" +) -> list[dict]: """Normalize a library manifest's ``dependencies`` to a list of dicts. - PIO's library.json accepts both the list-of-dicts form and the shorthand - dict form (``{"owner/Name": "version_spec"}``); normalize the latter so - callers see a uniform list. + PIO's library.json accepts the list-of-dicts form, the shorthand dict + form (``{"owner/Name": "version_spec"}``), bare name strings inside the + list, and a plain (possibly comma-separated) string; normalize them all + so callers see a uniform list. ``manifest_name`` names the manifest in the + warning for entries that cannot be normalized. """ if not dependencies: return [] + if isinstance(dependencies, str): + # A plain string is one or more comma-separated names; iterating it + # as a list would shred it into one-character "libraries" + return [{"name": n.strip()} for n in dependencies.split(",") if n.strip()] if isinstance(dependencies, dict): normalized = [] for raw_name, spec in dependencies.items(): - if "/" in raw_name: + if isinstance(raw_name, str) and "/" in raw_name: owner, pkgname = raw_name.split("/", 1) else: owner, pkgname = None, raw_name @@ -574,9 +744,31 @@ def _normalize_dependencies(dependencies: Any) -> list[dict]: entry.update(spec) else: entry["version"] = spec - normalized.append(entry) + if _valid_dependency_entry(entry, manifest_name): + normalized.append(entry) return normalized - return [d for d in dependencies if isinstance(d, dict)] + if not isinstance(dependencies, (list, tuple)): + _LOGGER.warning( + "Ignoring unrecognized dependencies %r of %s", + dependencies, + manifest_name, + ) + return [] + normalized = [] + for entry in dependencies: + if isinstance(entry, dict): + if _valid_dependency_entry(entry, manifest_name): + normalized.append(entry) + elif isinstance(entry, str) and entry: + # PIO also accepts a bare list of names ("dependencies": ["Wire"]) + normalized.append({"name": entry}) + else: + _LOGGER.warning( + "Ignoring unrecognized dependency entry %r of %s", + entry, + manifest_name, + ) + return normalized @dataclass @@ -688,6 +880,111 @@ def _node_key( return name, "registry", (owner, pkgname) +def lib_ignore_set() -> set[str]: + """The ``lib_ignore`` names from ``esphome->platformio_options``, + normalized to lowercase short names (the part after the ``/``).""" + return { + name.split("/")[-1].lower() + for name in CORE.platformio_options.get("lib_ignore", []) + } + + +def is_lib_ignored(name: str | None, lib_ignore: set[str]) -> bool: + """Whether ``name`` matches the normalized ``lib_ignore`` set.""" + return ( + bool(lib_ignore) + and name is not None + and (name.split("/")[-1].lower() in lib_ignore) + ) + + +def _content_lengths(urls: list[str]) -> list[int | None]: + """Content-Length per URL via HEAD requests; None when unknown.""" + import requests + + def head(url: str) -> int | None: + try: + resp = requests.head(url, timeout=10, allow_redirects=True) + if not resp.ok: + _LOGGER.debug("HEAD %s returned %s", url, resp.status_code) + return None + return int(resp.headers.get("content-length", 0)) or None + except (requests.RequestException, ValueError) as err: + _LOGGER.debug("HEAD %s failed: %s", url, err) + return None + + with ThreadPoolExecutor(max_workers=min(BATCH_DOWNLOAD_WORKERS, len(urls))) as ex: + return list(ex.map(head, urls)) + + +def _prefetch_wave( + wave: list[tuple[str, ConvertedLibrary]], salt: str, namespace: str +) -> None: + """Best-effort parallel download of a wave's registry archives. + + The walk's own ``download()`` call stays authoritative (it surfaces real + failures, with resume); bars are suppressed since parallel bars would + interleave. Duplicate URLs prefetch once so two threads never extract + into the same cache directory. + """ + components: list[ConvertedLibrary] = [] + seen: set[str] = set() + for _key, component in wave: + if not isinstance(component.source, URLSource): + continue + if component.source.url in seen: + continue + seen.add(component.source.url) + try: + cached = component.source.is_cached( + component.get_sanitized_name(), salt=salt, namespace=namespace + ) + except Exception as err: # noqa: BLE001 # pylint: disable=broad-exception-caught + # Best-effort: a failing probe prefetches (and re-downloads) + _LOGGER.debug("Cache probe for %s failed: %s", component.name, err) + cached = False + if cached: + # A completed extraction downloads nothing; a warm build must + # stay silent + continue + components.append(component) + if len(components) < 2: + return + _LOGGER.info( + "Downloading %d libraries: %s", + len(components), + ", ".join(c.name for c in components), + ) + # One combined bar over the batch, sized by HEAD requests. An unknown + # size would mean a silent multi-MB download; fall back to sequential + # downloads with their per-file bars instead. + sizes = _content_lengths([c.source.url for c in components]) + if not all(sizes): + # Name the culprits so the fallback is distinguishable from a hang + _LOGGER.debug( + "No Content-Length for %s; downloading sequentially", + ", ".join( + c.source.url + for c, size in zip(components, sizes, strict=True) + if not size + ), + ) + return + + def _fetch(component: ConvertedLibrary): + return lambda tracker: component.download( + salt=salt, namespace=namespace, progress=tracker + ) + + failures = run_batch_downloads( + BatchDownloadProgress("Downloading libraries", sum(sizes)), + [(component.name, _fetch(component)) for component in components], + ) + for name, err in failures: + # The sequential call below retries and raises the real error + _LOGGER.warning("Prefetch of %s failed (retrying sequentially): %s", name, err) + + def convert_libraries( libraries: list[Library], backend: LibraryBackend ) -> list[ConvertedLibrary]: @@ -713,10 +1010,7 @@ def convert_libraries( """ nodes: dict[str, _LibNode] = {} - lib_ignore = { - name.split("/")[-1].lower() - for name in CORE.platformio_options.get("lib_ignore", []) - } + lib_ignore = lib_ignore_set() # The generated build files inside the shared cache bake in the dependency # wiring, which lib_ignore changes; salt the cache path so configs with @@ -728,11 +1022,6 @@ def convert_libraries( else "" ) - def is_ignored(name: str | None) -> bool: - if not lib_ignore or name is None: - return False - return name.split("/")[-1].lower() in lib_ignore - def add_spec(name: str | None, version: str | None, repository: str | None) -> str: key, kind, locator = _node_key(name, version, repository) node = nodes.get(key) or _LibNode(key=key, is_git=kind == "git") @@ -781,7 +1070,7 @@ def convert_libraries( top_level = [ add_spec(library.name, library.version, library.repository) for library in libraries - if not is_ignored(library.name) + if not is_lib_ignored(library.name, lib_ignore) ] # Collect + resolve to a fixpoint: a node is (re)resolved whenever its @@ -792,105 +1081,126 @@ def convert_libraries( top_level_keys = set(top_level) worklist = deque(dict.fromkeys(top_level)) while worklist: - key = worklist.popleft() - node = nodes[key] + # Drain the frontier sequentially (spec resolution mutates shared + # node state), then prefetch the wave's registry archives in + # parallel; the per-component download() below stays authoritative. + wave: list[tuple[str, ConvertedLibrary]] = [] + while worklist: + key = worklist.popleft() + node = nodes[key] - # A node is queued once per referring edge; skip the (uncached) registry - # lookup + download + dependency walk unless its requirement set grew - # since the last resolve. Requirements only ever grow, so this still - # converges the fixpoint and terminates dependency cycles. - requirements = frozenset(node.requirements) - if resolved_requirements.get(key) == requirements: - continue - resolved_requirements[key] = requirements + # Re-resolve only when the requirement set grew; requirements + # only ever grow, so the fixpoint converges and cycles terminate + requirements = frozenset(node.requirements) + if resolved_requirements.get(key) == requirements: + continue + resolved_requirements[key] = requirements - if node.is_git: - component = ConvertedLibrary(key, "*", GitSource(node.url, node.ref)) - elif node.is_local: - component = ConvertedLibrary(key, "*", LocalSource(node.local_path)) - else: - owner, name, version, url = _resolve_registry_version( - node.owner, node.pkgname, node.requirements - ) - component = ConvertedLibrary( - _owner_pkgname_to_name(owner, name), version, URLSource(url) - ) - component.download(salt=salt, namespace=backend.cache_key) + if node.is_git: + component = ConvertedLibrary(key, "*", GitSource(node.url, node.ref)) + elif node.is_local: + component = ConvertedLibrary(key, "*", LocalSource(node.local_path)) + else: + owner, name, version, url = _resolve_registry_version( + node.owner, node.pkgname, node.requirements + ) + component = ConvertedLibrary( + _owner_pkgname_to_name(owner, name), version, URLSource(url) + ) + wave.append((key, component)) + _prefetch_wave(wave, salt, backend.cache_key) + for key, component in wave: + node = nodes[key] + component.download(salt=salt, namespace=backend.cache_key) - source_dir = component.source_dir - library_json_path = source_dir / "library.json" - library_properties_path = source_dir / "library.properties" - has_json = library_json_path.is_file() - has_properties = library_properties_path.is_file() - if not has_json and not has_properties and not node.is_local: - # The shared cache can hold a broken copy (e.g. a clone or an - # extraction interrupted by a killed process). Force one - # re-download so a bad cache entry self-heals instead of failing - # every build until the user runs a full clean. A local source is - # read in place, so there is nothing to re-download. - _LOGGER.warning( - "Library %s at %s is missing library.json and library.properties; " - "re-downloading", - key, - source_dir, - ) - component.download(force=True, salt=salt, namespace=backend.cache_key) + source_dir = component.source_dir + library_json_path = source_dir / "library.json" + library_properties_path = source_dir / "library.properties" has_json = library_json_path.is_file() has_properties = library_properties_path.is_file() - if has_json: - component.data = _parse_library_json(library_json_path) - elif has_properties: - component.data = _parse_library_properties(library_properties_path) - else: - # For a local library a missing manifest is user input, so raise - # EsphomeError (clean CLI message) like the missing-directory case; - # for registry/git a missing manifest means a corrupt cache, which - # is not user error, so keep RuntimeError. - error_cls = EsphomeError if node.is_local else RuntimeError - raise error_cls( - f"Invalid PIO library {key}: missing library.json and " - f"library.properties in {source_dir}" - ) + if not has_json and not has_properties and not node.is_local: + # An interrupted clone/extraction self-heals with one forced + # re-download; a local source has nothing to re-download + _LOGGER.warning( + "Library %s at %s is missing library.json and library.properties; " + "re-downloading", + key, + source_dir, + ) + component.download(force=True, salt=salt, namespace=backend.cache_key) + has_json = library_json_path.is_file() + has_properties = library_properties_path.is_file() + if has_json: + component.data = parse_library_json(library_json_path) + elif has_properties: + component.data = parse_library_properties(library_properties_path) + else: + # Local sources are user input (EsphomeError); a registry/git + # miss means a corrupt cache (RuntimeError) + error_cls = EsphomeError if node.is_local else RuntimeError + raise error_cls( + f"Invalid PIO library {key}: missing library.json and " + f"library.properties in {source_dir}" + ) - try: - check_library_data(component.data, backend.platform, backend.framework) - except InvalidLibrary as e: - # Skip an incompatible transitive dependency, but fail fast if a - # top-level library the build explicitly requested is incompatible. - if key in top_level_keys: - raise RuntimeError( - f"Requested library {key} is not compatible with " - f"{backend.framework}: {e}" - ) from e - _LOGGER.debug("Skip incompatible dependency %s: %s", key, str(e)) - continue - components[key] = component + if not isinstance(component.data, dict) or not isinstance( + component.data.get("build", {}), dict + ): + # A bare json.load imposes no shape; every backend dereferences + # data/build, so validate once here and name the library + raise EsphomeError(f"Library {key} has a malformed manifest") + warn_properties_depends(component.name, component.data) - # Requirements changed (we got past the short-circuit above), so - # (re)walk this component's dependencies. - node.edges = set() - for dependency in _normalize_dependencies(component.data.get("dependencies")): - if "name" not in dependency or "version" not in dependency: - continue try: - check_library_data(dependency, backend.platform, backend.framework) + check_library_data(component.data, backend.platform, backend.framework) except InvalidLibrary as e: - _LOGGER.debug("Skip dependency %s: %s", dependency.get("name"), str(e)) + # An explicitly requested library fails fast; the routine + # cross-platform skip stays at debug, other causes warn + if key in top_level_keys: + raise RuntimeError( + f"Requested library {key} is not compatible with " + f"{backend.framework}: {e}" + ) from e + if isinstance(e, IncompatiblePlatform): + _LOGGER.debug("Skip incompatible dependency %s: %s", key, str(e)) + else: + _LOGGER.warning("Skipping dependency %s: %s", key, str(e)) continue - dep_name = _owner_pkgname_to_name( - dependency.get("owner"), dependency.get("name") - ) - if is_ignored(dep_name): - _LOGGER.debug("Skip ignored dependency %s", dep_name) - continue - # The version field may actually be a URL (git/archive dependency). - dep_version = dependency["version"] - dep_url = _url_or_none(dep_version) - if dep_url is not None: - dep_version = None - dep_key = add_spec(dep_name, dep_version, dep_url) - node.edges.add(dep_key) - worklist.append(dep_key) + components[key] = component + + # Requirements changed (we got past the short-circuit above), so + # (re)walk this component's dependencies. + node.edges = set() + for dependency in normalize_dependencies( + component.data.get("dependencies"), component.name + ): + if "version" not in dependency: + # Cannot resolve from the registry; the arduino-backend + # PR adds the reconciliation that reports real drops + _LOGGER.debug( + "Skip version-less dependency %r of %s", + dependency.get("name"), + component.name, + ) + continue + if not dependency_is_usable( + dependency, backend.platform, backend.framework, component.name + ): + continue + dep_name = _owner_pkgname_to_name( + dependency.get("owner"), dependency.get("name") + ) + if is_lib_ignored(dep_name, lib_ignore): + _LOGGER.debug("Skip ignored dependency %s", dep_name) + continue + # The version field may actually be a URL (git/archive dependency). + dep_version = dependency["version"] + dep_url = _url_or_none(dep_version) + if dep_url is not None: + dep_version = None + dep_key = add_spec(dep_name, dep_version, dep_url) + node.edges.add(dep_key) + worklist.append(dep_key) # A git or local source wins over the same component requested from the # registry. That's intentional, but warn so the dropped registry spec isn't diff --git a/esphome/platformio/toolchain.py b/esphome/platformio/toolchain.py index d76581d032..a98ef3e9fe 100644 --- a/esphome/platformio/toolchain.py +++ b/esphome/platformio/toolchain.py @@ -254,6 +254,9 @@ def _ccache_runs(ccache: str) -> bool: stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, timeout=15, + # Repo-wide convention (posix_spawn fast path); see the + # close_fds=False call sites across esphome/ and script/helpers.py + close_fds=False, ) except (OSError, subprocess.SubprocessError): _LOGGER.warning( diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index f9e048f6f4..c5f889a37a 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -1,12 +1,12 @@ import glob import hashlib import json -import os from pathlib import Path from unittest.mock import MagicMock import pytest +from esphome.components import esp32 as esp32_module from esphome.const import ( KEY_CORE, KEY_TARGET_FRAMEWORK, @@ -16,6 +16,7 @@ from esphome.const import ( ) from esphome.core import CORE, Library from esphome.espidf.component import ( + _emit_idf_component, generate_cmakelists_txt, generate_idf_component_yml, generate_idf_components, @@ -26,11 +27,11 @@ from esphome.platformio.library import ( GitSource, URLSource, _node_key, - _normalize_dependencies, - _parse_library_json, - _parse_library_properties, _resolve_registry_version, collect_filtered_files, + normalize_dependencies, + parse_library_json, + parse_library_properties, split_list_by_condition, ) @@ -369,133 +370,11 @@ def test_generate_idf_component_yml_missing_path_raises(tmp_component): generate_idf_component_yml(tmp_component) -def test_extra_script_captures_libpath_libs_and_defines(tmp_path): - from esphome.espidf.extra_script import captured_as_build_flags, run_extra_script - - (tmp_path / "src" / "esp32").mkdir(parents=True) - script = tmp_path / "extra_script.py" - script.write_text( - "Import('env')\n" - "mcu = env.get('BOARD_MCU')\n" - "env.Append(\n" - " LIBPATH=[join('src', mcu)],\n" - " LIBS=['algobsec'],\n" - " CPPDEFINES=['FOO', ('BAR', '1')],\n" - " LINKFLAGS=['-Wl,--gc-sections'],\n" - ")\n" - ) - # The script uses bare ``join`` (PIO's extra-scripts run inside SCons - # where this is in scope). Inject it via the script header so the - # shim's exec namespace can resolve it. - script.write_text("from os.path import join\n" + script.read_text()) - - result = run_extra_script(script, library_dir=tmp_path, idf_target="esp32") - - assert result.libpath == [str(Path("src") / "esp32")] - assert result.libs == ["algobsec"] - assert ("BAR", "1") in result.cppdefines - assert "FOO" in result.cppdefines - assert result.linkflags == ["-Wl,--gc-sections"] - - flags = captured_as_build_flags(result, library_dir=tmp_path) - sep = os.sep - assert f"-Lsrc{sep}esp32" in flags - assert "-lalgobsec" in flags - assert "-DFOO" in flags - assert "-DBAR=1" in flags - assert "-Wl,--gc-sections" in flags - - -def test_extra_script_libpath_relative_resolves_against_library_dir( - tmp_path, monkeypatch -): - """Relative LIBPATH entries must resolve against ``library_dir``, not the - caller's CWD (the shim restores CWD before ``captured_as_build_flags`` - runs).""" - from esphome.espidf.extra_script import ExtraScriptResult, captured_as_build_flags - - (tmp_path / "lib" / "esp32").mkdir(parents=True) - elsewhere = tmp_path.parent / "not_the_library_dir" - elsewhere.mkdir(exist_ok=True) - monkeypatch.chdir(elsewhere) - - result = ExtraScriptResult(libpath=["lib/esp32"]) - flags = captured_as_build_flags(result, library_dir=tmp_path) - - sep = os.sep - assert flags == [f"-Llib{sep}esp32"] - - -def test_extra_script_libpath_absolute_outside_library_dir(tmp_path): - from esphome.espidf.extra_script import ExtraScriptResult, captured_as_build_flags - - outside = tmp_path.parent / "system_lib" - outside.mkdir(exist_ok=True) - result = ExtraScriptResult(libpath=[str(outside)]) - - flags = captured_as_build_flags(result, library_dir=tmp_path) - assert flags == [f"-L{outside.resolve()}"] - - -def test_extra_script_failure_returns_empty_result(tmp_path, caplog): - from esphome.espidf.extra_script import run_extra_script - - script = tmp_path / "broken.py" - script.write_text("raise RuntimeError('boom')\n") - - with caplog.at_level("WARNING"): - result = run_extra_script(script, library_dir=tmp_path, idf_target="esp32") - - assert result.libpath == [] - assert result.libs == [] - assert "broken.py" in caplog.text - - -def test_apply_extra_script_path_traversal_is_rejected(tmp_path): - from esphome.espidf.component import _apply_extra_script - - library_dir = tmp_path / "lib" - library_dir.mkdir() - outside = tmp_path / "evil.py" - outside.write_text("env.Append(LIBS=['pwned'])\n") - - c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) - c.path = library_dir - c.data = {"build": {"extraScript": "../evil.py"}} - - _apply_extra_script(c) - - # Nothing was folded into flags: the traversal was rejected before - # the script could run. - assert "flags" not in c.data["build"] - - -def test_apply_extra_script_merges_into_existing_flags(tmp_path, monkeypatch): - from esphome.components import esp32 as esp32_module - - monkeypatch.setattr(esp32_module, "get_esp32_variant", lambda: "ESP32") - - from esphome.espidf.component import _apply_extra_script - - (tmp_path / "src").mkdir() - script = tmp_path / "extra.py" - script.write_text("env.Append(LIBS=['algobsec'])\n") - - c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) - c.path = tmp_path - c.data = {"build": {"extraScript": "extra.py", "flags": ["-DEXISTING"]}} - - _apply_extra_script(c) - - assert "-DEXISTING" in c.data["build"]["flags"] - assert "-lalgobsec" in c.data["build"]["flags"] - - def test_parse_library_json(tmp_path): f = tmp_path / "library.json" f.write_text(json.dumps({"name": "test"})) - result = _parse_library_json(f) + result = parse_library_json(f) assert result["name"] == "test" @@ -510,7 +389,7 @@ empty= """ ) - result = _parse_library_properties(f) + result = parse_library_properties(f) assert result["name"] == "Test" assert result["version"] == "1.0" @@ -680,22 +559,22 @@ def test_node_key_registry_bare_name(): def test_normalize_dependencies_none(): - assert _normalize_dependencies(None) == [] + assert normalize_dependencies(None) == [] def test_normalize_dependencies_list_form(): deps = [{"name": "foo", "version": "1.0"}] - assert _normalize_dependencies(deps) == [{"name": "foo", "version": "1.0"}] + assert normalize_dependencies(deps) == [{"name": "foo", "version": "1.0"}] def test_normalize_dependencies_dict_form(): - out = _normalize_dependencies({"nanopb/Nanopb": "^0.4.91", "BareName": "1.2.3"}) + out = normalize_dependencies({"nanopb/Nanopb": "^0.4.91", "BareName": "1.2.3"}) assert {"name": "Nanopb", "owner": "nanopb", "version": "^0.4.91"} in out assert {"name": "BareName", "owner": None, "version": "1.2.3"} in out def test_normalize_dependencies_dict_form_nested_spec(): - out = _normalize_dependencies( + out = normalize_dependencies( {"nanopb/Nanopb": {"version": "^0.4.91", "platforms": "espidf"}} ) assert out == [ @@ -1187,6 +1066,36 @@ def test_idf_component_download_passes_salt() -> None: c.download(force=True, salt="abcd1234", namespace="idf") source.download.assert_called_once_with( - "owner/name", force=True, salt="abcd1234", namespace="idf" + "owner/name", force=True, salt="abcd1234", namespace="idf", progress=None ) assert c.path == Path("/converted/owner/name") + + +def test_emit_idf_component_wires_esp32_target(tmp_path, monkeypatch): + """Emitting a component resolves the esp32 variant into the shared + extraScript helper.""" + + monkeypatch.setattr(esp32_module, "get_esp32_variant", lambda: "ESP32") + (tmp_path / "src").mkdir() + script = tmp_path / "extra.py" + script.write_text("env.Append(LIBS=[env.get('BOARD_MCU')])\n") + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py"}} + _emit_idf_component(c) + assert c.data["build"]["flags"] == ["-lesp32"] + + +def test_build_flags_dangling_flag_does_not_cross_entries( + tmp_path, caplog: pytest.LogCaptureFixture +) -> None: + """Each entry is lexed independently, as ParseFlags does: a dangling -I ending one + entry warns instead of absorbing the next entry's first token.""" + (tmp_path / "src").mkdir() + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"flags": ["-Wall -I", "-DFOO=1"]}} + content = generate_cmakelists_txt(c) + assert "FOO=1" in content + assert "-I-DFOO" not in content + assert "Ignoring trailing '-I'" in caplog.text diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index d8e7738569..b5296def66 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -2,6 +2,7 @@ # pylint: disable=protected-access +from concurrent.futures import ThreadPoolExecutor from contextlib import contextmanager import importlib.util import io @@ -14,7 +15,7 @@ import subprocess import sys import tarfile from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest @@ -887,6 +888,58 @@ _PREFETCH_JSON = json.dumps( ) +def test_prefetch_leaves_unverifiable_entries_to_the_installer( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """An entry missing sha256 or size must not download unverified; the + installer handles it and fails loudly on a bad archive.""" + entries = json.loads(_PREFETCH_JSON) + del entries[0]["sha256"] + del entries[1]["size"] + entries.append( + { + "name": "gcc@14.2.0", + "url": "https://example.com/gcc.tar.gz", + "size": 67, + "sha256": "ef" * 32, + "dest": "gcc.tar.gz", + } + ) + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, json.dumps(entries), ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch("esphome.espidf.framework.BatchDownloadProgress") as progress_cls, + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + assert [call[0][0] for call in download.call_args_list] == [ + "https://example.com/gcc.tar.gz" + ] + assert download.call_args[1]["sha256"] == "ef" * 32 + progress_cls.assert_called_once_with("Downloading ESP-IDF tools", 67) + assert "cmake@3.30.2 has no sha256/size" in caplog.text + assert "ninja@1.12.1 has no sha256/size" in caplog.text + + +def test_prefetch_all_entries_unverifiable_is_a_noop(tmp_path: Path) -> None: + entries = json.loads(_PREFETCH_JSON) + for entry in entries: + del entry["sha256"] + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, json.dumps(entries), ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + download.assert_not_called() + + def test_prefetch_downloads_each_archive_with_resume(tmp_path: Path) -> None: with ( patch( @@ -895,16 +948,73 @@ def test_prefetch_downloads_each_archive_with_resume(tmp_path: Path) -> None: ), patch("esphome.espidf.framework.download_with_resume") as download, patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch("esphome.espidf.framework.BatchDownloadProgress") as progress_cls, ): + # Materialize the lazy mock before threads race its first creation + tracker = progress_cls.return_value.tracker.return_value _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) dist = get_idf_tools_path() / "dist" - assert download.call_count == 2 - assert download.call_args_list[0][0] == ( - "https://example.com/cmake.tar.gz", - dist / "cmake-3.30.2.tar.gz", - ) - assert download.call_args_list[0][1] == {"sha256": "ab" * 32, "size": 123} + # Archives download concurrently, so the call order is not fixed. + calls = {call[0]: call[1] for call in download.call_args_list} + assert set(calls) == { + ("https://example.com/cmake.tar.gz", dist / "cmake-3.30.2.tar.gz"), + ("https://example.com/ninja.zip", dist / "ninja.zip"), + } + kwargs = calls[("https://example.com/cmake.tar.gz", dist / "cmake-3.30.2.tar.gz")] + assert kwargs["sha256"] == "ab" * 32 + assert kwargs["size"] == 123 + # every archive reports into the one combined progress bar + progress_cls.assert_called_once_with("Downloading ESP-IDF tools", 123 + 45) + assert all(kw["progress"] is tracker for kw in calls.values()) + + +def test_prefetch_downloads_archives_concurrently(tmp_path: Path) -> None: + """More than one archive fans out over a bounded thread pool.""" + entries = [ + { + "name": f"tool{i}@1", + "url": f"https://example.com/tool{i}.tar.gz", + "size": 10, + "sha256": "ab" * 32, + "dest": f"tool{i}.tar.gz", + } + for i in range(6) + ] + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, json.dumps(entries), ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch( + "esphome.framework_helpers.ThreadPoolExecutor", wraps=ThreadPoolExecutor + ) as pool, + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + + pool.assert_called_once_with(max_workers=4) + assert download.call_count == 6 + + +def test_prefetch_single_archive_uses_one_worker(tmp_path: Path) -> None: + entries = json.loads(_PREFETCH_JSON)[:1] + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, json.dumps(entries), ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch( + "esphome.framework_helpers.ThreadPoolExecutor", wraps=ThreadPoolExecutor + ) as pool, + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + + pool.assert_called_once_with(max_workers=1) + assert download.call_count == 1 def test_prefetch_skips_already_downloaded_archives(tmp_path: Path) -> None: @@ -964,6 +1074,11 @@ def test_prefetch_one_failed_archive_does_not_stop_the_rest( ) -> None: """A single archive failing its download must not abort the prefetch of the remaining archives.""" + + def _fail_cmake_download(url: str, *args, **kwargs) -> None: + if "cmake" in url: + raise OSError("network down") + with ( patch( "esphome.espidf.framework.run_command", @@ -971,7 +1086,7 @@ def test_prefetch_one_failed_archive_does_not_stop_the_rest( ), patch( "esphome.espidf.framework.download_with_resume", - side_effect=[OSError("network down"), None], + side_effect=_fail_cmake_download, ) as download, patch("esphome.espidf.framework.get_system_python_path", return_value="python"), ): @@ -981,6 +1096,29 @@ def test_prefetch_one_failed_archive_does_not_stop_the_rest( assert "Could not prefetch cmake@3.30.2" in caplog.text +def test_prefetch_finishes_progress_bar_and_cancels_queue(tmp_path: Path) -> None: + """The batch bar is closed out after the pool, and the pool is shut down + with cancel_futures so Ctrl-C does not drain every queued archive.""" + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, _PREFETCH_JSON, ""), + ), + patch("esphome.espidf.framework.download_with_resume"), + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch("esphome.espidf.framework.BatchDownloadProgress") as progress_cls, + patch( + "esphome.framework_helpers.ThreadPoolExecutor", wraps=ThreadPoolExecutor + ) as pool_cls, + ): + pool = MagicMock(wraps=ThreadPoolExecutor(max_workers=2)) + pool_cls.return_value = pool + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + + pool.shutdown.assert_called_once_with(wait=True, cancel_futures=True) + progress_cls.return_value.done.assert_called_once_with() + + def test_prefetch_passes_targets_and_tools_to_script(tmp_path: Path) -> None: with ( patch( diff --git a/tests/unit_tests/test_framework_helpers.py b/tests/unit_tests/test_framework_helpers.py index 2022c15bfe..098dcb7725 100644 --- a/tests/unit_tests/test_framework_helpers.py +++ b/tests/unit_tests/test_framework_helpers.py @@ -21,6 +21,7 @@ import requests as req from esphome import framework_helpers from esphome.core import EsphomeError from esphome.framework_helpers import ( + BatchDownloadProgress, _7z_extract_all, _detect_archive_root, _is_transient_download_error, @@ -1112,6 +1113,108 @@ class TestDownloadWithResume: assert mock_get.call_args[1]["headers"] == {} assert dest.read_bytes() == b"data" + def test_progress_callback_reports_absolute_bytes(self, tmp_path: Path) -> None: + """With a callback no bar is drawn; the callback sees the running + byte count of this file, then its final verified size.""" + dest = tmp_path / "tool.tar.gz" + resp = _mock_response(b"") + resp.headers = {"content-length": "7"} + resp.iter_content.return_value = [b"1234", b"567"] + seen: list[int] = [] + with ( + patch("requests.get", return_value=resp), + patch("esphome.framework_helpers.ProgressBar") as bar, + ): + download_with_resume( + "https://example.com/t", dest, size=7, progress=seen.append + ) + assert seen == [0, 4, 7, 7] + bar.assert_not_called() + + def test_progress_callback_seeds_with_resume_offset(self, tmp_path: Path) -> None: + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"12345") + good = hashlib.sha256(b"12345678").hexdigest() + seen: list[int] = [] + with patch("requests.get", return_value=_resumed_response(b"678")): + download_with_resume( + "https://example.com/t", dest, sha256=good, size=8, progress=seen.append + ) + assert seen[0] == 5 + assert seen[-1] == 8 + + def test_progress_callback_credits_already_complete_download( + self, tmp_path: Path + ) -> None: + """A verified dest from an earlier run still counts toward the batch.""" + dest = tmp_path / "tool.tar.gz" + dest.write_bytes(b"12345678") + seen: list[int] = [] + with patch("requests.get") as mock_get: + download_with_resume( + "https://example.com/t", dest, size=8, progress=seen.append + ) + mock_get.assert_not_called() + assert seen == [8] + + +class TestBatchDownloadProgress: + def test_sums_trackers_into_one_bar(self) -> None: + with patch("esphome.framework_helpers.ProgressBar") as bar_cls: + progress = BatchDownloadProgress("Downloading", 100) + a = progress.tracker() + b = progress.tracker() + a(10) + b(20) + a(30) + a(0) # a restart from zero takes that file's bytes back out + bar_cls.assert_called_once_with("Downloading") + updates = [c[0][0] for c in bar_cls.return_value.update.call_args_list] + assert updates == [0.1, 0.3, 0.5, 0.2] + + def test_clamps_at_one(self) -> None: + """Sizes are advisory; an over-delivering server never pushes past 100%.""" + with patch("esphome.framework_helpers.ProgressBar") as bar_cls: + progress = BatchDownloadProgress("Downloading", 10) + progress.tracker()(25) + assert bar_cls.return_value.update.call_args[0][0] == 1 + + def test_unknown_total_draws_nothing(self) -> None: + with patch("esphome.framework_helpers.ProgressBar") as bar_cls: + progress = BatchDownloadProgress("Downloading", 0) + progress.tracker()(5) + progress.done() + bar_cls.assert_not_called() + + def test_done_ends_an_unfinished_bar(self) -> None: + """A batch that stops short of 100% (a failed archive) still ends its + line so the next log message starts on a fresh row.""" + stream = io.StringIO() + stream.isatty = lambda: True # type: ignore[method-assign] + with patch("esphome.helpers.sys.stderr", stream): + progress = BatchDownloadProgress("Downloading", 10) + progress.tracker()(5) + progress.done() + assert stream.getvalue().endswith("50% \n") + + def test_done_before_any_frame_writes_nothing(self) -> None: + """A batch aborted before any tracker fired must not emit a stray + newline for a bar that was never drawn.""" + stream = io.StringIO() + stream.isatty = lambda: True # type: ignore[method-assign] + with patch("esphome.helpers.sys.stderr", stream): + BatchDownloadProgress("Downloading", 10).done() + assert stream.getvalue() == "" + + def test_done_after_full_bar_adds_nothing(self) -> None: + stream = io.StringIO() + stream.isatty = lambda: True # type: ignore[method-assign] + with patch("esphome.helpers.sys.stderr", stream): + progress = BatchDownloadProgress("Downloading", 10) + progress.tracker()(10) + progress.done() + assert stream.getvalue().endswith("100% Done...\r\n") + class TestDownloadFromMirrors: def test_success_returns_url_and_writes_content(self, tmp_path: Path) -> None: diff --git a/tests/unit_tests/test_platformio_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py new file mode 100644 index 0000000000..5df8e3c9c5 --- /dev/null +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -0,0 +1,377 @@ +"""Tests for the shared extraScript machinery (platformio.extra_script).""" + +from __future__ import annotations + +import logging +import os +from pathlib import Path +from unittest.mock import patch + +import pytest + +from esphome.core import EsphomeError +from esphome.platformio.extra_script import ( + ExtraScriptResult, + _FakeSConsEnv, + apply_extra_script, + captured_as_build_flags, + run_extra_script, +) +from esphome.platformio.library import ConvertedLibrary as IDFComponent, URLSource + + +def test_extra_script_captures_libpath_libs_and_defines(tmp_path): + + (tmp_path / "src" / "esp32").mkdir(parents=True) + script = tmp_path / "extra_script.py" + script.write_text( + "Import('env')\n" + "mcu = env.get('BOARD_MCU')\n" + "env.Append(\n" + " LIBPATH=[join('src', mcu)],\n" + " LIBS=['algobsec'],\n" + " CPPDEFINES=['FOO', ('BAR', '1')],\n" + " LINKFLAGS=['-Wl,--gc-sections'],\n" + ")\n" + ) + # The script uses bare ``join`` (PIO's extra-scripts run inside SCons + # where this is in scope). Inject it via the script header so the + # shim's exec namespace can resolve it. + script.write_text("from os.path import join\n" + script.read_text()) + + result = run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) + + assert result.libpath == [str(Path("src") / "esp32")] + assert result.libs == ["algobsec"] + assert ("BAR", "1") in result.cppdefines + assert "FOO" in result.cppdefines + assert result.linkflags == ["-Wl,--gc-sections"] + + flags = captured_as_build_flags(result, library_dir=tmp_path) + sep = os.sep + assert f"-Lsrc{sep}esp32" in flags + assert "-lalgobsec" in flags + assert "-DFOO" in flags + assert "-DBAR=1" in flags + assert "-Wl,--gc-sections" in flags + + +def test_extra_script_libpath_relative_resolves_against_library_dir( + tmp_path, monkeypatch +): + """Relative LIBPATH entries must resolve against ``library_dir``, not the + caller's CWD (the shim restores CWD before ``captured_as_build_flags`` + runs).""" + + (tmp_path / "lib" / "esp32").mkdir(parents=True) + elsewhere = tmp_path.parent / "not_the_library_dir" + elsewhere.mkdir(exist_ok=True) + monkeypatch.chdir(elsewhere) + + result = ExtraScriptResult(libpath=["lib/esp32"]) + flags = captured_as_build_flags(result, library_dir=tmp_path) + + sep = os.sep + assert flags == [f"-Llib{sep}esp32"] + + +def test_extra_script_libpath_absolute_outside_library_dir(tmp_path): + + outside = tmp_path.parent / "system_lib" + outside.mkdir(exist_ok=True) + result = ExtraScriptResult(libpath=[str(outside)]) + + flags = captured_as_build_flags(result, library_dir=tmp_path) + assert flags == [f"-L{outside.resolve()}"] + + +def test_extra_script_failure_returns_empty_result(tmp_path, caplog): + + script = tmp_path / "broken.py" + script.write_text("raise RuntimeError('boom')\n") + + with caplog.at_level("WARNING"): + result = run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) + + assert result.libpath == [] + assert result.libs == [] + assert "broken.py" in caplog.text + + +def test_apply_extra_script_path_traversal_is_rejected(tmp_path): + + library_dir = tmp_path / "lib" + library_dir.mkdir() + outside = tmp_path / "evil.py" + outside.write_text("env.Append(LIBS=['pwned'])\n") + + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = library_dir + c.data = {"build": {"extraScript": "../evil.py"}} + + with pytest.raises(EsphomeError, match="escapes the library directory"): + apply_extra_script(c, board_mcu=lambda: "esp32", pio_platform="espressif32") + # Nothing was folded into flags: the traversal was rejected before + # the script could run. + assert "flags" not in c.data["build"] + + +def test_apply_extra_script_merges_into_existing_flags(tmp_path): + + (tmp_path / "src").mkdir() + script = tmp_path / "extra.py" + script.write_text("env.Append(LIBS=['algobsec'])\n") + + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py", "flags": ["-DEXISTING"]}} + + apply_extra_script(c, board_mcu=lambda: "esp32", pio_platform="espressif32") + + assert "-DEXISTING" in c.data["build"]["flags"] + assert "-lalgobsec" in c.data["build"]["flags"] + + +def test_apply_extra_script_malformed_flags_raises(tmp_path) -> None: + """A null/dict build.flags fails naming the library instead of injecting + a non-string into the compiler command line.""" + + (tmp_path / "src").mkdir() + script = tmp_path / "extra.py" + script.write_text("env.Append(LIBS=['algobsec'])\n") + + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py", "flags": None}} + + with pytest.raises(EsphomeError, match="malformed build.flags"): + apply_extra_script(c, board_mcu=lambda: "esp32", pio_platform="espressif32") + + +def test_apply_extra_script_callable_target_and_str_flags(tmp_path) -> None: + """The shared helper resolves the board_mcu callable lazily and normalizes + a string ``build.flags`` value into a list before extending it.""" + + (tmp_path / "src").mkdir() + script = tmp_path / "extra.py" + script.write_text("env.Append(LIBS=[env.get('BOARD_MCU')])\n") + + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py", "flags": "-DBASE=1"}} + + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + + assert c.data["build"]["flags"] == ["-DBASE=1", "-lesp8266"] + + +def test_captured_dict_cppdefines_warn_and_skip(tmp_path, caplog) -> None: + """A dict CPPDEFINES entry (legal SCons) must warn and skip; formatting + it blind would hand the compiler -D{'FOO': '1'} garbage.""" + (tmp_path / "src").mkdir() + script = tmp_path / "extra.py" + script.write_text( + "env.Append(CPPDEFINES=[{'FOO': '1'}, ('BAR', 2), ['BAZ', 3], 'PLAIN'])\n" + ) + + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py"}} + + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + + assert c.data["build"]["flags"] == ["-DBAR=2", "-DBAZ=3", "-DPLAIN"] + assert "Ignoring unsupported CPPDEFINES entry" in caplog.text + + +def test_apply_extra_script_subscript_env_read(tmp_path) -> None: + """Scripts also read env["BOARD_MCU"]; the subscript form must work or + the broad handler discards every flag the script captured.""" + (tmp_path / "src").mkdir() + script = tmp_path / "extra.py" + script.write_text("env.Append(LIBS=[env['BOARD_MCU']])\n") + + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py"}} + + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + + assert c.data["build"]["flags"] == ["-lesp8266"] + + +def test_apply_extra_script_no_script_and_no_flags(tmp_path) -> None: + + # No extraScript declared: nothing happens, the target is never resolved + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {}} + apply_extra_script( + c, + board_mcu=lambda: pytest.fail("target resolved without a script"), + pio_platform="espressif8266", + ) + + # A script that captures nothing leaves the flags untouched + script = tmp_path / "noop.py" + script.write_text("pass\n") + c.data = {"build": {"extraScript": "noop.py"}} + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + assert "flags" not in c.data["build"] + + +def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path, caplog) -> None: + """Un-captured env vars and unsupported env methods are skipped but + diagnosable from the build log.""" + + caplog.set_level(logging.DEBUG) + script = tmp_path / "extra.py" + script.write_text( + "env.Replace(CC='clang')\nenv.Append(UNCAPTURED=['x'], LIBS='single')\n" + ) + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py"}} + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + assert c.data["build"]["flags"] == ["-lsingle"] + assert "env.Append(UNCAPTURED=...) is not captured" in caplog.text + assert "env.Replace(...) is not supported" in caplog.text + + +def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None: + """A raising extra-script is best-effort: logged and skipped.""" + + script = tmp_path / "extra.py" + script.write_text("raise RuntimeError('boom')\n") + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py"}} + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + assert "flags" not in c.data["build"] + assert "ignoring its output" in caplog.text + + +def test_apply_extra_script_pio_platform(tmp_path) -> None: + """The backend's platform token is exposed to the script as PIOPLATFORM.""" + + script = tmp_path / "extra.py" + script.write_text("env.Append(LIBS=[env.get('PIOPLATFORM')])\n") + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py"}} + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + assert c.data["build"]["flags"] == ["-lespressif8266"] + + +def test_apply_extra_script_missing_script_raises(tmp_path) -> None: + """A declared but absent extraScript is a broken package and fails by + name, as it would under PlatformIO.""" + + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "nope.py"}} + with pytest.raises(EsphomeError, match="nope.py of library owner/name not found"): + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + + +def test_run_extra_script_failure_discards_partial_capture(tmp_path, caplog) -> None: + """A crashed script yields an empty result: half-applied flags could + build wrong-output firmware that links cleanly.""" + + script = tmp_path / "extra.py" + script.write_text("env.Append(LIBS=['algobsec'])\nraise RuntimeError('boom')\n") + result = run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) + assert result.libs == [] + assert "ignoring its output" in caplog.text + + +def test_run_extra_script_syntax_error_is_best_effort(tmp_path, caplog) -> None: + """A vendored script that does not even compile warns and skips instead + of aborting the build.""" + + script = tmp_path / "extra.py" + script.write_text("def broken(:\n") + result = run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) + assert result.libs == [] + assert "ignoring its output" in caplog.text + + +def test_unsupported_env_method_warns_once(caplog) -> None: + """Repeated calls to the same unsupported method warn only once.""" + + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + env.Replace(CC="clang") + env.Replace(CC="gcc") + assert caplog.text.count("env.Replace(...) is not supported") == 1 + + +def test_run_extra_script_sys_exit_is_best_effort(tmp_path, caplog) -> None: + """A nonzero sys.exit() in a vendored script must not kill the esphome + run, and its output is discarded.""" + + script = tmp_path / "extra.py" + script.write_text("import sys\nenv.Append(LIBS=['x'])\nsys.exit(3)\n") + result = run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) + assert result.libs == [] + assert "exited with status 3" in caplog.text + + +def test_run_extra_script_sys_exit_zero_is_success(tmp_path, caplog) -> None: + """sys.exit(0) is a normal PlatformIO script ending: the capture is kept.""" + + script = tmp_path / "extra.py" + script.write_text("import sys\nenv.Append(LIBS=['algobsec'])\nsys.exit(0)\n") + result = run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) + assert result.libs == ["algobsec"] + assert "ignoring its output" not in caplog.text + + +def test_run_extra_script_unreadable_raises(tmp_path) -> None: + """An unreadable declared script is a broken package, like a missing one.""" + + script = tmp_path / "extra.py" + script.write_text("") + with ( + patch("pathlib.Path.read_text", side_effect=OSError("denied")), + pytest.raises(EsphomeError, match="is unreadable"), + ): + run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) + + +def test_run_extra_script_bad_encoding_is_best_effort(tmp_path, caplog) -> None: + """Undecodable content warns and skips, like a SyntaxError.""" + + script = tmp_path / "extra.py" + script.write_bytes(b"\xff\xfe\x00bad") + result = run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) + assert result.libs == [] + assert "is not UTF-8" in caplog.text + + +def test_uncaptured_append_key_warns_once(caplog) -> None: + """A loop of Appends to the same uncaptured key warns once.""" + + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + env.Append(CPPPATH=["a"]) + env.Append(CPPPATH=["b"]) + assert caplog.text.count("env.Append(CPPPATH=...) is not captured") == 1 diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 0eede78656..25d074e26f 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -4,15 +4,18 @@ Covers the shared download/parse/resolve/dependency-walk paths in ``esphome.platformio.library`` directly (the ESP-IDF and Zephyr backends are exercised in their own test modules).""" +from contextlib import contextmanager import json import logging from pathlib import Path +from types import SimpleNamespace import pytest from esphome.core import EsphomeError, Library import esphome.platformio.library as lib from esphome.platformio.library import ( + SOURCE_KIND_FOR_SUFFIX, ConvertedLibrary, GitSource, InvalidLibrary, @@ -23,6 +26,8 @@ from esphome.platformio.library import ( _resolve_registry_version, check_library_data, convert_libraries, + join_flag_args, + split_flag_entry, ) @@ -150,11 +155,30 @@ def test_localsource_download_returns_empty_build_dir(setup_core: Path) -> None: assert plain != out +@contextmanager +def caplog_at_info(): + records: list[logging.LogRecord] = [] + handler = logging.Handler() + handler.emit = records.append + logger = logging.getLogger("esphome.platformio.library") + logger.addHandler(handler) + # The level must actually admit INFO or the no-INFO assertions are vacuous + old_level = logger.level + logger.setLevel(logging.INFO) + try: + yield records + finally: + logger.setLevel(old_level) + logger.removeHandler(handler) + + def test_urlsource_download_extracts_then_reuses_marker(setup_core, monkeypatch): monkeypatch.setattr(lib, "rmdir", lambda path, msg="": None) dl_calls: list[list[str]] = [] monkeypatch.setattr( - lib, "download_from_mirrors", lambda urls, headers, f: dl_calls.append(urls) + lib, + "download_from_mirrors", + lambda urls, headers, f, progress=None: dl_calls.append(urls), ) def fake_extract(fileobj, path): @@ -173,6 +197,12 @@ def test_urlsource_download_extracts_then_reuses_marker(setup_core, monkeypatch) assert out2 == out assert len(dl_calls) == 1 + # A batch caller passes a tracker and owns the messaging; no per-file INFO + with caplog_at_info() as records: + src.download("mylib-batch", progress=lambda done: None) + assert len(dl_calls) == 2 + assert not [r for r in records if "Downloading" in r.message] + def test_resolve_registry_version_raises_without_pkg_file(monkeypatch): registry = lib._make_registry_client() @@ -213,7 +243,7 @@ def _patch_registry_resolve(monkeypatch: pytest.MonkeyPatch) -> None: def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properties=()): """Fake ConvertedLibrary.download to materialize canned manifests on disk.""" - def fake_download(self, force=False, salt="", namespace=""): + def fake_download(self, force=False, salt="", namespace="", progress=None): self.path = tmp_path / self.get_require_name() self.path.mkdir(parents=True, exist_ok=True) if self.name in properties: @@ -292,7 +322,11 @@ def _patch_download_without_manifest( calls: list[bool] = [] def fake_download( - self: ConvertedLibrary, force: bool = False, salt: str = "", namespace: str = "" + self: ConvertedLibrary, + force: bool = False, + salt: str = "", + namespace: str = "", + progress=None, ) -> None: calls.append(force) self.path = tmp_path / self.get_require_name() @@ -531,3 +565,322 @@ def test_convert_libraries_skips_incompatible_dependency(tmp_path, monkeypatch): top = convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) assert top[0].dependencies == [] + + +def test_split_flag_entry_unbalanced_quote_is_clean() -> None: + """A malformed flags entry raises EsphomeError, not a raw ValueError.""" + + assert split_flag_entry('-DX="a b"', "library x") == ["-DX=a b"] + with pytest.raises(EsphomeError, match=r"Malformed build flag.*library x"): + split_flag_entry('-DX="unclosed', "library x") + + +def test_join_flag_args_reglues_spaced_define() -> None: + """A spaced -D re-glues to its argument, as ParseFlags does.""" + + assert join_flag_args(["-D", "FOO=1", "-Os"], "x") == ["-DFOO=1", "-Os"] + + +def test_join_flag_args_trailing_bare_flag_warns( + caplog: pytest.LogCaptureFixture, +) -> None: + + assert join_flag_args(["-Os", "-l"], "library x") == ["-Os"] + assert "Ignoring trailing '-l'" in caplog.text + + +def test_lex_build_flags_dangling_flag_does_not_cross_entries( + caplog: pytest.LogCaptureFixture, +) -> None: + """Each entry is lexed independently, as ParseFlags does: a dangling -I + ending one entry warns instead of absorbing the next entry's first token.""" + from esphome.platformio.library import lex_build_flags + + assert lex_build_flags(["-Wall -I", "-DFOO=1"], "lib x") == ["-Wall", "-DFOO=1"] + assert "Ignoring trailing '-I'" in caplog.text + + +def test_prefetch_wave_downloads_registry_archives_in_parallel( + setup_core, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """Registry archives in one wave download concurrently, deduped by URL; + git/local sources and failures are left to the sequential call.""" + calls: list[str] = [] + + def fake_download(self, force=False, salt="", namespace="", progress=None): + calls.append(self.source.url) + if progress is not None: + progress(0) + if "boom" in self.source.url: + raise RuntimeError("boom") + + monkeypatch.setattr(ConvertedLibrary, "download", fake_download) + monkeypatch.setattr(lib, "_content_lengths", lambda urls: [1] * len(urls)) + wave = [ + ("a", ConvertedLibrary("a", "1.0", URLSource("https://x/a.tar.gz"))), + ("b", ConvertedLibrary("b", "1.0", URLSource("https://x/b.tar.gz"))), + # Duplicate URL must prefetch once (two threads must never extract + # into the same cache directory) + ("b2", ConvertedLibrary("b2", "1.0", URLSource("https://x/b.tar.gz"))), + ("c", ConvertedLibrary("c", "1.0", URLSource("https://x/boom.tar.gz"))), + ("g", ConvertedLibrary("g", "*", lib.GitSource("https://x/g.git", None))), + ] + lib._prefetch_wave(wave, "", "idf") + assert sorted(calls) == [ + "https://x/a.tar.gz", + "https://x/b.tar.gz", + "https://x/boom.tar.gz", + ] + # The failure surfaces at default verbosity, after the bar + assert "Prefetch of c failed (retrying sequentially)" in caplog.text + + +def test_prefetch_wave_unknown_size_falls_back_to_sequential( + setup_core, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """Any unknown HEAD size skips the parallel prefetch entirely so the + sequential downloads keep their per-file bars.""" + + def fail_download(self, force=False, salt="", namespace="", progress=None): + raise AssertionError("prefetched despite unknown size") + + monkeypatch.setattr(ConvertedLibrary, "download", fail_download) + monkeypatch.setattr(lib, "_content_lengths", lambda urls: [1, None]) + wave = [ + ("a", ConvertedLibrary("a", "1.0", URLSource("https://x/a.tar.gz"))), + ("b", ConvertedLibrary("b", "1.0", URLSource("https://x/b.tar.gz"))), + ] + caplog.set_level("DEBUG") + lib._prefetch_wave(wave, "", "idf") + # The culprit URL is named so the fallback is traceable + assert "No Content-Length for https://x/b.tar.gz" in caplog.text + + +def test_content_lengths_head_requests(monkeypatch: pytest.MonkeyPatch) -> None: + """Sizes come from HEAD Content-Length; a failing HEAD reads as 0 so + the combined bar is skipped rather than wrong.""" + import requests + + def fake_head(url, timeout, allow_redirects): + if "bad" in url: + raise requests.ConnectionError("down") + if "gone" in url: + return SimpleNamespace(ok=False, status_code=404, headers={}) + if "garbage" in url: + # A proxy/CDN doubling the header ("123, 123") or emitting junk + # must degrade to unknown, not ValueError the build + return SimpleNamespace(ok=True, headers={"content-length": "123, 123"}) + return SimpleNamespace(ok=True, headers={"content-length": "123"}) + + monkeypatch.setattr( + lib.requests if hasattr(lib, "requests") else requests, "head", fake_head + ) + # None marks an unknown size (probe failure or non-2xx), distinct + # from a genuine zero + assert lib._content_lengths( + ["https://x/a", "https://x/bad", "https://x/gone", "https://x/garbage"] + ) == [ + 123, + None, + None, + None, + ] + + +def test_prefetch_wave_cache_probe_failure_still_prefetches( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The cache probe is best-effort; a failing probe prefetches anyway.""" + calls: list[str] = [] + monkeypatch.setattr( + ConvertedLibrary, + "download", + lambda self, **kw: calls.append(self.source.url), + ) + monkeypatch.setattr( + URLSource, + "is_cached", + lambda self, *a, **kw: (_ for _ in ()).throw(RuntimeError("no core")), + ) + monkeypatch.setattr(lib, "_content_lengths", lambda urls: [1] * len(urls)) + wave = [ + ("a", ConvertedLibrary("a", "1.0", URLSource("https://x/a.tar.gz"))), + ("b", ConvertedLibrary("b", "1.0", URLSource("https://x/b.tar.gz"))), + ] + lib._prefetch_wave(wave, "", "idf") + assert sorted(calls) == ["https://x/a.tar.gz", "https://x/b.tar.gz"] + + +def test_prefetch_wave_warm_cache_is_silent( + setup_core, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """Already-extracted archives download nothing; a warm build must not + print a Downloading line or draw a bar.""" + monkeypatch.setattr( + ConvertedLibrary, + "download", + lambda self, **kw: (_ for _ in ()).throw(AssertionError("downloaded")), + ) + wave = [] + for name in ("a", "b", "c"): + comp = ConvertedLibrary(name, "1.0", URLSource(f"https://x/{name}.tar.gz")) + marker_dir = comp.source._cache_dir(comp.get_sanitized_name(), "", "idf") + marker_dir.mkdir(parents=True) + (marker_dir / ".esphome_extracted").touch() + wave.append((name, comp)) + lib._prefetch_wave(wave, "", "idf") + assert "Downloading" not in caplog.text + + +def test_prefetch_wave_single_archive_skips_the_pool( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """One archive gains nothing from a pool; the sequential call keeps its + progress bar.""" + monkeypatch.setattr( + ConvertedLibrary, + "download", + lambda self, **kw: (_ for _ in ()).throw(AssertionError("prefetched")), + ) + lib._prefetch_wave( + [("a", ConvertedLibrary("a", "1.0", URLSource("https://x/a.tar.gz")))], + "", + "idf", + ) + + +def test_normalize_dependencies_forms(caplog) -> None: + """Every PIO-legal spelling normalizes; unrecognizable entries warn.""" + from esphome.platformio.library import normalize_dependencies + + assert normalize_dependencies( + ["Wire", {"name": "SPI"}, 5, "", {"version": "1.0"}], "libx" + ) == [ + {"name": "Wire"}, + {"name": "SPI"}, + ] + # The int, the empty string, and the nameless dict all warn + assert caplog.text.count("unrecognized dependency entry") == 3 + # A plain string is names, never iterated into characters + assert normalize_dependencies("Wire, SPI") == [ + {"name": "Wire"}, + {"name": "SPI"}, + ] + assert normalize_dependencies("Wire") == [{"name": "Wire"}] + # A non-iterable value fails by manifest name, never a bare TypeError + assert normalize_dependencies(5, "libx") == [] + assert "Ignoring unrecognized dependencies 5 of libx" in caplog.text + # The dict-shorthand form validates names like the list form: an empty + # key and a spec overriding name with a non-string both warn and drop + assert normalize_dependencies( + {"": "1.0", "Wire": {"name": 123, "version": "1.0"}, "SPI": "*"}, "libx" + ) == [{"name": "SPI", "owner": None, "version": "*"}] + assert caplog.text.count("unrecognized dependency entry") == 5 + # A container or numeric version would raise from set.add() or fail + # opaquely in the registry; both spellings warn and drop + assert normalize_dependencies({"Foo": ["1.0", "2.0"]}, "libx") == [] + assert normalize_dependencies([{"name": "Foo", "version": 1}], "libx") == [] + assert caplog.text.count("unrecognized dependency entry") == 7 + + +@pytest.mark.parametrize( + "manifest", [["not", "a", "manifest"], {"name": "A", "build": "src"}] +) +def test_convert_libraries_malformed_manifest_raises( + tmp_path, monkeypatch, manifest +) -> None: + """A manifest without the expected dict shape fails by library name + before any backend dereferences data/build.""" + _patch_download_with_manifests(monkeypatch, tmp_path, {"esphome/A": manifest}) + with pytest.raises(EsphomeError, match="has a malformed manifest"): + convert_libraries([Library("esphome/A", None, None)], _backend()) + + +def test_walk_warns_for_properties_only_depends( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A manifest declaring dependencies only as library.properties depends= + warns in the shared walk, so every backend reports the drop.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + {"esphome/A": "name=A\nversion=1.0\ndepends=Wire, SPI\n"}, + properties=("esphome/A",), + ) + convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + assert "declares dependencies via library.properties" in caplog.text + + +def test_walk_warns_for_nonplatform_invalid_library( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A dependency dropped for any cause other than the routine platform + filter is visible in every backend.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + {"esphome/A": {"name": "A", "dependencies": [{"name": "B", "version": "1.0"}]}}, + ) + calls = {"n": 0} + real = lib.check_library_data + + def flaky(data, platform, framework): + calls["n"] += 1 + if calls["n"] > 1: + raise InvalidLibrary("manifest is corrupt") + return real(data, platform, framework) + + monkeypatch.setattr(lib, "check_library_data", flaky) + convert_libraries([Library("esphome/A", None, None)], _backend()) + assert "Skipping dependency B of esphome/A: manifest is corrupt" in caplog.text + + +def test_convert_libraries_warns_for_nonplatform_invalid_dependency_component( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A dependency component dropped for any cause other than the platform + filter warns; only the routine cross-platform skip stays at debug.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + { + "esphome/A": { + "name": "A", + "dependencies": [{"name": "C", "owner": "esphome", "version": "1.0"}], + }, + "esphome/C": {"name": "C"}, + }, + ) + real = lib.check_library_data + + def flaky(data, platform, framework): + # Fail only on C's resolved manifest, not on A's dependency entry + if data.get("name") == "C" and "version" not in data: + raise InvalidLibrary("manifest is corrupt") + return real(data, platform, framework) + + monkeypatch.setattr(lib, "check_library_data", flaky) + convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + assert "manifest is corrupt" in caplog.text + assert "Skipping dependency" in caplog.text + + +def test_split_flag_entry_non_string_is_clean() -> None: + """A dict or number from a third-party manifest fails naming the entry, + not with an opaque shlex traceback.""" + + with pytest.raises(EsphomeError, match="Malformed build flag"): + split_flag_entry({"esp32": ["-DX"]}, "lib x") + with pytest.raises(EsphomeError, match="Malformed build flag 5"): + split_flag_entry(5, "lib x") + + +def test_source_kind_map_shape() -> None: + """The kind values the native compile rules key on, and the deliberate + AS/ASPP merge (.s and .S both map to asm).""" + + assert set(SOURCE_KIND_FOR_SUFFIX.values()) == {"c", "cxx", "asm"} + assert SOURCE_KIND_FOR_SUFFIX[".s"] == "asm" + assert SOURCE_KIND_FOR_SUFFIX[".S"] == "asm" + assert SOURCE_KIND_FOR_SUFFIX[".c"] == "c" + assert SOURCE_KIND_FOR_SUFFIX[".cpp"] == "cxx" diff --git a/tests/unit_tests/test_platformio_toolchain.py b/tests/unit_tests/test_platformio_toolchain.py index 172b288c25..28304270a4 100644 --- a/tests/unit_tests/test_platformio_toolchain.py +++ b/tests/unit_tests/test_platformio_toolchain.py @@ -1977,3 +1977,10 @@ def test_run_platformio_cli_invokes_heal( with patch.object(toolchain, "heal_platformio_python_env") as mock_heal: toolchain.run_platformio_cli("test") mock_heal.assert_called_once() + + +def test_ccache_probe_spawns_with_close_fds_false() -> None: + """The probe follows the repo-wide posix_spawn convention.""" + with patch("subprocess.run") as mock_run: + assert toolchain._ccache_runs("/usr/bin/ccache") is True + assert mock_run.call_args.kwargs["close_fds"] is False