From c206f8352879eadeb3cd3d810b61cc20d1d083a5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 12:08:46 -0500 Subject: [PATCH] Make the remaining silent drops visible across backends The depends=-without-dependencies warning moves into the shared walk so espidf builds report it too; the arduino backend keeps it only for bundled libraries the walk never sees. Version-less dependencies now pass the platform filter before the reconciliation records them, so a platform-excluded entry no longer draws a spurious drop warning. A converted library whose matched files all fall through the source suffix map warns instead of silently producing an empty archive, and unhonored precompiled/ldflags properties fields warn by library name. The ar rspfile parser undoes ninja's POSIX inner-quote escape and the module docstring records the one-object-per-line contract. The includeDir comment now states that warn-and-drop is intended. --- esphome/arduino/library.py | 64 ++++++++++++--------- esphome/build_gen/build_tool.py | 7 ++- esphome/platformio/library.py | 24 +++++++- tests/unit_tests/test_arduino_library.py | 37 ++++++++---- tests/unit_tests/test_platformio_library.py | 34 +++++++++++ 5 files changed, 125 insertions(+), 41 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index db9480e629..e706f66ff0 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -6,7 +6,9 @@ library builds its own archive; all include dirs join one global path. Deviations from PlatformIO: flat-layout libraries get the recursive default source filter; ``dot_a_linkage`` is honored; bundled libraries never run a -manifest ``extraScript``; manifest ``-I`` flags join the global include path. +manifest ``extraScript``; manifest ``-I`` flags join the global include path; +``precompiled``/``ldflags`` properties are not honored (a warning names the +library). """ from __future__ import annotations @@ -36,6 +38,7 @@ from esphome.platformio.library import ( normalize_dependencies, parse_library_json, parse_library_properties, + warn_properties_depends, ) _LOGGER = logging.getLogger(__name__) @@ -72,21 +75,6 @@ def _is_safe_library_name(name: object) -> bool: ) -def _warn_properties_depends(name: str, data: object) -> None: - """Warn when a manifest declares dependencies only as ``depends=``. - - The dependency walk reads the JSON ``dependencies`` key; the raw - ``library.properties`` spelling would otherwise drop silently. - """ - if isinstance(data, dict) and not data.get("dependencies") and data.get("depends"): - _LOGGER.warning( - "Library %s declares dependencies via library.properties " - "depends=, which are not resolved automatically; add them with " - "add_library() if needed", - name, - ) - - def _manifest_build(name: str, data: object) -> dict: """The manifest's ``build`` section; a malformed manifest must fail naming the library, not with an AttributeError.""" @@ -114,6 +102,15 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: else: src_dir = next((d for d in ("src", "Src") if (read_path / d).is_dir()), ".") + for dropped_key in ("precompiled", "ldflags"): + if data.get(dropped_key): + # PIO's Arduino lib builder honors these; building without them + # would fail far away at link with no stated cause + _LOGGER.warning( + "Library %s declares %s, which this backend does not honor", + name, + dropped_key, + ) 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") @@ -170,24 +167,38 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: if (path := (read_path / d)).is_dir(): lib.include_dirs.append(path.resolve()) elif explicit: - # The includeDir/srcDir defaults are probes; an explicitly - # declared path that does not resolve is a manifest error + # Warn-and-drop is intended (unlike srcDir, which raises): a + # missing include dir is harmless until a header is actually + # needed, and the compile names it then _LOGGER.warning( "Library %s declares include dir %s which does not exist", name, d ) + matched = collect_filtered_files(read_path / src_dir, src_filter) lib.sources = sorted( path.resolve() - for f in collect_filtered_files(read_path / src_dir, src_filter) + for f in matched if (path := Path(f)).suffix in SRC_FILE_EXTENSIONS ) - if not lib.sources 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. - _LOGGER.warning( - "Library %s declares srcFilter/srcDir but no source files matched", - name, + if skipped := [f for f in matched if Path(f).suffix not in SRC_FILE_EXTENSIONS]: + _LOGGER.debug( + "Library %s: %d matched files are not sources", name, len(skipped) ) + if not lib.sources: + if matched: + # Every matched file fell through the suffix map: an empty + # archive would fail far away at link + _LOGGER.warning( + "Library %s: no matched file has a recognized source suffix", + name, + ) + elif "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. + _LOGGER.warning( + "Library %s declares srcFilter/srcDir but no source files matched", + name, + ) return lib @@ -213,7 +224,7 @@ def _bundled_library(framework_path: Path, name: str) -> ArduinoLibrary: "resolved automatically; add them with add_library() if needed", name, ) - _warn_properties_depends(name, data) + warn_properties_depends(name, data) build = data.get("build") if isinstance(build, dict) and build.get("extraScript"): # apply_extra_script only runs on the converted path; a bundled @@ -302,7 +313,6 @@ def resolve_libraries( # A version-less bare-name dependency ("Hash" in ESPAsyncWebServer) # is a core-bundled library; the shared converter skips it because # it cannot be resolved from the registry. - _warn_properties_depends(component.name, component.data) for dep in normalize_dependencies( component.data.get("dependencies"), component.name ): diff --git a/esphome/build_gen/build_tool.py b/esphome/build_gen/build_tool.py index 6395f84dd0..5dcadc9671 100644 --- a/esphome/build_gen/build_tool.py +++ b/esphome/build_gen/build_tool.py @@ -6,6 +6,9 @@ started esphome and must not depend on the package being importable. Subcommands: ar remove stale archive, then ``ar rc`` copy copy a file + +The ar rspfile carries one object path per line (the generating rule must +use ``$in_newline``, never ``$in``). """ from pathlib import Path @@ -24,8 +27,10 @@ def main() -> int: # GNU ar treats backslashes in response files as escapes (corrupts # Windows paths), so expand the rspfile into argv, stripping the # simple surrounding quote ninja adds to special paths. + # After stripping the outer pair, undo ninja's POSIX escape for an + # embedded quote ('a'\''b.o' -> a'b.o) objects = [ - line[1:-1] + line[1:-1].replace("'\\''", "'") if len(line) >= 2 and line[0] == line[-1] and line[0] in "'\"" else line for line in Path(rspfile).read_text(encoding="utf-8").splitlines() diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 5fdf46b464..8a21e131a1 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -628,6 +628,21 @@ def join_flag_args(tokens: Iterable[str], owner: str) -> list[str]: return out +def warn_properties_depends(name: str, data: object) -> None: + """Warn when a manifest declares dependencies only as ``depends=``. + + The dependency walk reads the JSON ``dependencies`` key; the raw + ``library.properties`` spelling would otherwise drop silently. + """ + if isinstance(data, dict) and not data.get("dependencies") and data.get("depends"): + _LOGGER.warning( + "Library %s declares dependencies via library.properties " + "depends=, which are not resolved automatically; add them with " + "add_library() if needed", + name, + ) + + def dependency_is_usable( dep: dict, platform: str | None, framework: str, requester: str ) -> bool: @@ -1001,6 +1016,7 @@ def convert_libraries( # A bare json.load imposes no shape; every backend dereferences # data/build, so validate once here and name the library raise EsphomeError(f"Library {key} has a malformed manifest") + warn_properties_depends(component.name, component.data) try: check_library_data(component.data, backend.platform, backend.framework) @@ -1036,7 +1052,13 @@ def convert_libraries( dependency.get("name"), component.name, ) - if not is_lib_ignored(dependency.get("name"), lib_ignore): + if not is_lib_ignored( + dependency.get("name"), lib_ignore + ) and dependency_is_usable( + dependency, backend.platform, backend.framework, component.name + ): + # A platform-filtered or ignored dep is deliberately + # absent, not a drop to reconcile skipped_versionless.append( ( dependency.get("name"), diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index 6c03bc9345..4deb842763 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -461,6 +461,31 @@ def test_library_info_lib_archive_parse( assert lib.lib_archive is expected +def test_library_info_dropped_link_fields_warn( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """precompiled/ldflags properties are not honored; the drop is named.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + component._library_info( + "x", read_path, {"precompiled": "true", "ldflags": "-lfoo", "build": {}} + ) + assert "declares precompiled, which this backend does not honor" in caplog.text + assert "declares ldflags, which this backend does not honor" in caplog.text + + +def test_library_info_unmapped_sources_warn( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """Matched files that all fall through the suffix map are visible; an + empty archive would fail far away at link.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + (read_path / "src" / "impl.CPP").write_text("") + component._library_info("x", read_path, {"build": {}}) + assert "no matched file has a recognized source suffix" in caplog.text + + def test_library_info_lib_archive_malformed_raises(tmp_path: Path) -> None: """A typo'd libArchive fails by name like the other build fields.""" read_path = tmp_path / "lib" @@ -645,18 +670,6 @@ def test_dict_shorthand_dependency_skips_registry_through_real_converter( assert any("locallib" in n.lower() for n in names) -def test_converted_properties_depends_warns( - tmp_path: Path, caplog: pytest.LogCaptureFixture -) -> None: - """A converted library shipping only the properties depends= spelling - is visible, not a silent bundled-dependency drop.""" - framework = _make_framework(tmp_path) - converted = _webserver(tmp_path, {"build": {}, "depends": "Wire,SPI"}) - with _emitting_converter(converted): - _resolve(framework) - assert "declares dependencies via library.properties" in caplog.text - - @pytest.mark.parametrize( ("bad_name", "message"), [ diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index fff7b0d677..7f497d0344 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -666,6 +666,40 @@ def test_convert_libraries_malformed_manifest_raises( convert_libraries([Library("esphome/A", None, None)], _backend()) +def test_walk_warns_for_properties_only_depends( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A manifest declaring dependencies only as library.properties depends= + warns in the shared walk, so every backend reports the drop.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + {"esphome/A": "name=A\nversion=1.0\ndepends=Wire, SPI\n"}, + properties=("esphome/A",), + ) + convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + assert "declares dependencies via library.properties" in caplog.text + + +def test_versionless_platform_filtered_dependency_stays_quiet( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A version-less dependency the platform filter excludes is + deliberately absent, not a drop to warn about.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + { + "esphome/A": { + "name": "A", + "dependencies": [{"name": "Hash", "platforms": "espressif8266"}], + } + }, + ) + convert_libraries([Library("esphome/A", None, None)], _backend()) + assert "has no version to resolve" not in caplog.text + + def test_versionless_ignored_dependency_stays_quiet( tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture ) -> None: