From 69179c95de53f7de44597d4710d622f3673ff660 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:30:11 +0200 Subject: [PATCH] fix: count only the files a stopped run wrote A run stopped after 8 of 118 files said 118 were installed. The count now comes from the downloads that finished, and a refused folder writes one downloaded event per file, as the events table says. --- install.ps1 | 2 +- install.py | 38 +++++++++---- install.sh | 2 +- tests/test_install_experience.py | 95 ++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 12 deletions(-) diff --git a/install.ps1 b/install.ps1 index 2ab5bf18..30bda6d9 100644 --- a/install.ps1 +++ b/install.ps1 @@ -11,7 +11,7 @@ $ErrorActionPreference = "Stop" $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" - $defaultInstallSha256 = "851c1d9e8b2b481adf50d1af2c67d1e44d9b50504fdef3d5b6514332ac4457eb" + $defaultInstallSha256 = "81b72f480566791ccf1db34d20635bc7886190f2a21b150507c526130af1d499" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 74807803..6c694f3a 100644 --- a/install.py +++ b/install.py @@ -377,6 +377,9 @@ class Progress: self.files = 0 self.bytes = 0 self.resumed = 0 + # Files written to their place, which a stopped run counts: the + # queue's length said "118 files installed" over 8 on disk. + self.installed = 0 self.start = time.monotonic() self._drawn = 0.0 self._evented = self.start @@ -4841,10 +4844,10 @@ def download_files( """Download files in parallel. Returns the destinations that failed, each with its reason, and whether - the run was interrupted. Every future is submitted up front, so a bare - KeyboardInterrupt would let the pool drain the queue in full: the stop - flag is what makes Ctrl+C end the run rather than only stop the - reporting. + the run was interrupted. progress.installed counts the files written. + Every future is submitted up front, so a bare KeyboardInterrupt would + let the pool drain the queue in full: the stop flag is what makes Ctrl+C + end the run rather than only stop the reporting. """ failed: list[tuple[str, str]] = [] interrupted = False @@ -4859,14 +4862,24 @@ def download_files( for f in files } refused = False + refused_events = 0 try: for future in concurrent.futures.as_completed(future_map): dest, success, reason = future.result() if refused: # The folder they all go in refused the first: each - # would fail alike, counted without a line of its own. + # would fail alike, counted without a line of its own, + # though a program reading the events still gets one. if not success: failed.append((dest, NOT_WRITABLE)) + refused_events += 1 + EVENTS.emit( + "downloaded", dest=dest, ok=False, reason=NOT_WRITABLE, + size=future_map[future].get("size", 0), + files_done=progress.files + refused_events, + files_total=progress.total_files, bytes_done=progress.bytes, + bytes_total=progress.expected_bytes, + ) continue if _stop.is_set(): continue @@ -4889,6 +4902,12 @@ def download_files( _stop.set() progress.finish() print("\n Stopping. Files already installed are kept.") + # The pool has run every future by now: a file that landed after Ctrl+C + # is on disk and counts. + progress.installed = sum( + 1 for future in future_map + if not future.cancelled() and future.exception() is None and future.result()[1] + ) progress.finish() return failed, interrupted @@ -5201,7 +5220,7 @@ def _install_standalone_folders( to_download, folder, jobs=args.jobs, verbose=args.verbose, term=term, progress=progress, ) - downloaded += len(to_download) - len(failed) + downloaded += progress.installed errors += len(failed) failures.extend((str(folder / dest), reason) for dest, reason in failed) transferred += progress.transferred() @@ -7646,10 +7665,9 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: term=term, progress=progress, ) bulk_lines += hint - total_downloaded += len(to_download) - len(failed) - files_pending += len(failed) - if interrupted: - files_pending += max(len(to_download) - progress.files, 0) + total_downloaded += progress.installed + files_pending += len(to_download) - progress.installed if interrupted \ + else len(failed) total_errors += len(failed) failures.extend(failed) total_bytes += progress.transferred() diff --git a/install.sh b/install.sh index b3217604..57190d69 100755 --- a/install.sh +++ b/install.sh @@ -18,7 +18,7 @@ esac TEMP_INSTALLER="" TEMP_DIRECTORY="" DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" -DEFAULT_INSTALL_SHA256="851c1d9e8b2b481adf50d1af2c67d1e44d9b50504fdef3d5b6514332ac4457eb" +DEFAULT_INSTALL_SHA256="81b72f480566791ccf1db34d20635bc7886190f2a21b150507c526130af1d499" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/tests/test_install_experience.py b/tests/test_install_experience.py index f356a2f3..eb3614c9 100644 --- a/tests/test_install_experience.py +++ b/tests/test_install_experience.py @@ -404,6 +404,101 @@ class ReviewCases(unittest.TestCase): progress.add(40) self.assertEqual(progress.transferred(), 40) +class _HeldHandler(http.server.BaseHTTPRequestHandler): + """Serves the manifest and first.bin at once, every other file once released.""" + + files: dict[str, bytes] = {} + manifest = b"" + release = threading.Event() + + def log_message(self, *args) -> None: # noqa: D102 + pass + + def do_GET(self) -> None: # noqa: N802, D102 + if self.path.endswith(".json"): + body = self.manifest + else: + name = self.path.rsplit("/", 1)[-1] + if name != "first.bin": + self.release.wait(30) + body = self.files[name] + self.send_response(200) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + +class AStoppedRunCountsWhatLanded(unittest.TestCase): + """Stopped at 8 of 118 files, the summary said "118 files installed": + every queued file was counted, written or not, and a user who read it + did not run the command again.""" + + @unittest.skipIf(os.name == "nt", "SIGINT is sent the POSIX way") + def test_only_the_written_file_is_counted(self): + import signal # noqa: PLC0415 + import time # noqa: PLC0415 + + files = {name: name.encode() * 64 for name in ("first.bin", "second.bin", "third.bin")} + manifest = {"manifest_version": 2, "platform": "retroarch", "files": [ + {"dest": name, "size": len(data), "sha1": hashlib.sha1(data).hexdigest(), + "repo_path": f"bios/{name}", "cores": None} + for name, data in files.items() + ]} + _HeldHandler.files, _HeldHandler.manifest = files, json.dumps(manifest).encode() + _HeldHandler.release = threading.Event() + httpd = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _HeldHandler) + threading.Thread(target=httpd.serve_forever, daemon=True).start() + self.addCleanup(httpd.server_close) + self.addCleanup(httpd.shutdown) + self.addCleanup(_HeldHandler.release.set) + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + dest = Path(tmp) / "bios" + env = dict(os.environ, + RETROBIOS_BASE_URL=f"http://127.0.0.1:{httpd.server_address[1]}") + proc = subprocess.Popen( + [sys.executable, str(REPO_ROOT / "install.py"), "--platform", "retroarch", + "--dest", str(dest), "--no-input"], + env=env, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, text=True, + ) + deadline = time.monotonic() + 30 + while not (dest / "first.bin").exists() and time.monotonic() < deadline: + time.sleep(0.05) + time.sleep(0.2) + proc.send_signal(signal.SIGINT) + time.sleep(0.3) + _HeldHandler.release.set() + out, err = proc.communicate(timeout=60) + written = sorted(p.name for p in dest.iterdir() if not p.name.startswith(".")) + self.assertEqual(proc.returncode, 130, out + err) + self.assertEqual(written, ["first.bin"]) + self.assertIn("Stopped. 1 file installed.", out) + + +class ARefusedFolderReportsEachFile(unittest.TestCase): + """A read-only folder ended the run on its first refusal, and --events + wrote one `downloaded` line for seven failed files, while the events + table promises one per finished file.""" + + @unittest.skipIf(os.name == "nt" or os.geteuid() == 0, + "a read-only folder is refused by POSIX permissions") + def test_one_event_per_file_with_the_reason(self): + with FakeRepository() as repo: + repo.dest.mkdir() + os.chmod(repo.dest, 0o555) + try: + proc = repo.run("--events") + finally: + os.chmod(repo.dest, 0o755) + events = [json.loads(line) for line in proc.stdout.splitlines()] + downloaded = [e for e in events if e["event"] == "downloaded"] + self.assertEqual(proc.returncode, 2, proc.stderr) + self.assertEqual(len(downloaded), len(FakeRepository.PAYLOADS)) + self.assertEqual({e["reason"] for e in downloaded}, {install.NOT_WRITABLE}) + self.assertEqual(sorted(e["files_done"] for e in downloaded), + list(range(1, len(downloaded) + 1))) + + class WindowsMandatoryLock(unittest.TestCase): """Issue 82: on Windows the partial copy's lock is mandatory, so hashing it by reopening its path failed with a sharing violation, and every file