From b439371579846fe5c3a011977acf3ec7334f311f Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:39:28 +0100 Subject: [PATCH] fix: take a backup folder on one-owner storage --- install.ps1 | 2 +- install.py | 21 ++++++++++++++----- install.sh | 2 +- install/launchers/index.json | 16 +++++++-------- tests/test_install.py | 35 +++++++++++++++++++++++++++----- tests/test_install_experience.py | 15 ++++++++++++++ 6 files changed, 71 insertions(+), 20 deletions(-) diff --git a/install.ps1 b/install.ps1 index 5ea6fd3b7..c85d9c23b 100644 --- a/install.ps1 +++ b/install.ps1 @@ -11,7 +11,7 @@ $ErrorActionPreference = "Stop" $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" - $defaultInstallSha256 = "b3363820bb97bec559287f72d9712281e22307387fa752b663f6f7af023a0e33" + $defaultInstallSha256 = "cc940a96dea46fb43d3940a13fddb4c9f1c02b8d81d1c810bcbddd5120143ce4" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 7acc9cbb4..ae27b2dbf 100644 --- a/install.py +++ b/install.py @@ -4468,15 +4468,19 @@ def _check_holder(holder: Path) -> os.stat_result: A link would send them wherever it points. On a system with user ids, a folder another user owns was not made by this user's runs, and its owner - could read or swap what is moved in. Returns what was checked, so the - caller can confirm the same folder is still there once it has written - into it. + could read or swap what is moved in. A storage that keeps no owner per + file gives every file one owner: Android's shared storage shows a folder + Termux has just made as another user's, as does a FAT card. A holder + owned like the folder it sits in is that storage's owner, not someone + else's folder. Returns what was checked, so the caller can confirm the + same folder is still there once it has written into it. """ info = os.lstat(holder) if not stat.S_ISDIR(info.st_mode): raise OSError(errno.ELOOP, f"{holder} is a link or not a folder") getuid = getattr(os, "getuid", None) - if getuid is not None and info.st_uid != getuid(): + if (getuid is not None and info.st_uid != getuid() + and info.st_uid != os.lstat(holder.parent).st_uid): raise OSError(errno.EPERM, f"{holder} belongs to another user") return info @@ -5497,6 +5501,12 @@ def _report_outcome( "(or delete them), then run again.") if any(isinstance(reason, NetworkReason) for _dest, reason in failures or []): advice.append(NETWORK_FAILURE_ADVICE) + if any(str(reason).startswith(NOT_KEPT) for _dest, reason in failures or []): + # The same run fails the same way: "retry only those" sent a phone + # user round the loop with nothing changed. + advice.append(" The files in place were left as they were, as no backup folder could " + "be made for them. Move them out of the folder, then run again to " + "install the new ones.") paths = paths + [folder for folder in elsewhere if folder not in paths] if offline: @@ -5554,7 +5564,8 @@ def _report_outcome( print(f"Location: {_locations(paths)}") for line in kept + advice: print(line) - print("Run the same command again to retry only those.") + if not all(str(reason).startswith(NOT_KEPT) for _dest, reason in failures or []): + print("Run the same command again to retry only those.") sys.exit(1) print(f"\nDone. {_plural(downloaded, 'file')} installed, " diff --git a/install.sh b/install.sh index 27bcceaf4..2bc9b6703 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="b3363820bb97bec559287f72d9712281e22307387fa752b663f6f7af023a0e33" +DEFAULT_INSTALL_SHA256="cc940a96dea46fb43d3940a13fddb4c9f1c02b8d81d1c810bcbddd5120143ce4" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/install/launchers/index.json b/install/launchers/index.json index c943862b0..8735664d5 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": 734772, - "sha256": "b3363820bb97bec559287f72d9712281e22307387fa752b663f6f7af023a0e33", + "size": 735645, + "sha256": "cc940a96dea46fb43d3940a13fddb4c9f1c02b8d81d1c810bcbddd5120143ce4", "executable": false }, { @@ -71,8 +71,8 @@ "muos": { "archive": { "dest": "ARCHIVE/RetroBIOS.muxapp", - "size": 787321, - "sha256": "45366e0a225d8143f7b359fd04cd6b341a21f43057b7d5a41ad8f734f765f142" + "size": 788194, + "sha256": "3a9bc441d4178cc15833dc8a3f28877370b4075a05131d209208186247756cc7" }, "members": [ { @@ -92,8 +92,8 @@ { "name": "RetroBIOS/install.py", "repo_path": "install.py", - "size": 734772, - "sha256": "b3363820bb97bec559287f72d9712281e22307387fa752b663f6f7af023a0e33", + "size": 735645, + "sha256": "cc940a96dea46fb43d3940a13fddb4c9f1c02b8d81d1c810bcbddd5120143ce4", "executable": false }, { @@ -166,8 +166,8 @@ { "dest": ".config/modules/retrobios/install.py", "repo_path": "install.py", - "size": 734772, - "sha256": "b3363820bb97bec559287f72d9712281e22307387fa752b663f6f7af023a0e33", + "size": 735645, + "sha256": "cc940a96dea46fb43d3940a13fddb4c9f1c02b8d81d1c810bcbddd5120143ce4", "executable": false }, { diff --git a/tests/test_install.py b/tests/test_install.py index 55151f917..72c85866b 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -9,6 +9,7 @@ import importlib.util import io import json import os +import stat import shlex import re import shutil @@ -5737,11 +5738,35 @@ class AFileYouHadIsKept(unittest.TestCase): @unittest.skipUnless(hasattr(os, "getuid"), "user ids are POSIX") def test_a_holder_another_user_owns_is_refused(self): - (self.home / install.BACKUP_DIR).mkdir() - with self.assertRaises(OSError): - with unittest.mock.patch.object(install.os, "getuid", return_value=os.getuid() + 1): - install._check_holder(self.home / install.BACKUP_DIR) - install._check_holder(self.home / install.BACKUP_DIR) + holder = self.home / install.BACKUP_DIR + holder.mkdir() + real = os.lstat + + def planted(path, *args, **kwargs): + info = real(path, *args, **kwargs) + if Path(path) != holder: + return info + fields = list(info) + fields[stat.ST_UID] = info.st_uid + 2 + return os.stat_result(fields) + + with self.assertRaises(OSError), \ + unittest.mock.patch.object(install.os, "lstat", side_effect=planted): + install._check_holder(holder) + install._check_holder(holder) + + @unittest.skipUnless(hasattr(os, "getuid"), "user ids are POSIX") + def test_a_storage_with_one_owner_for_every_file_takes_the_holder(self): + # Android's shared storage, seen from Termux on Android 17: a folder + # the run has just made is shown as another user's, like every other + # file there. Refusing it left 25 files unreplaced on a real phone. + holder = self.home / install.BACKUP_DIR + holder.mkdir() + with unittest.mock.patch.object(install.os, "getuid", return_value=os.getuid() + 1): + install._check_holder(holder) + (self.bios / "x.bin").write_bytes(self.OLD) + self.assertTrue(self._download("x.bin")[1]) + self.assertEqual((self._run_folder() / "system" / "x.bin").read_bytes(), self.OLD) def test_restore_refuses_a_folder_the_installer_did_not_write(self): planted = self.home / install.BACKUP_DIR / "20260101-000000" diff --git a/tests/test_install_experience.py b/tests/test_install_experience.py index b74cfb7d9..34d5a9af8 100644 --- a/tests/test_install_experience.py +++ b/tests/test_install_experience.py @@ -337,6 +337,21 @@ class ProgressOutput(unittest.TestCase): self.assertIn("failed: x.bin (HTTP 404)", out) self.assertIn("2.0 KB downloaded in 0:02", out) + def test_files_not_set_aside_get_a_remedy_not_a_retry(self): + # A real Android run: 25 files whose backup folder was refused, told + # to run again, which refuses them the same way. + reason = f"{install.NOT_KEPT} (/sdcard/RetroArch/retrobios-backup belongs to another user)" + buf = io.StringIO() + with contextlib.redirect_stdout(buf), self.assertRaises(SystemExit) as end: + install._report_outcome( + [Path("/b")], 3, 0, 2, 0, False, + failures=[("a.zip", reason), ("b.zip", reason)], + ) + out = buf.getvalue() + self.assertEqual(end.exception.code, 1) + self.assertIn("The files in place were left as they were", out) + self.assertNotIn("retry only those", out) + class CommandLine(unittest.TestCase): def test_a_dry_run_writes_nothing(self):