From 333f1ac3cabd8317007f7cd58821789e8d029784 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:04:52 +0100 Subject: [PATCH] fix: resume a cut data archive A data archive cut mid-download was deleted on every failure: the retry and the next run started from zero, while the run said nothing was lost. The archive is now kept beside its tree with the validator its server sent, and the rest is asked for with Range and If-Range. Checked against the release: fbneo-samples stopped at 16 MB, the next run downloaded 34.6 MB of 50.5. --- install.ps1 | 2 +- install.py | 114 ++++++++++++++++++++++++++++++----- install.sh | 2 +- install/launchers/index.json | 16 ++--- tests/test_install.py | 104 ++++++++++++++++++++++++++++++-- wiki/installer.md | 6 +- 6 files changed, 210 insertions(+), 34 deletions(-) diff --git a/install.ps1 b/install.ps1 index 7225221c0..c8d2c3399 100644 --- a/install.ps1 +++ b/install.ps1 @@ -16,7 +16,7 @@ # and Invoke-WebRequest throws on a cut transfer before the hash is read: # the site's build rewrites this line there (scripts/sitebootstrap.py). $mismatchAdvice = "The download was most likely cut short: try again. Within minutes of an update of this project the two files can come from caches of different ages. Waiting five minutes settles that." - $defaultInstallSha256 = "567d810f5c4f5cbb6793d6f0c96a7492d90b53664d74f045aae4ef75d6058a0f" + $defaultInstallSha256 = "fdccf00cb56c4caf3ddf916e126d538539e0d9efa527fb808cde954b6b0d29b1" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 8a4f6cb3c..16c011ea6 100644 --- a/install.py +++ b/install.py @@ -7086,20 +7086,72 @@ def download_files( return failed, interrupted -def _fetch_archive(url: str, directory: Path, progress: Progress | None = None) -> Path: +def _archive_validator(headers: object) -> str: + """What names this revision of an archive to its server: an If-Range + with it gets the rest of the same bytes, or the whole new archive.""" + if not headers: + return "" + etag = headers.get("ETag", "") or "" + # A weak tag never answers a range (RFC 9110 13.1.5). + return etag if etag and not etag.startswith("W/") else headers.get("Last-Modified", "") or "" + + +def _fetch_archive(url: str, directory: Path, progress: Progress | None = None, + keep: Path | None = None) -> Path: """A data directory's source archive, in *directory*, retried like files. - progress draws its bytes as they come, an attempt that failed taken back.""" + progress draws its bytes as they come, an attempt that failed taken back. + + With keep, the archive is written there and kept when the transfer is + cut, beside a note of its address and the validator its server sent: + the next attempt, or the next run, asks for the rest with Range and + If-Range, and a server that changed the archive sends it whole. A cut + 50 MB archive started again from zero, and a link that drops more often + than one archive takes never installed the tree. + """ _network_allowed(url) + note = keep.with_name(keep.name + ".source") if keep is not None else None reason: Exception | None = None for attempt in range(1, MAX_RETRIES + 1): - handle = tempfile.NamedTemporaryFile(dir=directory, suffix=".archive", delete=False) + offset = 0 + validator = "" + if keep is not None: + try: + recorded = note.read_text(encoding="utf-8").split("\n") + if recorded[0] == url and len(recorded) > 1 and recorded[1]: + validator = recorded[1] + offset = keep.stat().st_size + except (OSError, UnicodeError): + offset = 0 + request = urllib.request.Request(url) + if offset: + request.add_header("Range", f"bytes={offset}-") + request.add_header("If-Range", validator) + handle = (open(keep, "r+b" if offset else "wb") if keep is not None else + tempfile.NamedTemporaryFile(dir=directory, suffix=".archive", delete=False)) received = 0 + resumed = 0 try: - with handle, urllib.request.urlopen(url, timeout=60) as resp: + with handle, urllib.request.urlopen(request, timeout=60) as resp: + headers = resp.headers or {} + ranged = (offset and getattr(resp, "status", 200) == 206 and str( + headers.get("Content-Range", "")).startswith(f"bytes {offset}-")) + if not ranged: + offset = 0 + handle.seek(offset) + handle.truncate() + if keep is not None and not offset: + # Written before the first byte: a cut transfer is then + # resumed only against the revision it holds. + note.write_text(f"{url}\n{_archive_validator(headers)}\n", + encoding="utf-8") if progress is not None: - length = resp.headers.get("Content-Length", "") + length = headers.get("Content-Length", "") if length.isdigit(): - progress.expect(int(length)) + progress.expect(offset + int(length)) + if offset: + progress.add(offset, resumed=True) + resumed = offset + total = offset while True: if _stop.is_set(): raise _Interrupted() @@ -7107,30 +7159,51 @@ def _fetch_archive(url: str, directory: Path, progress: Progress | None = None) if not chunk: break received += len(chunk) - if received > MAX_DATA_ARCHIVE_SIZE: + total += len(chunk) + if total > MAX_DATA_ARCHIVE_SIZE: raise ValueError("archive exceeds the size limit") handle.write(chunk) if progress is not None: progress.add(len(chunk)) - return Path(handle.name) + return keep if keep is not None else Path(handle.name) except urllib.error.HTTPError as exc: - Path(handle.name).unlink(missing_ok=True) - if exc.code < 500 and exc.code != 429: + if keep is None or exc.code == 416 or exc.code < 500 and exc.code != 429: + # Refused, or a range past an archive that shrank: no + # attempt resumes it. + _drop_archive(Path(handle.name), note) + if exc.code < 500 and exc.code not in (416, 429): raise reason = exc except (urllib.error.URLError, OSError, http.client.HTTPException) as exc: - Path(handle.name).unlink(missing_ok=True) + if keep is None: + Path(handle.name).unlink(missing_ok=True) reason = exc if isinstance(exc, OSError) else ConnectionError(repr(exc)) - except BaseException: - Path(handle.name).unlink(missing_ok=True) + except ValueError: + _drop_archive(Path(handle.name), note) raise - if progress is not None and received: - progress.add(-received) + except BaseException: + # Stopped: a kept archive waits for the next run. + if keep is None: + Path(handle.name).unlink(missing_ok=True) + raise + if progress is not None: + if received: + progress.add(-received) + if resumed: + progress.add(-resumed) + progress.forget_resumed(resumed) if attempt < MAX_RETRIES: time.sleep(_retry_delay(attempt, None)) raise reason if reason else OSError("download failed") +def _drop_archive(path: Path, note: Path | None) -> None: + """Remove an archive and the note of where it came from.""" + path.unlink(missing_ok=True) + if note is not None: + note.unlink(missing_ok=True) + + def _archive_members( archive: Path, entry: dict ) -> Iterator[tuple[str, int, Callable[[], BinaryIO]]]: @@ -7223,6 +7296,7 @@ def install_data_directory(entry: dict, bios_path: Path, if not needed: return counts _writing.set() + fetched: Path | None = None try: root.mkdir(parents=True, exist_ok=True) with tempfile.TemporaryDirectory(dir=root) as scratch: @@ -7230,7 +7304,11 @@ def install_data_directory(entry: dict, bios_path: Path, # The pack holds the tree where it extracts, already mapped. members = LOCAL.pack.tree(entry.get("dest", "")) else: - archive = _fetch_archive(entry["url"], Path(scratch), progress) + # Beside the tree, outside the scratch folder: a transfer + # cut short is resumed by the next attempt or the next run. + kept = root / f".{entry['ref']}.archive.part" + archive = _fetch_archive(entry["url"], Path(scratch), progress, kept) + fetched = archive members = _archive_members(archive, entry) for rel, declared, opener in members: meta = needed.pop(rel, None) @@ -7264,6 +7342,10 @@ def install_data_directory(entry: dict, bios_path: Path, except _MEMBER_ERRORS as exc: # A damaged, encrypted or unsupported member, in words. counts["error"] = unreadable_words(exc) + finally: + if fetched is not None: + # Read whole: the next run fetches the archive afresh. + _drop_archive(fetched, fetched.with_name(fetched.name + ".source")) counts["absent"] = len(needed) return counts diff --git a/install.sh b/install.sh index fe3d4fd6e..4e6c1560d 100755 --- a/install.sh +++ b/install.sh @@ -23,7 +23,7 @@ DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/ins # curl or wget stop on a cut transfer before the hash is read: the site's # build rewrites this line there (scripts/sitebootstrap.py). MISMATCH_ADVICE="The download was most likely cut short: try again. Within minutes of an update of this project the two files can come from caches of different ages. Waiting five minutes settles that." -DEFAULT_INSTALL_SHA256="567d810f5c4f5cbb6793d6f0c96a7492d90b53664d74f045aae4ef75d6058a0f" +DEFAULT_INSTALL_SHA256="fdccf00cb56c4caf3ddf916e126d538539e0d9efa527fb808cde954b6b0d29b1" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/install/launchers/index.json b/install/launchers/index.json index acfb61130..2f435fcb1 100644 --- a/install/launchers/index.json +++ b/install/launchers/index.json @@ -27,8 +27,8 @@ { "dest": "roms/ports/retrobios/install.py", "repo_path": "install.py", - "size": 1108265, - "sha256": "567d810f5c4f5cbb6793d6f0c96a7492d90b53664d74f045aae4ef75d6058a0f", + "size": 1112094, + "sha256": "fdccf00cb56c4caf3ddf916e126d538539e0d9efa527fb808cde954b6b0d29b1", "executable": false }, { @@ -71,8 +71,8 @@ "muos": { "archive": { "dest": "ARCHIVE/RetroBIOS.muxapp", - "size": 1161892, - "sha256": "1031750dcd599e0c4dd2e6c39296b3bcada68494a4837ad442150da64b67a55c" + "size": 1165721, + "sha256": "1b6ea28a5fc2f413aa7444ddb9ebe00b4b89c29ef639d62e3e1cf4d10793ed16" }, "members": [ { @@ -92,8 +92,8 @@ { "name": "RetroBIOS/install.py", "repo_path": "install.py", - "size": 1108265, - "sha256": "567d810f5c4f5cbb6793d6f0c96a7492d90b53664d74f045aae4ef75d6058a0f", + "size": 1112094, + "sha256": "fdccf00cb56c4caf3ddf916e126d538539e0d9efa527fb808cde954b6b0d29b1", "executable": false }, { @@ -166,8 +166,8 @@ { "dest": ".config/modules/retrobios/install.py", "repo_path": "install.py", - "size": 1108265, - "sha256": "567d810f5c4f5cbb6793d6f0c96a7492d90b53664d74f045aae4ef75d6058a0f", + "size": 1112094, + "sha256": "fdccf00cb56c4caf3ddf916e126d538539e0d9efa527fb808cde954b6b0d29b1", "executable": false }, { diff --git a/tests/test_install.py b/tests/test_install.py index a918d7d06..13deeaa5c 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -3867,6 +3867,96 @@ class DataDirectoriesAreInstalled(unittest.TestCase): (len(body), len(body))) emit.assert_not_called() + def test_a_cut_archive_resumes_where_it_stopped(self): + """A data archive cut at 90% was deleted and fetched again from + zero, on the retry and on the next run, while the run said that + nothing was lost.""" + work = Path(_tempdir(self)) + body = self._zip(work).read_bytes() + keep = work / ".dolphin-sys.archive.part" + requests: list[dict] = [] + + class Response(io.BytesIO): + def __init__(self, data, status, headers): + super().__init__(data) + self.status, self.headers = status, headers + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def urlopen(request, timeout=None): + sent = dict(request.header_items()) + requests.append(sent) + if "Range" in sent: + start = int(sent["Range"][len("bytes="):-1]) + if sent.get("If-range") != '"v1"': + return Response(body, 200, {"ETag": '"v2"'}) + return Response(body[start:], 206, { + "Content-Range": f"bytes {start}-{len(body) - 1}/{len(body)}", + "Content-Length": str(len(body) - start), "ETag": '"v1"'}) + cut = len(body) * 9 // 10 + + class Cut(Response): + def read(self, size=-1): + if self.tell() >= cut: + raise ConnectionResetError("cut") + return super().read(min(size, cut - self.tell())) + return Cut(body, 200, {"Content-Length": str(len(body)), "ETag": '"v1"'}) + + with unittest.mock.patch.object(install.urllib.request, "urlopen", urlopen), \ + unittest.mock.patch.object(install, "_network_allowed", lambda url: None), \ + unittest.mock.patch.object(install, "_retry_delay", lambda *a: 0): + # The next run, after a stop: what the cut attempt wrote is kept. + with unittest.mock.patch.object(install, "MAX_RETRIES", 1), \ + self.assertRaises(ConnectionResetError): + install._fetch_archive("https://example.invalid/D.zip", work, None, keep) + self.assertEqual(keep.stat().st_size, len(body) * 9 // 10) + path = install._fetch_archive("https://example.invalid/D.zip", work, None, keep) + self.assertEqual(path.read_bytes(), body) + self.assertEqual(requests[-1]["Range"], f"bytes={len(body) * 9 // 10}-") + self.assertEqual(requests[-1]["If-range"], '"v1"') + + def test_a_changed_archive_is_fetched_whole(self): + work = Path(_tempdir(self)) + body = self._zip(work).read_bytes() + keep = work / ".dolphin-sys.archive.part" + keep.write_bytes(b"older revision") + keep.with_name(keep.name + ".source").write_text( + 'https://example.invalid/D.zip\n"v0"\n') + + class Whole(io.BytesIO): + status, headers = 200, {"ETag": '"v1"'} + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + with unittest.mock.patch.object(install.urllib.request, "urlopen", + lambda request, timeout=None: Whole(body)), \ + unittest.mock.patch.object(install, "_network_allowed", lambda url: None): + path = install._fetch_archive("https://example.invalid/D.zip", work, None, keep) + self.assertEqual(path.read_bytes(), body) + + def test_a_tree_read_whole_leaves_no_archive(self): + work = Path(_tempdir(self)) + archive = self._zip(work) + bios = work / "system" + bios.mkdir() + + def fetch(url, directory, progress=None, keep=None): + shutil.copy(archive, keep) + keep.with_name(keep.name + ".source").write_text(f"{url}\n\"v1\"\n") + return keep + + with unittest.mock.patch.object(install, "_fetch_archive", fetch): + install.install_data_directory(self._entry(), bios) + self.assertEqual([p.name for p in (bios / "dolphin-emu").iterdir()], ["Sys"]) + def test_the_run_ends_on_the_archive_bytes_too(self): from test_install_selection import FakeRepository # noqa: PLC0415 @@ -3925,7 +4015,7 @@ class DataDirectoriesAreInstalled(unittest.TestCase): bios = work / "system" bios.mkdir() - def fetch(url, directory, progress=None): + def fetch(url, directory, progress=None, keep=None): copy = Path(directory) / "a.zip" shutil.copy(archive, copy) return copy @@ -3961,8 +4051,9 @@ class DataDirectoriesAreInstalled(unittest.TestCase): def members(_archive, _entry): yield "Sys/GC/font.bin", 4, lambda: io.BufferedReader(Endless()) - with unittest.mock.patch.object(install, "_fetch_archive", - lambda url, d, progress=None: Path(d) / "a"), \ + with unittest.mock.patch.object( + install, "_fetch_archive", + lambda url, d, progress=None, keep=None: Path(d) / "a"), \ unittest.mock.patch.object(install, "_archive_members", members): counts = install.install_data_directory(entry, work) self.assertEqual(counts["changed"], 1) @@ -3989,8 +4080,9 @@ class DataDirectoriesAreInstalled(unittest.TestCase): def members(_archive, _entry): yield "Sys/GC/font.bin", 4, lambda: io.BufferedReader(Broken()) - with unittest.mock.patch.object(install, "_fetch_archive", - lambda url, d, progress=None: Path(d) / "a"), \ + with unittest.mock.patch.object( + install, "_fetch_archive", + lambda url, d, progress=None, keep=None: Path(d) / "a"), \ unittest.mock.patch.object(install, "_archive_members", members): counts = install.install_data_directory(entry, work) self.assertTrue(counts["error"]) @@ -6714,7 +6806,7 @@ class AFileYouHadIsKept(unittest.TestCase): "type": "zip", "files": {"Sys/a.ini": { "sha1": hashlib.sha1(self.NEW).hexdigest(), "size": len(self.NEW)}}} - def fetch(url, directory, progress=None): + def fetch(url, directory, progress=None, keep=None): copy = Path(directory) / "a.zip" shutil.copy(archive, copy) return copy diff --git a/wiki/installer.md b/wiki/installer.md index 796938117..1aa89e133 100644 --- a/wiki/installer.md +++ b/wiki/installer.md @@ -352,8 +352,10 @@ file list is downloaded once and handed to each run. times, after 1, 2 then 4 seconds or the delay a `Retry-After` header asks. A file of 16 MB or more keeps its partial copy (`..part`) when a transfer stops, and the next attempt or the next run requests only the - rest. A file already there with other contents is moved aside first, see - [Files you already had](#files-you-already-had). + rest. A data tree's archive does the same beside the tree + (`..archive.part`), asking with `If-Range` so that a server whose + archive changed sends it whole. A file already there with other contents + is moved aside first, see [Files you already had](#files-you-already-had). 8. Copies into standalone emulator directories, only with `--standalone-copies`. The file list names the version of the platform it was built for (Batocera