diff --git a/install.ps1 b/install.ps1 index ae967ff4..52ac1e68 100644 --- a/install.ps1 +++ b/install.ps1 @@ -8,7 +8,7 @@ param( $ErrorActionPreference = "Stop" $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" -$defaultInstallSha256 = "6245355aa8a65872b761229f2429ca6e0953573f0b75f6f30286a2f067c83afd" +$defaultInstallSha256 = "22737fbb470a63a8d3630f52ffee1535e57f96a2f1fea969e58368e06ca35f06" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index df299e90..3cde59b4 100644 --- a/install.py +++ b/install.py @@ -2339,11 +2339,17 @@ def _digest_file(path: Path, algorithms: tuple[str, ...]) -> dict[str, str]: every file twice, single threaded. Hashing is cheap next to the I/O, so the read is what has to happen once. """ - hashers = {name: hashlib.new(name) for name in algorithms} with open(path, "rb") as fh: - for chunk in iter(lambda: fh.read(1024 * 1024), b""): - for hasher in hashers.values(): - hasher.update(chunk) + return _digest_handle(fh, algorithms) + + +def _digest_handle(fh: BinaryIO, algorithms: tuple[str, ...]) -> dict[str, str]: + """Digests of an open file, read from its start.""" + hashers = {name: hashlib.new(name) for name in algorithms} + fh.seek(0) + for chunk in iter(lambda: fh.read(1024 * 1024), b""): + for hasher in hashers.values(): + hasher.update(chunk) return {name: hasher.hexdigest() for name, hasher in hashers.items()} @@ -2512,12 +2518,21 @@ def _download_one( f"downloaded {downloaded} bytes, expected {expected_size}" ) - expected_sha256 = f.get("sha256", "") - expected_sha1 = f.get("sha1", "") - if expected_sha256 and _sha256_file(tmp_path) != expected_sha256.lower(): + expected = { + name: f[name].lower() for name in ("sha256", "sha1") if f.get(name) + } + # A large file is hashed through the handle that holds its lock. + # On Windows that lock is mandatory: reopening the path to read + # it failed with a sharing violation, which read as an unwritable + # folder for every file of 16 MB or more (issue 82). + if partial is not None: + actual = _digest_handle(partial, tuple(expected)) + else: + actual = _digest_file(tmp_path, tuple(expected)) + if "sha256" in expected and actual["sha256"] != expected["sha256"]: reason = "SHA-256 mismatch" raise _Corrupt() - if expected_sha1 and _sha1_file(tmp_path) != expected_sha1.lower(): + if "sha1" in expected and actual["sha1"] != expected["sha1"]: reason = "SHA-1 mismatch" raise _Corrupt() @@ -2525,6 +2540,7 @@ def _download_one( # file is: the temporary file is private, and a BIOS folder # written with sudo has to stay readable by the emulator's user. os.chmod(tmp_path, (0o777 if f.get("executable") else 0o666) & ~_UMASK) + _clear_read_only(dest) if partial is not None and os.name == "nt": # Windows renames no open file. Elsewhere the lock is held # until the copy has its final name. @@ -2571,8 +2587,10 @@ def _download_one( except (urllib.error.URLError, OSError, ValueError, http.client.HTTPException) as exc: if (isinstance(exc, OSError) and not isinstance(exc, urllib.error.URLError) and _write_refused(exc)): - # No attempt can change a refused write. - reason, refused = NOT_WRITABLE, True + # No attempt can change a refused write. A read-only file in + # place is not a folder this user cannot write. + reason = READ_ONLY if _read_only_file(dest) else NOT_WRITABLE + refused = True else: reason = str(getattr(exc, "reason", "") or exc) if verbose: @@ -2684,6 +2702,33 @@ def _write_refused(exc: OSError) -> bool: return isinstance(exc, PermissionError) or exc.errno == errno.EROFS +# A file already at the destination, marked read-only, whose content differs, +# and whose attribute could not be cleared: replacing it is refused while its +# folder stays writable. +READ_ONLY = "file in place is read-only" + + +def _clear_read_only(dest: Path) -> None: + """Let the verified copy replace a read-only file the user already has. + + Windows refuses to replace a file carrying the read-only attribute + (WinError 5) even in a writable folder. The file is about to be replaced + by the copy the manifest names, so its attribute is cleared first. + Elsewhere a rename only needs the folder to be writable. + """ + if os.name != "nt": + return + try: + if dest.is_file() and not os.access(dest, os.W_OK): + os.chmod(dest, stat.S_IREAD | stat.S_IWRITE) + except OSError: + pass # the replace that follows reports the refusal with its reason + + +def _read_only_file(dest: Path) -> bool: + return dest.is_file() and not os.access(dest, os.W_OK) and _folder_writable(dest.parent) + + def _folder_writable(path: Path) -> bool: """Whether a file can be created under path, read from its nearest existing folder: a BIOS folder not created yet is as writable as the @@ -2779,6 +2824,9 @@ def _report_outcome( Path(dest).parent for dest, reason in failures or [] if Path(dest).is_absolute() and reason == NOT_WRITABLE })] + if any(reason == READ_ONLY for _dest, reason in failures or []): + advice.append(" Files marked read-only are in the way: clear that attribute " + "(or delete them), then run again.") paths = paths + [folder for folder in elsewhere if folder not in paths] if errors and not downloaded: diff --git a/install.sh b/install.sh index aa69bfa5..360630eb 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="6245355aa8a65872b761229f2429ca6e0953573f0b75f6f30286a2f067c83afd" +DEFAULT_INSTALL_SHA256="22737fbb470a63a8d3630f52ffee1535e57f96a2f1fea969e58368e06ca35f06" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/tests/test_install_experience.py b/tests/test_install_experience.py index cb257561..4a0aaae6 100644 --- a/tests/test_install_experience.py +++ b/tests/test_install_experience.py @@ -377,5 +377,61 @@ class ReviewCases(unittest.TestCase): progress.add(40) self.assertEqual(progress.transferred(), 40) +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 + of 16 MB or more was reported as "folder not writable".""" + + def test_the_locked_partial_is_hashed_through_its_handle(self): + real_open = open + + def locked_open(path, *args, **kwargs): + if str(path).endswith(".part"): + raise PermissionError(33, "The process cannot access the file") + return real_open(path, *args, **kwargs) + + with _Server(), tempfile.TemporaryDirectory() as tmp, \ + mock.patch.object(install, "RESUME_FROM_SIZE", 1024), \ + mock.patch("builtins.open", locked_open): + dest, ok, reason = install._download_one(dict(ENTRY), Path(tmp)) + self.assertEqual((dest, ok, reason), ("big.bin", True, "")) + + def test_a_read_only_file_in_place_is_named_as_such(self): + with _Server(), tempfile.TemporaryDirectory() as tmp, \ + mock.patch.object(install.os, "replace", + side_effect=PermissionError(5, "Access is denied")), \ + mock.patch.object(install, "_read_only_file", return_value=True): + _dest, ok, reason = install._download_one(dict(ENTRY), Path(tmp)) + self.assertFalse(ok) + self.assertEqual(reason, install.READ_ONLY) + + def test_a_read_only_file_in_place_is_made_writable_on_windows(self): + with tempfile.TemporaryDirectory() as tmp: + dest = Path(tmp) / "big.bin" + dest.write_bytes(b"old") + os.chmod(dest, 0o444) + with mock.patch.object(install.os, "name", "nt"): + install._clear_read_only(dest) + self.assertTrue(os.access(dest, os.W_OK)) + + def test_elsewhere_the_attribute_is_left_alone(self): + with tempfile.TemporaryDirectory() as tmp: + dest = Path(tmp) / "big.bin" + dest.write_bytes(b"old") + os.chmod(dest, 0o444) + with mock.patch.object(install.os, "name", "posix"): + install._clear_read_only(dest) + self.assertFalse(os.access(dest, os.W_OK)) + os.chmod(dest, 0o644) + + def test_a_refused_folder_keeps_its_own_reason(self): + with _Server(), tempfile.TemporaryDirectory() as tmp, \ + mock.patch.object(install.os, "replace", + side_effect=PermissionError(13, "Permission denied")), \ + mock.patch.object(install, "_read_only_file", return_value=False): + _dest, ok, reason = install._download_one(dict(ENTRY), Path(tmp)) + self.assertEqual((ok, reason), (False, install.NOT_WRITABLE)) + + if __name__ == "__main__": unittest.main() diff --git a/wiki/troubleshooting.md b/wiki/troubleshooting.md index a740cc82..5cba726f 100644 --- a/wiki/troubleshooting.md +++ b/wiki/troubleshooting.md @@ -256,6 +256,11 @@ The installer needs write access to the target directory. permissions. Avoid running as root unless the target directory requires it. - On Windows: run PowerShell as Administrator if installing to a protected directory. +A file already in the BIOS folder with the read-only attribute is replaced +like any other: on Windows the installer clears the attribute before putting +the verified copy in place. `file in place is read-only` only appears when +Windows refuses that too, which a file owned by another account can cause. + ### Platform not detected `install.py` auto-detects your platform by checking for known config files. If