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

@@ -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 }

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"]

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() {

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
         },
         {

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": []}
 

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

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