fix: tell an unreadable copy from a locked folder

A copy that could not be read when it was copied was reported as a
folder not writable. The source is now opened first and its errors are
its own, and a download failing after it names both reasons. A run
reading a pack falls back to the pack, never to the network.

install.ps1

@@ -11,7 +11,7 @@
 
     $ErrorActionPreference = "Stop"
     $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py"
-    $defaultInstallSha256 = "bbb64b0bf499e079e5c6166c2907834d65a437438aeb4b4511cebc76728b8bb0"
+    $defaultInstallSha256 = "baeaf6acc5a7813df508d997c72ce934fcf3010b953f278426ccb95fd9a4abf8"
     $maximumInstallerBytes = 2MB
     $installerExit = 0
     $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null }

install.py

@@ -4221,9 +4221,17 @@ def _download_one(
         if result[2] in ("stopped", NOT_WRITABLE, READ_ONLY) or result[2].startswith(NOT_KEPT):
             return result
         # The copy went or changed since it was read (a card pulled out, a
-        # file another run replaced): the file is fetched as if never found.
+        # file another run replaced): the file is fetched as if never found,
+        # and a fetch that fails too names both reasons.
         if verbose:
             print(f"    {f['dest']}: {result[2]}, fetching it instead", file=sys.stderr)
+        fetched = _download_one(f, bios_path, verbose, progress)
+        why = fetched[2]
+        if (fetched[1] or why in ("stopped", NOT_WRITABLE, READ_ONLY)
+                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
 
     if LOCAL.pack is not None:
         return _extract_from_pack(f, bios_path, dest, progress)
@@ -6529,7 +6537,7 @@ def _extract_from_pack(
         return f["dest"], False, NOT_IN_PACK
     archive, info = held
     return _write_verified(f, bios_path, dest, lambda: archive.open(info), progress,
-                           "the pack")
+                           "the pack", named=f"{info.filename} inside {archive.filename}")
 
 
 @contextlib.contextmanager
@@ -6549,30 +6557,67 @@ def _copy_local(
     """Copy one file found on this machine to its place, checked like a
     download. Its bytes count as already here, not in the download speed."""
     return _write_verified(f, bios_path, dest, lambda: _open_copy(copy), progress,
-                           f"the copy in {copy.index.label}", local=True)
+                           f"the copy in {copy.index.label}", local=True,
+                           named=copy.named())
+
+
+class _SourceUnreadable(Exception):
+    """The bytes to write could not be read: the source's fault, never the
+    folder's, so it falls back like any copy that failed."""
+
+
+def unreadable_words(exc: BaseException) -> str:
+    """Why a file or a ZIP member could not be read, in words."""
+    if isinstance(exc, RuntimeError) and "encrypted" in str(exc):
+        return "it is encrypted, a password is needed"
+    if isinstance(exc, NotImplementedError):
+        return "it is packed with a compression method this installer cannot read"
+    if isinstance(exc, (zipfile.BadZipFile, zlib.error, EOFError)):
+        return "it is damaged"
+    if isinstance(exc, OSError) and exc.strerror:
+        return exc.strerror
+    return str(exc) or type(exc).__name__
 
 
 def _write_verified(
     f: dict, bios_path: Path, dest: Path,
     source: Callable[[], ContextManager[BinaryIO]], progress: Progress | None,
-    held_in: str, local: bool = False,
+    held_in: str, local: bool = False, named: str = "",
 ) -> tuple[str, bool, str]:
     """Write the bytes source opens at dest once they match the list's size
-    and hashes, the file in place kept aside first when it differs."""
+    and hashes, the file in place kept aside first when it differs.
+
+    The source is opened before the temporary file, and its read errors
+    are told apart from the write errors: a copy that could not be read
+    was reported as a folder not writable. named is the source in the
+    reason then, held_in when empty."""
     expected = {name: f[name].lower() for name in ("sha256", "sha1") if f.get(name)}
     tmp_path: Path | None = None
     counted = 0
+
+    def unreadable(exc: BaseException) -> _SourceUnreadable:
+        return _SourceUnreadable(f"{named or held_in} could not be read: "
+                                 f"{unreadable_words(exc)}")
+
     try:
-        with source() as src, tempfile.NamedTemporaryFile(
-            mode="wb", dir=dest.parent, prefix=f".{dest.name}.", suffix=".part",
-            delete=False,
-        ) as out:
+        with contextlib.ExitStack() as stack:
+            try:
+                src = stack.enter_context(source())
+            except _MEMBER_ERRORS as exc:
+                raise unreadable(exc) from exc
+            out = stack.enter_context(tempfile.NamedTemporaryFile(
+                mode="wb", dir=dest.parent, prefix=f".{dest.name}.", suffix=".part",
+                delete=False,
+            ))
             tmp_path = Path(out.name)
             size = 0
             while True:
                 if _stop.is_set():
                     raise _Interrupted()
-                chunk = src.read(min(1024 * 1024, f["size"] + 1 - size))
+                try:
+                    chunk = src.read(min(1024 * 1024, f["size"] + 1 - size))
+                except _MEMBER_ERRORS as exc:
+                    raise unreadable(exc) from exc
                 if not chunk:
                     break
                 size += len(chunk)
@@ -6598,7 +6643,7 @@ def _write_verified(
         return f["dest"], False, "stopped"
     except _NotKept as exc:
         return f["dest"], False, f"{NOT_KEPT} ({exc})"
-    except (ValueError, *_MEMBER_ERRORS) as exc:
+    except (_SourceUnreadable, ValueError, *_MEMBER_ERRORS) as exc:
         if progress is not None and counted:
             progress.add(-counted, resumed=local)
         if isinstance(exc, OSError) and _write_refused(exc):
@@ -6647,6 +6692,10 @@ class LocalCopy(NamedTuple):
         rel = self.index.relative(self.path)
         return f"{rel}, inside as {self.member}" if self.member else rel
 
+    def named(self) -> str:
+        """The copy by its full path, the member first when it is one."""
+        return f"{self.member} inside {self.path}" if self.member else str(self.path)
+
 
 class FolderIndex:
     """The files under a folder, by name and by size, hashed on demand.

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="bbb64b0bf499e079e5c6166c2907834d65a437438aeb4b4511cebc76728b8bb0"
+DEFAULT_INSTALL_SHA256="baeaf6acc5a7813df508d997c72ce934fcf3010b953f278426ccb95fd9a4abf8"
 MAX_INSTALLER_BYTES=2097152
 
 cleanup() {

tests/test_install_from.py

@@ -13,12 +13,15 @@ from __future__ import annotations
 
 import contextlib
 import copy
+import errno
 import io
 import json
+import os
 import subprocess
 import sys
 import time
 import unittest
+import urllib.error
 import zipfile
 from pathlib import Path
 from unittest import mock
@@ -481,6 +484,70 @@ class MisplacedInTheBiosFolder(Scratch):
             self.assertNotIn("copied from", proc.stdout)
 
 
+def _unprivileged(test: unittest.TestCase) -> None:
+    if os.name != "posix" or os.geteuid() == 0:
+        test.skipTest("a file of mode 000 is read here anyway")
+
+
+class ACopyThatFails(FromFolder):
+    """A copy found by the plan that cannot be read when it is copied:
+    it read as "folder not writable", and a download failing after it lost
+    the first reason."""
+
+    def setUp(self):
+        super().setUp()
+        self.entry = json.loads(self.list.read_text())["files"][0]
+        self.source = self.keep("old/scph5501.bin", US)
+        self.copy = install.FolderIndex(self.old).find(self.entry)
+        self.assertIsNotNone(self.copy)
+        self.addCleanup(install.LOCAL.reset)
+        self.dest.mkdir()
+
+    def test_an_unreadable_source_falls_back_and_names_both_reasons(self):
+        _unprivileged(self)
+        self.source.chmod(0)
+        self.addCleanup(self.source.chmod, 0o644)
+        refused = urllib.error.URLError(ConnectionRefusedError(111, "Connection refused"))
+        with mock.patch.object(install, "_retry_delay", return_value=0), \
+                mock.patch.object(install.urllib.request, "urlopen", side_effect=refused):
+            dest, ok, reason = install._download_one(self.entry, self.dest, copy=self.copy)
+        self.assertFalse(ok)
+        self.assertTrue(reason.startswith(
+            f"{self.source.resolve()} could not be read: Permission denied, and "), reason)
+        self.assertNotIn(install.NOT_WRITABLE, reason)
+        self.assertIsInstance(reason, install.NetworkReason)
+
+    def test_a_read_error_is_the_source_and_not_the_folder(self):
+        class Failing(io.BytesIO):
+            def read(self, *_args):
+                raise OSError(errno.EIO, "Input/output error")
+
+        for source, words in (
+                (lambda: Failing(b""), "Input/output error"),
+                (mock.Mock(side_effect=PermissionError(errno.EACCES, "Permission denied")),
+                 "Permission denied")):
+            with self.subTest(words=words):
+                result = install._write_verified(
+                    self.entry, self.dest, self.dest / "scph5501.bin", source, None,
+                    "the copy in E:\\", named="E:\\bios\\a.bin")
+                self.assertEqual(result, ("scph5501.bin", False,
+                                          f"E:\\bios\\a.bin could not be read: {words}"))
+        self.assertEqual(list(self.dest.iterdir()), [], "no temporary file is left")
+
+    def test_a_run_reading_a_pack_falls_back_to_the_pack_only(self):
+        _unprivileged(self)
+        self.source.chmod(0)
+        self.addCleanup(self.source.chmod, 0o644)
+        install.LOCAL.pack = mock.Mock(find=mock.Mock(return_value=None))
+        with mock.patch.object(install.urllib.request, "urlopen",
+                               side_effect=AssertionError("network")):
+            _dest, ok, reason = install._download_one(self.entry, self.dest, copy=self.copy)
+        install.LOCAL.pack = None
+        self.assertFalse(ok)
+        self.assertEqual(reason, f"{self.source.resolve()} could not be read: "
+                                 f"Permission denied, and {install.NOT_IN_PACK}")
+
+
 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."""