From 7e504e8469a4bc933878d3d57eeee6f548260780 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 03:51:36 +0100 Subject: [PATCH] fix: show and count data archive downloads A data tree's archive came with no progress line, and the run ended on the BIOS bytes alone (310.5 KB where 82 MB arrived). Its download now draws the progress line and adds its bytes and time to the end line, a cut attempt taken back. It writes no progress event: those count the download event's files. --- install.ps1 | 2 +- install.py | 47 +++++++++++++++++---- install.sh | 2 +- install/launchers/index.json | 16 +++---- scripts/generate_installer_doc.py | 3 +- tests/test_install.py | 69 +++++++++++++++++++++++++++++-- wiki/installer.md | 4 +- 7 files changed, 119 insertions(+), 24 deletions(-) diff --git a/install.ps1 b/install.ps1 index a96745599..896cdaf76 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 = "c8cf4b65474279c30c2e1781028d3e05f195e26dd759a98a37db3477b6dd3014" + $defaultInstallSha256 = "34fd006cc0456c61b9a621e52f35cee6d3fd55ff14bbc6cea207ddc7bc57d858" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index fe18bfa61..01c0b9425 100644 --- a/install.py +++ b/install.py @@ -528,7 +528,8 @@ class Progress: BAR = 24 - def __init__(self, total_files: int, total_bytes: int, term: Terminal) -> None: + def __init__(self, total_files: int, total_bytes: int, term: Terminal, + events: bool = True) -> None: self.total_files = total_files self.expected_bytes = total_bytes self.total_bytes = max(total_bytes, 1) @@ -548,6 +549,15 @@ class Progress: self._drawn = 0.0 self._evented = self.start self._lock = threading.Lock() + # A data tree's archive draws the line and writes no event: the + # progress events count the files of the download event. + self.events = events + + def expect(self, total_bytes: int) -> None: + """The size the transfer turns out to have, once its server says it.""" + with self._lock: + self.expected_bytes = total_bytes + self.total_bytes = max(total_bytes, 1) def add(self, count: int, resumed: bool = False, network: bool | None = None) -> None: """count bytes written, resumed ones left out of the speed. network @@ -563,7 +573,7 @@ class Progress: if self.term.live and not self.term.quiet and now - self._drawn >= 0.1: self._drawn = now self._draw() - if EVENTS.on and now - self._evented >= EVENT_PROGRESS_INTERVAL: + if self.events and EVENTS.on and now - self._evented >= EVENT_PROGRESS_INTERVAL: self._evented = now EVENTS.emit("progress", files_done=self.files, files_total=self.total_files, bytes_done=self.bytes, bytes_total=self.expected_bytes) @@ -7052,15 +7062,20 @@ def download_files( return failed, interrupted -def _fetch_archive(url: str, directory: Path) -> Path: - """A data directory's source archive, in *directory*, retried like files.""" +def _fetch_archive(url: str, directory: Path, progress: Progress | 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.""" _network_allowed(url) reason: Exception | None = None for attempt in range(1, MAX_RETRIES + 1): handle = tempfile.NamedTemporaryFile(dir=directory, suffix=".archive", delete=False) + received = 0 try: with handle, urllib.request.urlopen(url, timeout=60) as resp: - received = 0 + if progress is not None: + length = resp.headers.get("Content-Length", "") + if length.isdigit(): + progress.expect(int(length)) while True: if _stop.is_set(): raise _Interrupted() @@ -7071,6 +7086,8 @@ def _fetch_archive(url: str, directory: Path) -> Path: if received > 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) except urllib.error.HTTPError as exc: Path(handle.name).unlink(missing_ok=True) @@ -7083,6 +7100,8 @@ def _fetch_archive(url: str, directory: Path) -> Path: except BaseException: Path(handle.name).unlink(missing_ok=True) raise + if progress is not None and received: + progress.add(-received) if attempt < MAX_RETRIES: time.sleep(_retry_delay(attempt, None)) raise reason if reason else OSError("download failed") @@ -7149,7 +7168,8 @@ def data_directory_state(entry: dict, bios_path: Path, def install_data_directory(entry: dict, bios_path: Path, - state: tuple[Path, dict[str, dict], int] | None = None) -> dict: + state: tuple[Path, dict[str, dict], int] | None = None, + progress: Progress | None = None) -> dict: """Rebuild one data tree from its source archive, file by file verified. state is the tree's data_directory_state when the run already read it: each read hashes every file in place. @@ -7173,7 +7193,7 @@ 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)) + archive = _fetch_archive(entry["url"], Path(scratch), progress) members = _archive_members(archive, entry) for rel, declared, opener in members: meta = needed.pop(rel, None) @@ -15856,7 +15876,18 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: if writable: print("\nData directories:") for entry, state in writable: - counts = install_data_directory(entry, bios_path, state) + # Its archive is drawn as the BIOS files were, and counted + # in the bytes and time the run ends on: 82 MB came in + # silence and the end said 310.5 KB. + fetching = bool(state[1]) and LOCAL.pack is None + progress = Progress(len(state[1]), 0, term, events=False) if fetching else None + try: + counts = install_data_directory(entry, bios_path, state, progress) + finally: + if progress is not None: + progress.finish() + total_bytes += progress.transferred() + total_seconds += progress.elapsed() total_downloaded += counts["installed"] data_landed += counts["installed"] total_up_to_date += counts["present"] diff --git a/install.sh b/install.sh index 556c6aec4..b085a4cbb 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="c8cf4b65474279c30c2e1781028d3e05f195e26dd759a98a37db3477b6dd3014" +DEFAULT_INSTALL_SHA256="34fd006cc0456c61b9a621e52f35cee6d3fd55ff14bbc6cea207ddc7bc57d858" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/install/launchers/index.json b/install/launchers/index.json index 3cba41420..358ee41c9 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": 1101457, - "sha256": "c8cf4b65474279c30c2e1781028d3e05f195e26dd759a98a37db3477b6dd3014", + "size": 1103111, + "sha256": "34fd006cc0456c61b9a621e52f35cee6d3fd55ff14bbc6cea207ddc7bc57d858", "executable": false }, { @@ -71,8 +71,8 @@ "muos": { "archive": { "dest": "ARCHIVE/RetroBIOS.muxapp", - "size": 1155084, - "sha256": "41b72bf029de9d855312e472cc9a10046f71692a55e02c4a959efdf4201505ad" + "size": 1156738, + "sha256": "35c0920008baadca997d0f778e99ce46d702bbe3d3f0d33104d7158422b292aa" }, "members": [ { @@ -92,8 +92,8 @@ { "name": "RetroBIOS/install.py", "repo_path": "install.py", - "size": 1101457, - "sha256": "c8cf4b65474279c30c2e1781028d3e05f195e26dd759a98a37db3477b6dd3014", + "size": 1103111, + "sha256": "34fd006cc0456c61b9a621e52f35cee6d3fd55ff14bbc6cea207ddc7bc57d858", "executable": false }, { @@ -166,8 +166,8 @@ { "dest": ".config/modules/retrobios/install.py", "repo_path": "install.py", - "size": 1101457, - "sha256": "c8cf4b65474279c30c2e1781028d3e05f195e26dd759a98a37db3477b6dd3014", + "size": 1103111, + "sha256": "34fd006cc0456c61b9a621e52f35cee6d3fd55ff14bbc6cea207ddc7bc57d858", "executable": false }, { diff --git a/scripts/generate_installer_doc.py b/scripts/generate_installer_doc.py index ee2973261..7fd134d65 100644 --- a/scripts/generate_installer_doc.py +++ b/scripts/generate_installer_doc.py @@ -362,7 +362,8 @@ def _child(spec: dict) -> int: def held(files: list[dict], _bios: Path, **_kwargs: object) -> tuple: return [], list(files), [] - def tree_landed(entry: dict, _bios: Path, state: object = None) -> dict: + def tree_landed(entry: dict, _bios: Path, state: object = None, + _progress: object = None) -> dict: return {"present": 0, "installed": len(entry["files"]), "changed": 0, "absent": 0, "error": "", "not_kept": []} diff --git a/tests/test_install.py b/tests/test_install.py index b86e1744c..e51432718 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -3707,13 +3707,74 @@ class DataDirectoriesAreInstalled(unittest.TestCase): }, } + def test_the_archive_is_drawn_and_counted(self): + """82 MB of archives came with no progress line, and the run ended + on "310.5 KB downloaded", the BIOS files alone.""" + work = Path(_tempdir(self)) + body = self._zip(work).read_bytes() + attempts: list[int] = [] + + class Response(io.BytesIO): + headers = {"Content-Length": str(len(body))} + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def urlopen(_url, timeout=None): + attempts.append(1) + if len(attempts) == 1: + class Cut(Response): + def read(self, size=-1): + if self.tell(): + raise ConnectionResetError("cut") + return super().read(5) + return Cut(body) + return Response(body) + + progress = install.Progress(1, 0, install.Terminal(io.StringIO()), events=False) + 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), \ + unittest.mock.patch.object(install.EVENTS, "emit") as emit: + path = install._fetch_archive("https://example.invalid/D.zip", work, progress) + self.assertEqual(path.read_bytes(), body) + self.assertEqual(len(attempts), 2) + # The cut attempt's five bytes are taken back. + self.assertEqual((progress.transferred(), progress.expected_bytes), + (len(body), len(body))) + emit.assert_not_called() + + def test_the_run_ends_on_the_archive_bytes_too(self): + from test_install_selection import FakeRepository # noqa: PLC0415 + + member = os.urandom(300_000) + with FakeRepository() as repo: + with zipfile.ZipFile(repo.root / "tree.zip", "w") as zf: + zf.writestr("Sys/big.bin", member) + archive = (repo.root / "tree.zip").stat().st_size + listed = json.loads((repo.root / "install" / "retroarch.json").read_text()) + listed["data_directories"] = [{ + "ref": "tree", "dest": "tree", "type": "zip", + "url": f"http://127.0.0.1:{repo.httpd.server_address[1]}/tree.zip", + "systems": ["nintendo-gba"], + "files": {"Sys/big.bin": {"sha1": hashlib.sha1(member).hexdigest(), + "size": len(member)}}}] + (repo.root / "install" / "retroarch.json").write_text(json.dumps(listed)) + proc = repo.run("--no-input") + bios = sum(len(payload) for payload, *_ in FakeRepository.PAYLOADS.values()) + self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr) + self.assertIn(f"{install.format_size(bios + archive)} downloaded in", proc.stdout) + def test_verified_members_land_the_rest_are_said(self): work = Path(_tempdir(self)) archive = self._zip(work) bios = work / "system" bios.mkdir() - def fetch(url, directory): + def fetch(url, directory, progress=None): copy = Path(directory) / "a.zip" shutil.copy(archive, copy) return copy @@ -3750,7 +3811,7 @@ class DataDirectoriesAreInstalled(unittest.TestCase): yield "Sys/GC/font.bin", 4, lambda: io.BufferedReader(Endless()) with unittest.mock.patch.object(install, "_fetch_archive", - lambda url, d: Path(d) / "a"), \ + lambda url, d, progress=None: Path(d) / "a"), \ unittest.mock.patch.object(install, "_archive_members", members): counts = install.install_data_directory(entry, work) self.assertEqual(counts["changed"], 1) @@ -3778,7 +3839,7 @@ class DataDirectoriesAreInstalled(unittest.TestCase): yield "Sys/GC/font.bin", 4, lambda: io.BufferedReader(Broken()) with unittest.mock.patch.object(install, "_fetch_archive", - lambda url, d: Path(d) / "a"), \ + lambda url, d, progress=None: Path(d) / "a"), \ unittest.mock.patch.object(install, "_archive_members", members): counts = install.install_data_directory(entry, work) self.assertTrue(counts["error"]) @@ -6458,7 +6519,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): + def fetch(url, directory, progress=None): copy = Path(directory) / "a.zip" shutil.copy(archive, copy) return copy diff --git a/wiki/installer.md b/wiki/installer.md index af2339730..dea79b4bd 100644 --- a/wiki/installer.md +++ b/wiki/installer.md @@ -342,7 +342,9 @@ file list is downloaded once and handed to each run. [Copy from an old card](#copy-from-an-old-card). 6. Downloads what is still missing or wrong, up to 8 files at a time. At a terminal one line shows the progress, the speed and the time left, while in a - log each finished file gets its own line. In an interactive run, a + log each finished file gets its own line. A data tree's archive gets the + same line at a terminal, and the bytes and time the run ends on count + it with the files. In an interactive run, a download of 1 GB or more is confirmed first (`--yes` skips the question). 7. Verifies each download against the declared size, SHA-256 and SHA-1 before it touches the destination, then moves it into place with `os.replace` so