From eb99131a6efa9419049393c75fbf52b15e6f116f Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 08:45:03 +0100 Subject: [PATCH] fix: stop advising --save-dir a list cannot serve RetroPie, ROCKNIX, AmberELEC, EmuELEC and RomM keep their save directory in no fixed place, and their lists leave those files out with no hash or address. The run said to name the folder with --save-dir, and said it again when --save-dir was given. It now says the list holds none of them. --- install.ps1 | 2 +- install.py | 20 +++++++++++++++++-- install.sh | 2 +- install/launchers/index.json | 16 +++++++-------- tests/test_install.py | 24 +++++++++++++++++++++++ tests/test_install_tui.py | 38 +++++++++++++++++++++++++++++++++++- wiki/installer.md | 5 ++++- 7 files changed, 93 insertions(+), 14 deletions(-) diff --git a/install.ps1 b/install.ps1 index dab7b3426..412a6456e 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 = "7622c5e1a9ec7a76bf3c325b1fd356a22b53aefe0b625d6c12447afee9414300" + $defaultInstallSha256 = "3aa392c9c4fd50ab2a8474bb71ef11be45945964fb147b2ade45296461c87f51" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 429dffa35..feabcc09e 100644 --- a/install.py +++ b/install.py @@ -6456,7 +6456,8 @@ def _report_outcome( kept = _kept_lines() + ([ f"{_plural(len(unplaced), 'file')} read from " f"{unplaced[0].get('frontend') or 'the frontend'}'s save directory left out: " - f"its place is not known. {LINE_SAVE_REMEDY}"] if unplaced else []) + + (f"{unplaced[0]['save_why']}." if unplaced[0].get("save_why") + else f"its place is not known. {LINE_SAVE_REMEDY}")] if unplaced else []) kept += list(notes or []) if interrupted: say(f"\nStopped. {_plural(downloaded, 'file')} installed.") @@ -8636,6 +8637,13 @@ def saves_line(display: str, count: int, where: str) -> str: f"{shown_path(folder)}") +def unlisted_saves_words(display: str) -> str: + """Why a file list holds none of the files read from the save + directory: the platform keeps it in no fixed place, and the list leaves + them out with no hash or address, which --save-dir cannot change.""" + return f"{display} keeps it in no fixed place, and its file list holds none of them" + + def unplaced_line(display: str, count: int, remedy: str) -> str: """The files read from an unknown save directory, left out.""" return (f"{_plural(count, 'file')} read from {display}'s save directory left out: " @@ -15395,6 +15403,10 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: display = display_name(plat_name, manifest) files, unplaced = place_save_entries( files, bios_path, manifest, args.save_dir, display, say=False) + if not manifest.get("save_directory"): + # Its own list leaves these files out: naming the folder with + # --save-dir installs none of them, and was advised in a loop. + _SAVE_WHY[bios_path] = unlisted_saves_words(display) # The games of the card, read before the choices: where it holds # some, what they need is the menu's first choice, and --for-games # narrows the list here, before the systems are counted. @@ -15597,7 +15609,8 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: total_omitted += len(omitted_files) # Named for the end of the run: ROCKNIX's own list leaves them out, # and the end said "the frontend's save directory". - omitted_seen.extend({**entry, "frontend": display} + omitted_seen.extend({**entry, "frontend": display, + "save_why": unlisted_saves_words(display)} if entry.get("reason") == "no_save_directory" and not entry.get("frontend") else entry for entry in omitted_files) @@ -21212,6 +21225,9 @@ def _unplaced_lines(summary: dict | None, state: TuiState, if not count: return [] name = display_name(state.platform) if state.platform else "the frontend" + if state.manifest and not state.manifest.get("save_directory"): + return [f"{_plural(count, 'file')} read from {name}'s save directory left out: " + f"{unlisted_saves_words(name)}."] return [f"{_plural(count, 'file')} read from {name}'s save directory left out: " f"its place is not known. {remedy or TUI_SAVE_REMEDY}"] diff --git a/install.sh b/install.sh index cbdaed395..c8cba304a 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="7622c5e1a9ec7a76bf3c325b1fd356a22b53aefe0b625d6c12447afee9414300" +DEFAULT_INSTALL_SHA256="3aa392c9c4fd50ab2a8474bb71ef11be45945964fb147b2ade45296461c87f51" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/install/launchers/index.json b/install/launchers/index.json index 2be9a4f89..cd32da514 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": 1106280, - "sha256": "7622c5e1a9ec7a76bf3c325b1fd356a22b53aefe0b625d6c12447afee9414300", + "size": 1107258, + "sha256": "3aa392c9c4fd50ab2a8474bb71ef11be45945964fb147b2ade45296461c87f51", "executable": false }, { @@ -71,8 +71,8 @@ "muos": { "archive": { "dest": "ARCHIVE/RetroBIOS.muxapp", - "size": 1159907, - "sha256": "6460af3bc1a9d27c7373b42cf45f19bf8619b8e55daac9892af9f57df21b258e" + "size": 1160885, + "sha256": "5ccc4ec3df34a1fef96caea9e84218de1fb92331664047c36d6863c11973dc3d" }, "members": [ { @@ -92,8 +92,8 @@ { "name": "RetroBIOS/install.py", "repo_path": "install.py", - "size": 1106280, - "sha256": "7622c5e1a9ec7a76bf3c325b1fd356a22b53aefe0b625d6c12447afee9414300", + "size": 1107258, + "sha256": "3aa392c9c4fd50ab2a8474bb71ef11be45945964fb147b2ade45296461c87f51", "executable": false }, { @@ -166,8 +166,8 @@ { "dest": ".config/modules/retrobios/install.py", "repo_path": "install.py", - "size": 1106280, - "sha256": "7622c5e1a9ec7a76bf3c325b1fd356a22b53aefe0b625d6c12447afee9414300", + "size": 1107258, + "sha256": "3aa392c9c4fd50ab2a8474bb71ef11be45945964fb147b2ade45296461c87f51", "executable": false }, { diff --git a/tests/test_install.py b/tests/test_install.py index 658385670..b8c72ae82 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -482,6 +482,30 @@ class ADestThatIsAFileIsNamed(unittest.TestCase): self.assertEqual((root / "scph5501.bin").read_bytes(), b"x") +class SaveFilesAListHoldsNoneOfAreNotPromised(unittest.TestCase): + """RetroPie, ROCKNIX, AmberELEC, EmuELEC and RomM keep their save + directory in no fixed place: their lists leave those files out with no + hash or address. The run said "Name it with --save-dir", and said it + again with --save-dir given, which installs none of them.""" + + def test_the_reason_is_said_and_no_option_is_offered(self): + manifest = json.loads((REPO_ROOT / "install" / "retropie.json").read_text()) + self.assertIsNone(manifest.get("save_directory")) + root = Path(_tempdir(self)) + env = dict(os.environ, RETROBIOS_OS="linux") + for extra in ([], ["--save-dir", str(root / "saves")]): + with self.subTest(extra=extra): + proc = _install_run( + env, "--platform", "retropie", "--dest", str(root / "bios"), + "--manifest", "install/retropie.json", "--offline", "--no-input", + "--yes", "--emulator", "azahar", *extra) + out = proc.stdout + proc.stderr + self.assertIn("read from RetroPie's save directory left out: RetroPie " + "keeps it in no fixed place, and its file list holds none " + "of them.", out) + self.assertNotIn("Name it with --save-dir", out) + + class AFolderThatRefusesWritesIsNamed(unittest.TestCase): """A read-only BIOS folder ended the run on a traceback (a subfolder to create), or on three retries per file and "once that is resolved", and a diff --git a/tests/test_install_tui.py b/tests/test_install_tui.py index 7e0b2e0b2..988b7234e 100644 --- a/tests/test_install_tui.py +++ b/tests/test_install_tui.py @@ -2967,12 +2967,44 @@ class SaveDirectoryLeftOut(unittest.TestCase): UNPLACED = {"reason": "no_save_directory", "label": "no save directory", "files": 2, "required": 0} + def _ended(self, manifest: dict) -> tuple: + state = fresh_state() + state.stack = ["setup"] + with mock.patch.object(install.Path, "exists", return_value=True): + install.tui_update(state, Key("enter")) + install.tui_receive(state, ("manifest", "retroarch", manifest)) + summary = {"event": "summary", "mode": "check", "installed": 0, "up_to_date": 0, + "pending": 5, "failed": [], "skipped": [self.UNPLACED], "left_out": [], + "kept": [], "interrupted": False} + for event in [*FILES, *SYSTEMS, CHECK, summary]: + install.tui_receive(state, ("event", state.run_id, event)) + with mock.patch.object(install, "_tui_folder_facts", return_value={ + "free": 50 * 1024 ** 3, "writable": True, "cores": []}): + install.tui_receive(state, ("ended", state.run_id, 1, [])) + return state, summary + + def test_a_list_that_holds_none_of_them_offers_no_folder(self): + """RetroPie's list leaves its save files out with no hash or address: + --save-dir installs none of them, and was advised again each run.""" + state, summary = self._ended(MANIFEST) + line = ("2 files read from RetroArch's save directory left out: RetroArch keeps " + "it in no fixed place, and its file list holds none of them.") + plan = " ".join(" ".join(install.tui_view(state, 120, 40)).split()) + self.assertIn(line, plan) + self.assertNotIn("--save-dir", plan) + state.stack.append("result") + state.code = 0 + state.summary = {**summary, "mode": "install", "installed": 5} + self.assertEqual([text for text in install.tui_outcome(state)[1] if "save" in text], + [line]) + def test_files_left_out_of_the_save_directory_are_said(self): state = fresh_state() state.stack = ["setup"] with mock.patch.object(install.Path, "exists", return_value=True): install.tui_update(state, Key("enter")) - install.tui_receive(state, ("manifest", "retroarch", MANIFEST)) + install.tui_receive(state, ("manifest", "retroarch", { + **MANIFEST, "save_directory": {"folder": "saves", "by_core": True}})) summary = {"event": "summary", "mode": "check", "installed": 0, "up_to_date": 0, "pending": 5, "failed": [], "skipped": [self.UNPLACED], "left_out": [], "kept": [], "interrupted": False} @@ -3834,6 +3866,8 @@ class BackTicksAndSaveFolder(unittest.TestCase): def test_the_save_folder_remedy_is_the_same_on_both_screens(self): summary = {"skipped": [{"reason": "no_save_directory", "files": 1}]} state = checked_state() + state.manifest = {**state.manifest, "save_directory": {"folder": "saves", + "by_core": True}} lines = install._unplaced_lines(summary, state) self.assertIn("Options, Save folder (--save-dir)", lines[0]) @@ -11846,6 +11880,8 @@ class OneChoicePerRun(unittest.TestCase): def test_the_farewell_names_the_option_not_the_screen(self): state = _installed_state(SYSTEM_DIR) + state.manifest = {**state.manifest, "save_directory": {"folder": "saves", + "by_core": True}} state.summary = {**state.summary, "skipped": [ {"reason": "no_save_directory", "label": "", "files": 1, "required": False}]} farewell = " ".join(install.tui_farewell(state)) diff --git a/wiki/installer.md b/wiki/installer.md index f66a8eb54..0d6e5141b 100644 --- a/wiki/installer.md +++ b/wiki/installer.md @@ -386,7 +386,10 @@ the save directory beside it (`saves`, `/storage/savefiles` on Lakka). applies, or when `savefiles_in_content_dir` or `sort_savefiles_by_content_enable` keeps saves beside each game, those files are left out and the run says why, like the other entries the list cannot -install. The full screen says so on its plan and its result. +install. The full screen says so on its plan and its result. RetroPie, ROCKNIX, +AmberELEC, EmuELEC and RomM keep the save directory in no fixed place: their +lists hold none of these files, so `--save-dir` installs none, and the run says +that instead of naming the option. Batocera, KNULLI, Recalbox, RetroBat and RetroDECK write the save directory at each launch, one folder per system (`by_system` in the file list). Their lists