From 9731c9527aa1e7b9523c53398b37b48a50591cfd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 23 Aug 2026 12:32:44 -0500 Subject: [PATCH] Harden the shared library converter Split out of #18662: the converter/manifest hardening without the download parallelization, which stays there. The PIO extra-script shim moves from espidf/ to platformio/ and gains type checks for every manifest field, capture of the Append variants and CPPPATH, warn-once diagnostics for unmodelled env reads, and uniform shlex quoting so captured values survive the lex_build_flags round-trip. The dependency walk normalizes every spelling PIO accepts, splits the routine cross-platform skip (IncompatiblePlatform) from real manifest problems, validates manifest shape once, and the shared flag lexer warns and drops trailing or empty glued arguments. --- esphome/espidf/component.py | 69 +-- esphome/espidf/extra_script.py | 161 ------ esphome/platformio/extra_script.py | 299 +++++++++++ esphome/platformio/library.py | 301 ++++++++--- esphome/platformio/toolchain.py | 3 + script/determine-jobs.py | 1 + tests/script/test_determine_jobs.py | 1 + tests/unit_tests/test_espidf_component.py | 173 ++----- .../test_platformio_extra_script.py | 478 ++++++++++++++++++ tests/unit_tests/test_platformio_library.py | 212 +++++++- tests/unit_tests/test_platformio_toolchain.py | 7 + 11 files changed, 1294 insertions(+), 411 deletions(-) delete mode 100644 esphome/espidf/extra_script.py create mode 100644 esphome/platformio/extra_script.py create mode 100644 tests/unit_tests/test_platformio_extra_script.py 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/platformio/extra_script.py b/esphome/platformio/extra_script.py new file mode 100644 index 0000000000..363c40561f --- /dev/null +++ b/esphome/platformio/extra_script.py @@ -0,0 +1,299 @@ +"""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 +import shlex +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 + if not isinstance(extra_script, str): + # A list/dict value would raise an opaque TypeError on the join below + raise EsphomeError( + f"extraScript of library {component.name} must be a string, " + f"got {type(extra_script).__name__}" + ) + # 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( + {"CPPPATH", "LIBPATH", "LIBS", "CPPDEFINES", "LINKFLAGS", "CPPFLAGS"} +) + + +@dataclass +class ExtraScriptResult: + """Build-var deltas captured from a PIO extra-script ``env.Append`` call.""" + + cpppath: list[str] = field(default_factory=list) + 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() + self._warned_gets: set[str] = set() + + # ----- SCons env API the common scripts use ----- + + def get(self, key: str, default: str | None = None) -> str | None: + if key not in self._vars and key not in self._warned_gets: + # A script branching on an unmodelled var silently takes the + # default branch; make that diagnosable from a normal build log + self._warned_gets.add(key) + _LOGGER.warning( + "PIO extra-script env.get(%r) is not modelled; returning the default", + key, + ) + 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) + + # Same keys, same flattened capture; ordering/dedup don't matter since + # the consumer re-orders anyway + Prepend = Append + AppendUnique = Append + PrependUnique = Append + + # ----- 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] = [] + + def _strs(bucket: list, kind: str) -> list[str]: + # Third-party scripts legally append SCons nodes, ints, or dicts; + # stringifying those into flags would hand the compiler garbage + good = [entry for entry in bucket if isinstance(entry, str)] + for entry in bucket: + if not isinstance(entry, str): + _LOGGER.warning("Ignoring unsupported %s entry %r", kind, entry) + return good + + library_root = library_dir.resolve() + + def _anchored(path: str) -> str: + # Anchor relative paths to library_dir; the script's CWD has been + # restored by now + resolved = (library_dir / path).resolve() + try: + return str(resolved.relative_to(library_root)) + except ValueError: + return str(resolved) + + # shlex.quote so a spaced path survives lex_build_flags as one token + flags.extend( + f"-I{shlex.quote(_anchored(path))}" for path in _strs(result.cpppath, "CPPPATH") + ) + flags.extend( + f"-L{shlex.quote(_anchored(path))}" for path in _strs(result.libpath, "LIBPATH") + ) + flags.extend(f"-l{shlex.quote(lib)}" for lib in _strs(result.libs, "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) + # Each captured entry is one argv token in SCons; quote so the + # lex_build_flags round-trip cannot split a spaced value into two + flags.extend(shlex.quote(f) for f in _strs(result.linkflags, "LINKFLAGS")) + flags.extend(shlex.quote(f) for f in _strs(result.cppflags, "CPPFLAGS")) + return flags diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index ee0a758a31..af3b05c9a0 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -13,7 +13,7 @@ regardless of which toolchain consumes the result. """ from collections import deque -from collections.abc import Callable +from collections.abc import Callable, Iterable from dataclasses import dataclass, field import glob import hashlib @@ -47,20 +47,29 @@ 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. +# The kind values drive the ESP8266 native ninja rules (later in this +# chain); existing backends consume only the keys. Note .C/.C++ join the +# suffix set here per CXXSUFFIXES; SCons demotes .C to C on +# case-insensitive filesystems, we always treat it as C++. +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" @@ -203,6 +212,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. @@ -432,7 +449,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 +472,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 +486,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 +570,139 @@ 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. + + A trailing or empty argument (``-D ""``) is warned and dropped: the + bare flag would make gcc eat the next flag as its argument (or add + the CWD for ``-L``); always a typo. + """ + 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 + if not arg: + _LOGGER.warning( + "Ignoring '%s' with empty argument in %s build flags", tok, owner + ) + continue + 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"): + # INFO: common and unactionable for transitive libraries; a WARNING + # on every build would train users to ignore the stream + _LOGGER.info( + "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 +711,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 +847,24 @@ 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 convert_libraries( libraries: list[Library], backend: LibraryBackend ) -> list[ConvertedLibrary]: @@ -713,10 +890,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 +902,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 +950,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 @@ -795,10 +964,8 @@ def convert_libraries( 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. + # 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 @@ -823,11 +990,8 @@ def convert_libraries( 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. + # 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", @@ -838,49 +1002,66 @@ def convert_libraries( 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) + component.data = parse_library_json(library_json_path) elif has_properties: - component.data = _parse_library_properties(library_properties_path) + 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. + # 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}" ) + 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) + 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. + # 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 - _LOGGER.debug("Skip incompatible dependency %s: %s", key, str(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 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")): - if "name" not in dependency or "version" not in dependency: + for dependency in normalize_dependencies( + component.data.get("dependencies"), component.name + ): + if "version" not in dependency: + # Cannot resolve from the registry; common for bundled + # names (Wire, SPI) -- add_library() is the fix if real + _LOGGER.info( + "Skip version-less dependency %r of %s", + dependency.get("name"), + component.name, + ) continue - try: - check_library_data(dependency, backend.platform, backend.framework) - except InvalidLibrary as e: - _LOGGER.debug("Skip dependency %s: %s", dependency.get("name"), str(e)) + 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_ignored(dep_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). 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/script/determine-jobs.py b/script/determine-jobs.py index 722e2370fe..3e11deeb9a 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -537,6 +537,7 @@ ESP_IDF_INFRA_TRIGGER_FILES = frozenset( "esphome/build_gen/espidf.py", "esphome/framework_helpers.py", "esphome/platformio/library.py", + "esphome/platformio/extra_script.py", } ) diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index 1a568ca6c6..b42c33de96 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -1124,6 +1124,7 @@ def test_should_run_esp32_platformio_with_branch() -> None: (["esphome/build_helpers/idedata.py"], True), (["esphome/platformio/library.py"], True), (["esphome/framework_helpers.py"], True), + (["esphome/platformio/extra_script.py"], True), # PlatformIO build gen, its toolchain, and the esp32 component are # NOT IDF-infra triggers (["esphome/platformio/toolchain.py"], False), diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index f9e048f6f4..e2884454e5 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 == [ @@ -1190,3 +1069,33 @@ def test_idf_component_download_passes_salt() -> None: "owner/name", force=True, salt="abcd1234", namespace="idf" ) 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_platformio_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py new file mode 100644 index 0000000000..a4eeb5bf41 --- /dev/null +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -0,0 +1,478 @@ +"""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, + lex_build_flags, +) + + +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"] + + # Lex like the consumer does: quoting makes raw strings platform-varying + tokens = lex_build_flags( + captured_as_build_flags(result, library_dir=tmp_path), "test" + ) + sep = os.sep + assert f"-Lsrc{sep}esp32" in tokens + assert "-lalgobsec" in tokens + assert "-DFOO" in tokens + assert "-DBAR=1" in tokens + assert "-Wl,--gc-sections" in tokens + + +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 lex_build_flags(flags, "test") == [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 lex_build_flags(flags, "test") == [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_nonstring_buckets_warn_and_skip(tmp_path, caplog) -> None: + """Non-string LIBS/LINKFLAGS/CPPFLAGS/LIBPATH entries (legal SCons + nodes) are skipped by name instead of stringified into garbage flags.""" + (tmp_path / "src").mkdir() + script = tmp_path / "extra.py" + script.write_text( + "env.Append(LIBS=['m', 42], LINKFLAGS=['-Wl,-x', {'no': 1}], " + "CPPFLAGS=['-Os', 3.5], LIBPATH=['libs', 7])\n" + ) + (tmp_path / "libs").mkdir() + + 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") + + flags = c.data["build"]["flags"] + assert "-lm" in flags and "-Wl,-x" in flags and "-Os" in flags + assert not any("42" in f or "no" in f or "3.5" in f for f in flags) + assert "Ignoring unsupported LIBS entry 42" in caplog.text + assert "Ignoring unsupported LIBPATH entry 7" in caplog.text + + +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") + + +@pytest.mark.parametrize("bad", (["a.py"], {"esp32": "a.py"}), ids=("list", "dict")) +def test_apply_extra_script_non_string_raises(tmp_path, bad) -> None: + """A non-string extraScript fails naming the library, not with a TypeError.""" + + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": bad}} + with pytest.raises(EsphomeError, match="of library owner/name must be a string"): + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + + +def test_extra_script_cpppath_captured_as_include_flags(tmp_path, monkeypatch): + """CPPPATH entries translate to -I flags anchored like LIBPATH.""" + + (tmp_path / "include").mkdir() + outside = tmp_path.parent / "system_inc" + outside.mkdir(exist_ok=True) + elsewhere = tmp_path.parent / "not_the_library_dir" + elsewhere.mkdir(exist_ok=True) + monkeypatch.chdir(elsewhere) + + result = ExtraScriptResult(cpppath=["include", str(outside), 7]) + flags = captured_as_build_flags(result, library_dir=tmp_path) + + assert lex_build_flags(flags, "test") == ["-Iinclude", f"-I{outside.resolve()}"] + + +def test_extra_script_spaced_paths_survive_relexing(tmp_path): + """-I/-L paths with spaces round-trip through lex_build_flags as one token.""" + (tmp_path / "my libs").mkdir() + result = ExtraScriptResult(cpppath=["my libs"], libpath=["my libs"]) + flags = captured_as_build_flags(result, library_dir=tmp_path) + assert lex_build_flags(flags, "test") == ["-Imy libs", "-Lmy libs"] + + +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 + + +@pytest.mark.parametrize("method", ("Prepend", "AppendUnique", "PrependUnique")) +def test_append_variants_capture_like_append(method: str) -> None: + """Prepend/AppendUnique/PrependUnique write the captured keys too.""" + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + getattr(env, method)(LIBS=["algobsec"], LIBPATH=["lib"]) + assert env.result.libs == ["algobsec"] + assert env.result.libpath == ["lib"] + + +def test_env_get_unknown_key_warns_once(caplog) -> None: + """A script branching on an unmodelled env var is diagnosable.""" + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + assert env.get("BOARD") is None + assert env.get("BOARD", "d1") == "d1" + assert env.get("BOARD_MCU") == "esp8266" + assert caplog.text.count("env.get('BOARD') is not modelled") == 1 + assert "BOARD_MCU" not in caplog.text + + +def test_spaced_linkflag_survives_relexing(tmp_path) -> None: + """A captured argv token with a space stays one token after lexing.""" + result = ExtraScriptResult( + linkflags=["-Wl,-T my linker.ld"], cppflags=["-include my hdr.h"] + ) + flags = captured_as_build_flags(result, library_dir=tmp_path) + assert lex_build_flags(flags, "test") == [ + "-Wl,-T my linker.ld", + "-include my hdr.h", + ] + + +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(RANLIBFLAGS=["a"]) + env.Append(RANLIBFLAGS=["b"]) + assert caplog.text.count("env.Append(RANLIBFLAGS=...) is not captured") == 1 diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 0eede78656..3eb4f2aa75 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -4,6 +4,7 @@ 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 @@ -13,6 +14,7 @@ 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 +25,8 @@ from esphome.platformio.library import ( _resolve_registry_version, check_library_data, convert_libraries, + join_flag_args, + split_flag_entry, ) @@ -150,11 +154,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: dl_calls.append(urls), ) def fake_extract(fileobj, path): @@ -213,7 +236,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 +315,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 +558,182 @@ 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_join_flag_args_empty_argument_warns_and_drops( + caplog: pytest.LogCaptureFixture, +) -> None: + """An empty glued argument is dropped: a bare -D would eat the next flag.""" + assert lib.lex_build_flags('-D "" -DFOO', "build_flags") == ["-DFOO"] + assert "Ignoring '-D' with empty argument in build_flags" in caplog.text + + +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",), + ) + caplog.set_level("INFO") + 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