Repository navigation
fix(tests): stage linked .prx runtime files next to test executables on Windows - #7645
akkadbakkad89 wants to merge 1 commit into
Conversation
|
I rechecked the submitted diff against the cross-target reproduction from #7637. The PR currently still uses |
|
One more edge case from the same cross-target: a test target with no linked runtime DLLs also fails in the submitted form. |
8ee1664 to
d2fbfd9
Compare
|
I tested the updated |
|
Thanks for the fast re-verification on the 3.25.1 cross-target - good that the |
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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
There was a problem hiding this comment.
One compatibility issue to resolve before merging: the root CMakeLists.txt declares cmake_minimum_required(VERSION 3.20), but $<TARGET_RUNTIME_DLLS:...> was added in CMake 3.21 (CMake 3.21 release notes). On Windows with CMake 3.20 this generator expression is evaluated during generation and is unknown, so the newly added if(WIN32) command can make configuration fail for a version the project currently claims to support. Could you either preserve 3.20 compatibility (for example, gate an alternate staging path by CMake version) or raise the project minimum to 3.21 and update the documented/tooling requirement? The current CMake 4.4.4 validation does not cover the declared minimum.
|
Overlap bot report checked: all 13 shared-file PRs touch |
…on Windows A test registration that omits the WIN32 PATH prepend for libs/unpatched makes the executable block on the Windows entry-point dialog (0xC0000139) until ctest reports a timeout; the dialog is invisible to headless and CI runs. The prepend is per-test boilerplate repeated across ~95 CMakeLists files, so new registrations keep missing it (boykopovar#7627). add_test_executable now copies $<TARGET_RUNTIME_DLLS> next to the exe at link time (copy_if_different rather than copy -t, whose -t option older cmake -E lacks; the exe itself is listed as a source so a target with no shared-library dependencies still receives a valid command and copies nothing). The loader resolves the .prx imports from the executable directory ahead of PATH, so no registration needs the prepend and a test exe can be run by hand with no environment setup. Fixes boykopovar#7637. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
d2fbfd9 to
ee69ac6
Compare
MarcZX10
left a comment
There was a problem hiding this comment.
Thanks for checking the real CMake 3.20.5 behavior; the version gate correctly avoids the unknown-generator-expression failure. One remaining scope point: on 3.20 the staging command is disabled, so a test registration without the manual PATH prepend can still hit the same loader dialog, and direct-running the exe still lacks the new guarantee. Could you either add a 3.20-compatible staging fallback, or state that this general protection applies only with CMake >=3.21 (and retain the per-test PATH setup as required below that)? The current reported CMake 3.20 validation proves configuration/build, not the no-PATH runtime behavior.
|
Good catch — fixed in if(WIN32 AND NOT CMAKE_VERSION VERSION_LESS 3.21)
add_custom_command(TARGET ${target} POST_BUILD ...)
endif()Chose the gate over bumping the minimum so this fix stays scoped. On <3.21 the command is never created, so the unknown genex can't fail generation — and the per-test Verified against a real CMake 3.20.5 binary (PyPI wheel) with a minimal fixture replicating the pattern:
Side note for context, not pushback: the repo already relies on |
MarcZX10
left a comment
There was a problem hiding this comment.
The current head addresses the CMake 3.20 generator-expression failure by gating the staging command at 3.21, and the updated body clearly records that the existing per-test PATH setup remains in effect below that version. The reported MinGW staging checks cover linked and no-runtime-DLL targets, and the current conventions, relinker platform, and ASan checks pass. I approve this focused test-infrastructure change.
|
Fair point — taken the second option and scoped the claim in the PR body: staging applies on One nuance worth noting on the fallback side: A true 3.20 fallback would need |
|
Full-suite Windows run, to cover every test executable now that each one gets the staging step (the description lists a 14-test sweep). Windows 11 Pro 26200, NVIDIA GeForce RTX 4080 (driver 617.42), WinLibs GCC 15.2.0 r7, CMake from WinLibs, Ninja, Release, bundled dependencies,
I did not repeat the scrubbed- AI-assisted: yes (runs done with Claude Code). |
THEMANnt
left a comment
There was a problem hiding this comment.
Reviewed ee69ac6d0e6f1fb9a08c9a6d9790b12d35da6985 against 8d17389a73852ee0e462c640589b47d49c68e8d6. No actionable findings in this focused review.
Source review: Inspected the complete helper change and its existing runtime output/wrapped-main setup. Windows CMake 3.21+ stages linked runtime dependencies beside the test. COMMAND_EXPAND_LISTS preserves the DLL list, VERBATIM handles paths, and including the executable itself keeps copy_if_different valid when the dependency list is empty. Older CMake and non-Windows behavior are unchanged.
Validation: Windows, CMake 4.3.1 / MinGW GCC 15.2: a disposable project using the exact head helper and WindowsTestExit source built a shared .prx plus linked and standalone tests in directories containing spaces. Both head executables ran with exit 0; the linked .prx was staged. The same baseline fixture built but did not stage that file. Conventions and whitespace passed. This older script-registration CLI rejects --scripts; no registration pass is claimed. Effective hosted relinker matrix and ASan succeeded; full-product jobs were skipped.
Scope: Actual bounded CMake/build/runtime fixture; full AnyPS5 libraries and all test targets were not rebuilt locally.
What
add_test_executablegains a POST_BUILD step that copies$<TARGET_RUNTIME_DLLS>next to the test exe on Windows. The step is gated onCMAKE_VERSION >= 3.21(the generator expression's floor) so the declaredcmake_minimum_required(VERSION 3.20)keeps configuring; on <3.21 the step is absent and the existing per-testENVIRONMENT_MODIFICATIONPATH prepends apply unchanged (note the prepend property itself is a 3.22 feature that ctest silently ignores on 3.20-3.21, so this widens rather than narrows the working range; the loader trap below 3.22 predates this PR). The executable directory outranks PATH in the DLL search order, so a test whose registration forgot theENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$<TARGET_FILE_DIR:libc>"boilerplate still resolveslibc.prx/libkernel.prx/libSceAgcDriver.prxat load - instead of spawning a modal "entry point not found" (0xC0000139) dialog that blocks until ctest reports a timeout.The prepend is manual per-test boilerplate repeated across ~95 CMakeLists files; new registrations keep missing it (live instance fixed in #7627's
agc_driver_depth_bounds_state). With staging, no registration needs it, future omissions stop mattering, and hand-running a test exe works with no environment setup. ExistingENVIRONMENT_MODIFICATIONlines stay harmless.Tested
Windows 11, MinGW (WinLibs GCC 15.2.0), Release, RTX 3070 (driver 617.42).
objdumpevidence + generated build line:agc_driver_graphics_testslinks libc/libkernel/libSceAgcDriver and the copy stages exactly those threeunpatched/*.prxintotests/; exes with no shared-library deps (nid_buffer_validation_tests) build clean (empty$<TARGET_RUNTIME_DLLS>is a no-op).PATHscrubbed toC:\Windows\System32;C:\Windows:agc_driver_graphics_tests.exepasses the full graphics suite - the loader resolves the.prximports from the exe directory.agc_write_tracking,agc_buffer_pool(+2 small-device variants),agc_import_watch,agc_import_watch_other_family,agc_host_import_unmap,agc_host_import_retirement,agc_host_import_second_mapping,agc_host_import_retire,guest_kernel_sys,guest_cpu_time_clocks,nid_buffer_validation- 14/14 pass,agc_host_import_sparseskipped by design.$<TARGET_RUNTIME_DLLS>form fails generation (Expression did not evaluate to a known generator expression); the gated form configures, generates and builds clean.python3 tools/check_conventions.py --base origin/main: clean;git diff --check: clean.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)