From efa3018123d45425b4ae3d918a12ec0db73cde0b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 13:10:44 -0500 Subject: [PATCH] Close the four diagnostic gaps in the library backend An owner-qualified dependency name (Owner/Pkg, both halves safe) is the converter's to resolve and no longer draws the malformed-entry warning, which stays for genuinely unsafe shapes. A declared srcFilter matching only inert files now warns like one matching nothing, while matched headers still read as a header-only library. The short-name suppression logs the assumed external at debug so an accidental collision is attributable. A failing ar batch unlinks the truncated archive before returning its exit code. --- esphome/arduino/library.py | 32 ++++++++++---- esphome/build_gen/build_tool.py | 2 + tests/unit_tests/build_gen/test_build_tool.py | 9 +++- tests/unit_tests/test_arduino_library.py | 42 ++++++++++++++++++- 4 files changed, 75 insertions(+), 10 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index a57bc7c5a3..e2e07b4551 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -219,9 +219,13 @@ def _collect_lib_sources( len(dropped), ", ".join(sorted(dropped)), ) - if not lib.sources and not matched and ("srcFilter" in build or "srcDir" in build): - # A default probe finding nothing is a header-only library; a - # declared filter matching nothing is a manifest/tree problem. + if ( + not lib.sources + and ("srcFilter" in build or "srcDir" in build) + and not any(Path(f).suffix.lower() in HEADER_FILE_EXTENSIONS for f in matched) + ): + # A declared filter matching nothing (or only inert files) is a + # manifest/tree problem; matched headers mean a header-only library _LOGGER.warning( "Library %s declares srcFilter/srcDir but no source files matched", name, @@ -361,6 +365,12 @@ def resolve_libraries( component.data.get("dependencies"), component.name ): name = dep.get("name") + if isinstance(name, str) and "/" in name: + owner, _, pkg = name.partition("/") + if _is_safe_library_name(owner) and _is_safe_library_name(pkg): + # PIO's owner-qualified spelling ("Owner/Pkg"); the + # converter resolves it from the registry + continue 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 @@ -370,11 +380,17 @@ def resolve_libraries( component.name, ) continue - if ( - name in bundled_names - or name in external_short_names - or is_lib_ignored(name, lib_ignore) - ): + if name in external_short_names: + # Deliberate when the external really is this library; + # attributable when the short-name match is accidental + _LOGGER.debug( + "Dependency %s of %s assumed satisfied by a requested " + "external library", + name, + component.name, + ) + continue + if name in bundled_names or is_lib_ignored(name, lib_ignore): continue if dep.get("owner") or not _provided(name): # Owner-less names in the framework tree prefer the bundled diff --git a/esphome/build_gen/build_tool.py b/esphome/build_gen/build_tool.py index 804b623d9d..3cbb4fb61f 100644 --- a/esphome/build_gen/build_tool.py +++ b/esphome/build_gen/build_tool.py @@ -57,6 +57,8 @@ def _run_ar(ar: str, archive: str, rspfile: str) -> int: [ar, op, archive, *batch], check=False, close_fds=False ).returncode if rc != 0: + # A failed batch must not leave a truncated archive behind + Path(archive).unlink(missing_ok=True) return rc op = "q" return 0 diff --git a/tests/unit_tests/build_gen/test_build_tool.py b/tests/unit_tests/build_gen/test_build_tool.py index 0e9d069b4a..9a361211ad 100644 --- a/tests/unit_tests/build_gen/test_build_tool.py +++ b/tests/unit_tests/build_gen/test_build_tool.py @@ -164,8 +164,15 @@ def test_ar_batch_failure_stops(tmp_path: Path) -> None: ["build_tool", "ar", "ar-bin", str(archive), str(rsp)], ), patch.object( - build_tool.subprocess, "run", return_value=MagicMock(returncode=3) + build_tool.subprocess, + "run", + side_effect=lambda cmd, **kw: ( + archive.write_text("partial"), + MagicMock(returncode=3), + )[1], ) as mock_run, ): assert build_tool.main() == 3 assert mock_run.call_count == 1 + # The failed batch must not leave a truncated archive behind + assert not archive.exists() diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index ef73c62314..0f9ec88634 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -490,6 +490,29 @@ def test_library_info_unmapped_sources_warn( assert "not compiled: impl.CPP, sketch.ino" in caplog.text +def test_library_info_inert_only_filter_warns( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A declared srcFilter matching only inert files (no sources, no + headers) warns like one matching nothing at all.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + (read_path / "src" / "keywords.txt").write_text("") + component._library_info("x", read_path, {"build": {"srcFilter": ["+<*>"]}}) + assert "no source files matched" in caplog.text + + +def test_library_info_declared_filter_matching_headers_stays_quiet( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A declared filter matching real headers is a header-only library.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + (read_path / "src" / "api.h").write_text("") + component._library_info("x", read_path, {"build": {"srcFilter": ["+<*>"]}}) + assert "no source files matched" not in caplog.text + + def test_library_info_header_only_src_stays_quiet( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: @@ -695,7 +718,6 @@ def test_dict_shorthand_dependency_skips_registry_through_real_converter( # A non-string name never leaves the shared normalizer (1, "Ignoring unrecognized dependency entry"), ("../escape", "Ignoring malformed dependency entry"), - ("a/b", "Ignoring malformed dependency entry"), ("..", "Ignoring malformed dependency entry"), ], ) @@ -713,6 +735,24 @@ def test_bundled_dependency_bad_name_is_malformed( assert message in caplog.text +def test_owner_qualified_dependency_is_silent( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """The owner-qualified dependency spelling (PIO's Owner/Pkg) resolves via + the converter; it must not draw the malformed-entry warning.""" + framework = _make_framework(tmp_path) + converted = _webserver( + tmp_path, + { + "build": {}, + "dependencies": [{"name": "ESP32Async/AsyncTCP", "version": "^3.0"}], + }, + ) + with _emitting_converter(converted): + _resolve(framework) + assert "malformed" not in caplog.text + + def test_bundled_dependency_string_list_form( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: