From f693cd3ec6d8f536a3485ba92701627ed201c3ac Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:46:38 +0100 Subject: [PATCH] fix: lock a data archive while a run holds it --- install.ps1 | 2 +- install.py | 25 +++++++++++++++++++- install.sh | 2 +- install/launchers/index.json | 16 ++++++------- tests/test_install.py | 44 ++++++++++++++++++++++++++++++++++++ wiki/installer.md | 3 ++- 6 files changed, 80 insertions(+), 12 deletions(-) diff --git a/install.ps1 b/install.ps1 index 28de54233..10ef0b596 100644 --- a/install.ps1 +++ b/install.ps1 @@ -16,7 +16,7 @@ # and Invoke-WebRequest throws on a cut transfer before the hash is read: # the site's build rewrites this line there (scripts/sitebootstrap.py). $mismatchAdvice = "The download was most likely cut short: try again. Within minutes of an update of this project the two files can come from caches of different ages. Waiting five minutes settles that." - $defaultInstallSha256 = "57e3cb6739a701181908773e603f17d2b10e1b0e2a5cde5cc4d5c97426da5dd4" + $defaultInstallSha256 = "1096705acbaa531b7e05458cc58b393426b7e1577d0db62bd2cd29e1c8257ca3" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 04a697af5..e84c01276 100644 --- a/install.py +++ b/install.py @@ -5970,6 +5970,21 @@ def _claim_partial(path: Path) -> BinaryIO | None: return handle +def _release_claim(handle: BinaryIO, path: Path) -> None: + """Free a file taken with _claim_partial and remove it. Removed while + still held where the system allows it, so no other run takes the name + in between. Windows refuses to remove an open file: there it is closed + first, and one another run opened meanwhile is left to that run.""" + if not _NO_RENAME_WHILE_OPEN: + path.unlink(missing_ok=True) + handle.close() + if _NO_RENAME_WHILE_OPEN: + try: + path.unlink(missing_ok=True) + except OSError: + pass # another run holds it now and removes it when done + + class _Unclosed: """The locked partial copy, written through without closing it.""" @@ -7302,6 +7317,8 @@ def install_data_directory(entry: dict, bios_path: Path, return counts _writing.set() fetched: Path | None = None + lock: BinaryIO | None = None + lock_path = root / f".{entry['ref']}.archive.lock" try: root.mkdir(parents=True, exist_ok=True) with tempfile.TemporaryDirectory(dir=root) as scratch: @@ -7311,7 +7328,11 @@ def install_data_directory(entry: dict, bios_path: Path, else: # Beside the tree, outside the scratch folder: a transfer # cut short is resumed by the next attempt or the next run. - kept = root / f".{entry['ref']}.archive.part" + # Two runs into one folder truncated or deleted the archive + # the other was still writing or reading: the run that finds + # it locked downloads into its scratch folder instead. + lock = _claim_partial(lock_path) + kept = root / f".{entry['ref']}.archive.part" if lock is not None else None archive = _fetch_archive(entry["url"], Path(scratch), progress, kept) fetched = archive members = _archive_members(archive, entry) @@ -7351,6 +7372,8 @@ def install_data_directory(entry: dict, bios_path: Path, if fetched is not None: # Read whole: the next run fetches the archive afresh. _drop_archive(fetched, fetched.with_name(fetched.name + ".source")) + if lock is not None: + _release_claim(lock, lock_path) counts["absent"] = len(needed) return counts diff --git a/install.sh b/install.sh index a03428f05..1aa344316 100755 --- a/install.sh +++ b/install.sh @@ -23,7 +23,7 @@ DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/ins # curl or wget stop on a cut transfer before the hash is read: the site's # build rewrites this line there (scripts/sitebootstrap.py). MISMATCH_ADVICE="The download was most likely cut short: try again. Within minutes of an update of this project the two files can come from caches of different ages. Waiting five minutes settles that." -DEFAULT_INSTALL_SHA256="57e3cb6739a701181908773e603f17d2b10e1b0e2a5cde5cc4d5c97426da5dd4" +DEFAULT_INSTALL_SHA256="1096705acbaa531b7e05458cc58b393426b7e1577d0db62bd2cd29e1c8257ca3" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/install/launchers/index.json b/install/launchers/index.json index f6dc104a6..29f624bbe 100644 --- a/install/launchers/index.json +++ b/install/launchers/index.json @@ -27,8 +27,8 @@ { "dest": "roms/ports/retrobios/install.py", "repo_path": "install.py", - "size": 1112438, - "sha256": "57e3cb6739a701181908773e603f17d2b10e1b0e2a5cde5cc4d5c97426da5dd4", + "size": 1113525, + "sha256": "1096705acbaa531b7e05458cc58b393426b7e1577d0db62bd2cd29e1c8257ca3", "executable": false }, { @@ -71,8 +71,8 @@ "muos": { "archive": { "dest": "ARCHIVE/RetroBIOS.muxapp", - "size": 1166065, - "sha256": "1c9e5737a0b32c85108f556c2799cf0ebb5ce52cbd2bf501bc24783fa3a04b9a" + "size": 1167152, + "sha256": "eaed4292f1bcfdbe613e377b5e6b69933819b71c50b1b2d2f162603743d6b17a" }, "members": [ { @@ -92,8 +92,8 @@ { "name": "RetroBIOS/install.py", "repo_path": "install.py", - "size": 1112438, - "sha256": "57e3cb6739a701181908773e603f17d2b10e1b0e2a5cde5cc4d5c97426da5dd4", + "size": 1113525, + "sha256": "1096705acbaa531b7e05458cc58b393426b7e1577d0db62bd2cd29e1c8257ca3", "executable": false }, { @@ -166,8 +166,8 @@ { "dest": ".config/modules/retrobios/install.py", "repo_path": "install.py", - "size": 1112438, - "sha256": "57e3cb6739a701181908773e603f17d2b10e1b0e2a5cde5cc4d5c97426da5dd4", + "size": 1113525, + "sha256": "1096705acbaa531b7e05458cc58b393426b7e1577d0db62bd2cd29e1c8257ca3", "executable": false }, { diff --git a/tests/test_install.py b/tests/test_install.py index 086d9b697..78db13fe4 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -4008,6 +4008,50 @@ class DataDirectoriesAreInstalled(unittest.TestCase): install.install_data_directory(self._entry(), bios) self.assertEqual([p.name for p in (bios / "dolphin-emu").iterdir()], ["Sys"]) + def test_a_second_run_leaves_the_kept_archive_alone(self): + """Two runs into one BIOS folder shared .dolphin-sys.archive.part: the + one that finished first deleted the archive the other was still + writing, and the other truncated the one being read.""" + work = Path(_tempdir(self)) + archive = self._zip(work) + bios = work / "system" + root = bios / "dolphin-emu" + root.mkdir(parents=True) + other = root / ".dolphin-sys.archive.part" + other.write_bytes(b"another run is writing this") + kept: list = [] + + def fetch(url, directory, progress=None, keep=None): + kept.append(keep) + copy = Path(directory) / "private.zip" + shutil.copy(archive, copy) + return copy + + held = install._claim_partial(root / ".dolphin-sys.archive.lock") + self.addCleanup(held.close) + with unittest.mock.patch.object(install, "_fetch_archive", fetch): + counts = install.install_data_directory(self._entry(), bios) + self.assertEqual(kept, [None]) + self.assertEqual(counts["installed"], 1) + self.assertEqual(other.read_bytes(), b"another run is writing this") + self.assertTrue((root / ".dolphin-sys.archive.lock").exists()) + + def test_a_lone_run_keeps_its_archive_beside_the_tree(self): + work = Path(_tempdir(self)) + archive = self._zip(work) + bios = work / "system" + kept: list = [] + + def fetch(url, directory, progress=None, keep=None): + kept.append(keep) + shutil.copy(archive, keep) + return keep + + with unittest.mock.patch.object(install, "_fetch_archive", fetch): + install.install_data_directory(self._entry(), bios) + self.assertEqual(kept, [bios / "dolphin-emu" / ".dolphin-sys.archive.part"]) + self.assertEqual(sorted(p.name for p in (bios / "dolphin-emu").iterdir()), ["Sys"]) + def test_the_run_ends_on_the_archive_bytes_too(self): from test_install_selection import FakeRepository # noqa: PLC0415 diff --git a/wiki/installer.md b/wiki/installer.md index 6378b191a..8dd96352c 100644 --- a/wiki/installer.md +++ b/wiki/installer.md @@ -354,7 +354,8 @@ file list is downloaded once and handed to each run. transfer stops, and the next attempt or the next run requests only the rest. A data tree's archive does the same beside the tree (`..archive.part`), asking with `If-Range` so that a server whose - archive changed sends it whole. A file already there with other contents + archive changed sends it whole. A second run into the same folder finds + that archive in use and downloads its own copy. A file already there with other contents is moved aside first, see [Files you already had](#files-you-already-had). 8. Copies into standalone emulator directories, only with `--standalone-copies`.