mirror of
https://github.com/esphome/esphome.git
synced 2026-08-22 22:26:21 +00:00
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.
This commit is contained in:
J. Nick Koston
+41
-29
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user