diff --git a/install.ps1 b/install.ps1 index 2bdb4e8d..3224f28a 100644 --- a/install.ps1 +++ b/install.ps1 @@ -11,7 +11,7 @@ $ErrorActionPreference = "Stop" $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" - $defaultInstallSha256 = "7065a9d52aa9016066c273ec8a5907b09fcce7eaad5a423583c79515f09edb99" + $defaultInstallSha256 = "079fcba3b83003932af8f5ed513e556675832ab20e55bbbbc356581b2daf1a14" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 33955194..de751a65 100644 --- a/install.py +++ b/install.py @@ -4497,7 +4497,9 @@ def _download_one( or why.startswith(NOT_KEPT)): return fetched both = f"{result[2]}, and {why}" - return fetched[0], False, NetworkReason(both) if isinstance(why, NetworkReason) else both + # The fetch's kind of failure stays, so the advice is the right one. + kind = type(why) if isinstance(why, (NetworkReason, OfflineReason)) else str + return fetched[0], False, kind(both) if LOCAL.pack is not None: return _extract_from_pack(f, bios_path, dest, progress) @@ -5599,40 +5601,62 @@ def install_data_directory(entry: dict, bios_path: Path) -> dict: continue target = _destination_path(root, rel) target.parent.mkdir(parents=True, exist_ok=True) - digest = hashlib.sha1() - size = 0 - with opener() as src, tempfile.NamedTemporaryFile( - dir=target.parent, prefix=f".{target.name}.", suffix=".part", - delete=False, - ) as out: - while size <= meta["size"]: - chunk = src.read(min(1024 * 1024, meta["size"] + 1 - size)) - if not chunk: - break - digest.update(chunk) - size += len(chunk) - out.write(chunk) - if digest.hexdigest() == meta["sha1"] and size == meta["size"]: - slot = f"{entry['dest']}/{rel}" if entry.get("dest") else rel - try: - KEPT.keep(*_kept_slot(bios_path, slot), target, - {"sha1": meta["sha1"]}) - except _NotKept as exc: - Path(out.name).unlink(missing_ok=True) - counts["not_kept"].append((rel, str(exc))) - continue - os.replace(out.name, target) - counts["installed"] += 1 - else: - Path(out.name).unlink(missing_ok=True) - counts["changed"] += 1 - except (urllib.error.URLError, OSError, ValueError, zipfile.BadZipFile, - tarfile.TarError) as exc: + outcome = _install_data_member( + opener, meta, target, bios_path, + f"{entry['dest']}/{rel}" if entry.get("dest") else rel, + rel, counts["not_kept"]) + if outcome: + counts[outcome] += 1 + except _Offline as exc: + # Kept apart, so the advice is the offline one, not the network's. + counts["error"] = OfflineReason(str(exc)) + except (urllib.error.URLError, OSError, ValueError, tarfile.TarError) as exc: counts["error"] = str(getattr(exc, "reason", None) or exc) + except _MEMBER_ERRORS as exc: + # A damaged, encrypted or unsupported member, in words. + counts["error"] = unreadable_words(exc) counts["absent"] = len(needed) return counts +def _install_data_member( + opener: Callable[[], ContextManager[BinaryIO]], meta: dict, target: Path, + bios_path: Path, slot: str, rel: str, not_kept: list[tuple[str, str]], +) -> str: + """Write one member of a data tree's archive at target once its bytes + are the recorded ones: "installed", "changed" when they are not, or "" + when the file in place could not be moved aside (named in not_kept). + A read or write error is raised, and leaves no .part behind.""" + scratch: Path | None = None + try: + digest = hashlib.sha1() + size = 0 + with opener() as src, tempfile.NamedTemporaryFile( + dir=target.parent, prefix=f".{target.name}.", suffix=".part", delete=False, + ) as out: + scratch = Path(out.name) + while size <= meta["size"]: + chunk = src.read(min(1024 * 1024, meta["size"] + 1 - size)) + if not chunk: + break + digest.update(chunk) + size += len(chunk) + out.write(chunk) + if digest.hexdigest() != meta["sha1"] or size != meta["size"]: + return "changed" + try: + KEPT.keep(*_kept_slot(bios_path, slot), target, {"sha1": meta["sha1"]}) + except _NotKept as exc: + not_kept.append((rel, str(exc))) + return "" + os.replace(scratch, target) + scratch = None + return "installed" + finally: + if scratch is not None: + scratch.unlink(missing_ok=True) + + def select_data_directories(manifest: dict, files: list[dict], narrowed: bool) -> list[dict]: """The data trees this install keeps: all, or those of a kept system.""" entries = manifest.get("data_directories") or [] diff --git a/install.sh b/install.sh index 559077c8..1cc244ce 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="7065a9d52aa9016066c273ec8a5907b09fcce7eaad5a423583c79515f09edb99" +DEFAULT_INSTALL_SHA256="079fcba3b83003932af8f5ed513e556675832ab20e55bbbbc356581b2daf1a14" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/tests/test_install.py b/tests/test_install.py index 4da52996..b7cb3285 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -3248,6 +3248,41 @@ class DataDirectoriesAreInstalled(unittest.TestCase): self.assertEqual(counts["changed"], 1) self.assertLess(Endless.served, 64 * 1024) + def test_a_damaged_member_is_a_reason_and_leaves_no_part(self): + """zlib.error and EOFError from a damaged member ended the run on a + traceback, and a failed write left its .part beside the tree.""" + import zlib + + work = Path(_tempdir(self)) + entry = self._entry() + entry["files"] = {"Sys/GC/font.bin": {"sha1": "1" * 40, "size": 4}} + for error in (zlib.error("invalid stored block lengths"), EOFError(), + OSError(28, "No space left on device")): + with self.subTest(error=type(error).__name__): + class Broken(io.RawIOBase): + def readable(self): + return True + + def readinto(self, buffer, error=error): + raise error + + 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: Path(d) / "a"), \ + unittest.mock.patch.object(install, "_archive_members", members): + counts = install.install_data_directory(entry, work) + self.assertTrue(counts["error"]) + self.assertEqual(counts["installed"], 0) + self.assertEqual([p.name for p in work.rglob("*.part")], []) + + def test_offline_keeps_its_reason(self): + work = Path(_tempdir(self)) + with unittest.mock.patch.dict(os.environ, {install.OFFLINE_VARIABLE: "1"}): + counts = install.install_data_directory(self._entry(), work) + self.assertIsInstance(counts["error"], install.OfflineReason) + def test_a_tarball_maps_through_source_path(self): import io as _io import tarfile diff --git a/tests/test_install_from.py b/tests/test_install_from.py index b4aef6ca..2f5f2880 100644 --- a/tests/test_install_from.py +++ b/tests/test_install_from.py @@ -520,6 +520,18 @@ class ACopyThatFails(FromFolder): self.assertNotIn(install.NOT_WRITABLE, reason) self.assertIsInstance(reason, install.NetworkReason) + def test_offline_the_fallback_keeps_the_offline_reason(self): + """The copy failed, the download was refused by --offline: the + joined reason lost its kind, and the advice named neither.""" + _unprivileged(self) + self.source.chmod(0) + self.addCleanup(self.source.chmod, 0o644) + with mock.patch.dict(os.environ, {install.OFFLINE_VARIABLE: "1"}): + _dest, ok, reason = install._download_one(self.entry, self.dest, copy=self.copy) + self.assertFalse(ok) + self.assertIsInstance(reason, install.OfflineReason) + self.assertIn("could not be read: Permission denied, and not fetched", reason) + def test_a_read_error_is_the_source_and_not_the_folder(self): class Failing(io.BytesIO): def read(self, *_args):