fix: keep why a data tree or a copy failed

A damaged member of a data tree's archive (zlib.error, EOFError) ended
the run on a traceback, and a failed write left its .part. --offline
refusing a data tree, or the download after a --from copy failed, lost
its kind, so the advice named the network instead of --offline.

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 }

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

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

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

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):