mirror of
https://github.com/esphome/esphome.git
synced 2026-08-22 22:26:21 +00:00
Never shred a string dependency list, defer and de-duplicate drop warnings
A plain-string dependencies value comma-splits into names instead of iterating into one-character libraries; entries that normalize to nothing warn naming the manifest, and the walk's version-less skip logs at debug for every backend. The not-bundled warning is deferred until after the walk and stays quiet for names resolved transitively, so a false skipping message cannot teach users to ignore the real one. An explicitly requested bundled library that resolves to no sources and no headers warns that the install may be incomplete.
This commit is contained in:
+34
-10
@@ -221,7 +221,20 @@ def _bundled_library(framework_path: Path, name: str) -> ArduinoLibrary:
|
||||
"run for bundled libraries",
|
||||
name,
|
||||
)
|
||||
return _library_info(name, lib_dir, data)
|
||||
lib = _library_info(name, lib_dir, data)
|
||||
if not lib.sources and not any(
|
||||
p.suffix in (".h", ".hpp", ".hh", ".inc")
|
||||
for d in lib.include_dirs
|
||||
for p in d.rglob("*")
|
||||
):
|
||||
# An empty or half-extracted bundled directory would otherwise
|
||||
# become a silent no-op that surfaces as undefined symbols at link
|
||||
_LOGGER.warning(
|
||||
"Bundled library %s has no sources or headers; the framework "
|
||||
"install may be incomplete (run 'esphome clean-all')",
|
||||
name,
|
||||
)
|
||||
return lib
|
||||
|
||||
|
||||
def resolve_libraries(
|
||||
@@ -272,6 +285,8 @@ def resolve_libraries(
|
||||
# "skipping" warning teaches users to ignore the real one)
|
||||
external_short_names = {lib.name.split("/")[-1] for lib in external if lib.name}
|
||||
|
||||
pending_drops: list[tuple[str, str]] = []
|
||||
|
||||
def _add_bundled_dependencies(component: ConvertedLibrary) -> None:
|
||||
# A version-less bare-name dependency ("Hash" in ESPAsyncWebServer)
|
||||
# is a core-bundled library; the shared converter skips it because
|
||||
@@ -327,15 +342,10 @@ def resolve_libraries(
|
||||
)
|
||||
continue
|
||||
if not bundled_dir.is_dir():
|
||||
# The shared converter skips version-less deps too, so this
|
||||
# is the only place the drop can be made visible before the
|
||||
# missing sources surface as link errors.
|
||||
_LOGGER.warning(
|
||||
"Dependency %s of library %s is not bundled with the "
|
||||
"framework and has no version to resolve; skipping",
|
||||
name,
|
||||
component.name,
|
||||
)
|
||||
# Deferred: the walk may still resolve this name as another
|
||||
# library's transitive registry dependency, and a false
|
||||
# "skipping" warning teaches users to ignore the real one
|
||||
pending_drops.append((name, component.name))
|
||||
continue
|
||||
try:
|
||||
check_library_data(dep, pio_platform, "arduino")
|
||||
@@ -406,4 +416,18 @@ def resolve_libraries(
|
||||
f"{', '.join(dropped) or 'unknown'})"
|
||||
)
|
||||
|
||||
# The shared converter skips version-less deps too, so this is the only
|
||||
# place a genuine drop can be made visible before the missing sources
|
||||
# surface as link errors; names the walk resolved anyway stay quiet.
|
||||
resolved_short_names = {c.name.split("__")[-1] for c in converted}
|
||||
for name, requester in pending_drops:
|
||||
if name in bundled_names or name in resolved_short_names:
|
||||
continue
|
||||
_LOGGER.warning(
|
||||
"Dependency %s of library %s is not bundled with the framework "
|
||||
"and has no version to resolve; skipping",
|
||||
name,
|
||||
requester,
|
||||
)
|
||||
|
||||
return bundled + converted
|
||||
|
||||
@@ -628,15 +628,21 @@ def join_flag_args(tokens: Iterable[str], owner: str) -> list[str]:
|
||||
return out
|
||||
|
||||
|
||||
def normalize_dependencies(dependencies: Any) -> list[dict]:
|
||||
def normalize_dependencies(dependencies: Any, owner: 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. ``owner`` 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():
|
||||
@@ -660,6 +666,10 @@ def normalize_dependencies(dependencies: Any) -> list[dict]:
|
||||
# ["Wire"]); dropping them here would hide a real dependency
|
||||
# from every caller's visibility warning
|
||||
normalized.append({"name": entry})
|
||||
else:
|
||||
_LOGGER.warning(
|
||||
"Ignoring unrecognized dependency entry %r of %s", entry, owner
|
||||
)
|
||||
return normalized
|
||||
|
||||
|
||||
@@ -973,8 +983,17 @@ def convert_libraries(
|
||||
# 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:
|
||||
# Version-less deps cannot resolve from the registry; the
|
||||
# arduino backend picks bundled ones up after emit
|
||||
_LOGGER.debug(
|
||||
"Skip version-less dependency %r of %s",
|
||||
dependency.get("name"),
|
||||
component.name,
|
||||
)
|
||||
continue
|
||||
try:
|
||||
check_library_data(dependency, backend.platform, backend.framework)
|
||||
|
||||
@@ -920,3 +920,48 @@ def test_pinned_bundled_dependency_substitution_warns(
|
||||
)
|
||||
assert "Wire" in [lib.name for lib in libs]
|
||||
assert "pins version ^2.0.0; using the library bundled" in caplog.text
|
||||
|
||||
|
||||
def test_transitively_resolved_dependency_does_not_warn(
|
||||
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A version-less dependency the walk resolved as another library's
|
||||
registry dependency is present in the build; the skipping warning must
|
||||
stay quiet for it."""
|
||||
framework = _make_framework(tmp_path)
|
||||
_add_library("ESP32Async/ESPAsyncWebServer", "3.9.6")
|
||||
ws_dir = tmp_path / "converted" / "webserver"
|
||||
(ws_dir / "src").mkdir(parents=True)
|
||||
tcp_dir = tmp_path / "converted" / "tcp"
|
||||
(tcp_dir / "src").mkdir(parents=True)
|
||||
ws = _converted(
|
||||
"esp32async__ESPAsyncWebServer",
|
||||
ws_dir,
|
||||
{"build": {}, "dependencies": [{"name": "ESPAsyncTCP"}]},
|
||||
)
|
||||
tcp = _converted("esp32async__ESPAsyncTCP", tcp_dir, {"build": {}})
|
||||
with _emitting_converter(ws, tcp):
|
||||
component.resolve_libraries(
|
||||
framework,
|
||||
pio_platform="espressif8266",
|
||||
board_mcu="esp8266",
|
||||
cache_key="arduino8266",
|
||||
)
|
||||
assert "is not bundled with the framework" not in caplog.text
|
||||
|
||||
|
||||
def test_empty_bundled_library_warns(
|
||||
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A bundled directory with no sources or headers is a broken install,
|
||||
not a silent no-op archive."""
|
||||
framework = _make_framework(tmp_path)
|
||||
(framework / "libraries" / "Empty").mkdir()
|
||||
_add_library("Empty", None)
|
||||
component.resolve_libraries(
|
||||
framework,
|
||||
pio_platform="espressif8266",
|
||||
board_mcu="esp8266",
|
||||
cache_key="arduino8266",
|
||||
)
|
||||
assert "Bundled library Empty has no sources or headers" in caplog.text
|
||||
|
||||
@@ -592,12 +592,18 @@ def test_source_kind_map_shape() -> None:
|
||||
assert SOURCE_KIND_FOR_SUFFIX[".cpp"] == "cxx"
|
||||
|
||||
|
||||
def test_normalize_dependencies_string_entries() -> None:
|
||||
def test_normalize_dependencies_forms(caplog) -> None:
|
||||
"""Every PIO-legal spelling normalizes; unrecognizable entries warn."""
|
||||
from esphome.platformio.library import normalize_dependencies
|
||||
|
||||
"""PIO's bare string-list form coerces to name dicts; other non-dict
|
||||
entries still drop."""
|
||||
assert normalize_dependencies(["Wire", {"name": "SPI"}, 5, ""]) == [
|
||||
assert normalize_dependencies(["Wire", {"name": "SPI"}, 5, ""], "libx") == [
|
||||
{"name": "Wire"},
|
||||
{"name": "SPI"},
|
||||
]
|
||||
assert caplog.text.count("unrecognized dependency entry") == 2
|
||||
# A plain string is names, never iterated into characters
|
||||
assert normalize_dependencies("Wire, SPI") == [
|
||||
{"name": "Wire"},
|
||||
{"name": "SPI"},
|
||||
]
|
||||
assert normalize_dependencies("Wire") == [{"name": "Wire"}]
|
||||
|
||||
Reference in New Issue
Block a user