From e18852e084290ac7439366922e8e84cde6d737fb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 21 Aug 2026 17:19:01 -0500 Subject: [PATCH] Fail on dropped requests, guard the last manifest fields, parse dot_a_linkage strictly A top-level library the converter dropped always makes the firmware wrong, so resolve_libraries raises naming the missing requests instead of warning toward link errors, and a resolved component without its node_key fails as a converter bug rather than silently substituting the mismatched canonical name. includeDir and srcFilter join srcDir and flags in raising a named error on malformed values, and dot_a_linkage parses through the same strict table as libArchive so a typo warns and keeps the archive default instead of flipping link semantics. --- esphome/arduino/library.py | 70 ++++++++------- tests/unit_tests/test_arduino_library.py | 103 +++++++++++++++++++---- 2 files changed, 127 insertions(+), 46 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index 5d2af680d1..7e79ac6b73 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -94,29 +94,33 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: 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)) + if not all(isinstance(entry, str) for entry in src_filter): + raise EsphomeError(f"Library {name} has a malformed srcFilter") # PlatformIO shell-lexes each build.flags entry flag_tokens = lex_build_flags(build.get("flags", []), f"library {name}") # build.libArchive is PIO behavior; dot_a_linkage is honored as a # deliberate extra (Arduino IDE's property, which PIO ignores) so - # properties-only libraries can opt out of archiving too + # properties-only libraries can opt out of archiving too. Both parse + # through the same strict table: bool("false") is True, and a typo'd + # value must not silently change link semantics. + def _parse_archive(key: str, raw: object) -> bool: + if isinstance(raw, bool): + return raw + if str(raw).strip().lower() in ("true", "false"): + return str(raw).strip().lower() == "true" + _LOGGER.warning( + "Library %s has an unrecognized %s value %r; assuming true", + name, + key, + raw, + ) + return True + if "libArchive" in build: - 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 + lib_archive = _parse_archive("libArchive", build["libArchive"]) elif "dot_a_linkage" in data: - lib_archive = str(data["dot_a_linkage"]).lower() == "true" + lib_archive = _parse_archive("dot_a_linkage", data["dot_a_linkage"]) else: lib_archive = True lib = ArduinoLibrary(name=name, lib_archive=lib_archive) @@ -143,6 +147,8 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: lib.flags.append(tok) include_dir = build.get("includeDir", DEFAULT_BUILD_INCLUDE_DIR) + if not isinstance(include_dir, str): + raise EsphomeError(f"Library {name} has a malformed includeDir") for d in [include_dir, src_dir, *include_flags]: if (path := (read_path / d)).is_dir(): lib.include_dirs.append(path.resolve()) @@ -320,20 +326,26 @@ 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; name the - # requests that went missing, not just the survivors - # ConvertedLibrary.name is canonical (bare "pngle" resolves to - # "bitbank2__pngle"); diff the request-side node keys instead - resolved_keys = {c.node_key or c.name for c in resolved} - dropped = [ + # A requested library the converter dropped always makes the + # firmware wrong (link errors far from the cause), so fail here + # naming the requests. ConvertedLibrary.name is canonical (bare + # "pngle" resolves to "bitbank2__pngle"); diff the request-side + # node keys instead. A None node_key is a converter construction + # path that forgot to set it: a programming error, never + # silently substituted with the mismatched canonical name. + if unkeyed := sorted(c.name for c in resolved if c.node_key is None): + raise EsphomeError( + "ConvertedLibrary without a node_key (a converter bug): " + + ", ".join(unkeyed) + ) + resolved_keys = {c.node_key for c in resolved} + dropped = sorted( str(lib) for lib in external if request_key(lib) not in resolved_keys - ] - _LOGGER.warning( - "%d of %d requested libraries were not resolved (missing: %s)", - len(external) - len(resolved), - len(external), - ", ".join(sorted(dropped)) or "unknown", + ) + raise EsphomeError( + f"{len(external) - len(resolved)} of {len(external)} requested " + f"libraries were not resolved (missing: " + f"{', '.join(dropped) or 'unknown'})" ) return bundled + converted diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index ebc03da497..9e53cd443a 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -163,7 +163,10 @@ def test_resolve_libraries_bare_registry_name_is_external(tmp_path: Path) -> Non latest version, matching PlatformIO and the documented libraries: key.""" framework = _make_framework(tmp_path) _add_library("pngle", None) - with patch.object(component, "convert_libraries", return_value=[]) as mock_convert: + with ( + patch.object(component, "convert_libraries", return_value=[]) as mock_convert, + pytest.raises(EsphomeError, match="not resolved"), + ): component.resolve_libraries( framework, pio_platform="espressif8266", @@ -254,7 +257,10 @@ def test_resolve_libraries_versioned_bare_name_is_external(tmp_path: Path) -> No framework = _make_framework(tmp_path) _add_library("pngle", "1.1.0") - with patch.object(component, "convert_libraries", return_value=[]) as mock_convert: + with ( + patch.object(component, "convert_libraries", return_value=[]) as mock_convert, + pytest.raises(EsphomeError, match="not resolved"), + ): component.resolve_libraries( framework, pio_platform="espressif8266", @@ -411,22 +417,23 @@ def test_resolve_libraries_dep_warnings( assert "owner but no version" in caplog.text -def test_resolve_libraries_warns_when_converter_drops_a_request( - tmp_path: Path, caplog: pytest.LogCaptureFixture +def test_resolve_libraries_raises_when_converter_drops_a_request( + tmp_path: Path, ) -> None: - """A requested external library the converter drops is named, not lost.""" + """A dropped top-level request always makes the firmware wrong; fail by + name instead of warning toward link errors far from the cause.""" framework = _make_framework(tmp_path) _add_library("pngle", "1.0.0") - with patch.object(component, "convert_libraries", return_value=[]): + with ( + patch.object(component, "convert_libraries", return_value=[]), + pytest.raises(EsphomeError, match="1 of 1 requested .*missing: pngle"), + ): component.resolve_libraries( framework, pio_platform="espressif8266", board_mcu="esp8266", 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( @@ -563,9 +570,7 @@ def test_library_info_malformed_manifest_is_named(tmp_path: Path, data: object) component._library_info("x", read_path, data) -def test_drop_warning_maps_requests_by_node_key( - tmp_path: Path, caplog: pytest.LogCaptureFixture -) -> None: +def test_drop_error_maps_requests_by_node_key(tmp_path: Path) -> None: """A bare request resolving to a canonical name is not falsely reported missing; the genuinely dropped request is the one named.""" framework = _make_framework(tmp_path) @@ -575,17 +580,40 @@ def test_drop_warning_maps_requests_by_node_key( (lib_dir / "src").mkdir(parents=True) resolved = _converted("bitbank2__pngle", lib_dir, {"build": {}}) resolved.node_key = "pngle" - with patch.object(component, "convert_libraries", return_value=[resolved]): + with ( + patch.object(component, "convert_libraries", return_value=[resolved]), + pytest.raises(EsphomeError) as excinfo, + ): + component.resolve_libraries( + framework, + pio_platform="espressif8266", + board_mcu="esp8266", + cache_key="arduino8266", + ) + missing = str(excinfo.value).split("missing:")[-1] + assert "gone/missing" in missing + assert "pngle@" not in missing + + +def test_drop_error_without_node_key_is_a_converter_bug(tmp_path: Path) -> None: + """A resolved component missing its node_key must fail as a programming + error, never silently substitute the mismatched canonical name.""" + framework = _make_framework(tmp_path) + _add_library("pngle", None) + _add_library("gone/missing", "1.0.0") + lib_dir = tmp_path / "converted" / "pngle" + (lib_dir / "src").mkdir(parents=True) + resolved = _converted("bitbank2__pngle", lib_dir, {"build": {}}) + with ( + patch.object(component, "convert_libraries", return_value=[resolved]), + pytest.raises(EsphomeError, match="node_key .*bitbank2__pngle"), + ): component.resolve_libraries( framework, pio_platform="espressif8266", board_mcu="esp8266", cache_key="arduino8266", ) - assert "1 of 2 requested libraries were not resolved" in caplog.text - assert "missing" in caplog.text - assert "pngle" not in caplog.text.split("missing:")[-1] - assert "gone/missing" in caplog.text.split("missing:")[-1] def test_bundled_library_with_declared_dependencies_warns( @@ -606,3 +634,44 @@ def test_bundled_library_with_declared_dependencies_warns( cache_key="arduino8266", ) assert "Bundled library Wire declares dependencies" in caplog.text + + +@pytest.mark.parametrize( + ("build", "match"), + [ + ({"includeDir": ["a", "b"]}, "malformed includeDir"), + ({"srcFilter": [123]}, "malformed srcFilter"), + ], +) +def test_library_info_malformed_build_fields_are_named( + tmp_path: Path, build: dict, match: str +) -> None: + """Malformed includeDir/srcFilter fail naming the library like srcDir.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + with pytest.raises(EsphomeError, match=match): + component._library_info("x", read_path, {"build": build}) + + +@pytest.mark.parametrize( + ("value", "expected", "warns"), + [ + ("true", True, False), + ("False", False, False), + ("yes", True, True), + ], +) +def test_library_info_dot_a_linkage_parses_strictly( + tmp_path: Path, + value: str, + expected: bool, + warns: bool, + caplog: pytest.LogCaptureFixture, +) -> None: + """The dot_a_linkage property uses the same strict table as libArchive; a typo warns + and keeps the archive default instead of silently flipping linkage.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + lib = component._library_info("x", read_path, {"dot_a_linkage": value, "build": {}}) + assert lib.lib_archive is expected + assert ("unrecognized dot_a_linkage" in caplog.text) is warns