mirror of
https://github.com/esphome/esphome.git
synced 2026-08-23 06:36:23 +00:00
Move version-less drop reconciliation into the walk, harden the name guards
The shared walk now defers every version-less skip and reconciles after emit against the final resolution set (request keys, resolved manifest names, and backend.provides(name) per name), so drop visibility is correct for every backend by construction instead of by a comment-level contract, and the arduino backend's duplicate pending_drops pass and its short-name suppression heuristics are deleted. The provides lambda applies _is_safe_library_name so an unsafe manifest name is simply not provided, and the guard also rejects drive-colon names that would escape the tree on Windows.
This commit is contained in:
+10
-24
@@ -80,6 +80,7 @@ def _is_safe_library_name(name: object) -> bool:
|
||||
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 (".", "..")
|
||||
)
|
||||
|
||||
@@ -294,8 +295,6 @@ 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
|
||||
@@ -347,10 +346,8 @@ def resolve_libraries(
|
||||
)
|
||||
continue
|
||||
if not bundled_dir.is_dir():
|
||||
# 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))
|
||||
# The shared walk's post-emit reconciliation reports drops
|
||||
# (it alone knows the final resolution set); nothing to add
|
||||
continue
|
||||
try:
|
||||
check_library_data(dep, pio_platform, "arduino")
|
||||
@@ -394,8 +391,13 @@ def resolve_libraries(
|
||||
cache_key=cache_key,
|
||||
# The graph walk must not resolve a bundled name from the
|
||||
# registry ({"Wire": "*"} in a manifest); the bundled copy
|
||||
# is added by _add_bundled_dependencies after emit
|
||||
provides=lambda name: (framework_path / "libraries" / name).is_dir(),
|
||||
# is added by _add_bundled_dependencies after emit. Unsafe
|
||||
# names are simply not provided (the malformed-entry warning
|
||||
# names them).
|
||||
provides=lambda name: (
|
||||
_is_safe_library_name(name)
|
||||
and (framework_path / "libraries" / name).is_dir()
|
||||
),
|
||||
),
|
||||
)
|
||||
if len(resolved) < len(external):
|
||||
@@ -421,20 +423,4 @@ 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.
|
||||
# Split once from the left: strip the sanitized owner prefix only, so a
|
||||
# library whose own name carries "__" still matches
|
||||
resolved_short_names = {c.name.split("__", 1)[-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
|
||||
|
||||
@@ -907,6 +907,8 @@ def convert_libraries(
|
||||
components: dict[str, ConvertedLibrary] = {}
|
||||
resolved_requirements: dict[str, frozenset[str]] = {}
|
||||
top_level_keys = set(top_level)
|
||||
# (name, requester) pairs reconciled against the final resolution set
|
||||
skipped_versionless: list[tuple[Any, str]] = []
|
||||
worklist = deque(dict.fromkeys(top_level))
|
||||
while worklist:
|
||||
key = worklist.popleft()
|
||||
@@ -991,40 +993,16 @@ def convert_libraries(
|
||||
component.data.get("dependencies"), component.name
|
||||
):
|
||||
if "name" not in dependency or "version" not in dependency:
|
||||
dep_name = dependency.get("name")
|
||||
if (
|
||||
isinstance(dep_name, str)
|
||||
# _node_key raises for a malformed URL-ish name; that
|
||||
# entry belongs to the warning below, not a traceback
|
||||
and "://" not in dep_name
|
||||
and _node_key(dep_name, None, None)[0] in top_level_keys
|
||||
):
|
||||
# Already requested top-level: present in the build, not
|
||||
# a drop (a false warning teaches users to ignore the
|
||||
# real one)
|
||||
_LOGGER.debug(
|
||||
"Version-less dependency %r of %s is requested top-level",
|
||||
dep_name,
|
||||
component.name,
|
||||
)
|
||||
elif backend.provides is None:
|
||||
# No backend tree can supply it and the registry cannot
|
||||
# resolve it: a real drop, not a routine skip
|
||||
_LOGGER.warning(
|
||||
"Dependency %r of %s has no version to resolve; skipping",
|
||||
dep_name,
|
||||
component.name,
|
||||
)
|
||||
else:
|
||||
# A provides backend owns the post-emit reporting (the
|
||||
# arduino backend defers drops and suppresses names the
|
||||
# walk resolved, which this layer cannot know yet), so
|
||||
# warning here would duplicate or false-positive
|
||||
_LOGGER.debug(
|
||||
"Skip version-less dependency %r of %s",
|
||||
dep_name,
|
||||
component.name,
|
||||
)
|
||||
# Version-less deps cannot resolve from the registry.
|
||||
# Deferred: only the final resolution set can tell a real
|
||||
# drop from a name another manifest resolves later, so the
|
||||
# reconciliation after emit owns the warning
|
||||
_LOGGER.debug(
|
||||
"Skip version-less dependency %r of %s",
|
||||
dependency.get("name"),
|
||||
component.name,
|
||||
)
|
||||
skipped_versionless.append((dependency.get("name"), component.name))
|
||||
continue
|
||||
try:
|
||||
check_library_data(dependency, backend.platform, backend.framework)
|
||||
@@ -1129,4 +1107,28 @@ def convert_libraries(
|
||||
for component in components.values():
|
||||
backend.emit(component)
|
||||
|
||||
# A version-less dependency is satisfied when its request key resolved,
|
||||
# a resolved component's manifest name matches, or the backend provides
|
||||
# it from its own tree (e.g. the arduino bundled libraries, added by the
|
||||
# backend after emit). Anything else is a real drop that would otherwise
|
||||
# surface 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, requester in skipped_versionless:
|
||||
if not isinstance(dep_name, str) or not dep_name or dep_name in warned:
|
||||
continue
|
||||
if "://" not in dep_name and _node_key(dep_name, None, None)[0] in components:
|
||||
continue
|
||||
if dep_name in resolved_manifest_names:
|
||||
continue
|
||||
if backend.provides is not None and backend.provides(dep_name):
|
||||
continue
|
||||
warned.add(dep_name)
|
||||
_LOGGER.warning(
|
||||
"Dependency %s of %s has no version to resolve and nothing "
|
||||
"provides it; skipping",
|
||||
dep_name,
|
||||
requester,
|
||||
)
|
||||
|
||||
return [components[key] for key in top_level if key in components]
|
||||
|
||||
@@ -620,7 +620,10 @@ def test_versionless_dependency_without_provider_warns(
|
||||
{"esphome/A": {"name": "A", "dependencies": [{"name": "Hash"}]}},
|
||||
)
|
||||
convert_libraries([Library("esphome/A", None, None)], _backend())
|
||||
assert "'Hash' of esphome/A has no version to resolve" in caplog.text
|
||||
assert (
|
||||
"Hash of esphome/A has no version to resolve and nothing provides it"
|
||||
in caplog.text
|
||||
)
|
||||
|
||||
|
||||
def test_walk_warns_for_nonplatform_invalid_library(
|
||||
@@ -678,4 +681,31 @@ def test_versionless_url_ish_dependency_name_warns_cleanly(
|
||||
{"esphome/A": {"name": "A", "dependencies": [{"name": "file://"}]}},
|
||||
)
|
||||
convert_libraries([Library("esphome/A", None, None)], _backend())
|
||||
assert "'file://' of esphome/A has no version to resolve" in caplog.text
|
||||
assert (
|
||||
"file:// of esphome/A has no version to resolve and nothing provides it"
|
||||
in caplog.text
|
||||
)
|
||||
|
||||
|
||||
def test_versionless_dependency_matching_resolved_manifest_name_stays_quiet(
|
||||
tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A bare dependency name satisfied by a component requested under an
|
||||
owner-qualified spec (manifest names match) is not a drop; a nameless
|
||||
entry is skipped without a reconciliation warning."""
|
||||
_patch_download_with_manifests(
|
||||
monkeypatch,
|
||||
tmp_path,
|
||||
{
|
||||
"esphome/A": {
|
||||
"name": "A",
|
||||
"dependencies": [{"name": "B"}, {"version": "1.0"}],
|
||||
},
|
||||
"esphome/B": {"name": "B"},
|
||||
},
|
||||
)
|
||||
convert_libraries(
|
||||
[Library("esphome/A", None, None), Library("esphome/B", None, None)],
|
||||
_backend(),
|
||||
)
|
||||
assert "has no version to resolve" not in caplog.text
|
||||
|
||||
Reference in New Issue
Block a user