fix: measure free space on the NUP extraction volume #1391

Merged

JooaoPedro15 merged 1 commit from fix/nup-extract-destination-space into main

2026-11-10 04:25:54 +00:00

JooaoPedro15 commented

2026-11-10 00:15:08 +00:00

(Migrated from github.com)

nup.py --extract out/new/nested --only romfs aborts with FileNotFoundError when out/new does not exist. The free-space message queries the immediate parent before the extractor can create it. For an existing destination on a mount or junction, the same lookup measures the parent volume.

Resolve the extraction destination and measure the nearest existing directory. The game-free CLI regression exercises an existing destination and one with two missing levels. Both cases check where free space is measured and that extraction writes the fixture.

All 22 cases in nx2/test_nup_paths.py passed on macOS. This change improves the destination-space check. The cause of the disk exhaustion reported in #1388 still needs a reproduction.

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

`nup.py --extract out/new/nested --only romfs` aborts with `FileNotFoundError` when `out/new` does not exist. The free-space message queries the immediate parent before the extractor can create it. For an existing destination on a mount or junction, the same lookup measures the parent volume. Resolve the extraction destination and measure the nearest existing directory. The game-free CLI regression exercises an existing destination and one with two missing levels. Both cases check where free space is measured and that extraction writes the fixture. All 22 cases in `nx2/test_nup_paths.py` passed on macOS. This change improves the destination-space check. The cause of the disk exhaustion reported in #1388 still needs a reproduction. AI assistance from OpenAI Codex is recorded in the commit trailers.

NotRexed commented

2026-11-10 03:50:06 +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:48 +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) approved these changes

2026-11-10 04:18:13 +00:00

hckrman101 (Migrated from github.com) left a comment

The test is good: it asserts where disk_usage is called for both an existing and a two-levels-missing destination, and that the file is written. The self.out.resolve() vs os.path.realpath comparison should hold on Windows too (both resolve 8.3 names). nx2/test_nup_paths.py is picked up by run_all.py's nx2/test_*.py glob, so nothing to register.

Overlaps, none blocking:

#1412 also edits nx2/nup.py (inside extract(), not main()) and adds tests to nx2/test_nup_paths.py (an extra import errno and a new test method further down the class). Different hunks from this one, so I'd expect them to merge cleanly in either order, but whichever lands second may need a trivial rebase of the test file.
#1393, #1395 and #1397 only touch nx2/make_game.py and tests/test_make_game_cache.py. No conflict with this PR. #1393 wraps the nx2/nup.py --extract call in try/except; that is independent of this change.
#1392 only touches scripts/prof.py and tests/test_profile_stream.py, unrelated.
No settings option ids involved.

Nit: the free-space message is only informational, so nothing enforces it. It's fine as is, just noting that a too-small volume still fails at write time (which #1412/#1393 handle by cleaning up).

CI was still pending on the Python source checks (ubuntu and windows) when I looked, so worth a glance before merging.

Looks good. The old line asked for the free space of the immediate parent, which fails (FileNotFoundError from disk_usage) when out/new doesn't exist yet. Resolving the destination and walking up to the nearest existing directory fixes that. realpath also follows a symlink or junction at the destination, so the volume you report is the one the writes land on. extraction_path() already does the same realpath of the root, so the two agree. The loop terminates at the filesystem root, and shutil.disk_usage works the same on Windows and Linux (GetDiskFreeSpaceEx vs statvfs), so nothing platform-specific here. The test is good: it asserts where disk_usage is called for both an existing and a two-levels-missing destination, and that the file is written. The `self.out.resolve()` vs `os.path.realpath` comparison should hold on Windows too (both resolve 8.3 names). nx2/test_nup_paths.py is picked up by run_all.py's nx2/test_*.py glob, so nothing to register. Overlaps, none blocking: - #1412 also edits nx2/nup.py (inside extract(), not main()) and adds tests to nx2/test_nup_paths.py (an extra `import errno` and a new test method further down the class). Different hunks from this one, so I'd expect them to merge cleanly in either order, but whichever lands second may need a trivial rebase of the test file. - #1393, #1395 and #1397 only touch nx2/make_game.py and tests/test_make_game_cache.py. No conflict with this PR. #1393 wraps the `nx2/nup.py --extract` call in try/except; that is independent of this change. - #1392 only touches scripts/prof.py and tests/test_profile_stream.py, unrelated. - No settings option ids involved. Nit: the free-space message is only informational, so nothing enforces it. It's fine as is, just noting that a too-small volume still fails at write time (which #1412/#1393 handle by cleaning up). CI was still pending on the Python source checks (ubuntu and windows) when I looked, so worth a glance before merging.

Commenting is not possible because the repository is archived.

No reviewers

hckrman101