From a48fbd1cd18734d7e0dd78b261ac072995f7f2c7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 09:41:35 -0500 Subject: [PATCH] Suppress the top-level-requested false positive, guard names once, fail empty bundled libraries The walk's version-less warning stays quiet for a name the config also requests top-level, and the provides-backend debug level carries the ownership rationale (the arduino backend defers drops and suppresses walk-resolved names this layer cannot know yet). _is_safe_library_name guards both the top-level bundled check and manifest dependencies, so a YAML libraries entry of '.' or a backslash name cannot join under the framework tree. The drop suppression strips only the sanitized owner prefix, keeping a library's own double underscores. An empty bundled directory raises naming the fix instead of warning into an unlinkable build, and the second real-converter test pins ESPHOME_DATA_DIR. --- esphome/arduino/library.py | 37 ++++++++++++--------- esphome/platformio/library.py | 25 +++++++++++--- tests/unit_tests/test_arduino_library.py | 23 +++++++------ tests/unit_tests/test_platformio_library.py | 20 +++++++++++ 4 files changed, 73 insertions(+), 32 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index a70938ef22..1cb0d48868 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -73,6 +73,17 @@ class ArduinoLibrary: link_flags: list[str] = field(default_factory=list) +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 name not in (".", "..") + ) + + def _manifest_build(name: str, data: object) -> dict: """The manifest's ``build`` section, validated by name. @@ -227,12 +238,11 @@ def _bundled_library(framework_path: Path, name: str) -> ArduinoLibrary: 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, + # An empty or half-extracted bundled directory can never link; a + # warning would scroll away and resurface as undefined symbols + raise EsphomeError( + f"Bundled library {name} has no sources or headers; the " + "framework install may be incomplete (run 'esphome clean-all')" ) return lib @@ -266,8 +276,7 @@ def resolve_libraries( if ( not library.repository and not library.version - and library.name - and "/" not in library.name + and _is_safe_library_name(library.name) and (framework_path / "libraries" / library.name).is_dir() ): # A bundled library's own manifest dependencies are not walked. @@ -304,13 +313,7 @@ def resolve_libraries( component.data.get("dependencies"), component.name ): name = dep.get("name") - if ( - not name - or not isinstance(name, str) - or "/" in name - or "\\" in name - or name in (".", "..") - ): + if not _is_safe_library_name(name): # The name becomes a path component under the framework # tree; never join a traversal or a non-string _LOGGER.warning( @@ -421,7 +424,9 @@ def resolve_libraries( # 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} + # 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 diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 90c1eb27ae..049d4f3b87 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -991,20 +991,35 @@ def convert_libraries( component.data.get("dependencies"), component.name ): if "name" not in dependency or "version" not in dependency: - if backend.provides is None: + dep_name = dependency.get("name") + if ( + isinstance(dep_name, str) + and _node_key(dep_name, None, None)[0] in nodes + ): + # 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", - dependency.get("name"), + dep_name, component.name, ) else: - # The backend (e.g. the arduino bundled tree) picks - # these up after emit + # 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", - dependency.get("name"), + dep_name, component.name, ) continue diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index 434ccd4cdf..863d8664b4 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -907,8 +907,7 @@ def test_pinned_bundled_dependency_substitution_warns( '{"name": "LocalLib", "version": "1.0.0", "dependencies": {"Wire": "^2.0.0"}}' ) _add_library(local_lib.as_uri(), None) - CORE.config_path = tmp_path / "test.yaml" - CORE.config_path.write_text("") + monkeypatch.setenv("ESPHOME_DATA_DIR", str(tmp_path / ".esphome")) with patch.object( pio_library, "_resolve_registry_version", @@ -955,18 +954,20 @@ def test_transitively_resolved_dependency_does_not_warn( 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.""" + """A bundled directory with no sources or headers is a broken install + that can never link; fail by name instead of warning into it.""" 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 + with pytest.raises( + EsphomeError, match="Bundled library Empty has no sources or headers" + ): + component.resolve_libraries( + framework, + pio_platform="espressif8266", + board_mcu="esp8266", + cache_key="arduino8266", + ) def test_versionless_dependency_with_provider_stays_quiet( diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 38d7942394..8202658675 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -645,3 +645,23 @@ def test_walk_warns_for_nonplatform_invalid_library( 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_versionless_dependency_requested_top_level_stays_quiet( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A version-less dependency the config also requests top-level is in + the build; no drop warning even without a provides backend.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + { + "esphome/A": {"name": "A", "dependencies": [{"name": "Hash"}]}, + "Hash": {"name": "Hash"}, + }, + ) + convert_libraries( + [Library("esphome/A", None, None), Library("Hash", None, None)], + _backend(), + ) + assert "has no version to resolve" not in caplog.text