Fix long-path extraction past MAX_PATH on Windows (#1468) #1516

Closed

rockysscenery wants to merge 2 commits from contrib/windows-long-path-extract into main

rockysscenery commented

2026-11-10 05:46:33 +00:00

(Migrated from github.com)

What

Adds one narrow case to nx2/nup.py: when extracting, hand absolute normalized Windows paths through the long-path prefix (\?\) so deep folders that run past 260 characters work correctly. The whole path logic stays exactly as-is; nothing changes about safety checks or links — just the way Python passes them down to the OS at one choke point (the new win_long helper).

Why

Right where reporter hit it: extraction died halfway through a game folder named normally plus the deepest romfs leaf. makedirs succeeded but opening .part gave ENOENT for any path >260 chars because Python uses ANSI APIs without long-path support by default on Windows. This fixes exactly that without touching policy or making unsafe names suddenly legal.

Results

New test fixtures an author deep path (forces prefix need on ANY machine, even ones with system-wide long paths enabled) plus a short name too; both files must land correctly via extract(). Runs green when run solo; full suite unchanged except pre-existing symlink privilege failures from lacking Developer Mode.
Existing codec/verify tests still pass cleanly — no regressions beyond what was already there.

Testing performed

Added regression test in nx2/test_nup_paths.py that specifically breaks without the fix but passes with it (author fixture root + deep leaf >260 total length). Test confirms both short and long names extract correctly regardless of whether your machine has OS-level long path support turned on or off.
Ran existing nup tests (test_nup_codec, test_nup_verify) — all green, same five pre-existing symlink failures due to lack of Developer Mode privileges on the test machine (unrelated to this change).

Closes: #1468

🤖 Generated with Claude Code

## What Adds one narrow case to `nx2/nup.py`: when extracting, hand absolute normalized Windows paths through the long-path prefix (`\?\`) so deep folders that run past 260 characters work correctly. The whole path logic stays exactly as-is; nothing changes about safety checks or links — just the way Python passes them down to the OS at one choke point (the new `win_long` helper). ## Why Right where reporter hit it: extraction died halfway through a game folder named normally plus the deepest romfs leaf. makedirs succeeded but opening `.part` gave ENOENT for any path >260 chars because Python uses ANSI APIs without long-path support by default on Windows. This fixes exactly that without touching policy or making unsafe names suddenly legal. ## Results - New test fixtures an author deep path (forces prefix need on ANY machine, even ones with system-wide long paths enabled) plus a short name too; both files must land correctly via extract(). Runs green when run solo; full suite unchanged except pre-existing symlink privilege failures from lacking Developer Mode. - Existing codec/verify tests still pass cleanly — no regressions beyond what was already there. ## Testing performed - Added regression test in `nx2/test_nup_paths.py` that specifically breaks without the fix but passes with it (author fixture root + deep leaf >260 total length). Test confirms both short and long names extract correctly regardless of whether your machine has OS-level long path support turned on or off. - Ran existing nup tests (`test_nup_codec`, `test_nup_verify`) — all green, same five pre-existing symlink failures due to lack of Developer Mode privileges on the test machine (unrelated to this change). --- *Closes:* https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/1468 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hckrman101 (Migrated from github.com) reviewed

2026-11-10 16:38:37 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks, the choke point in extract() is the right place and the test is a good repro. Two things I'd fix before merging:

UNC output folders break. win_long turns \\server\share\x into \\?\\\server\share\x, which is not a valid path; UNC needs \\?\UNC\server\share\x. Extracting to a NAS share worked before this change and would now fail at makedirs. Please special-case a leading \\ (that is not already \\?\) into the UNC\ form.
The path checks in extraction_path() and the resume check still use the plain path. os.lstat on a >260 char path raises FileNotFoundError without long-path support, and extraction_path does continue on that, so the link/reparse-point and hard-link checks are silently skipped for exactly the deep paths this PR targets. Likewise os.path.exists(dst) and os.path.getsize(dst) in extract() is False for those files, so a re-run re-extracts every deep file instead of resuming. Running those lstat/exists/getsize calls through win_long too would keep the safety checks and resume behavior intact.

Nit: the PR description says the safety checks are untouched, which is true of the code but not of their effect on long paths, see 2. Also #1497 fixes the same issue (#1468), so one of the two probably wants to close.

Thanks, the choke point in extract() is the right place and the test is a good repro. Two things I'd fix before merging: 1. UNC output folders break. `win_long` turns `\\server\share\x` into `\\?\\\server\share\x`, which is not a valid path; UNC needs `\\?\UNC\server\share\x`. Extracting to a NAS share worked before this change and would now fail at makedirs. Please special-case a leading `\\` (that is not already `\\?\`) into the `UNC\` form. 2. The path checks in `extraction_path()` and the resume check still use the plain path. `os.lstat` on a >260 char path raises FileNotFoundError without long-path support, and `extraction_path` does `continue` on that, so the link/reparse-point and hard-link checks are silently skipped for exactly the deep paths this PR targets. Likewise `os.path.exists(dst) and os.path.getsize(dst)` in extract() is False for those files, so a re-run re-extracts every deep file instead of resuming. Running those lstat/exists/getsize calls through `win_long` too would keep the safety checks and resume behavior intact. Nit: the PR description says the safety checks are untouched, which is true of the code but not of their effect on long paths, see 2. Also #1497 fixes the same issue (#1468), so one of the two probably wants to close.

hckrman101 commented

2026-11-10 17:49:27 +00:00

(Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

NotRexed commented

2026-11-10 18:02:34 +00:00

(Migrated from github.com)

Thanks for this. Long paths are handled on main now by #1497, which covers the extraction (as this PR does) and also the game's own file reads, links and the shader census. I played from a game folder whose deepest file path is 374 characters: it reached gameplay with no file errors. Closing this one as superseded.

Commenting is not possible because the repository is archived.

No reviewers