diff --git a/esphome/components/zephyr/library.py b/esphome/components/zephyr/library.py index 0e6551ccf1..d09f0afedb 100644 --- a/esphome/components/zephyr/library.py +++ b/esphome/components/zephyr/library.py @@ -28,6 +28,7 @@ from esphome.platformio.library import ( collect_filtered_files, convert_libraries, ensure_list, + lex_build_flags, split_list_by_condition, ) @@ -38,11 +39,13 @@ ZEPHYR_FRAMEWORK = "zephyr" def _escape(p: PathType) -> str: - # In CMakeLists.txt, backslashes need to be escaped (mirrors the ESP-IDF - # backend's escape_entry). Doubling -- rather than rewriting '\' -> '/' -- - # preserves content, so it's safe for arbitrary build flags (e.g. a -D value - # containing a backslash) as well as Windows paths. - return f'"{str(p)}"'.replace("\\", "\\\\") + # In CMakeLists.txt, backslashes and embedded quotes need escaping + # (mirrors the ESP-IDF backend's escape_entry; the lex round-trip makes + # a literal quote in a -D value reachable). Doubling backslashes -- + # rather than rewriting '\' -> '/' -- preserves content, so it's safe + # for arbitrary build flags as well as Windows paths. + escaped = str(p).replace("\\", "\\\\").replace('"', '\\"') + return f'"{escaped}"' def generate_module_yml(component: ConvertedLibrary) -> str: @@ -80,7 +83,11 @@ def generate_cmakelists_txt(component: ConvertedLibrary) -> str: build_include_dir = build.get("includeDir", DEFAULT_BUILD_INCLUDE_DIR) build_src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) - build_flags = ensure_list(build.get("flags", DEFAULT_BUILD_FLAGS)) + # The shared lexer re-glues spaced entries and drops bare/empty + # arguments, same as the espidf emitter + build_flags = lex_build_flags( + build.get("flags", DEFAULT_BUILD_FLAGS), component.name + ) src_files = collect_filtered_files( read_path / Path(build_src_dir), build_src_filter diff --git a/esphome/espidf/component.py b/esphome/espidf/component.py index aa6f10c261..bed999656b 100644 --- a/esphome/espidf/component.py +++ b/esphome/espidf/component.py @@ -20,6 +20,7 @@ from esphome.platformio.library import ( DEFAULT_BUILD_SRC_FILTER, ESPHOME_DATA_EXTRA_CMAKE_KEY, ESPHOME_DATA_KEY, + ESPHOME_DATA_LINK_FLAGS_KEY, SRC_FILE_EXTENSIONS, ConvertedLibrary as IDFComponent, LibraryBackend, @@ -27,6 +28,7 @@ from esphome.platformio.library import ( collect_filtered_files, convert_libraries, ensure_list, + lex_build_flags, split_list_by_condition, ) @@ -40,37 +42,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,14 +56,12 @@ 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 - return f'"{str(p)}"'.replace("\\", "\\\\") + # In CMakeLists.txt, backslashes and embedded quotes need escaping + # (a quoted define value reaches here via the shlex round-trip) + escaped = str(p).replace("\\", "\\\\").replace('"', '\\"') + return f'"{escaped}"' def escape_path(p: PathType) -> str: # CMake uses forward slashes for paths on every platform and treats @@ -122,26 +91,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( @@ -253,6 +208,16 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: content += f" {str_build_flag}\n" content += ")\n" + # Extra-script LINKFLAGS: routed to the link line; in + # target_compile_options they would be silently ineffective + if link_flags := component.data.get(ESPHOME_DATA_KEY, {}).get( + ESPHOME_DATA_LINK_FLAGS_KEY, [] + ): + content += "target_link_options(${COMPONENT_LIB} INTERFACE\n" + for link_flag in link_flags: + content += f" {escape_entry(link_flag)}\n" + content += ")\n" + # Add custom CMake scripts content += "\n".join( component.data.get(ESPHOME_DATA_KEY, {}).get(ESPHOME_DATA_EXTRA_CMAKE_KEY, []) @@ -275,12 +240,17 @@ def generate_idf_component_yml(component: IDFComponent) -> str: data = {} + # Metadata only: tolerate malformed shapes instead of crashing on a + # third-party manifest (repository may legally be {"url": ...} or a + # plain URL string) description = component.data.get("description") - if description: + if isinstance(description, str) and description: data["description"] = description - repository = component.data.get("repository", {}).get("url", None) - if repository: + repository = component.data.get("repository") + if isinstance(repository, dict): + repository = repository.get("url") + if isinstance(repository, str) and repository: data["repository"] = repository for dependency in component.dependencies: @@ -299,7 +269,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..e04ccd1f55 --- /dev/null +++ b/esphome/platformio/extra_script.py @@ -0,0 +1,365 @@ +"""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, Any, NamedTuple + +from esphome.core import EsphomeError +from esphome.platformio.library import ESPHOME_DATA_KEY, ESPHOME_DATA_LINK_FLAGS_KEY + +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 + ``build.flags``; ``board_mcu`` is a callable so it resolves lazily.""" + extra_script = component.data.get("build", {}).get("extraScript") + if extra_script is None or 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, + ) + if link_flags := _str_entries(result.linkflags, "LINKFLAGS"): + # Kept apart from build.flags: the CMake emitters route those to + # target_compile_options, where a link flag is silently ineffective + esphome_data = component.data.setdefault(ESPHOME_DATA_KEY, {}) + esphome_data.setdefault(ESPHOME_DATA_LINK_FLAGS_KEY, []).extend(link_flags) + 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[CppDefine] = field(default_factory=list) + linkflags: list[str] = field(default_factory=list) + cppflags: list[str] = field(default_factory=list) + + +class CppDefine(NamedTuple): + """One normalized CPPDEFINES entry; a ``value`` of None is a bare -DNAME.""" + + name: str + value: str | None = None + + +def _cppdefine(entry: Any) -> CppDefine | None: + """Normalize one CPPDEFINES element, or warn and drop an unsupported + shape; formatting those blind would hand the compiler garbage like + ``-D{'FOO': '1'}``.""" + if isinstance(entry, str): + return CppDefine(entry) + if ( + isinstance(entry, (tuple, list)) + and len(entry) == 2 + and isinstance(entry[0], (str, int)) + and isinstance(entry[1], (str, int, type(None))) + ): + value = entry[1] + return CppDefine(str(entry[0]), None if value is None else str(value)) + _LOGGER.warning("Ignoring unsupported CPPDEFINES entry %r", entry) + return None + + +def _cppdefines_items(value: Any) -> list[CppDefine]: + """Normalize SCons ``processDefines`` spellings into ``CppDefine``s: a + bare 2-tuple is one ``name=value`` pair, a dict maps names to values, a + list is element-wise.""" + if isinstance(value, tuple) and len(value) == 2: + elements: list[Any] = [value] + elif isinstance(value, dict): + elements = list(value.items()) + else: + elements = list(value) if isinstance(value, (list, tuple)) else [value] + return [d for e in elements if (d := _cppdefine(e)) is not None] + + +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 __contains__(self, key: object) -> bool: + # Without this, "KEY" in env falls back to the legacy sequence + # protocol: __getitem__(0), (1), ... never raises, so it loops + # forever flooding the log + return key in self._vars + + def __iter__(self): + return iter(self._vars) + + def __getitem__(self, key: str) -> str: + # Scripts also read env["BOARD_MCU"]; an unmodelled subscript + # degrades one branch instead of discarding the whole capture + if key not in self._vars and key not in self._warned_gets: + self._warned_gets.add(key) + _LOGGER.warning( + "PIO extra-script env[%r] is not modelled; returning ''", key + ) + return self._vars.get(key, "") + + def Append(self, **kwargs) -> None: # noqa: N802 (SCons API name) + self._add(kwargs, prepend=False) + + def Prepend(self, **kwargs) -> None: # noqa: N802 (SCons API name) + self._add(kwargs, prepend=True) + + def _add(self, kwargs: dict[str, Any], *, prepend: bool) -> None: + 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 + if key == "CPPDEFINES": + items = _cppdefines_items(value) + else: + items = list(value) if isinstance(value, (list, tuple)) else [value] + bucket = getattr(self.result, key.lower()) + if prepend: + # SCons order: new values ahead of what is already there + # (scripts prepend LIBS for static-link symbol resolution) + bucket[:0] = items + else: + bucket.extend(items) + + # Dedup is not modelled; a repeated flag is harmless on the command line + AppendUnique = Append + PrependUnique = Prepend + + # ----- Everything else is a no-op so unsupported scripts don't crash ----- + + def __getattr__(self, name: str): + if name.startswith("__") and name.endswith("__"): + # Protocol probes (copy, pickle, iteration) are not script calls + raise AttributeError(name) + if name not in self._warned_methods: + # Warn on access, not call: hasattr()/truthiness branches would + # otherwise silently take the wrong path; a script whose whole + # effect is env.Replace() stays diagnosable either way + self._warned_methods.add(name) + _LOGGER.warning("PIO extra-script env.%s is not supported; ignoring", name) + + def _noop(*args, **kwargs): + return None + + 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, ``library_dir`` as CWD. + + 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 _str_entries(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 + + +def captured_as_build_flags( + result: ExtraScriptResult, *, library_dir: Path +) -> list[str]: + """Translate captured env vars into -L/-l/-D/raw build flags; path + entries anchor to ``library_dir`` so the build files stay portable.""" + flags: list[str] = [] + 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 _str_entries(result.cpppath, "CPPPATH") + ) + flags.extend( + f"-L{shlex.quote(_anchored(path))}" + for path in _str_entries(result.libpath, "LIBPATH") + ) + flags.extend(f"-l{shlex.quote(lib)}" for lib in _str_entries(result.libs, "LIBS")) + for define in result.cppdefines: + if define.value is None: + # {"FOO": None} / ("FOO", None) is a bare -DFOO in SCons + flags.append(shlex.quote(f"-D{define.name}")) + else: + flags.append(shlex.quote(f"-D{define.name}={define.value}")) + # Each captured entry is one argv token in SCons; quote so the + # lex_build_flags round-trip cannot split a spaced value into two. + # LINKFLAGS are deliberately absent: they travel via + # ESPHOME_DATA_LINK_FLAGS_KEY straight to the link line. + flags.extend(shlex.quote(f) for f in _str_entries(result.cppflags, "CPPFLAGS")) + return flags diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index ee0a758a31..a3899fa860 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,25 +47,34 @@ 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. Per CXXSUFFIXES .C/.C++ are C++ +# here, even where SCons demotes .C on case-insensitive filesystems. +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" ESPHOME_DATA_KEY = "ESPHOME" ESPHOME_DATA_EXTRA_CMAKE_KEY = "EXTRA_CMAKE" +# Captured extra-script LINKFLAGS; kept apart from build.flags so they reach +# the link line (target_link_options), not target_compile_options +ESPHOME_DATA_LINK_FLAGS_KEY = "LINK_FLAGS" class Source: @@ -203,6 +212,11 @@ class InvalidLibrary(Exception): pass +class IncompatiblePlatform(InvalidLibrary): + """The routine cross-platform skip, typed so callers need not match + message text.""" + + class ConvertedLibrary: """A resolved PlatformIO library plus its parsed manifest and on-disk path. @@ -427,17 +441,22 @@ def check_library_data(data: dict, platform: str | None, framework: str): if isinstance(platforms, str): platforms = [a.strip() for a in platforms.split(",")] platforms = ensure_list(platforms) + if not all(isinstance(pf, str) for pf in platforms): + # A real (non-platform) manifest problem; callers warn, not skip + raise InvalidLibrary(f"Malformed platforms value: {platforms!r}") # Check if library supports the target platform 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): frameworks = [a.strip() for a in frameworks.split(",")] frameworks = ensure_list(frameworks) + if not all(isinstance(fw, str) for fw in frameworks): + raise InvalidLibrary(f"Malformed frameworks value: {frameworks!r}") # Check if library declares the active framework. PIO library manifests # often list only "arduino" even when the library actually compiles fine @@ -455,7 +474,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 +488,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 +572,131 @@ 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.""" + # Lex per entry as ParseFlags does: a dangling -I 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, as + PlatformIO's ParseFlags does. A trailing or empty argument is warned and + dropped: the bare flag would make gcc eat the next flag.""" + 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 for ``depends=``-only manifests; the walk reads only the JSON + ``dependencies`` key, so they 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 (non-empty string), + version (string, if present), and owner (string, if present); invalid + entries warn naming the manifest.""" + name = entry.get("name") + owner = entry.get("owner") + name_ok = isinstance(name, str) and name + version_ok = "version" not in entry or isinstance(entry["version"], str) + owner_ok = owner is None or isinstance(owner, str) + if name_ok and version_ok and owner_ok: + 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. + + Bare-name spellings carry no version; the dependency walk later drops + version-less entries at DEBUG, since they are usually names of bundled + framework libraries (Wire, SPI) that need no registry install. """ - if not dependencies: + if dependencies is None: 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 +705,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 +841,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 +884,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 +896,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 +944,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 +958,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 +984,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 +996,85 @@ 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}" ) + # A bare json.load imposes no shape; every backend dereferences + # these fields, so validate once here and name the library + malformed = not isinstance(component.data, dict) + if not malformed: + build = component.data.get("build", {}) + esphome_data = component.data.get(ESPHOME_DATA_KEY, {}) + malformed = ( + not isinstance(build, dict) + or not isinstance(esphome_data, dict) + or not isinstance( + esphome_data.get(ESPHOME_DATA_LINK_FLAGS_KEY, []), list + ) + or not isinstance(build.get("srcDir", ""), str) + or not isinstance(build.get("includeDir", ""), str) + or not isinstance(build.get("srcFilter", ""), (str, list)) + ) + if malformed: + # Fail fast only for a library the user asked for; a defect in + # an unrequested corner of the graph must not block the build + if key in top_level_keys: + raise EsphomeError(f"Library {key} has a malformed manifest") + _LOGGER.warning("Skipping dependency %s: malformed manifest", key) + continue + 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)) + reason = ( + f"is not compatible with {backend.framework}" + if isinstance(e, IncompatiblePlatform) + else "has a malformed manifest" + ) + raise RuntimeError(f"Requested library {key} {reason}: {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 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) -- unactionable noise above debug + _LOGGER.debug( + "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..7b4f848979 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,21 +16,24 @@ 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, ) import esphome.platformio.library from esphome.platformio.library import ( + ESPHOME_DATA_KEY, + ESPHOME_DATA_LINK_FLAGS_KEY, ConvertedLibrary as IDFComponent, 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, ) @@ -291,6 +294,38 @@ def test_generate_cmakelists_txt_multi_token_flag(tmp_component): assert ' "-include"\n "cp_custom_alloc.h"\n' in content +def test_generate_cmakelists_txt_escapes_embedded_quotes(tmp_component): + """A define value carrying a literal quote survives into CMake as an + escaped quote, not a prematurely-terminated string.""" + src_dir = tmp_component.path / "src" + src_dir.mkdir() + (src_dir / "main.c").write_text("int main() {}") + # shlex keeps the backslash-escaped quotes as literal characters + tmp_component.data = {"build": {"flags": ['-DMSG=\\"hi\\"']}} + + content = generate_cmakelists_txt(tmp_component) + assert '"-DMSG=\\"hi\\""' in content + + +def test_generate_cmakelists_txt_extra_script_link_flags(tmp_component): + """Captured extra-script LINKFLAGS come out as target_link_options, not + compile options where they would be silently ineffective.""" + src_dir = tmp_component.path / "src" + src_dir.mkdir() + (src_dir / "main.c").write_text("int main() {}") + + tmp_component.data = { + ESPHOME_DATA_KEY: {ESPHOME_DATA_LINK_FLAGS_KEY: ["-Wl,--gc-sections"]} + } + + content = generate_cmakelists_txt(tmp_component) + assert ( + 'target_link_options(${COMPONENT_LIB} INTERFACE\n "-Wl,--gc-sections"\n)' + in content + ) + assert "target_compile_options" not in content + + def test_generate_cmakelists_txt_space_separated_classified_flags(tmp_component): # Space-separated -I/-L/-l entries routed to INCLUDE_DIRS and the link # handling before the shlex split was added; splitting must not leak @@ -339,6 +374,18 @@ def test_generate_idf_component_yml_basic(tmp_component): assert result == "description: test\nrepository: http://aaa\n" +def test_generate_idf_component_yml_tolerates_malformed_metadata(tmp_component): + """A string repository is the URL itself; junk shapes drop instead of + crashing on a third-party manifest.""" + tmp_component.data = {"description": "test", "repository": "http://aaa"} + assert ( + generate_idf_component_yml(tmp_component) + == "description: test\nrepository: http://aaa\n" + ) + tmp_component.data = {"description": {"en": "x"}, "repository": 123} + assert generate_idf_component_yml(tmp_component) == "{}\n" + + def test_generate_idf_component_yml_with_dependencies(tmp_component, tmp_path): dep = IDFComponent("dep", "1.0", source=URLSource("http://dummy.com")) dep.path = tmp_path / "dep" @@ -369,133 +416,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 +435,7 @@ empty= """ ) - result = _parse_library_properties(f) + result = parse_library_properties(f) assert result["name"] == "Test" assert result["version"] == "1.0" @@ -680,22 +605,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 +1115,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..07f0e78500 --- /dev/null +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -0,0 +1,563 @@ +"""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 ( + CppDefine, + ExtraScriptResult, + _FakeSConsEnv, + apply_extra_script, + captured_as_build_flags, + run_extra_script, +) +from esphome.platformio.library import ( + ESPHOME_DATA_KEY, + ESPHOME_DATA_LINK_FLAGS_KEY, + 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 CppDefine("BAR", "1") in result.cppdefines + assert CppDefine("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 + # LINKFLAGS travel via the link-flags channel, never the compile flags + assert "-Wl,--gc-sections" not 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 "-Os" in flags + assert c.data[ESPHOME_DATA_KEY][ESPHOME_DATA_LINK_FLAGS_KEY] == ["-Wl,-x"] + 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 LINKFLAGS entry {'no': 1}" 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"] + + +@pytest.mark.parametrize("method", ("Prepend", "PrependUnique")) +def test_prepend_inserts_ahead_of_existing(method: str) -> None: + """Prepend keeps SCons order: new values land ahead of what is already + captured (scripts prepend LIBS for static-link symbol resolution).""" + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + env.Append(LIBS=["m"]) + getattr(env, method)(LIBS=["algobsec", "bsec"]) + assert env.result.libs == ["algobsec", "bsec", "m"] + + +def test_env_membership_and_iteration(tmp_path) -> None: + """Membership tests and for-loops must use the mapping protocol; the + legacy sequence fallback through __getitem__ would loop forever.""" + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + assert "BOARD_MCU" in env + assert "NOPE" not in env + assert sorted(env) == ["BOARD_MCU", "PIOENV", "PIOPLATFORM"] + + +def test_apply_extra_script_non_string_falsey_raises(tmp_path) -> None: + """A falsey non-string extraScript (false, 0, []) is a malformed + manifest, not an absent script.""" + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": False}} + with pytest.raises(EsphomeError, match="must be a string"): + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + + +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_cppflag_survives_relexing(tmp_path) -> None: + """A captured argv token with a space stays one token after lexing.""" + result = ExtraScriptResult( + cppflags=["-include my hdr.h"], + cppdefines=[CppDefine("MSG", '"hello world"'), CppDefine("PLAIN")], + ) + flags = captured_as_build_flags(result, library_dir=tmp_path) + assert lex_build_flags(flags, "test") == [ + '-DMSG="hello world"', + "-DPLAIN", + "-include my hdr.h", + ] + + +def test_env_attribute_access_warns_without_call(caplog) -> None: + """hasattr()/truthiness on an unsupported method is diagnosable; dunder + protocol probes stay silent.""" + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + assert env.GetProjectOption + assert caplog.text.count("env.GetProjectOption is not supported") == 1 + assert not hasattr(env, "__deepcopy__") + assert "__deepcopy__" not in caplog.text + + +def test_env_unmodelled_subscript_degrades_one_branch(caplog) -> None: + """env[...] on an unmodelled var returns '' instead of KeyError + discarding the whole capture.""" + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + assert env["PIOFRAMEWORK"] == "" + assert env["PIOFRAMEWORK"] == "" + assert caplog.text.count("env['PIOFRAMEWORK'] is not modelled") == 1 + assert env["BOARD_MCU"] == "esp8266" + env.Append(LIBS=["still_captured"]) + assert env.result.libs == ["still_captured"] + + +def test_cppdefines_scons_spellings(tmp_path) -> None: + """A bare 2-tuple is one name=value pair, a dict maps names to values, + and a None value is a bare define (SCons processDefines).""" + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + env.Append(CPPDEFINES=("FOO", "1")) + env.Append(CPPDEFINES={"BAR": "2", "BAZ": None}) + env.Append(CPPDEFINES=["PLAIN"]) + flags = captured_as_build_flags(env.result, library_dir=tmp_path) + assert lex_build_flags(flags, "test") == [ + "-DFOO=1", + "-DBAR=2", + "-DBAZ", + "-DPLAIN", + ] + + +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..bf8340cea0 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -13,6 +13,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 +24,8 @@ from esphome.platformio.library import ( _resolve_registry_version, check_library_data, convert_libraries, + join_flag_args, + split_flag_entry, ) @@ -154,7 +157,9 @@ 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): @@ -292,7 +297,10 @@ 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 = "", ) -> None: calls.append(force) self.path = tmp_path / self.get_require_name() @@ -531,3 +539,210 @@ 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 + # A non-string owner would stringify into a malformed registry name + assert ( + normalize_dependencies( + [{"name": "Foo", "owner": {"bad": 1}, "version": "1.0"}], "libx" + ) + == [] + ) + # A falsey scalar (0, false) is malformed, not an empty list + assert normalize_dependencies(0, "libx") == [] + assert "Ignoring unrecognized dependencies 0 of libx" in caplog.text + + +@pytest.mark.parametrize( + "manifest", + [ + ["not", "a", "manifest"], + {"name": "A", "build": "src"}, + {"name": "A", "ESPHOME": "yes"}, + {"name": "A", "build": {"srcDir": 123}}, + {"name": "A", "build": {"includeDir": ["inc"]}}, + {"name": "A", "build": {"srcFilter": {"+": "src"}}}, + {"name": "A", "ESPHOME": {"LINK_FLAGS": "-Wl,-x"}}, + ], +) +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_convert_libraries_malformed_transitive_dep_skips( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A malformed manifest on a dependency the user never asked for warns + and skips; only a top-level library fails the build.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + { + "esphome/A": { + "name": "A", + "dependencies": [{"name": "B", "owner": "esphome", "version": "1.0"}], + }, + "esphome/B": {"name": "B", "build": {"srcDir": 123}}, + }, + ) + components = convert_libraries([Library("esphome/A", None, None)], _backend()) + names = [c.name for c in components] + assert "esphome/A" in names + assert "esphome/B" not in names + assert "Skipping dependency esphome/B: malformed manifest" in caplog.text + + +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", "platforms": [123]}], + } + }, + ) + convert_libraries([Library("esphome/A", None, None)], _backend()) + assert "Skipping dependency B of esphome/A: Malformed platforms" 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", "frameworks": [None]}, + }, + ) + convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + assert "Malformed frameworks" 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 diff --git a/tests/unit_tests/test_zephyr_library.py b/tests/unit_tests/test_zephyr_library.py index b370fe0c47..765e505c38 100644 --- a/tests/unit_tests/test_zephyr_library.py +++ b/tests/unit_tests/test_zephyr_library.py @@ -66,6 +66,34 @@ def test_generate_cmakelists_txt_flags_and_includes(tmp_path): assert "-lm" in out +def test_generate_cmakelists_txt_escapes_embedded_quotes(tmp_path): + """A define value carrying a literal quote (reachable via the lex + round-trip) survives as an escaped quote, not a broken CMake string.""" + c = _make_component(tmp_path) + (tmp_path / "src").mkdir() + (tmp_path / "src" / "a.c").write_text("") + c.data = {"build": {"flags": ['-DMSG=\\"hi\\"']}} + + out = generate_cmakelists_txt(c) + assert '"-DMSG=\\"hi\\""' in out + + +def test_generate_cmakelists_txt_lexes_spaced_flags(tmp_path): + """A spaced -I entry routes to include dirs instead of landing verbatim + in compile options; same shared lexer as the espidf emitter.""" + c = _make_component(tmp_path) + (tmp_path / "src").mkdir() + (tmp_path / "src" / "a.c").write_text("") + (tmp_path / "include").mkdir() + c.data = {"build": {"flags": "-I include -DBAR=1"}} + + out = generate_cmakelists_txt(c) + + assert str((tmp_path / "include").resolve()).replace("\\", "\\\\") in out + assert "-DBAR=1" in out + assert "-I include" not in out + + def test_generate_zephyr_modules_collects_all_dirs_and_writes(tmp_path, monkeypatch): # Two converted libraries: one top-level, one transitive dependency. The # converter calls backend.emit for both; generate_zephyr_modules must return