Tighten the review-round edges

Only IncompatiblePlatform counts as a knowing skip; other malformed
manifest data stays out of the satisfied set so the reconciliation can
still flag a broken promise. Backend-provided names are recorded before
the manifest-name evidence so the overlap warns once, in the backend's
own suppression loop. ar runs rcs/qs so the symbol index is explicit,
and the copy shim prints and returns 1 like its siblings instead of
re-raising.
This commit is contained in:
J. Nick Koston
2026-08-25 23:42:13 -05:00
parent b957fb0712
commit 21fd74b201
4 changed files with 40 additions and 32 deletions
+7 -1
View File
@@ -28,6 +28,7 @@ from esphome.platformio.library import (
LIBRARY_HEADER_SUFFIXES,
SRC_FILE_EXTENSIONS,
ConvertedLibrary,
IncompatiblePlatform,
InvalidLibrary,
LibraryBackend,
_url_or_none,
@@ -457,11 +458,16 @@ def resolve_libraries(
# causes; debug keeps one fault from warning twice (pinned
# by test_nonplatform_rejection_warns_once_through_real_converter)
check_library_data(dep, pio_platform, None)
except InvalidLibrary as err:
except IncompatiblePlatform as err:
# A knowing skip (platform filter), not a broken promise
knowingly_skipped.add(name)
_LOGGER.debug("Skip bundled candidate %s: %s", name, err)
continue
except InvalidLibrary as err:
# Malformed manifest data never counts as satisfied; the
# walk owns the warning (see the warns-once test above)
_LOGGER.debug("Skip malformed bundled candidate %s: %s", name, err)
continue
# Deferred: a later manifest name may satisfy this
pending_bundled.setdefault(name)
+6 -5
View File
@@ -44,9 +44,9 @@ def _run_ar(ar: str, archive: str, rspfile: str) -> int:
print(f"ar: no objects listed in {rspfile} for {archive}", file=sys.stderr)
return 1
# Batch by argv length: expanding the rspfile gives back the Windows
# 32767-char command-line limit it existed to avoid. "rc" creates,
# "q" appends the remainder.
op = "rc"
# 32767-char command-line limit it existed to avoid. "rcs" creates,
# "qs" appends; the s keeps the symbol index explicit on every ar.
op = "rcs"
ok = False
try:
while objects:
@@ -60,7 +60,7 @@ def _run_ar(ar: str, archive: str, rspfile: str) -> int:
).returncode
if rc != 0:
return rc
op = "q"
op = "qs"
ok = True
return 0
finally:
@@ -78,7 +78,8 @@ def _run_copy(src: str, dst: str) -> int:
# SameFileError means dst IS src, where unlinking destroys the input
if not isinstance(err, shutil.SameFileError):
Path(dst).unlink(missing_ok=True)
raise
print(f"copy: {src} -> {dst} failed: {err}", file=sys.stderr)
return 1
return 0
+11 -9
View File
@@ -943,6 +943,17 @@ def _reconcile_versionless_skips(
if dep_name in components:
# A version-less dep's request key is the name itself
continue
if (
not dep_owner
and backend.provides is not None
and backend.provides(dep_name)
):
# provides() only satisfies owner-less names (same guard as
# the walk's skip); record for the post-emit reconciliation.
# Checked before the manifest-name evidence so the overlap
# case warns once, in the backend's own suppression loop
backend.provided_requests.add(dep_name)
continue
if dep_name in resolved_manifest_names:
# Name-only evidence: a coincidental collision must stay
# visible where the user could pin it
@@ -954,15 +965,6 @@ def _reconcile_versionless_skips(
requester,
)
continue
if (
not dep_owner
and backend.provides is not None
and backend.provides(dep_name)
):
# provides() only satisfies owner-less names (same guard as
# the walk's skip); record for the post-emit reconciliation
backend.provided_requests.add(dep_name)
continue
warned.add(dep_name)
log(
"Dependency %s of %s has no version to resolve and nothing "
+16 -17
View File
@@ -3,7 +3,6 @@
from __future__ import annotations
from pathlib import Path
import shutil
import subprocess
import sys
from unittest.mock import MagicMock, patch
@@ -31,7 +30,7 @@ def test_ar_removes_stale_archive(tmp_path: Path) -> None:
assert build_tool.main() == 0
assert not archive.exists()
# The rspfile is expanded by the shim (GNU ar would escape backslashes)
assert mock_run.call_args[0][0] == ["ar-bin", "rc", str(archive), "a.o"]
assert mock_run.call_args[0][0] == ["ar-bin", "rcs", str(archive), "a.o"]
def test_copy(tmp_path: Path) -> None:
@@ -83,7 +82,7 @@ def test_ar_expands_rspfile_without_escaping(tmp_path) -> None:
assert build_tool.main() == 0
assert mock_run.call_args[0][0] == [
"ar-bin",
"rc",
"rcs",
str(tmp_path / "lib.a"),
"obj/a.o",
"sub\\b.o",
@@ -106,7 +105,7 @@ def test_ar_unquotes_ninja_escaped_paths(tmp_path: Path) -> None:
assert rc == 0
assert mock_run.call_args.args[0] == [
"/usr/bin/ar",
"rc",
"rcs",
"lib.a",
"obj/a b.o",
"obj/c.o",
@@ -129,7 +128,7 @@ def test_ar_empty_object_list_fails(
def test_ar_batches_long_object_lists(tmp_path: Path) -> None:
"""The expanded argv must stay under the Windows 32767-char limit: a
long object list creates with rc, then appends with q."""
long object list creates with rcs, then appends with qs."""
archive = tmp_path / "lib.a"
rsp = tmp_path / "lib.a.rsp"
objects = [f"dir/{'x' * 120}_{i}.o" for i in range(400)]
@@ -147,8 +146,8 @@ def test_ar_batches_long_object_lists(tmp_path: Path) -> None:
assert build_tool.main() == 0
calls = [c[0][0] for c in mock_run.call_args_list]
assert len(calls) > 1
assert calls[0][1] == "rc"
assert all(c[1] == "q" for c in calls[1:])
assert calls[0][1] == "rcs"
assert all(c[1] == "qs" for c in calls[1:])
assert [o for c in calls for o in c[3:]] == objects
assert all(sum(len(a) + 1 for a in c) < 32000 for c in calls)
@@ -215,18 +214,19 @@ def test_surplus_arguments_error(capsys: pytest.CaptureFixture[str]) -> None:
assert "expected 2 arguments, got 3" in capsys.readouterr().err
def test_copy_same_file_keeps_the_input(tmp_path: Path) -> None:
"""A same-file copy (dst IS src) must not unlink the input."""
def test_copy_same_file_keeps_the_input(
tmp_path: Path, capsys: pytest.CaptureFixture[str]
) -> None:
"""A same-file copy (dst IS src) must not unlink the input, and fails
with a message and exit code like the other shim paths."""
src = tmp_path / "firmware.bin"
src.write_bytes(b"image")
with (
patch.object(
build_tool.sys, "argv", ["build_tool", "copy", str(src), str(src)]
),
pytest.raises(shutil.SameFileError),
with patch.object(
build_tool.sys, "argv", ["build_tool", "copy", str(src), str(src)]
):
build_tool.main()
assert build_tool.main() == 1
assert src.read_bytes() == b"image"
assert "failed" in capsys.readouterr().err
def test_copy_failure_leaves_no_partial_output(tmp_path: Path) -> None:
@@ -241,7 +241,6 @@ def test_copy_failure_leaves_no_partial_output(tmp_path: Path) -> None:
"argv",
["build_tool", "copy", str(tmp_path / "src.bin"), str(dst)],
),
pytest.raises(OSError),
):
build_tool.main()
assert build_tool.main() == 1
assert not dst.exists()