diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index 5ed263eee6..456a8ff0d1 100644 --- a/esphome/arduino/library.py +++ b/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] = [] diff --git a/esphome/build_gen/build_tool.py b/esphome/build_gen/build_tool.py index d2497acce6..eada061c14 100644 --- a/esphome/build_gen/build_tool.py +++ b/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: diff --git a/tests/unit_tests/build_gen/test_build_tool.py b/tests/unit_tests/build_gen/test_build_tool.py index 05bc2483de..ba47d0a9b7 100644 --- a/tests/unit_tests/build_gen/test_build_tool.py +++ b/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."""