From 0cb307aea573ef72ee3d35dbf23d7bd87f54a911 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 21 Aug 2026 12:53:39 -0500 Subject: [PATCH] Type the platform rejection, prefer bundled deps, parse manifests strictly check_library_data's platform filter raises IncompatiblePlatform (an InvalidLibrary subclass) so the arduino backend branches on the type instead of substring-matching the message. The {"Wire": "*"} dict shorthand resolves to the bundled library like PIO's process_dependencies instead of a registry lookup. libArchive parses booleans and true/false strings and warns on anything else (bool(str) made "false" archive). A declared-but-falsy srcDir raises instead of silently probing. The converter-drop warning names the missing requests, and the bundled-walk comment states what PIO actually does. --- esphome/arduino/library.py | 71 ++++++++++++------ esphome/platformio/library.py | 10 ++- tests/unit_tests/test_arduino_library.py | 96 +++++++++++++++++++++++- 3 files changed, 151 insertions(+), 26 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index dee58b5fed..f699d79704 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -32,6 +32,7 @@ from esphome.platformio.library import ( DEFAULT_BUILD_SRC_FILTER, SRC_FILE_EXTENSIONS, ConvertedLibrary, + IncompatiblePlatform, InvalidLibrary, LibraryBackend, check_library_data, @@ -73,16 +74,19 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: build = data.get("build", {}) # PIO's source-dir resolution: manifest srcDir, else src/Src, else the root - src_dir = build.get("srcDir") or next( - (d for d in ("src", "Src") if (read_path / d).is_dir()), "." - ) - if "srcDir" in build and not (read_path / src_dir).is_dir(): - # Unlike the default probes, an explicitly declared srcDir that does - # not resolve is unambiguously a manifest/tree error; a silently - # empty source set would surface as link errors far from the cause - raise EsphomeError( - f"Library {name} declares srcDir {src_dir} which does not exist" - ) + if "srcDir" in build: + # An explicitly declared srcDir (falsy included) that does not + # resolve is unambiguously a manifest/tree error; a silently empty + # source set would surface as link errors far from the cause + 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()), ".") src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) # PlatformIO shell-lexes each build.flags entry @@ -92,7 +96,20 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: # deliberate extra (Arduino IDE's property, which PIO ignores) so # properties-only libraries can opt out of archiving too if "libArchive" in build: - lib_archive = bool(build["libArchive"]) + raw_archive = build["libArchive"] + if isinstance(raw_archive, bool): + lib_archive = raw_archive + elif str(raw_archive).strip().lower() in ("true", "false"): + lib_archive = str(raw_archive).strip().lower() == "true" + else: + # bool("false") is True; an unparsable value must not silently + # archive a library whose author disabled archiving + _LOGGER.warning( + "Library %s has an unrecognized libArchive value %r; assuming true", + name, + raw_archive, + ) + lib_archive = True elif "dot_a_linkage" in data: lib_archive = str(data["dot_a_linkage"]).lower() == "true" else: @@ -192,9 +209,10 @@ def resolve_libraries( and "/" not in library.name and (framework_path / "libraries" / library.name).is_dir() ): - # A bundled library's own manifest dependencies are deliberately - # not walked (PlatformIO's lib_ldf_mode=off does not either); - # core add_library() calls list what they need explicitly. + # A bundled library's own manifest dependencies are not walked. + # PlatformIO would walk them even under lib_ldf_mode=off, but no + # library bundled with the ESP8266 core declares any, so the walk + # is a no-op there; core add_library() calls list what they need. bundled.append(_bundled_library(framework_path, library.name)) else: external.append(library) @@ -217,11 +235,13 @@ def resolve_libraries( continue if name in bundled_names or is_lib_ignored(name, lib_ignore): continue - if "version" in dep: - # The converter resolves versioned deps from the registry. - # Note: the dict shorthand {"Wire": "*"} normalizes to - # version="*" and takes this path even for a bundled name; - # use the list form for bundled dependencies. + bundled_dir = framework_path / "libraries" / name + if "version" in dep and (dep.get("owner") or not bundled_dir.is_dir()): + # The converter resolves versioned deps from the registry. An + # owner-less versioned name that exists in the framework tree + # ({"Wire": "*"} normalizes to version="*") falls through to + # the bundled path below, matching PlatformIO's + # process_dependencies preference for bundled builders. continue if dep.get("owner"): # Owner but no version: the converter skips it too, so this @@ -233,7 +253,7 @@ def resolve_libraries( component.name, ) continue - if not (framework_path / "libraries" / name).is_dir(): + 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. @@ -251,7 +271,7 @@ def resolve_libraries( # manifest is routine (every ESPAsyncWebServer build hits # it), so the platform filter stays at debug; any other # cause means a dropped dependency and must be visible - if "platform" in str(err).lower(): + if isinstance(err, IncompatiblePlatform): _LOGGER.debug("Skipping bundled dependency %s: %s", name, err) else: _LOGGER.warning( @@ -287,12 +307,15 @@ def resolve_libraries( ) if len(resolved) < len(external): # A requested library the converter dropped would otherwise - # surface only as link errors far from the cause + # surface only as link errors far from the cause; name the + # requests that went missing, not just the survivors + resolved_names = {c.name for c in resolved} + dropped = [str(lib) for lib in external if lib.name not in resolved_names] _LOGGER.warning( - "%d of %d requested libraries were not resolved (resolved: %s)", + "%d of %d requested libraries were not resolved (missing: %s)", len(external) - len(resolved), len(external), - ", ".join(sorted(c.name for c in resolved)) or "none", + ", ".join(sorted(dropped)) or "unknown", ) return bundled + converted diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 7349c46144..34c7a272cc 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -209,6 +209,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. @@ -438,7 +446,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): diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index f0bff7b729..59a1f9fa45 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -291,7 +291,7 @@ def test_library_info_missing_declared_src_dir_raises(tmp_path: Path) -> None: """An explicitly declared srcDir that does not exist is a manifest error.""" read_path = tmp_path / "lib" read_path.mkdir() - with pytest.raises(EsphomeError, match="srcDir nosrc which does not exist"): + with pytest.raises(EsphomeError, match="srcDir 'nosrc' which does not exist"): component._library_info("x", read_path, {"build": {"srcDir": "nosrc"}}) @@ -425,6 +425,8 @@ def test_resolve_libraries_warns_when_converter_drops_a_request( cache_key="arduino8266", ) assert "1 of 1 requested libraries were not resolved" in caplog.text + # The actionable fact is which request went missing, not the survivors + assert "missing: pngle" in caplog.text def test_bundled_dependency_nonplatform_rejection_warns( @@ -458,3 +460,95 @@ def test_bundled_dependency_nonplatform_rejection_warns( ) assert "Skipping bundled dependency Wire" in caplog.text assert "manifest is corrupt" in caplog.text + + +@pytest.mark.parametrize("declared", ["", None]) +def test_library_info_falsy_declared_src_dir_raises( + tmp_path: Path, declared: str | None +) -> None: + """A declared-but-falsy srcDir must not silently fall back to the probe.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + with pytest.raises(EsphomeError, match="does not exist"): + component._library_info("x", read_path, {"build": {"srcDir": declared}}) + + +@pytest.mark.parametrize( + ("value", "expected", "warns"), + [ + (False, False, False), + ("false", False, False), + ("False", False, False), + ("true", True, False), + ("archive-me", True, True), + ], +) +def test_library_info_lib_archive_parse( + tmp_path: Path, + value: object, + expected: bool, + warns: bool, + caplog: pytest.LogCaptureFixture, +) -> None: + """bool("false") is True; the string forms must parse, not coerce.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + lib = component._library_info("x", read_path, {"build": {"libArchive": value}}) + assert lib.lib_archive is expected + assert ("unrecognized libArchive" in caplog.text) is warns + + +def test_bundled_dependency_dict_shorthand_prefers_bundled(tmp_path: Path) -> None: + """The {"Wire": "*"} dict shorthand (version="*", no owner) must resolve + to the bundled library, matching PIO's process_dependencies, instead of + being routed to the registry.""" + framework = _make_framework(tmp_path) + _add_library("ESP32Async/ESPAsyncWebServer", "3.9.6") + lib_dir = tmp_path / "converted" / "webserver" + (lib_dir / "src").mkdir(parents=True) + converted = _converted( + "esp32async__ESPAsyncWebServer", + lib_dir, + {"build": {}, "dependencies": {"Wire": "*"}}, + ) + with _emitting_converter(converted): + libs = component.resolve_libraries( + framework, + pio_platform="espressif8266", + board_mcu="esp8266", + cache_key="arduino8266", + ) + assert "Wire" in [lib.name for lib in libs] + + +def test_bundled_dependency_platform_rejection_is_debug( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """The typed IncompatiblePlatform (the routine cross-platform skip) + stays at debug regardless of message wording.""" + from esphome.platformio.library import IncompatiblePlatform + + framework = _make_framework(tmp_path) + _add_library("ESP32Async/ESPAsyncWebServer", "3.9.6") + lib_dir = tmp_path / "converted" / "webserver" + (lib_dir / "src").mkdir(parents=True) + converted = _converted( + "esp32async__ESPAsyncWebServer", + lib_dir, + {"build": {}, "dependencies": [{"name": "Wire"}]}, + ) + with ( + _emitting_converter(converted), + patch.object( + component, + "check_library_data", + side_effect=IncompatiblePlatform("nothing about the p-word here"), + ), + ): + component.resolve_libraries( + framework, + pio_platform="espressif8266", + board_mcu="esp8266", + cache_key="arduino8266", + ) + assert "Skipping bundled dependency Wire" not in caplog.text