Skip to content

fix(saves): a link to an entry that moved first follows it to the new place - #2832

Draft
nullifyshot wants to merge 2 commits into
NotRexed:mainfrom
nullifyshot:fix/saves-move-link-order
Draft

nullifyshot wants to merge 2 commits into
NotRexed:mainfrom
nullifyshot:fix/saves-move-link-order

Conversation

@nullifyshot

Copy link
Copy Markdown
Contributor

test_linux_saves_move fails now and then on CI's ubuntu part 1/2 ("a link to an entry that moves too: last names another folder", e.g. on #2822, which only touches release scripts), and the same bug can point a player's saves link at a folder that no longer exists after the first start on Linux.

Cause. When user/ moves to $XDG_DATA_HOME/hookshot, copy_link (host/hos_main.c) makes each link again. A link to an entry that moves too is pointed at that entry's new place, but only when realpath resolves it. An absolute link (user/last -> <game>/user/save/<id>) listed after save/ names nothing once save/ has gone. realpath then fails, the link's text is copied unchanged, and the new link names the old place, which is removed at the end of the move. Which entry comes first depends on the file system's directory order, so the check passes on most runners and fails on some.

Fix. The link's text is checked against the folder on the move both as realpath names it and as given, after readlink too. Links written from where they stand and links that name nothing are unchanged.

Test. The case "a link to an entry that moves too" now makes 36 more absolute links to save/<id> (last-a ... last-9), so on any directory order some are listed after save/. Without the fix, those name the old place. The check is Linux only (it skips on Windows), so the run on this PR's ubuntu job is the proof. python -I -B tests/run_all.py on Windows: 419 passed; the 5 that fail (fpx_sticky, bake_portability, input_background, lift_cache, spawn_emit) fail on main on this PC too.

Overlap. #2317 moves this code (copy_link, move_old/move_new) into the per-user data folder work and has the same realpath-only check. Whichever merges second should carry the change across.

Not tested in the game. The Linux move runs once, at a first start with a user/ beside the game, which can't be run on this PC (Windows). Wanted: someone on Linux with a user/ folder holding an absolute symlink to a file in user/save/ to start the game once and check that the link in ~/.local/share/hookshot points inside that folder (readlink).

🤖 Generated with Claude Code

… place

copy_link re-pointed a link to an entry of the moving user/ folder only when realpath resolved it. An absolute link listed
after that entry named nothing by then, so its text was copied as it was and the new link named the removed old place.
The directory order is the file system's, so test_linux_saves_move failed on some CI runners. The link's text is now
checked against the folder as given and as realpath names it; the test makes 36 more such links so some come after save/.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bug NotRexed#2832 fixes was found while fixing another one and had no row; the
work list asks for a scored row for every new problem, owned in the PR that
does the work.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant