Repository navigation
build: add ANYPS5_USE_SYSTEM_DEPS option - #921
Conversation
4b25f1f to
35ae2eb
Compare
|
The overlap with #674 is textual, not a dependency. #674 flips |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks, the scope matches what was agreed in #400 and the default build is unchanged (merged onto current main, full ctest green here). Three things before it can go in:
- Bundled headers are still on the include path. Many targets keep hard-coded submodule include dirs, e.g.
core/libs/prx/libSceAgcDriver/CMakeLists.txt:179-180,292,306,378,455-456,480,493-494,510,599-600,660(3rdparty/Vulkan-Headers/include,3rdparty/SPIRV-Headers/include),core/shader/recompiler/CMakeLists.txt:154-155(3rdparty/SPIRV-Headers/include,3rdparty/glslang), and3rdparty/SDL2/includein libSceAudioOut, libSceAudioIn, libSceVideoOut, libScePad, libSceMouse and libSceKeyboard. The submodules are still checked out for ffmpeg-core/libjpeg-turbo/stb/VMA/LibAtrac9, so those dirs exist, and-Iis searched before theinclude_directories(SYSTEM ...)you add. The ON build therefore compiles against the bundled Vulkan/SPIRV/SDL2/glslang headers and links the system libraries. TheVK_KHR_maintenance8check tests headers that are not the ones used, and a system SDL2/glslang older or newer than the submodule is an ABI mismatch nobody sees. That is a partial fallback to the submodules, which #400 asked to avoid. Suggested direction: route those paths through one variable or target per package (e.g. linkVulkan::Headers/SPIRV-Headers::SPIRV-Headers, or setANYPS5_*_INCLUDE_DIRin one place to either the submodule or the system path). Then a build with those submodules deinitialised can prove nothing reads them. - Shader cache key (
core/shader/recompiler/CMakeLists.txt:185-203): with the flag ON, thesubmodulessection of the manifest is empty and there are no stamps, so upgrading the system glslang/SPIRV-Headers/SPIRV-Tools leaves the cache version unchanged and old cached shaders get reused. Adding the found package versions (glslang_VERSION,SPIRV-Headers_VERSION,SPIRV-Tools-opt_VERSION, Vulkan-Headers) toshaderCacheExtrawhen the flag is ON would cover it. cmake/SystemDeps.cmake:1-8,15-16,57-59: CONTRIBUTING allows no code comments except technical debt. The rationale already lives indocs/dev/BUILD.md, so those comments can go.
35ae2eb to
a38f294
Compare
|
Rebased onto current main and applied all three points.
Tested (Linux container, gcc 13, cmake 4, x86-64):
Not tested on Windows (no supported MinGW-w64 toolchain here). AI-assisted: yes. |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks, all three earlier points are addressed. The default build is unchanged: merged onto current main, full ctest is green here (400/400). Tried -DANYPS5_USE_SYSTEM_DEPS=ON -DBUILD_TESTING=ON on Arch (sdl2-compat 2.32.72, freetype 2.14.3, glslang/SPIRV-Tools 1.4.357, Vulkan-Headers 1.4.335). It configures, and all + libs build, but:
-
ctest is red with the system SDL2 (2 of 400). Arch and Fedora now ship SDL2 as
sdl2-compat(SDL3 underneath):pad_sdlfails: itsSDL_config.hdoesn't defineSDL_JOYSTICK_HIDAPI(core/libs/prx/libSceVideoOut/tests/PadSdl.cpp:6).audio_out_mix_level_pad_spkaborts: the disk driver opens the output aschannels=2, and with the bundled SDL2 the same test passes.
Either reject sdl2-compat at configure time with a named error, like the
VK_KHR_maintenance8check, or make both tests pass with it. -
ANYPS5_ENABLE_SPIRV_TOOLS=ONdoesn't configure on Arch (cmake/SystemDeps.cmake:80). SPIRV-Tools only exportsSPIRV-Tools-staticwhen it is built withSPIRV_TOOLS_BUILD_STATIC=ON(source/CMakeLists.txt); otherwise there is onlySPIRV-Tools(shared on Arch) plusSPIRV-Tools-shared. Falling back toSPIRV-ToolswhenSPIRV-Tools-staticis missing would cover both layouts. -
docs/dev/BUILD.md:48says the forced SDL/FreeType settings can't be overridden. With ON they don't apply at all: the system SDL2 comes with whatever its distro enabled (e.g. HIDAPI). Worth one sentence next to the new paragraph.
FYI, #1048 touches the line next to yours in core/libs/prx/libSceVideoOut/CMakeLists.txt:30, so whichever PR lands second needs a trivial rebase.
|
Second round applied; rebased on current
|
260c83f to
a70bc5c
Compare
|
Conventions job fixed; rebased on current
Verified: |
|
Confirming the point the earlier review left as "the real Arch environment is yours to confirm", on current Arch (CachyOS, x86-64), branch The configure fails on Arch as designed, with the new message.
Everything after the SDL2 probe passes on Arch. With a stub SDL2 package whose No The glslang layout. Arch exports only namespaced targets — The header checks hold ( The SPIRV-Tools fallback is the branch that is actually taken. Arch installs One consequence worth a sentence in BUILD.md: Arch and Fedora ship Not tested: a complete configure and build (the bundled |
MarcZX10
left a comment
There was a problem hiding this comment.
I checked this head against current upstream main (d70b8998). The existing Linux and Windows Build results are green for a70bc5c8, but git merge-tree upstream/main upstream/pr-921 reports content conflicts in CMakeLists.txt, core/libs/prx/libSceAgcDriver/CMakeLists.txt, and core/libs/prx/libSceVideoOut/CMakeLists.txt. CONTRIBUTING asks PR branches to stay current with main; please rebase/resolve those files and rerun the checks on the rebased head. The current green results do not cover that merge result.
a70bc5c to
a236308
Compare
|
@MarcZX100 Rebased onto current
One note: the Conventions job now runs the trusted base script ( |
|
@StarMrGamer Thanks for the Arch confirmation. The head moved to |
|
@011010 thanks — re-verified on the new head
So glslang, SPIRV-Headers, Vulkan-Headers and the Nothing further needed from me. For the record on the remaining blocker: I checked and there is no open pull request adding the |
|
@StarMrGamer thanks for the re-verification. Opened #2373 against |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the update. Before this can go in:
- Conflicts with
main.CMakeLists.txtconflicts, andmainhas added more hard-coded bundled include dirs since the last rebase (e.g.core/libs/prx/libSceAgcDriver/CMakeLists.txt,agc_buffer_pool_testsand the other3rdparty/Vulkan-Headers/include/3rdparty/SPIRV-Headers/include/3rdparty/SDL2/includeentries). Please rebase and route every remaining one through theANYPS5_*_INCLUDE_DIRvariables (git grep '3rdparty/\(Vulkan-Headers\|SPIRV-Headers\|SDL2/include\|glslang\)' -- '*CMakeLists.txt'should only matchcmake/SystemDeps.cmakeand the default-pathadd_subdirectorycalls). - Conventions job is red. The
tools/check_conventions.pyexemption is also in #2373. Keep it in only one of the two: once #2373 is merged, rebase and drop the hunk here. - After the rebase, rerun the
ANYPS5_USE_SYSTEM_DEPS=ONconfigure pluslibsbuild to confirm nothing else slipped past.
The approach itself matches what was agreed in #400.
a236308 to
6dadeb3
Compare
|
@adriadam10 pushed the rebase onto current Routing: the only remaining hard-coded include was Reran The |
|
Checked on current Two things before this can land:
Not verified on our side: the |
|
@adriadam10 thanks. On the duplicate: I'll land #2373 first, then rebase #921 and drop the hunk here, so the exemption lives only in #2373. On the @boykopovar on the rule change: the opt-in is off by default (the default build never reads the system), the resolution is confined to |
6dadeb3 to
f99c761
Compare
|
Rebased onto current
The |
|
Additional platform validation on standalone PR head Windows: passed — Windows 11, supported WinLibs GCC 15.2.0 UCRT/POSIX/SEH r7; CMake 4.3.1 / Ninja. Release / Tests: Reproduce after building the test targets: Default bundled-dependency mode passed. Integration limitation: a local merge against main |
Resolve SDL2, freetype, Vulkan-Headers, SPIRV-Headers, glslang and SPIRV-Tools from the system instead of the bundled submodules when the option is ON. It is OFF by default, so the bundled build is unchanged. Every package is required: a missing one, or Vulkan-Headers older than 1.4.310 (the agc driver uses VK_KHR_maintenance8), fails the configure with an error and nothing falls back to the submodule. The system packages are exposed under the same target names the bundled path uses, so the rest of the CMake is untouched. SDL2 is linked from the system as a shared library because the prx libraries are shared objects and distribution static SDL2 is usually not PIC. ffmpeg-core, libjpeg-turbo, stb, VulkanMemoryAllocator and LibAtrac9 stay bundled.
… to package versions
A system SDL2 that does not define SDL_JOYSTICK_HIDAPI (sdl2-compat over SDL3 ships no such macro, and neither does a build with the driver disabled) fails pad_sdl and makes the disk audio driver open the port with a different channel count, so ANYPS5_USE_SYSTEM_DEPS=ON now rejects it with a named error, like the VK_KHR_maintenance8 check. Ubuntu's real SDL2 defines the macro through its SDL_config.h indirection and is unaffected. SPIRV-Tools only exports SPIRV-Tools-static when built with SPIRV_TOOLS_BUILD_STATIC=ON; distributions that ship it shared expose SPIRV-Tools instead, so a shared target is aliased to the static name.
…se through ANYPS5_VULKAN_HEADERS_INCLUDE_DIR Co-Authored-By: Claude <noreply@anthropic.com>
f99c761 to
abcfa7c
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Rebased onto current main (it conflicted in docs/dev/BUILD.md and libSceAgcDriver/CMakeLists.txt) and added one commit:
- Routed the four
3rdparty/Vulkan-Headers/includeentries thatmainadded to AgcDriver tests since the last rebase throughANYPS5_VULKAN_HEADERS_INCLUDE_DIR.
Checked on the rebased head: default build (flag OFF) configures, builds, full ctest passes (795/795); git grep for hard-coded bundled include dirs only matches cmake/SystemDeps.cmake and the default-path glslang add_subdirectory calls. The ANYPS5_USE_SYSTEM_DEPS=ON path relies on the Ubuntu 24.04 and Windows runs above (this host's SDL2 is sdl2-compat, which the check rejects by design).
The conventions job is red only because the exemption lives in #2373; merging after it.
What
Adds
ANYPS5_USE_SYSTEM_DEPS, OFF by default (implements #400). With it ON,SDL2,freetype,Vulkan-Headers,SPIRV-Headers,glslangandSPIRV-Toolscome from the system instead of the bundled submodules. The default build is unchanged: the option only wraps the existingadd_subdirectorycalls and skips the bundled shader-cache submodule stamps.Design:
cmake/SystemDeps.cmakeresolves each package withfind_package(... CONFIG)(freetypethroughFindFreetype) and re-exposes them under the target names the rest of the build already uses:SDL2-static,freetype,glslang,SPIRV,glslang-standalone,SPIRV-Tools-opt,SPIRV-Tools-static. Because of that, no other CMake file changed except the two submodule stamps incore/shader/recompiler/CMakeLists.txt, which would otherwise depend on the missing bundled paths.find_package(... REQUIRED)), and ifglslangis found without the tools, or the headers are incomplete, the configure fails with a named error. Nothing falls back to the submodule, as you asked.<prefix>/include/glslang/Public/...and<prefix>/include/glslang/SPIRV/GlslangToSpv.h), unlike the bundled single root; the helper adds both.VK_KHR_maintenance8, which the agc driver uses. The helper checks it at configure withcheck_cxx_source_compilesonVkPhysicalDeviceMaintenance8FeaturesKHRand fails with a clear message when the headers are older. Ubuntu 24.04 (headers 1.4.304) fails there; Ubuntu 25.10 (1.4.321) passes.SDL2is linked from the system as a shared library (SDL2::SDL2). The prx libraries are shared objects and distribution static SDL2 is built without PIC, solibSDL2.acannot be linked into them (relocation R_X86_64_PC32 ... can not be used when making a shared object). The default build keeps its own static SDL2. The other five packages link fine from the system.ffmpeg-core(carriesffmpeg.patch),libjpeg-turbo,stb,VulkanMemoryAllocatorandLibAtrac9always stay bundled and still needgit submodule update --init.On your Linux-only vs Windows question: the interface is platform-neutral, but I only exercised the Linux packages, so I would call the Windows side untested rather than claim it. If you prefer to scope it to Linux for now, that is a small change to the helper.
Tested
Ubuntu 25.10 and 24.04 in linux/amd64 containers (the host is arm64; Docker Desktop runs the containers through Rosetta). Packages:
libsdl2-dev libfreetype-dev libvulkan-dev spirv-headers glslang-dev glslang-tools spirv-tools, GCC 15, Ninja, x86-64 as the project requires.-DANYPS5_USE_SYSTEM_DEPS=ON: configure OK andcmake --build . --target libsbuilds.-DANYPS5_USE_SYSTEM_DEPS=ON -DBUILD_TESTING=ON: configure OK;pad_sdl_testsbuilds (the executables keep-static-libgcc -static-libstdc++and link the shared system SDL2).VK_KHR_maintenance8message, at configure time.-DCMAKE_DISABLE_FIND_PACKAGE_glslang=ON, and separatelySDL2andSPIRV-Tools-opt): configure fails with a named error each time.add_subdirectory(3rdparty/SDL2)andadd_subdirectory(3rdparty/freetype).Not tested: Windows, and no PS5 title.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)