Repository navigation
fix(shader): give a device-path shader cache root the long-path form on Windows - #13004
Conversation
…on Windows NativeCachePath_nid_no_patch returned a path that already started with \.\ unchanged. Unlike \?\, a \.\ path is still parsed by Win32 and limited to MAX_PATH, so with ANYPS5_SHADER_CACHE_DIR=\.\C:\... and a moderately deep root (a ~140-character TEMP is enough) the versioned cache entries crossed 260 characters and could not be written. A \.\ path now goes through GetFullPathNameW like the others, and a drive path in that form (\.\X:\...) becomes the equivalent \?\X:\...; other device names are kept as they are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — looks good
The change handles drive paths in the \\.\ namespace while preserving existing extended paths and other device names; I found no blocking issue in the exact head reviewed.
What I ran
- PR head:
35efedd74799182c9e9091d7b7cd1ac649764a24. - Base:
upstream/mainatd820fef1827f27247cd28da7b4eec7891904bae5; the merge preview completed cleanly as treeb24d94e2162b33b9d3c62bd8e65a25a522c960e3. - Environment: Linux x86_64, GCC 12.2.0; no project build or tests run locally.
git diff --check upstream/main...refs/review/pr-13004-head
git merge-tree --write-tree upstream/main refs/review/pr-13004-head
python tools/check_conventions.py --base upstream/mainResults
| Check | Result |
|---|---|
| Patch whitespace | Pass; command exited 0 with no output. |
| Merge preview | Pass; clean merge tree b24d94e2162b33b9d3c62bd8e65a25a522c960e3. |
| Repository conventions | Pass. |
| Source review | Pass; the conversion is limited to normalized \\.\X:\... drive paths; existing \\?\ paths and non-drive device names retain their intended handling. |
| Hosted CI | Eligibility, conventions, intake, labels, Ubuntu and Windows relinker, and ASan passed. macOS relinker jobs were queued at final refresh; product linux and windows jobs were skipped. |
| Windows behavior test | The PR description reports full Windows 11 builds for the base and this head, with agc_shader_prebuilt_discovery_device failing on the base and passing 3/3 on this head under a 140-character TEMP. I did not independently run these tests. |
Unedited local command output
python tools/check_conventions.py --base upstream/main
All conventions checks passed.
git merge-tree --write-tree upstream/main refs/review/pr-13004-head
b24d94e2162b33b9d3c62bd8e65a25a522c960e3
git diff --check upstream/main...refs/review/pr-13004-head exited 0 and produced no output.
Not verified
- I did not build or run tests locally. The Windows test results above are author-reported, not independently reproduced.
- macOS CI was still queued; product Linux and Windows CI jobs were skipped. The changed implementation is Windows-only.
|
Linux run at this PR's exact head Arch Linux, kernel 7.2.9, Ryzen 9 7950X, g++ (GCC) 16.2.1 20260810, glibc 2.44, x86-64. Release,
The change is in the Windows-only device-path branch; this run shows the Linux build and the shader cache tests are unaffected at this head. On Linux CMake registers only the |
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (2, not counted against this PR)Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validation IDs already present on main (4 IDs in 391 tests)Skipped (41)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
Checked the |
What
ShaderRecompiler::NativeCachePath_nid_no_patchturns a cache path into the long-path form (\\?\…or\\?\UNC\…) before the shader cache, pipeline cache and pipeline journal touch it. It returned a path that already started with\\.\unchanged. Unlike\\?\, a\\.\path is still parsed by Win32 and limited toMAX_PATH. SoANYPS5_SHADER_CACHE_DIR=\\.\C:\…, which the prebuilt-discovery test'sdevicemode exercises, could not write entries once a path crossed 260 characters. The versioned layout's entry path crosses it with a root only moderately deep: a 140-character%TEMP%is enough foragc_shader_prebuilt_discovery_deviceto fail withdevice versioned: cannot seed the versioned prebuilt entry.A
\\.\path now goes throughGetFullPathNameWlike the other paths, so.,..and/are resolved as Win32 resolves them for that form. A drive path in that form (\\.\X:\…) then becomes the equivalent\\?\X:\…. Other device names (\\.\pipe\…and so on) are returned asGetFullPathNameWgives them. A path that already starts with\\?\is still returned unchanged.I found this with a full suite run on a 140-character
TEMP. Only this test failed there besides ones already failing onmain(ngs2_pitchshift,guest_filesystem) and one that passed on rerun. Earlier long-path fixes in the same area: #7933, #8554.Tested
Windows 11 Enterprise 10.0.22621, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release,
BUILD_TESTING=ON, no Vulkan driver visible. This head (35efedd747) and its basemain951f374c87, both full builds.main951f374c87agc_shader_prebuilt_discovery_device, 140-characterTEMP,--repeat until-fail:3cannot seed the versioned prebuilt entry)agc_shader_disk_cache_large_filealone, normalTEMPctest -R 'shader_disk_cache|shader_cache|prebuilt_discovery|pipeline_cache|pipeline_journal|pipeline_store' -j8 --repeat until-fail:3(46 tests) under both the 140-character and the normalTEMP: everything that runs here passes, exceptagc_shader_disk_cache_large_file, which hit its 240 s limit in that parallel run under bothTEMPvalues. Alone it passes in 47 s, against 55.8 s onmain. The fouragc_pipeline_cache*/agc_driver_pipeline_journalentries skip without Vulkan.python tools/check_conventions.py --base origin/main: all checks passed.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code