Repository navigation
fix(tests): open AGC and SceNet scratch streams under a Unicode TEMP on Windows - #13704
Conversation
…on Windows On Windows, std::tmpfile() returns NULL with errno 22 when %TEMP% contains a character outside the ANSI code page. Thirteen more test files still called it: the AGC async log, frame dump, PM4 replay and recorder tests, eight AGC execution tests and the SceNet test. They now use TemporaryFile::Open(), as boykopovar#13104 did for the libc tests. Part of boykopovar#13083 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 replaces remaining test-only std::tmpfile() calls with the repository’s Unicode-safe temporary-file helper; I found no blocking issue in the inspected call sites.
What I ran
Reviewed head 48191a538bcdb91812320df8d38c0c7936b69b18 against PR base bff0289787c98570491c8f175179c8a5651c8017. The fresh merge preview against current main 65f68c1d5cc7ae468b771e9e6c84fc5e9a52e450 was clean. Environment: Debian 6.1 x86_64, GCC 12.2.0. I did not build or run C++ tests locally.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 13704
python3 tools/pr_stack_check.py 13704 --repo boykopovar/AnyPS5Results
| Check | Result |
|---|---|
| Diff and conventions | Passed. |
| Current-base merge preview | Passed; exact head merges cleanly against current main. |
| Hosted CI | Conventions, intake, and labels passed. No hosted build/test job was present in the snapshot. |
| Windows test evidence | The PR description reports a full Windows build and 1,184/1,184 tests on the normal profile, and five unchanged failures on the Unicode profile (the four known path tests plus guest_syslog). This is author-reported, not independently reproduced here. |
Raw local check output
Precheck stdout:
/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13704-48191a538bcd/report.md
/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13704-48191a538bcd/report.json
Conclusion: checks failed
The precheck's ABI and test-quality helper invocations could not run because those workspace-local scripts are absent from the isolated PR checkout. The changed files contain no ABI changes. Its diff and conventions checks passed.
Conventions stdout:
All conventions checks passed.
Sources
- Existing
TemporaryFile::Open()helper at the reviewed base - Updated AGC and SceNet test call sites at the reviewed head
Not verified
I did not independently run the Windows suite or test Linux/macOS. The PR reports that its non-Windows helper path delegates to std::tmpfile(); those platforms therefore retain the prior behavior.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Windows check under a Unicode The CTest entries of
The execution-test files this PR changes need a GPU and were not run here. AI-assisted: yes (Claude Code). |
|
The GPU half of the Unicode
AI-assisted: yes (Claude Code). |
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Validation IDs already present on main (3 IDs in 190 tests)Skipped (10)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 18 |
What
On Windows,
std::tmpfile()returnsNULLwitherrno22 when%TEMP%contains a character outside the ANSI code page, whereGetTempPathAreturns?in its place (#13083). #13104 addedTemporaryFile::Open()and converted the libc, filesystem and trophy tests. Thirteen test files still calledstd::tmpfile(), and the tests that capture output through them abort withcannot open a temporary file for stderror fail theirtemporary filecheck:core/libs/prx/libSceAgcDriver/tests:AsyncLog.cpp,FrameDumpIo.cpp,Pm4Replay.cpp,Recorder.cppcore/libs/prx/libSceAgcDriver/tests/execution:ComputeTableCalls.cpp,DescriptorWebFaults.cpp,GuardedNullPointer.cpp,ImageTable.cpp,LdsMixedWidthAtomics.cpp,PreparedShaders.cpp,SampleMultisampleTarget.cpp,WindowLaneSpill.cppcore/libs/tests/GuestSceNet.cppEach file includes
tests/TemporaryFile.hppand replaces everystd::tmpfile()withTemporaryFile::Open()(18 calls). No other line changes, and no production code changes.core/libs/tests/GuestSyslog.cppis left out again, because #9929 rewrites the lines around its call.Still failing under such a
TEMP, for a later pull request:agc_pipeline_cache_header,agc_driver_lazy_resolved_abiandagc_driver_lazy_resolved_abi_switch_setpass a directory to the driver aspath.string()through_putenv_s, so the driver writes the cache toD:\tmp-unicode\zh_Жinstead ofzh_Ж. The driver readsANYPS5_SHADER_CACHE_DIRwith_wgetenv, so the fix is in the tests.agc_driver_skip_shader_file: the driver readsAPS5_SKIP_SHADER_FILEwithstd::getenv(Execution/src/Driver/Diagnostics.cpp), so the fix needs a driver change.guest_syslog, after fix(libc): restore syslog program-name fallback #9929.Tested
Windows 11 Pro 10.0.26200 (ANSI code page 1252), i7-12700K, NVIDIA GeForce RTX 5070 Ti (driver 621.01), Vulkan SDK 1.4.363.0, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release,
BUILD_TESTING=ON, full build including all AGC execution tests.main93c71cc and this branch were built in the same build directory, and each ran:TEMPandTMPmain93c71ccD:\tmp-unicode\zh_Ж(outside code page 1252)guest_syslogand the four tests aboveTEMPagc_driver_pm4_stall_watchdog_invalid(Attempt to access invalid address, unchanged source, then 200 of 200 passes with--repeat until-fail:200)The last run in each row is of head 48191a5; the earlier runs differ from it only in where
Recorder.cppandImageTable.cppplace the include.The 21 tests that pass under
zh_Жwith this change:agc_async_log,agc_frame_dump_io,agc_pipeline_cache_file,agc_recorder_deferred_failure_report,agc_driver_pm4_replay,agc_driver_pm4_replay_tool(not run onmainbecause its fixture failed),agc_driver_compute_table_calls,agc_driver_descriptor_web_faults,agc_driver_guarded_null_pointer,agc_driver_guarded_direct_root,agc_driver_image_table,agc_driver_lds_lock_budget,agc_driver_sample_multisample_target,agc_driver_window_lane_spill, the sixagc_driver_prepared_shaders_abi_deferred_*tests, andguest_sce_net.was not found but was requested by env var VK_INSTANCE_LAYERS. Every run, onmainand on this branch, under bothTEMPvalues, logs the same 18VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605lines fromagc_driver_prepared_exportsandagc_driver_srgb8_color_target, which this change does not touch. No newVUID-errors.TEMPholds ananyps5-test-*file.python tools/check_conventions.py --base origin/main: all checks passed.python tools/pr_overlap.py --branch claude/tests-unicode-tmpfile-3e21f5: no line conflicts. The first draft added the include at the end of the include block inRecorder.cppandImageTable.cpp, where perf(agc): copy only the vertex records a draw fetches #9875 and fix(agc): span table-reading wave64 programs by default on Intel drivers that share handles across fused threads #10846 add theirs. It now goes after theprx/libcincludes in both files. Comparinggit merge-treeagainst the 70 open pull requests that touch those two files, the move removed those two conflicts and added none. It also listed 34 pull requests that conflict withmainand share a file but could not be compared line by line: feat(agc): retain and refresh sampled depth array storage #1368, feat(agc): add experimental depth clear and copy recognition #1395, feat(aot): consolidate native ARM64 compiler, runtime and consumers #5021, fix(libSceNet): stop Sackboy loading retries on peer sockets #5836, perf(agc): build the pipelines of a submission ahead on a compile pool #5925, fix(agc): retire the window imports that pin a range a guest unmap waits for #6182, fix(agc): copy only the dwords a shader changed back from a GPU-copied buffer #6185, fix(agc): keep CPU stores made after a deferred DCC key store was recorded #6186, perf(agc): an untracked comparison memo holds until memory it covers is forgotten #6187, perf(agc): compile uncached draws ahead of queue 0 #6188, perf(agc): fast revalidation over untracked memory proves a surface by its stamp until the next ordering point #6192, perf(agc): a DCC key proof over untracked memory holds until the next ordering point #6209, fix(recompiler): defer the TYPE check of a guarded read-only static V# #6412, fix(agc): fault image table entries the device or guest memory cannot hold #6481, feat(agc): reuse a native multisample colour target and honour guest HTILE clears #6808, perf(agc): wait at submit only for the preparations of the programs a submission binds #6867, perf(agc): store whole words of 1- and 2-byte texels in the detile shader #6905, perf(shader): keep a binding's element flags inline #6970, fix(agc): sample multisampled targets in read-only layout #6971, fix(agc): save the guest bytes of results that become pending over a surface that is no longer fully write-watched #7048, perf(agc): remember structural host import misses #7457, feat(agc): trace ranges that leave the write watch #7604, fix(shader): bound image-table snapshots by table-key ranges #7684, perf(agc): expand single-key DCC storage uploads on the GPU from the host import #8484, perf(agc): keep draw snapshots in resizable BAR memory #9712, perf(agc): read dropped sampled snapshots straight into their upload staging buffer #9884, fix(agc): follow RDNA 2's range checks for strided vertex fetches with OOB_SELECT 2 and 3 #9915, fix(libSceNet): report the receive queue length in sceNetGetSockInfo #9932, fix(agc): hash an idle unit when its range leaves the write watch instead of at every upload #10026, feat(agc): serve lane-keyed, pointer and runtime image-table columns #10193, fix(agc): read the compressed FMASK of a resident two-sample color target #10233, perf(agc): store an unimported label at once when unfinished work leaves guest memory alone #10442, test(agc): enable VK_KHR_portability_subset in the tests that create their own device #10920, feat(agc): upload varying guest samples of a multisampled texture no draw rendered to #13146.std::tmpfile()calls in new execution tests (UnregisteredLabelOnGpu.cpp,BdaStoreHeapMirror.cpp). They do not conflict with this change, but those tests would fail the same way.TemporaryFile::Open()returnsstd::tmpfile(), which is what these tests called before.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)guest_syslog)