Fix Python 3.11 incompatibility in sandbox_destroy

shutil.rmtree's error callback was renamed onerror -> onexc in 3.12 and the
two receive different third arguments, so sandbox_destroy raised TypeError on
every 3.11 run despite requires-python = ">=3.11". Route both through one
helper and test it.

Found by running the suite under 3.11 before publishing; the whole suite now
passes on 3.11 and 3.13.
This commit is contained in:
Ryan Malloy 2026-08-17 17:56:11 -06:00
parent 8c619a1674
commit 1103b7649f
2 changed files with 41 additions and 3 deletions

View File

@ -7,6 +7,7 @@ import logging
import os import os
import shutil import shutil
import signal import signal
import sys
from pathlib import Path from pathlib import Path
from fastmcp import Context, FastMCP from fastmcp import Context, FastMCP
@ -42,6 +43,24 @@ async def _await_exit(pid: int | None, seconds: float) -> bool:
return not pid_alive(pid) return not pid_alive(pid)
def rmtree_collecting(target: Path, errors: list[str]) -> None:
"""shutil.rmtree that records failures instead of raising or hiding them.
The callback keyword changed name in 3.12 (onerror -> onexc) and the two
are passed different third arguments, so normalize both.
"""
def record(_func, path, exc):
if isinstance(exc, tuple): # 3.11's onerror passes sys.exc_info()
exc = exc[1]
errors.append(f"{path}: {exc}")
if sys.version_info >= (3, 12):
shutil.rmtree(target, onexc=record)
else:
shutil.rmtree(target, onerror=record)
def _tail_log(path: str | None, lines: int = 3) -> str: def _tail_log(path: str | None, lines: int = 3) -> str:
if not path: if not path:
return "(no log)" return "(no log)"
@ -227,7 +246,7 @@ async def sandbox_destroy(name: str, ctx: Context = None) -> dict:
for target in targets: for target in targets:
if not target.exists(): if not target.exists():
continue # nothing to remove is success, not a failure continue # nothing to remove is success, not a failure
shutil.rmtree(target, onexc=lambda _f, path, exc: errors.append(f"{path}: {exc}")) rmtree_collecting(target, errors)
overlay_gone = not Path(overlay).exists() overlay_gone = not Path(overlay).exists()
log.info("sandbox_destroy: removed %s (overlay_deleted=%s)", name, overlay_gone) log.info("sandbox_destroy: removed %s (overlay_deleted=%s)", name, overlay_gone)
result = { result = {

View File

@ -161,8 +161,9 @@ async def test_sandbox_destroy_reports_cleanup_failure_honestly(dirs, all_pids_d
seeded_record(dirs, "sb", config={"disks": [], "sandbox_overlay": str(overlay)}), seeded_record(dirs, "sb", config={"disks": [], "sandbox_overlay": str(overlay)}),
) )
def boom(target, onexc=None): def boom(target, onexc=None, onerror=None):
onexc(None, str(target), PermissionError("read-only filesystem")) # rmtree's callback keyword differs by Python version.
(onexc or onerror)(None, str(target), PermissionError("read-only filesystem"))
monkeypatch.setattr("mcqemu.tools.sandbox.shutil.rmtree", boom) monkeypatch.setattr("mcqemu.tools.sandbox.shutil.rmtree", boom)
async with Client(mcp) as client: async with Client(mcp) as client:
@ -460,3 +461,21 @@ def test_attached_vm_identity_is_not_judged_by_name(dirs, monkeypatch):
spawned = VMRecord(name="ours", source="spawned", qmp_socket="/tmp/q.sock", pid=1234) spawned = VMRecord(name="ours", source="spawned", qmp_socket="/tmp/q.sock", pid=1234)
assert registry.is_process_alive(spawned) is False assert registry.is_process_alive(spawned) is False
def test_rmtree_helper_works_on_this_python(tmp_path):
"""The rmtree callback keyword changed name in 3.12; support both."""
from mcqemu.tools.sandbox import rmtree_collecting
victim = tmp_path / "tree"
(victim / "sub").mkdir(parents=True)
(victim / "sub" / "f.txt").write_text("x")
errors: list[str] = []
rmtree_collecting(victim, errors)
assert not victim.exists()
assert errors == []
# A path that cannot be removed must be recorded, not raised or swallowed.
missing = tmp_path / "not-there"
rmtree_collecting(missing, errors)
assert len(errors) == 1 and str(missing) in errors[0]