From 47e87d7eb24115def0c141cb0546a37952d5cec3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 13:50:29 -0500 Subject: [PATCH] Dedupe the rejection warning and make bundled-name membership case-exact The backend-side bundled filter now checks compatibility silently (the shared walk owns the warning), so one manifest fault warns once instead of twice. Bundled-name membership uses the exact on-disk directory names instead of a per-name is_dir() probe, so a case-insensitive filesystem cannot match a case-mismatched dependency and build the same bundled library twice. --- esphome/arduino/library.py | 31 ++++++++++----- tests/unit_tests/test_arduino_library.py | 50 +++++++++++++++++++++--- 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index e2e07b4551..625ac865d7 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -14,7 +14,6 @@ library). from __future__ import annotations from dataclasses import dataclass, field -import functools import logging from pathlib import Path import re @@ -28,10 +27,11 @@ from esphome.platformio.library import ( HEADER_FILE_EXTENSIONS, SRC_FILE_EXTENSIONS, ConvertedLibrary, + InvalidLibrary, LibraryBackend, + check_library_data, collect_filtered_files, convert_libraries, - dependency_is_usable, ensure_list, is_lib_ignored, lex_build_flags, @@ -326,14 +326,20 @@ def resolve_libraries( # PlatformIO's lib_ignore covers framework-bundled libraries too; the # shared converter only filters the registry/git ones. lib_ignore = lib_ignore_set() - # Memoized "does the framework bundle this name?"; the safety guard and - # dir probe must stay fused (path traversal) - _provided = functools.cache( - lambda name: ( - _is_safe_library_name(name) - and (framework_path / "libraries" / name).is_dir() - ) + # Exact on-disk directory names, so membership is case-sensitive on + # every filesystem (a per-name is_dir() probe would match "wire" on + # macOS/Windows and build the bundled Wire twice); the safety guard + # stays fused with the lookup (path traversal) + libraries_dir = framework_path / "libraries" + bundled_dir_names = ( + frozenset(p.name for p in libraries_dir.iterdir() if p.is_dir()) + if libraries_dir.is_dir() + else frozenset() ) + + def _provided(name: object) -> bool: + return _is_safe_library_name(name) and name in bundled_dir_names + for library in CORE.platformio_libraries.values(): if is_lib_ignored(library.name, lib_ignore): continue @@ -397,7 +403,12 @@ def resolve_libraries( # copy (PIO's process_dependencies); everything else resolves # via the converter, and the walk reports any real drops continue - if not dependency_is_usable(dep, pio_platform, "arduino", component.name): + try: + check_library_data(dep, pio_platform, "arduino") + except InvalidLibrary as err: + # The shared walk already reported any non-platform cause; + # warning again here would read as two distinct failures + _LOGGER.debug("Skip bundled candidate %s: %s", name, err) continue # Deferred: a later-emitted library's manifest name may satisfy # this; adding now could double the archive diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index 0f9ec88634..349e0cd7f1 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -410,23 +410,63 @@ def test_resolve_libraries_dep_warnings( assert "Orphan" not in caplog.text -def test_bundled_dependency_nonplatform_rejection_warns( +def test_bundled_dependency_nonplatform_rejection_is_silent_here( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: - """An InvalidLibrary whose cause is not the platform filter is visible.""" + """The shared walk owns the rejection warning; the backend-side filter + stays at debug so one manifest fault never warns twice.""" framework = _make_framework(tmp_path) converted = _webserver(tmp_path, {"build": {}, "dependencies": [{"name": "Wire"}]}) with ( _emitting_converter(converted), patch.object( - pio_library, + component, "check_library_data", side_effect=InvalidLibrary("manifest is corrupt"), ), + ): + libs = _resolve(framework) + assert "Wire" not in [lib.name for lib in libs] + assert "manifest is corrupt" not in caplog.text + + +def test_nonplatform_rejection_warns_once_through_real_converter( + tmp_path: Path, + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """One manifest fault produces exactly one warning across the walk and + the backend-side bundled filter.""" + framework = _make_framework(tmp_path) + _local_lib(tmp_path, [{"name": "Wire"}]) + monkeypatch.setenv("ESPHOME_DATA_DIR", str(tmp_path / ".esphome")) + real = pio_library.check_library_data + + def flaky(data, platform, framework_name): + if data.get("name") == "Wire": + raise InvalidLibrary("manifest is corrupt") + return real(data, platform, framework_name) + + monkeypatch.setattr(pio_library, "check_library_data", flaky) + monkeypatch.setattr(component, "check_library_data", flaky) + with patch.object( + pio_library, + "_resolve_registry_version", + side_effect=AssertionError("registry touched"), ): _resolve(framework) - assert "Skipping dependency Wire" in caplog.text - assert "manifest is corrupt" in caplog.text + assert caplog.text.count("manifest is corrupt") == 1 + + +def test_provided_is_case_sensitive(tmp_path: Path) -> None: + """Membership uses the exact on-disk names, so a case-insensitive + filesystem cannot add the same bundled library twice.""" + framework = _make_framework(tmp_path) + converted = _webserver(tmp_path, {"build": {}, "dependencies": [{"name": "wire"}]}) + with _emitting_converter(converted): + libs = _resolve(framework) + assert "wire" not in [lib.name for lib in libs] + assert "Wire" not in [lib.name for lib in libs] @pytest.mark.parametrize("declared", ["", None])