Repository navigation
fix(relinker): reject an existing directory at any output path before writing - #11084
Conversation
… writing Only the main executable output was checked for being a directory. A directory at a guest module, extra executable, registry, needed-library, patch manifest, icon, desktop entry or diagnostics path failed only when it was opened, in most cases after earlier outputs had been written. checkOutputInputs now rejects an existing directory for every output kind it checks, as 286cf28 did for the main executable. Such runs still exit 2, now before any output is written.
|
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.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the output-path preflight and regression cases at this exact head. The directory check runs through the existing all-output validation before any output writes; the cases verify exit status, diagnostic, and unchanged tree for Linux and Windows output variants. I independently built the relinker-only target with GCC 12.2 and ran ctest --test-dir build-review -R '^output_collisions$' --output-on-failure; it passed 1/1. Hosted eligibility, conventions, intake, and the Linux, Windows, macOS ARM, macOS x86_64 and ASan relinker checks pass. Product/library build jobs are skipped by path policy.
What
Only the main executable output is checked for being an existing directory before anything is written (
main.cpp:276, from #5359). A directory at any other output path passes every check incheckOutputInputs, whose only early return is for a path that does not exist, and fails only whenFileWriter(Cannot open output file) or the desktop entry writer (Failed to write icon,Failed to write desktop entry) opens it. On untouched 4017310, 13 of the 15 cases in the new test exited 2 after writing earlier outputs: for example a directory atoutput.registry.jsonleftoutput.elfandapp0/sce_module/sample.prx.guest.prx, and a directory atoutput.elf.desktopleft those plusoutput.elf.pngand the four registry and needed-library files.checkOutputInputs(core/relinker/main.cpp:349) throws<kind> output is a directory: <path>right after itsexists()early return, using the labels it already has: Executable (extra--executableoutputs; a directory at the main output is still caught first by line 276), Icon, Desktop entry, Windows import diagnostics, Registry, Needed libraries, Patch manifest, Guest module. Every output path goes through it before the first write, so such a run still exits 2 but writes nothing. The test isis_directory, as in fix(relinker): reject a directory as the executable output path #5359. Successful conversions and their output bytes do not change, because a directory at an output path never converted.docs/user/USAGE.mdgets one sentence in the existing collisions paragraph.test_output_collisions.pygetsdirectory_outputs: for Linux the guest module, registry, module registry, needed, module needed, icon, desktop entry and an--executable extra/b.binoutput, and for--windows --windows-diagnosticsthe same without icon and desktop entry plus the diagnostics file. Each case creates a directory at that path and asserts exit 2,FAIL: <kind> output is a directory:, the blocked file name in stderr, and an unchanged tree. On untouched code the Linux guest-module and Windows diagnostics cases already wrote nothing (they are the first write) and differ only in the message; the other 13 left partial outputs. The patch manifest goes through the same check but is not in the test, because it needs the patch-file fixtures.pr_overlap.pyreports no shared lines with open pull requests that merge withmain. It cannot compare this branch with feat(tools): add a graphical launcher #3786, feat(aot): consolidate native ARM64 compiler, runtime and consumers #5021, perf(agc): build the pipelines of a submission ahead on a compile pool #5925, fix(tools): recognize guest module identities in import audit #6171, perf(recompiler): keep runtime-specialized shader modules in the shader disk cache #6924, feat(agc): look up table images at run time through a device image table on the GPU heap (APS5_GPU_PATH) #7642, feat(libSceUserService): take the user name from ANYPS5_USER_NAME #9345, feat(relinker): trace function imports on macOS #9408 and feat(agc): serve lane-keyed, pointer and run-time-address image table columns through the device image table #10193 (docs/user/USAGE.md), because those do not merge withmain. Checked by hand: this PR adds one sentence to the output collision paragraph (line 56); the others edit other sections: feat(aot): consolidate native ARM64 compiler, runtime and consumers #5021 the macOS output example and packaging text (lines 36 to 41 of its base), perf(agc): build the pipelines of a submission ahead on a compile pool #5925 the new pipeline journal and submission lookahead sections (line 167), fix(tools): recognize guest module identities in import audit #6171 the module dependency paragraph of the audit section (line 384), perf(recompiler): keep runtime-specialized shader modules in the shader disk cache #6924 the shader cache paragraphs (line 282), feat(agc): look up table images at run time through a device image table on the GPU heap (APS5_GPU_PATH) #7642 and feat(agc): serve lane-keyed, pointer and run-time-address image table columns through the device image table #10193 the pipeline journal paragraph and a new GPU descriptor heap section (lines 294 to 326), feat(libSceUserService): take the user name from ANYPS5_USER_NAME #9345 a new user name section (line 236), feat(tools): add a graphical launcher #3786 a new graphical launcher section (after line 243) and feat(relinker): trace function imports on macOS #9408 the--trace-importsrow (line 70) and the trace summary paragraph (line 349). The trial merges of this branch with them conflict in the same files asmaindoes (for perf(agc): build the pipelines of a submission ahead on a compile pool #5925 once inUSAGE.md, as onmain, outside the changed sentence); feat(tools): add a graphical launcher #3786, feat(relinker): trace function imports on macOS #9408 and feat(agc): serve lane-keyed, pointer and run-time-address image table columns through the device image table #10193 apply to this branch without a conflict inUSAGE.md.Tested
Windows 11, GCC 15.2.0 (WinLibs UCRT), relinker-only Release build (
-DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON); synthetic fixtures only, no title or homebrew.output_collisionswith the new cases on untouched 4017310 fails at the first case withAssertionError: (False, 'module', 2, ..., 'FAIL: Cannot open output file: ...\app0/sce_module\sample.prx.guest.prx\n'). Running all 15 cases one by one on untouched code: every case exited 2 withCannot open output file,Failed to write iconorFailed to write desktop entry; Linux guest module and Windows diagnostics wrote nothing, the other 13 left between 1 and 7 output files. With the change all 15 exit 2 with<kind> output is a directoryand write nothing;output_collisionspasses in 7.7 s.ctest -R '^(output_collisions|secondary_output_collisions|input_preservation|linux_desktop_entry_output|relinker_executables|game_patches|guest_registry|call_registry)$' -j 3: 8 tests passed in 12.1 s (output_collisions8.5 s,secondary_output_collisions8.7 s,input_preservation6.8 s,call_registry3.5 s,relinker_executables1.9 s,game_patches1.3 s,guest_registry1.0 s,linux_desktop_entry_output0.3 s) on 4017310. After the rebase onto 3a2c51a,ctest -R 'output|cli_args|relinker_conversion|desktop|registry|diagnostics' -j 3: 16 tests passed in 19.1 s (output_collisions11.8 s,secondary_output_collisions12.0 s,call_registry6.4 s,cli_args0.2 s,relinker_conversion0.5 s,linux_desktop_entry_output0.4 s,guest_registry1.2 s).ctest --test-dir build-relinker -j 4on 3a2c51a: 292 tests, 289 passed,long_pathsskipped; failures known on this machine onmainas well:select_tests(Windows symlink fixture),patch_advisor_unit(path with spaces, fixed by test(tools): compare quoted and resolved paths in the patch advisor test #10667); both failed again when rerun alone. No timeoutspython tools/check_conventions.py --base upstream/mainwith this title and description 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)