From 2da724879beb755908b3df0dbe6e5e82a1eda53e Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:08:15 +0200 Subject: [PATCH] fix: name --from copies that could not be read A file, ZIP member or folder below a --from folder that could not be read was taken as no match. When its size is that of a file the run still needs, or unknown, --check, --dry-run and an install now name it with the reason, and an unreadable event carries it. --- install.ps1 | 2 +- install.py | 96 ++++++++++++++++++++++++++++++++-- install.sh | 2 +- tests/test_install_from.py | 102 +++++++++++++++++++++++++++++++++++++ wiki/installer.md | 1 + 5 files changed, 196 insertions(+), 7 deletions(-) diff --git a/install.ps1 b/install.ps1 index ab2f66bd..cb079f36 100644 --- a/install.ps1 +++ b/install.ps1 @@ -11,7 +11,7 @@ $ErrorActionPreference = "Stop" $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" - $defaultInstallSha256 = "baeaf6acc5a7813df508d997c72ce934fcf3010b953f278426ccb95fd9a4abf8" + $defaultInstallSha256 = "4b658dc29528e94460995eaf844b1fc1cca9040ffa10cd71ac6656b4db5f1b34" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index b3a02e6d..5e17f22a 100644 --- a/install.py +++ b/install.py @@ -510,6 +510,10 @@ EVENT_KEYS: dict[str, tuple[str, ...]] = { # a folder named with --from (reason empty). member names the ZIP member # when path is an archive, else "". Its copy is a downloaded event. "found": ("platform", "dest", "folder", "path", "member", "reason"), + # What a folder named with --from holds that could not be read, of the + # size of a file no copy was found for, or of no known size (a folder, + # a ZIP that would not open). member as in found, reason in words. + "unreadable": ("platform", "folder", "path", "member", "reason"), "download": ("platform", "files", "bytes"), "progress": ("files_done", "files_total", "bytes_done", "bytes_total"), "downloaded": ("dest", "ok", "reason", "size", "files_done", "files_total", @@ -6732,12 +6736,20 @@ class FolderIndex: self._loose_named: dict[tuple[int, str], list[Path]] | None = None self._member_named: dict[int, dict[str, list[tuple[Path, str]]]] = {} self._scans: dict[tuple[str, int, str], _HashScan] = {} + self._member_size: dict[tuple[Path, str], int] = {} + # (path, member or "") -> (its size or None when unknown, why) of each + # file, member or folder below root that could not be read. + self.unreadable: dict[tuple[Path, str], tuple[int | None, str]] = {} if root.is_file(): if root.name.casefold().endswith(".zip"): self.archives.append(root) return seen = 0 - for folder, dirs, names in os.walk(root): + + def unlisted(exc: OSError) -> None: + self._unread(Path(exc.filename or root), "", None, exc) + + for folder, dirs, names in os.walk(root, onerror=unlisted): dirs[:] = sorted(d for d in dirs if d != BACKUP_DIR and d.casefold() not in SYSTEM_FOLDERS) for name in sorted(names): @@ -6753,7 +6765,8 @@ class FolderIndex: if not path.is_file(): continue size = path.stat().st_size - except OSError: + except OSError as exc: + self._unread(path, "", None, exc) continue self.by_name.setdefault(name.casefold(), []).append(path) self.by_size.setdefault(size, []).append(path) @@ -6769,11 +6782,17 @@ class FolderIndex: """A SHA-1 already known, so the file is not read again.""" self._sha1[path] = sha1 + def _unread(self, path: Path, member: str, size: int | None, exc: BaseException) -> None: + """Note a file, a member or a folder that could not be read: a copy + the run needs may be the one, and it is named instead of dropped.""" + self.unreadable.setdefault((path, member), (size, unreadable_words(exc))) + def sha1(self, path: Path) -> str: if path not in self._sha1: try: self._sha1[path] = _sha1_file(path) - except OSError: + except OSError as exc: + self._unread(path, "", self.size.get(path), exc) self._sha1[path] = "" return self._sha1[path] @@ -6789,11 +6808,21 @@ class FolderIndex: value = _digest_handle(handle, (algorithm,))[algorithm] else: value = _digest_file(path, (algorithm,))[algorithm] - except _MEMBER_ERRORS: + except _MEMBER_ERRORS as exc: + size = self._member_size.get((path, member)) if member else self.size.get(path) + self._unread(path, member, size, exc) value = "" self._digests[key] = value return self._digests[key] + def unreadable_for(self, sizes: Collection[int]) -> list[tuple[Path, str, str]]: + """(path, member, why) of what could not be read and may hold a file + of one of sizes: its size is one of them, or unknown (a folder, an + archive whose list of members could not be read).""" + return sorted((path, member, why) + for (path, member), (size, why) in self.unreadable.items() + if size is None or size in sizes) + def _scan(self, kind: str, size: int, algorithm: str) -> _HashScan: """The files (kind "loose") or ZIP members ("member") of one size, hashed in the order the folder was read, each read once.""" @@ -6839,12 +6868,18 @@ class FolderIndex: try: with zipfile.ZipFile(path) as archive: infos = archive.infolist() - except (OSError, zipfile.BadZipFile): + except OSError as exc: + # A ZIP that will not open may hold anything: named. A + # damaged one is a download cut short, not a copy. + self._unread(path, "", None, exc) + continue + except zipfile.BadZipFile: continue for info in infos: if not info.is_dir(): self._member_sizes.setdefault(info.file_size, []).append( (path, info.filename)) + self._member_size[(path, info.filename)] = info.file_size return self._member_sizes.get(size, []) def find(self, entry: dict, skip: Collection[Path] = ()) -> LocalCopy | None: @@ -7461,6 +7496,47 @@ def emit_copies( path=str(copy.path), member=copy.member, reason=reason) +class Unreadable(NamedTuple): + """A file, ZIP member or folder below a --from folder that could not be + read, and may be a file the run still needs.""" + index: FolderIndex + path: Path + member: str + why: str + + def line(self) -> str: + where = f"{self.member} inside {self.path}" if self.member else str(self.path) + return f"{where} could not be read: {self.why}" + + +def unreadable_copies( + entries: list[dict], copies: dict[int, LocalCopy], folders: list[FolderIndex], +) -> list[Unreadable]: + """What the --from folders hold that could not be read, of the size of + a file no copy was found for, or of no known size: dropped silently, + a file a user had read as one to download.""" + sizes = {e["size"] for e in entries + if id(e) not in copies and isinstance(e.get("size"), int)} + if not sizes: + return [] + return [Unreadable(index, *held) for index in folders + for held in index.unreadable_for(sizes)] + + +def unreadable_lines(found: list[Unreadable], every: bool) -> list[str]: + lines = [f" {item.line()}" for item in (found if every else found[:10])] + if len(found) > len(lines): + lines.append(f" ... and {len(found) - len(lines)} more could not be read " + "(--verbose lists them all)") + return lines + + +def emit_unreadable(plat: str, found: list[Unreadable]) -> None: + for item in found: + EVENTS.emit("unreadable", platform=plat, folder=item.index.label, + path=str(item.path), member=item.member, reason=item.why) + + def sources_line( folders: list[tuple[str, int]], fixed: int, downloaded: int, missing: int, fetched: str = "downloaded", shared: int = 0, @@ -8553,6 +8629,7 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: if bios_index is not None: bios_indexes.append(bios_index) copies = plan_local_copies(to_download, bios_path, bios_index, LOCAL.folders) + unread = unreadable_copies(to_download, copies, LOCAL.folders) hint = [] if LOCAL.pack is not None else bulk_hint( to_download, manifest, systems_chosen=bool(selection and selection[0]), @@ -8577,6 +8654,7 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: chosen=selection[0] if selection else None, ) emit_copies(plat_name, to_download, copies, bios_index, bios_path, files) + emit_unreadable(plat_name, unread) total_verified += len(up_to_date) if args.check: @@ -8595,6 +8673,8 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: for line in check_lines(missing, mismatched, files, manifest, bios_path, args.verbose, up_to_date, bios_index, copies): print(line) + for line in unreadable_lines(unread, args.verbose): + print(line) print(" Run the same command without --check to fetch them; " "a file with a wrong hash is replaced, and the one in place " f"kept in {BACKUP_DIR} beside the BIOS folder.") @@ -8628,6 +8708,8 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: _print_plan([f for f in to_download if id(f) not in copies], bios_path, term, trees, kept, copy_plan_lines(to_download, copies, bios_index, bios_path, files, args.verbose, True)) + for line in unreadable_lines(unread, args.verbose): + print(line) for line in hint: print(line) continue @@ -8653,6 +8735,10 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: files_pending += len(to_download) continue EVENTS.emit("download", platform=plat_name, files=len(to_download), bytes=dl_size) + if unread: + print() + for line in unreadable_lines(unread, args.verbose): + print(line) if copies: print() for line in copy_plan_lines(to_download, copies, bios_index, bios_path, diff --git a/install.sh b/install.sh index 3d9f51d5..d9ca30f3 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="baeaf6acc5a7813df508d997c72ce934fcf3010b953f278426ccb95fd9a4abf8" +DEFAULT_INSTALL_SHA256="4b658dc29528e94460995eaf844b1fc1cca9040ffa10cd71ac6656b4db5f1b34" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/tests/test_install_from.py b/tests/test_install_from.py index 4655e476..ebca9a29 100644 --- a/tests/test_install_from.py +++ b/tests/test_install_from.py @@ -548,6 +548,108 @@ class ACopyThatFails(FromFolder): f"Permission denied, and {install.NOT_IN_PACK}") +def _patched_zip(path: Path, name: str, data: bytes, flags: int = 0, + method: int | None = None) -> Path: + """A stored ZIP of one member, its headers then given other flags or + another compression method: zipfile writes neither encryption nor + methods it cannot read.""" + _zip(path, {}) + with zipfile.ZipFile(path, "w", zipfile.ZIP_STORED) as archive: + archive.writestr(name, data) + raw = bytearray(path.read_bytes()) + for signature, at in ((b"PK\x03\x04", 6), (b"PK\x01\x02", 8)): + start = raw.find(signature) + raw[start + at] |= flags + if method is not None: + raw[start + at + 2:start + at + 4] = method.to_bytes(2, "little") + path.write_bytes(bytes(raw)) + return path + + +class UnreadableCopies(FromFolder): + """A copy that could not be read was cached as no match and dropped: + the user saw a file to download that the old card held.""" + + def setUp(self): + super().setUp() + self.keep("scph5500.bin", JP) + self.keep("pcsx/extra.bin", EXTRA) + self.keep("gba_bios.bin", GBA) + + def locked(self, relative: str, data: bytes) -> Path: + _unprivileged(self) + path = self.keep(relative, data) + path.chmod(0) + self.addCleanup(path.chmod, 0o644) + return path + + def test_a_file_of_mode_000_is_named_in_every_mode(self): + path = self.locked("psx/scph5501.bin", US).resolve() + self.locked("notes.txt", b"not the size of any file") + line = f"{path} could not be read: Permission denied" + for options in (("--check",), ("--dry-run",), ()): + with self.subTest(options=options): + proc = self.install(*options) + self.assertEqual(proc.returncode, 1 if options != ("--dry-run",) else 0, + proc.stdout + proc.stderr) + self.assertIn(line, proc.stdout) + self.assertNotIn("notes.txt", proc.stdout) + + def test_an_unreadable_copy_is_an_event(self): + path = self.locked("psx/scph5501.bin", US).resolve() + proc = self.install("--check", "--events") + events = _events(proc) + for event in events: + self.assertEqual(list(event), ["event", *install.EVENT_KEYS[event["event"]]]) + self.assertEqual([e for e in events if e["event"] == "unreadable"], [{ + "event": "unreadable", "platform": "retroarch", "folder": str(self.old), + "path": str(path), "member": "", "reason": "Permission denied"}]) + kinds = [e["event"] for e in events] + self.assertLess(kinds.index("found"), kinds.index("unreadable")) + + def test_a_copy_found_elsewhere_leaves_the_unreadable_one_unnamed(self): + self.locked("psx/scph5501.bin", US) + self.keep("psx/copy.bin", US) + proc = self.install("--check") + self.assertNotIn("could not be read", proc.stdout) + + def test_a_folder_of_mode_000_is_named(self): + _unprivileged(self) + folder = self.old / "psx" + folder.mkdir() + folder.chmod(0) + self.addCleanup(folder.chmod, 0o755) + proc = self.install("--check") + self.assertIn(f"{folder.resolve()} could not be read: Permission denied", proc.stdout) + + def test_members_that_cannot_be_read_say_why(self): + for flags, method, words in ( + (0x1, None, "it is encrypted, a password is needed"), + (0, 6, "it is packed with a compression method this installer " + "cannot read")): + with self.subTest(words=words): + archive = _patched_zip(self.old / "psx.zip", "scph5501.bin", US, + flags, method).resolve() + proc = self.install("--check") + self.assertIn(f"scph5501.bin inside {archive} could not be read: {words}", + proc.stdout) + + def test_an_input_output_error_is_named(self): + self.keep("psx/scph5501.bin", US) + index = install.FolderIndex(self.old) + entry = json.loads(self.list.read_text())["files"][0] + failing = OSError(errno.EIO, "Input/output error") + with mock.patch.object(install, "_sha1_file", side_effect=failing): + self.assertIsNone(index.find(entry)) + found = install.unreadable_copies([entry], {}, [index]) + # scph5500.bin holds another region of the same size: read too. + self.assertEqual([item.line() for item in found], [ + f"{(self.old / 'psx' / 'scph5501.bin').resolve()} could not be read: " + "Input/output error", + f"{(self.old / 'scph5500.bin').resolve()} could not be read: " + "Input/output error"]) + + class ManySameSizedFiles(FromFolder): """A card of 16 KB ROMs and a list of 16 KB entries: each entry sorted and walked every file of its size.""" diff --git a/wiki/installer.md b/wiki/installer.md index 40098811..3b14d35c 100644 --- a/wiki/installer.md +++ b/wiki/installer.md @@ -1191,6 +1191,7 @@ kind, and each kind has exactly the keys below, in this order. The schema is | `system` | `platform`, `system`, `name`, `files`, `present`, `missing`, `wrong_hash`, `bytes_needed` | Each system, sorted by id, and with `--system` only the systems named. Files with no system count under `other`. `name` is the platform's own name for it | | `check` | `platform`, `name`, `bios_dir`, `files`, `present`, `missing`, `wrong_hash`, `bytes_needed`, `data_missing`, `not_installable` | The platform's totals. `data_missing` counts data-tree files to fetch, `not_installable` the files the collection cannot supply | | `found` | `platform`, `dest`, `folder`, `path`, `member`, `reason` | Each file to install that this machine already holds, after `check` and before anything is written, in `--check` and `--dry-run` too. `folder` is the `--from` folder as given, or the `bios_dir` of the check when the BIOS folder holds the file at another place, and then `reason` says where in words (`nested folder: ...` for a misplaced file, `the same file is at ...` for one the list installs there too). `path` is the file, and `member` the ZIP member holding the bytes when `path` is an archive, an empty string otherwise. Its copy is reported by a `downloaded` event | +| `unreadable` | `platform`, `folder`, `path`, `member`, `reason` | Each file in a `--from` folder that could not be read and may be one the run still needs, right after the `found` events: its size is that of a file no copy was found for, or unknown (a folder that could not be listed, a ZIP that would not open). `member` names the ZIP member, or is empty. `reason` says why in words (`Permission denied`, `it is encrypted, a password is needed`) | | `download` | `platform`, `files`, `bytes` | Before the downloads start. Counts the files copied from this machine too | | `progress` | `files_done`, `files_total`, `bytes_done`, `bytes_total` | At most every half second while bytes arrive | | `downloaded` | `dest`, `ok`, `reason`, `size`, `files_done`, `files_total`, `bytes_done`, `bytes_total` | Each finished file, downloaded or copied from where a `found` event named it. `reason` says why one failed, and is empty otherwise |