mirror of
https://github.com/esphome/esphome.git
synced 2026-08-23 06:36:23 +00:00
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.
This commit is contained in:
@@ -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(
|
||||
|
||||
+135
-15
@@ -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")
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user