From 77704c09095b867f3c0a4effd59a03da5e7f05ed Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 15:57:30 -0500 Subject: [PATCH] Refuse unsupported manifest features by name; drop the dead warn branch A bundled manifest relying on an extraScript, or any manifest declaring precompiled/ldflags, now raises naming the library instead of warning into a link failure far from the cause. The backend-side usability re-check logs all InvalidLibrary causes at debug with an accurate comment: the walk runs dependency_is_usable on every versioned entry before the provides() skip, so the fault always warns there first (a real-converter test pins the once-only warning). build_tool handles a missing subcommand with its named error instead of an IndexError. --- esphome/arduino/library.py | 40 +++++---------- esphome/build_gen/build_tool.py | 2 +- tests/unit_tests/test_arduino_library.py | 65 ++++++++++++------------ 3 files changed, 47 insertions(+), 60 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index 2c849a2fb0..e8c1f4c3f5 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -27,7 +27,6 @@ from esphome.platformio.library import ( LIBRARY_HEADER_SUFFIXES, SRC_FILE_EXTENSIONS, ConvertedLibrary, - IncompatiblePlatform, InvalidLibrary, LibraryBackend, _url_or_none, @@ -104,11 +103,10 @@ def _warn_dropped_link_fields(name: str, data: dict) -> None: 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, + # would fail at link with no stated cause + raise EsphomeError( + f"Library {name} declares {dropped_key}, which this backend " + "does not support" ) @@ -278,12 +276,11 @@ def _bundled_library(framework_path: Path, name: str) -> ArduinoLibrary: 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 - # manifest relying on one would build with missing flags - _LOGGER.warning( - "Bundled library %s declares an extraScript, which is not " - "run for bundled libraries", - name, + # apply_extra_script only runs on the converted path; building + # without the script's flags would miscompile + raise EsphomeError( + f"Bundled library {name} declares an extraScript, which is " + "not run for bundled libraries" ) lib = _library_info(name, lib_dir, data) _assert_tree_has_code( @@ -430,23 +427,12 @@ def resolve_libraries( # via the converter, and the walk reports any real drops continue try: - # framework=None: the walk already warned for a frameworks - # mismatch; re-checking would warn twice + # framework=None: the walk already ran dependency_is_usable + # on this entry (and warned for any non-platform cause); + # re-checking with a framework would warn twice check_library_data(dep, pio_platform, None) except InvalidLibrary as err: - if isinstance(err, IncompatiblePlatform) or "version" not in dep: - # The platform skip is routine; the walk's version-less - # filter already warned for other version-less causes - _LOGGER.debug("Skip bundled candidate %s: %s", name, err) - else: - # Versioned deps skip the walk's filter via provides(); - # this is the only place the fault can be seen - _LOGGER.warning( - "Skipping bundled dependency %s of %s: %s", - name, - component.name, - err, - ) + _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/esphome/build_gen/build_tool.py b/esphome/build_gen/build_tool.py index f77cd9d497..dbf988c1c7 100644 --- a/esphome/build_gen/build_tool.py +++ b/esphome/build_gen/build_tool.py @@ -80,7 +80,7 @@ _MODES = {"ar": (_run_ar, 3), "copy": (_run_copy, 2)} def main() -> int: - mode = sys.argv[1] + mode = sys.argv[1] if len(sys.argv) > 1 else "" if entry := _MODES.get(mode): handler, argc = entry args = sys.argv[2:] diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index 75204e6816..c99a62816f 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -490,27 +490,33 @@ def test_url_pinned_bundled_name_not_doubled(tmp_path: Path) -> None: assert "Wire" not in [lib.name for lib in libs] -def test_versioned_bundled_candidate_fault_warns( - tmp_path: Path, caplog: pytest.LogCaptureFixture +def test_versioned_bundled_candidate_fault_warns_once( + tmp_path: Path, + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, ) -> None: - """A versioned bundled-name dependency skips the walk's usability filter - via provides(), so a non-platform fault warns here.""" + """A versioned bundled-name dependency with a manifest fault warns once, + from the walk's usability filter; the backend-side re-check stays quiet.""" framework = _make_framework(tmp_path) - converted = _webserver( - tmp_path, - {"build": {}, "dependencies": [{"name": "Wire", "version": "*"}]}, - ) - with ( - _emitting_converter(converted), - patch.object( - component, - "check_library_data", - side_effect=InvalidLibrary("manifest is corrupt"), - ), + _local_lib(tmp_path, [{"name": "Wire", "version": "*"}]) + 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"), ): libs = _resolve(framework) assert "Wire" not in [lib.name for lib in libs] - assert "Skipping bundled dependency Wire" in caplog.text + assert caplog.text.count("manifest is corrupt") == 1 def test_short_name_collision_with_bundled_name_warns( @@ -589,18 +595,15 @@ 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.""" +def test_library_info_unsupported_link_fields_raise(tmp_path: Path) -> None: + """precompiled/ldflags properties are not supported; refuse by name.""" read_path = tmp_path / "lib" (read_path / "src").mkdir(parents=True) (read_path / "src" / "stub.cpp").write_text("") - 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 + with pytest.raises(EsphomeError, match="declares precompiled"): + component._library_info("x", read_path, {"precompiled": "true", "build": {}}) + with pytest.raises(EsphomeError, match="declares ldflags"): + component._library_info("x", read_path, {"ldflags": "-lfoo", "build": {}}) def test_library_info_unmapped_sources_warn( @@ -779,19 +782,17 @@ def test_bundled_library_properties_depends_warns( assert "Library Wire declares dependencies via library.properties" in caplog.text -def test_bundled_library_extra_script_warns( - tmp_path: Path, caplog: pytest.LogCaptureFixture -) -> None: - """A bundled manifest relying on an extraScript is a named deviation, - not a silently miscompiled library.""" +def test_bundled_library_extra_script_raises(tmp_path: Path) -> None: + """A bundled manifest relying on an extraScript would miscompile; + refuse by name.""" framework = _make_framework(tmp_path) wire = framework / "libraries" / "Wire" (wire / "library.json").write_text( '{"name": "Wire", "build": {"extraScript": "extra.py"}}' ) _add_library("Wire", None) - _resolve(framework) - assert "declares an extraScript" in caplog.text + with pytest.raises(EsphomeError, match="Wire declares an extraScript"): + _resolve(framework) def test_dependency_requested_top_level_is_not_a_drop(