fix: keep interrupted ROMFS copies out of the destination #1395

2026-11-10T00:18:12Z

JooaoPedro15 commented

2026-11-10 00:18:12 +00:00

(Migrated from github.com)

When the Builder receives extracted ROMFS folders on a different drive, a hard link fails and it copies each asset. An interrupted copy leaves a partial file at the final path. A retry skips that path because it already exists.

Stage each fallback copy beside its destination and move it into place after the copy and metadata update succeed. Clean up failed stages, including copies that inherited a read-only attribute on Windows. Existing destination assets still follow the Builder's resume policy.

Game-free fixtures simulate a cross-volume disk-full error, then retry and check the complete asset and an existing destination file. A second test simulates a denied replacement of a read-only copy. All ten cases in tests/test_make_game_cache.py passed on macOS. Windows execution is pending CI.

AI assistance from OpenAI Codex is recorded in the commit trailers.

When the Builder receives extracted ROMFS folders on a different drive, a hard link fails and it copies each asset. An interrupted copy leaves a partial file at the final path. A retry skips that path because it already exists. Stage each fallback copy beside its destination and move it into place after the copy and metadata update succeed. Clean up failed stages, including copies that inherited a read-only attribute on Windows. Existing destination assets still follow the Builder's resume policy. Game-free fixtures simulate a cross-volume disk-full error, then retry and check the complete asset and an existing destination file. A second test simulates a denied replacement of a read-only copy. All ten cases in `tests/test_make_game_cache.py` passed on macOS. Windows execution is pending CI. AI assistance from OpenAI Codex is recorded in the commit trailers.

NotRexed commented

2026-11-10 03:50:00 +00:00

(Migrated from github.com)

This landed in the v2.0.12 branch (merged there for testing; it reaches main with that release), so closing. Thanks!

This landed in the v2.0.12 branch (merged there for testing; it reaches `main` with that release), so closing. Thanks!

NotRexed commented

2026-11-10 03:54:56 +00:00

(Migrated from github.com)

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

hckrman101 (Migrated from github.com) reviewed

2026-11-10 04:17:50 +00:00

hckrman101 (Migrated from github.com) left a comment

Nice, small fix, and the approach is right: staging in the destination's own directory keeps the rename on one volume, so it's atomic on both Linux and Windows. os.replace onto an existing file also works on Windows (the exists() guard means it should never have to). mkstemp is O_EXCL so name collisions aren't a concern, the BaseException cleanup covers Ctrl-C, the read-only chmod before unlink on nt is correct, and the tests are picked up by run_all.py's tests/test_*.py glob. Windows CI is green; the ubuntu job was still pending when I looked.

One thing I'd like handled (inline): the cleanup only runs if the process gets to run Python again. The scenario in the description is an interrupted copy, and for a kill/power loss/crash, the .romfs-* stage is left in the romfs tree forever (retry skips it, nothing sweeps it). It's a full-size asset sitting in the game's romfs, and anything that walks that tree will see it. A sweep of stale .romfs-* files at the start of link_or_copy (or in the retry path) would close that.

Nit: no fsync before the replace, so after a power loss the final name can exist with empty or partial data on some filesystems. Probably not worth the cost for 26 GB, just noting.

Overlap with the sibling PRs: #1393 also edits nx2/make_game.py main() (the check_cache_paths/link_or_copy call site, a different hunk from yours) and tests/test_make_game_cache.py; #1397 edits the same test file with a hunk right at the end of file, next to where you add RomfsCopyTests, so expect a textual conflict in the tests whichever lands second. #1391 and #1412 only touch nx2/nup.py and nx2/test_nup_paths.py, so no overlap. No settings option ids involved.

Nice, small fix, and the approach is right: staging in the destination's own directory keeps the rename on one volume, so it's atomic on both Linux and Windows. os.replace onto an existing file also works on Windows (the exists() guard means it should never have to). mkstemp is O_EXCL so name collisions aren't a concern, the BaseException cleanup covers Ctrl-C, the read-only chmod before unlink on nt is correct, and the tests are picked up by run_all.py's tests/test_*.py glob. Windows CI is green; the ubuntu job was still pending when I looked. One thing I'd like handled (inline): the cleanup only runs if the process gets to run Python again. The scenario in the description is an interrupted copy, and for a kill/power loss/crash, the `.romfs-*` stage is left in the romfs tree forever (retry skips it, nothing sweeps it). It's a full-size asset sitting in the game's romfs, and anything that walks that tree will see it. A sweep of stale `.romfs-*` files at the start of link_or_copy (or in the retry path) would close that. Nit: no fsync before the replace, so after a power loss the final name can exist with empty or partial data on some filesystems. Probably not worth the cost for 26 GB, just noting. Overlap with the sibling PRs: #1393 also edits nx2/make_game.py main() (the check_cache_paths/link_or_copy call site, a different hunk from yours) and tests/test_make_game_cache.py; #1397 edits the same test file with a hunk right at the end of file, next to where you add RomfsCopyTests, so expect a textual conflict in the tests whichever lands second. #1391 and #1412 only touch nx2/nup.py and nx2/test_nup_paths.py, so no overlap. No settings option ids involved.

nx2/make_game.py

					
				@ -826,1 +826,3 @@

				                shutil.copy2(s, t)

				                # A retry skips existing targets, so publish the copy only after it is complete.

				                fd, staged = tempfile.mkstemp(prefix=".romfs-", dir=os.path.dirname(t))

				                try:

If the process dies here (SIGKILL, power loss, OOM) the .romfs-XXXX stage is never removed, and the next run won't touch it. Consider deleting leftover .romfs-* files under dst before the loop starts.

If the process dies here (SIGKILL, power loss, OOM) the `.romfs-XXXX` stage is never removed, and the next run won't touch it. Consider deleting leftover `.romfs-*` files under dst before the loop starts.

Commenting is not possible because the repository is archived.

No reviewers