From 09a875f359bb6d9963e1ce8131cc774fdf4ba5f4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 12:43:04 -0500 Subject: [PATCH 1/3] Refactor the library backend into focused functions build_tool gains one function per mode plus a shared rspfile reader. _library_info becomes an orchestrator over _resolve_src_dir, _resolve_lib_archive, _classify_build_flags, _resolve_include_dirs, and _collect_lib_sources. The version-less reconciliation moves into _warn_unsatisfied_versionless. _is_safe_library_name switches from a character denylist to a filename-plain allowlist, which excludes separators, drive colons, and dot-only names by shape. --- esphome/arduino/library.py | 117 +++++++++++++++++++++----------- esphome/build_gen/build_tool.py | 92 ++++++++++++++----------- esphome/platformio/library.py | 66 ++++++++++-------- 3 files changed, 167 insertions(+), 108 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index 8260f17c52..a57bc7c5a3 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -17,6 +17,7 @@ from dataclasses import dataclass, field import functools import logging from pathlib import Path +import re from esphome.core import CORE, EsphomeError, Library from esphome.helpers import walk_files @@ -63,16 +64,15 @@ class ArduinoLibrary: link_flags: list[str] = field(default_factory=list) +# Filename-plain names only: leading alnum/underscore, then word chars, +# dot, space, plus, or hyphen. An allowlist excludes separators, drive +# colons, and dot-only names by shape instead of enumerating them. +_SAFE_LIBRARY_NAME_RE = re.compile(r"[A-Za-z0-9_][A-Za-z0-9_. +-]*\Z") + + def _is_safe_library_name(name: object) -> bool: """Whether a name may be joined under the framework's libraries dir.""" - return ( - isinstance(name, str) - and bool(name) - and "/" not in name - and "\\" not in name - and ":" not in name # a Windows drive-relative name escapes the tree - and name not in (".", "..") - ) + return isinstance(name, str) and _SAFE_LIBRARY_NAME_RE.fullmatch(name) is not None def _manifest_build(name: str, data: object) -> dict: @@ -84,24 +84,21 @@ def _manifest_build(name: str, data: object) -> dict: return build -def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: - """Resolve one library's sources, include dirs, and flags (PIO semantics).""" - build = _manifest_build(name, data) +def _resolve_src_dir(name: str, read_path: Path, build: dict) -> str: + """Resolve PIO's source dir: manifest srcDir, else src/Src, else the root.""" + if "srcDir" not in build: + return next((d for d in ("src", "Src") if (read_path / d).is_dir()), ".") + # A declared srcDir (falsy included) that does not resolve is a + # manifest error + src_dir = build["srcDir"] + if not (isinstance(src_dir, str) and src_dir and (read_path / src_dir).is_dir()): + raise EsphomeError( + f"Library {name} declares srcDir {src_dir!r} which does not exist" + ) + return src_dir - # PIO's source-dir resolution: manifest srcDir, else src/Src, else the root - if "srcDir" in build: - # A declared srcDir (falsy included) that does not resolve is a - # manifest error - src_dir = build["srcDir"] - if not ( - isinstance(src_dir, str) and src_dir and (read_path / src_dir).is_dir() - ): - raise EsphomeError( - f"Library {name} declares srcDir {src_dir!r} which does not exist" - ) - else: - src_dir = next((d for d in ("src", "Src") if (read_path / d).is_dir()), ".") +def _warn_dropped_link_fields(name: str, data: dict) -> None: for dropped_key in ("precompiled", "ldflags"): if data.get(dropped_key): # PIO's Arduino lib builder honors these; building without them @@ -111,15 +108,14 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: name, dropped_key, ) - src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) - if not all(isinstance(entry, str) for entry in src_filter): - raise EsphomeError(f"Library {name} has a malformed srcFilter") - # PlatformIO shell-lexes each build.flags entry - flag_tokens = lex_build_flags(build.get("flags", []), f"library {name}") - # dot_a_linkage (Arduino IDE's property, ignored by PIO) is a deliberate - # extra. Strict parse: bool("false") is True. - def _parse_archive(key: str, raw: object) -> bool: + +def _resolve_lib_archive(name: str, data: dict, build: dict) -> bool: + """build.libArchive, else dot_a_linkage (Arduino IDE's property, ignored + by PIO -- a deliberate extra), else archive.""" + + # Strict parse: bool("false") is True + def _parse(key: str, raw: object) -> bool: if isinstance(raw, bool): return raw value = str(raw).strip().lower() @@ -128,12 +124,19 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: raise EsphomeError(f"Library {name} has a malformed {key} value {raw!r}") if "libArchive" in build: - lib_archive = _parse_archive("libArchive", build["libArchive"]) - elif "dot_a_linkage" in data: - lib_archive = _parse_archive("dot_a_linkage", data["dot_a_linkage"]) - else: - lib_archive = True - lib = ArduinoLibrary(name=name, lib_archive=lib_archive) + return _parse("libArchive", build["libArchive"]) + if "dot_a_linkage" in data: + return _parse("dot_a_linkage", data["dot_a_linkage"]) + return True + + +def _classify_build_flags( + name: str, read_path: Path, lib: ArduinoLibrary, flag_tokens: list[str] +) -> list[str]: + """Route the lexed build.flags into the library's flag lists. + + Returns the ``-I`` arguments for the include-dir resolution. + """ include_flags: list[str] = [] for tok in flag_tokens: if tok.startswith("-I"): @@ -155,13 +158,23 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: lib.link_flags.append(tok) else: lib.flags.append(tok) + return include_flags + +def _resolve_include_dirs( + name: str, + read_path: Path, + lib: ArduinoLibrary, + build: dict, + src_dir: str, + include_flags: list[str], +) -> None: include_dir = build.get("includeDir", DEFAULT_BUILD_INCLUDE_DIR) if not isinstance(include_dir, str): raise EsphomeError(f"Library {name} has a malformed includeDir") for d, explicit in [ (include_dir, "includeDir" in build), - (src_dir, False), # the srcDir guard above already validated it + (src_dir, False), # _resolve_src_dir already validated it *((flag, True) for flag in include_flags), ]: if (path := (read_path / d)).is_dir(): @@ -174,6 +187,15 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: "Library %s declares include dir %s which does not exist", name, d ) + +def _collect_lib_sources( + name: str, + read_path: Path, + lib: ArduinoLibrary, + build: dict, + src_dir: str, + src_filter: list[str], +) -> None: matched = collect_filtered_files(read_path / src_dir, src_filter) lib.sources = sorted( path.resolve() @@ -204,6 +226,23 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: "Library %s declares srcFilter/srcDir but no source files matched", name, ) + + +def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: + """Resolve one library's sources, include dirs, and flags (PIO semantics).""" + build = _manifest_build(name, data) + _warn_dropped_link_fields(name, data) + src_dir = _resolve_src_dir(name, read_path, build) + src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) + if not all(isinstance(entry, str) for entry in src_filter): + raise EsphomeError(f"Library {name} has a malformed srcFilter") + lib = ArduinoLibrary(name=name, lib_archive=_resolve_lib_archive(name, data, build)) + # PlatformIO shell-lexes each build.flags entry + include_flags = _classify_build_flags( + name, read_path, lib, lex_build_flags(build.get("flags", []), f"library {name}") + ) + _resolve_include_dirs(name, read_path, lib, build, src_dir, include_flags) + _collect_lib_sources(name, read_path, lib, build, src_dir, src_filter) return lib diff --git a/esphome/build_gen/build_tool.py b/esphome/build_gen/build_tool.py index c19c4dc220..804b623d9d 100644 --- a/esphome/build_gen/build_tool.py +++ b/esphome/build_gen/build_tool.py @@ -17,50 +17,62 @@ import subprocess import sys +def _read_rspfile(rspfile: str) -> list[str]: + r"""The object paths listed in ``rspfile``, unquoted. + + GNU ar treats backslashes in response files as escapes (corrupts + Windows paths), so the caller expands the list into argv; strip the + simple surrounding quote ninja adds to special paths, then undo + ninja's POSIX escape for an embedded quote ('a'\\''b.o' -> a'b.o). + """ + return [ + line[1:-1].replace("'\\''", "'") + if len(line) >= 2 and line[0] == line[-1] and line[0] in "'\"" + else line + for line in Path(rspfile).read_text(encoding="utf-8").splitlines() + if line + ] + + +def _run_ar(ar: str, archive: str, rspfile: str) -> int: + # Remove first: ``ar rc`` replaces members but never drops ones whose + # source was removed from the build, which would leak stale objects. + Path(archive).unlink(missing_ok=True) + objects = _read_rspfile(rspfile) + if not objects: + # An empty archive would "succeed" here and fail far away at link + print(f"ar: no objects listed in {rspfile} for {archive}", file=sys.stderr) + return 1 + # Batch by argv length: expanding the rspfile gives back the Windows + # 32767-char command-line limit it existed to avoid. "rc" creates, + # "q" appends the remainder. + op = "rc" + while objects: + batch = [objects.pop(0)] + batch_len = len(batch[0]) + while objects and batch_len + len(objects[0]) < 25000: + batch_len += len(objects[0]) + 1 + batch.append(objects.pop(0)) + rc = subprocess.run( + [ar, op, archive, *batch], check=False, close_fds=False + ).returncode + if rc != 0: + return rc + op = "q" + return 0 + + +def _run_copy(src: str, dst: str) -> int: + shutil.copyfile(src, dst) + return 0 + + def main() -> int: mode = sys.argv[1] if mode == "ar": - ar, archive, rspfile = sys.argv[2:5] - # Remove first: ``ar rc`` replaces members but never drops ones whose - # source was removed from the build, which would leak stale objects. - Path(archive).unlink(missing_ok=True) - # GNU ar treats backslashes in response files as escapes (corrupts - # Windows paths), so expand the rspfile into argv, stripping the - # simple surrounding quote ninja adds to special paths. - # After stripping the outer pair, undo ninja's POSIX escape for an - # embedded quote ('a'\''b.o' -> a'b.o) - objects = [ - line[1:-1].replace("'\\''", "'") - if len(line) >= 2 and line[0] == line[-1] and line[0] in "'\"" - else line - for line in Path(rspfile).read_text(encoding="utf-8").splitlines() - if line - ] - if not objects: - # An empty archive would "succeed" here and fail far away at link - print(f"ar: no objects listed in {rspfile} for {archive}", file=sys.stderr) - return 1 - # Batch by argv length: expanding the rspfile gives back the Windows - # 32767-char command-line limit it existed to avoid. "rc" creates, - # "q" appends the remainder. - op = "rc" - while objects: - batch = [objects.pop(0)] - batch_len = len(batch[0]) - while objects and batch_len + len(objects[0]) < 25000: - batch_len += len(objects[0]) + 1 - batch.append(objects.pop(0)) - rc = subprocess.run( - [ar, op, archive, *batch], check=False, close_fds=False - ).returncode - if rc != 0: - return rc - op = "q" - return 0 + return _run_ar(*sys.argv[2:5]) if mode == "copy": - src, dst = sys.argv[2:4] - shutil.copyfile(src, dst) - return 0 + return _run_copy(*sys.argv[2:4]) print(f"unknown build_tool mode: {mode}", file=sys.stderr) return 1 diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 6c0b0d09cf..8c683ba540 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -859,6 +859,42 @@ def is_lib_ignored(name: str | None, lib_ignore: set[str]) -> bool: ) +def _warn_unsatisfied_versionless( + skipped_versionless: list[tuple[Any, Any, str]], + components: dict[str, ConvertedLibrary], + backend: LibraryBackend, +) -> None: + """Warn for version-less deps nothing satisfied (request key, manifest + name, or backend provides()); a silent drop surfaces as link errors far + from the cause.""" + resolved_manifest_names = {c.data.get("name") for c in components.values()} + warned: set[str] = set() + for dep_name, dep_owner, requester in skipped_versionless: + if not isinstance(dep_name, str) or not dep_name or dep_name in warned: + continue + if dep_name in components: + # A version-less dep's request key is the name itself + continue + if dep_name in resolved_manifest_names: + continue + if ( + not dep_owner + and backend.provides is not None + and backend.provides(dep_name) + ): + # provides() only satisfies owner-less names: the walk's + # backend-provided skip has the same owner guard, so an + # owner-qualified version-less dep was added by nobody + continue + warned.add(dep_name) + _LOGGER.warning( + "Dependency %s of %s has no version to resolve and nothing " + "provides it; skipping", + dep_name, + requester, + ) + + def convert_libraries( libraries: list[Library], backend: LibraryBackend ) -> list[ConvertedLibrary]: @@ -1161,34 +1197,6 @@ def convert_libraries( for component in components.values(): backend.emit(component) - # Warn for version-less deps nothing satisfied (request key, manifest - # name, or backend provides()); a silent drop surfaces as link errors - # far from the cause. - resolved_manifest_names = {c.data.get("name") for c in components.values()} - warned: set[str] = set() - for dep_name, dep_owner, requester in skipped_versionless: - if not isinstance(dep_name, str) or not dep_name or dep_name in warned: - continue - if dep_name in components: - # A version-less dep's request key is the name itself - continue - if dep_name in resolved_manifest_names: - continue - if ( - not dep_owner - and backend.provides is not None - and backend.provides(dep_name) - ): - # provides() only satisfies owner-less names: the walk's - # backend-provided skip has the same owner guard, so an - # owner-qualified version-less dep was added by nobody - continue - warned.add(dep_name) - _LOGGER.warning( - "Dependency %s of %s has no version to resolve and nothing " - "provides it; skipping", - dep_name, - requester, - ) + _warn_unsatisfied_versionless(skipped_versionless, components, backend) return [components[key] for key in top_level if key in components] From 65609bb94b7a3b0b3acd2c804156fe175ecddf31 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 12:46:16 -0500 Subject: [PATCH 2/3] Move the backend-agnostic library hardening down from the arduino backend PR The typed IncompatiblePlatform exception, dependency_is_usable, warn_properties_depends, the lex_build_flags helper (espidf switches to it), the normalize_dependencies validation, the manifest shape check, the component-level drop-warning split, and the SCons case-sensitive .C/.C++ suffixes all harden the shared converter independently of the arduino backend, so they belong in this PR; the provides hook and the version-less reconciliation stay with the backend that needs them. --- esphome/espidf/component.py | 21 +-- esphome/platformio/library.py | 150 ++++++++++++++++++-- tests/unit_tests/test_platformio_library.py | 122 ++++++++++++++++ 3 files changed, 262 insertions(+), 31 deletions(-) diff --git a/esphome/espidf/component.py b/esphome/espidf/component.py index 567cde65e2..4eeaa30e7f 100644 --- a/esphome/espidf/component.py +++ b/esphome/espidf/component.py @@ -27,8 +27,7 @@ from esphome.platformio.library import ( collect_filtered_files, convert_libraries, ensure_list, - join_flag_args, - split_flag_entry, + lex_build_flags, split_list_by_condition, ) @@ -89,22 +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. - # Joined 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. - build_flags = [ - token - for entry in build_flags - for token in join_flag_args( - split_flag_entry(entry, f"library {component.name}"), - f"library {component.name}", - ) - ] + build_flags = lex_build_flags( + component.data.get("build", {}).get("flags", DEFAULT_BUILD_FLAGS), + f"library {component.name}", + ) # List all sources files build_src_files = collect_filtered_files( diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 7ce45a7fca..fb12f6f92b 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -55,6 +55,8 @@ SOURCE_KIND_FOR_SUFFIX: dict[str, str] = { ".cc": "cxx", ".cxx": "cxx", ".c++": "cxx", + ".C": "cxx", + ".C++": "cxx", ".S": "asm", ".spp": "asm", ".SPP": "asm", @@ -206,6 +208,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. @@ -435,7 +445,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): @@ -571,6 +581,24 @@ def split_flag_entry(entry: Any, owner: str) -> list[str]: 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 a manifest ``build.flags`` list into joined tokens. + + Each entry is lexed the way PlatformIO's ParseFlags does, and bare + ``-I``/``-L``/``-l``/``-D`` tokens re-glue to their argument across the + whole stream. Used by the espidf and arduino backends; zephyr still + classifies raw entries. + """ + # 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"}) @@ -591,19 +619,60 @@ def join_flag_args(tokens: Iterable[str], owner: str) -> list[str]: return out -def normalize_dependencies(dependencies: Any) -> list[dict]: +def warn_properties_depends(name: str, data: object) -> None: + """Warn when a manifest declares dependencies only as ``depends=``. + + The dependency walk reads the JSON ``dependencies`` key; the raw + ``library.properties`` spelling would otherwise drop silently. + """ + if isinstance(data, dict) and not data.get("dependencies") and data.get("depends"): + _LOGGER.warning( + "Library %s declares dependencies via library.properties " + "depends=, which are not resolved automatically; add them with " + "add_library() if needed", + name, + ) + + +def dependency_is_usable( + dep: dict, platform: str | None, framework: str, requester: str +) -> bool: + """Compatibility filter for a manifest dependency: platform mismatches + skip at debug, any other ``InvalidLibrary`` warns naming the requester.""" + try: + check_library_data(dep, platform, framework) + except IncompatiblePlatform as e: + _LOGGER.debug("Skip dependency %s of %s: %s", dep.get("name"), requester, e) + return False + except InvalidLibrary as e: + _LOGGER.warning( + "Skipping dependency %s of %s: %s", dep.get("name"), requester, e + ) + return False + return True + + +def 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 @@ -612,9 +681,46 @@ def normalize_dependencies(dependencies: Any) -> list[dict]: entry.update(spec) else: entry["version"] = spec + if not isinstance(name := entry.get("name"), str) or not name: + _LOGGER.warning( + "Ignoring unrecognized dependency entry %r of %s", + entry, + manifest_name, + ) + continue 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): + name = entry.get("name") + if not isinstance(name, str) or not name: + # A dependency name must be a non-empty string; every + # consumer indexes or joins it + _LOGGER.warning( + "Ignoring unrecognized dependency entry %r of %s", + entry, + manifest_name, + ) + continue + 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 @@ -900,30 +1006,44 @@ def convert_libraries( 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. + # Fail fast if a top-level library the build explicitly requested + # is incompatible; the routine cross-platform skip stays at + # debug, any other cause warns (a silent drop resurfaces as + # undefined symbols at link) 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")): + for dependency in normalize_dependencies( + component.data.get("dependencies"), component.name + ): if "name" not in dependency or "version" not in dependency: 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") diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index d050acad9e..60520f3df1 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -558,6 +558,128 @@ def test_join_flag_args_trailing_bare_flag_warns( 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_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 + + +@pytest.mark.parametrize( + "manifest", [["not", "a", "manifest"], {"name": "A", "build": "src"}] +) +def test_convert_libraries_malformed_manifest_raises( + tmp_path, monkeypatch, manifest +) -> None: + """A manifest without the expected dict shape fails by library name + before any backend dereferences data/build.""" + _patch_download_with_manifests(monkeypatch, tmp_path, {"esphome/A": manifest}) + with pytest.raises(EsphomeError, match="has a malformed manifest"): + convert_libraries([Library("esphome/A", None, None)], _backend()) + + +def test_walk_warns_for_properties_only_depends( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A manifest declaring dependencies only as library.properties depends= + warns in the shared walk, so every backend reports the drop.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + {"esphome/A": "name=A\nversion=1.0\ndepends=Wire, SPI\n"}, + properties=("esphome/A",), + ) + convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + assert "declares dependencies via library.properties" in caplog.text + + +def test_walk_warns_for_nonplatform_invalid_library( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A dependency dropped for any cause other than the routine platform + filter is visible in every backend.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + {"esphome/A": {"name": "A", "dependencies": [{"name": "B", "version": "1.0"}]}}, + ) + calls = {"n": 0} + real = lib.check_library_data + + def flaky(data, platform, framework): + calls["n"] += 1 + if calls["n"] > 1: + raise InvalidLibrary("manifest is corrupt") + return real(data, platform, framework) + + monkeypatch.setattr(lib, "check_library_data", flaky) + convert_libraries([Library("esphome/A", None, None)], _backend()) + assert "Skipping dependency B of esphome/A: manifest is corrupt" in caplog.text + + +def test_convert_libraries_warns_for_nonplatform_invalid_dependency_component( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A dependency component dropped for any cause other than the platform + filter warns; only the routine cross-platform skip stays at debug.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + { + "esphome/A": { + "name": "A", + "dependencies": [{"name": "C", "owner": "esphome", "version": "1.0"}], + }, + "esphome/C": {"name": "C"}, + }, + ) + real = lib.check_library_data + + def flaky(data, platform, framework): + # Fail only on C's resolved manifest, not on A's dependency entry + if data.get("name") == "C" and "version" not in data: + raise InvalidLibrary("manifest is corrupt") + return real(data, platform, framework) + + monkeypatch.setattr(lib, "check_library_data", flaky) + convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + assert "manifest is corrupt" in caplog.text + assert "Skipping dependency" in caplog.text + + def test_split_flag_entry_non_string_is_clean() -> None: """A dict or number from a third-party manifest fails naming the entry, not with an opaque shlex traceback.""" From e52703617e83328b7dd413d6ea984443254e73fe Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 12:47:05 -0500 Subject: [PATCH 3/3] Name the env boolean spelling tables like device-builder does parse_enable_env's inline tuples become TRUTHY_ENV_STRINGS and FALSY_ENV_STRINGS frozensets mirroring cv.boolean's spellings (enable and disable included) plus the 1/0 env convention, matching device-builder's TRUTHY_BOOL_STRINGS pattern. --- esphome/build_helpers/ccache.py | 8 ++++++-- tests/unit_tests/build_helpers/test_ccache.py | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index 8804e5028d..dfceaabf1c 100644 --- a/esphome/build_helpers/ccache.py +++ b/esphome/build_helpers/ccache.py @@ -11,6 +11,10 @@ from esphome.framework_helpers import strip_win_long_path_prefix, tool_version_r _LOGGER = logging.getLogger(__name__) +# esphome cv.boolean's spelling tables plus the 1/0 env convention +TRUTHY_ENV_STRINGS = frozenset({"1", "true", "yes", "on", "enable"}) +FALSY_ENV_STRINGS = frozenset({"0", "false", "no", "off", "disable"}) + def _ccache_runs(ccache: str) -> bool: """Return True when the ``ccache`` found on PATH actually runs.""" @@ -31,9 +35,9 @@ def parse_enable_env(name: str) -> bool | None: if raw is None: return None lowered = raw.strip().lower() - if lowered in ("1", "true", "yes", "on"): + if lowered in TRUTHY_ENV_STRINGS: return True - if lowered in ("0", "false", "no", "off"): + if lowered in FALSY_ENV_STRINGS: return False _LOGGER.warning("Ignoring unrecognized %s=%r; use 1 or 0", name, raw) return None diff --git a/tests/unit_tests/build_helpers/test_ccache.py b/tests/unit_tests/build_helpers/test_ccache.py index c612f79554..619a1a3476 100644 --- a/tests/unit_tests/build_helpers/test_ccache.py +++ b/tests/unit_tests/build_helpers/test_ccache.py @@ -97,3 +97,23 @@ def test_resolve_unrecognized_value_warns_and_probes( assert ccache.resolve_ccache_path() is None mock_probe.assert_called_once() assert "unrecognized ESPHOME_CCACHE_ENABLE" in caplog.text + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ("1", True), + ("enable", True), + ("ON", True), + ("0", False), + ("disable", False), + ("Off", False), + ("maybe", None), + ], +) +def test_parse_enable_env_spelling_tables( + monkeypatch: pytest.MonkeyPatch, raw: str, expected: bool | None +) -> None: + """cv.boolean's spelling tables plus the 1/0 env convention.""" + monkeypatch.setenv("ESPHOME_CCACHE_ENABLE", raw) + assert ccache.parse_enable_env("ESPHOME_CCACHE_ENABLE") is expected