Clean up the archive on any ar failure; state the link-order contract plainly

esphome/arduino/library.py

@@ -326,9 +326,9 @@ def resolve_libraries(
     for that core (e.g. ``espressif8266``/``esp8266``); ``cache_key`` keys the
     shared converter's download cache.
 
-    The returned order is unordered with respect to link dependencies
-    (bundled dependencies precede their dependents); the caller must link
-    the archives inside one ``--start-group``/``--end-group`` pair.
+    The returned list is not topologically sorted, so the caller must link
+    the archives inside one ``--start-group``/``--end-group`` pair (the
+    bundled-first grouping is incidental).
     """
     bundled: list[ArduinoLibrary] = []
     external: list[Library] = []

esphome/build_gen/build_tool.py

@@ -47,21 +47,27 @@ def _run_ar(ar: str, archive: str, rspfile: str) -> int:
     # 32767-char command-line limit it existed to avoid. "rc" creates,
     # "q" appends the remainder.
     op = "rc"
-    while objects:
-        batch = [objects.pop(0)]
-        batch_len = len(batch[0])
-        while objects and batch_len + len(objects[0]) < 25000:
-            batch_len += len(objects[0]) + 1
-            batch.append(objects.pop(0))
-        rc = subprocess.run(
-            [ar, op, archive, *batch], check=False, close_fds=False
-        ).returncode
-        if rc != 0:
-            # A failed batch must not leave a truncated archive behind
+    ok = False
+    try:
+        while objects:
+            batch = [objects.pop(0)]
+            batch_len = len(batch[0])
+            while objects and batch_len + len(objects[0]) < 25000:
+                batch_len += len(objects[0]) + 1
+                batch.append(objects.pop(0))
+            rc = subprocess.run(
+                [ar, op, archive, *batch], check=False, close_fds=False
+            ).returncode
+            if rc != 0:
+                return rc
+            op = "q"
+        ok = True
+        return 0
+    finally:
+        if not ok:
+            # Any failure (bad exit, missing ar binary, interrupt) must not
+            # leave a truncated archive behind
             Path(archive).unlink(missing_ok=True)
-            return rc
-        op = "q"
-    return 0
 
 
 def _run_copy(src: str, dst: str) -> int:

tests/unit_tests/build_gen/test_build_tool.py

@@ -179,6 +179,32 @@ def test_ar_batch_failure_stops(tmp_path: Path) -> None:
     assert not archive.exists()
 
 
+def test_ar_exception_leaves_no_partial_archive(tmp_path: Path) -> None:
+    """A missing ar binary mid-loop must not leave a truncated archive from
+    earlier successful batches."""
+    archive = tmp_path / "lib.a"
+    rsp = tmp_path / "lib.a.rsp"
+    rsp.write_text("a.o\n")
+    with (
+        patch.object(
+            build_tool.sys,
+            "argv",
+            ["build_tool", "ar", "ar-bin", str(archive), str(rsp)],
+        ),
+        patch.object(
+            build_tool.subprocess,
+            "run",
+            side_effect=lambda cmd, **kw: (
+                archive.write_text("partial"),
+                (_ for _ in ()).throw(FileNotFoundError("no ar")),
+            ),
+        ),
+        pytest.raises(FileNotFoundError),
+    ):
+        build_tool.main()
+    assert not archive.exists()
+
+
 def test_surplus_arguments_error(capsys: pytest.CaptureFixture[str]) -> None:
     """A mis-specified ninja rule passing extra operands errors instead of
     silently dropping them."""