yuzu
yuzu copied to clipboard
build: simplify find modules
With this patch I've deleted a few find modules that are now unused since the vcpkg transition, as the CMake code now forces CONFIG
mode for Catch2, fmt and nlohmann_json.
I've then simplified the lz4, opus, and zstd modules by exclusively using pkg-config. They were using it already, but were ignoring the result. Also, I believe that manually looking for libraries was required for Conan to work, and it is thus not needed anymore.
Lastly, I believe that there is no platform that ships these system libs without pkg-config/pkgconf, so requiring it should be fine.
I already did this in https://github.com/yuzu-emu/yuzu/pull/6833.
Oh ok @abouvier, I did not see it. Clean ups are a bit unrelated to unbundling dependencies though, do you think it makes sense to split these changes in these two different PRs? It'll make reviewing easier for the yuzu team.
I don't know, whatever is merged first :p
But a few remarks:
- Alias of non-global imported library is only supported since cmake 3.18, hence the
IMPORTED_TARGET GLOBAL
inpkg_search_module
, because the build environment of yuzu is on cmake 3.16 - The content of the first variable listed in REQUIRED_VARS will be displayed to the user, so if you use
xXx_FOUND
the message will be:Found lz4: 1 (found suitable version "1.9.3", minimum required is "1.8")
instead ofFound lz4: /usr/lib/liblz4.so (found suitable version "1.9.3", minimum required is "1.8")
for example withxXx_LINK_LIBRARIES
. - The prefix used in
pkg_search_module
andfind_package_handle_standard_args
cannot be the same or some variables generated by cmake will conflict.
I had replied to this more than one hour ago via email, but for some reason GitHub dropped my reply...
- Alias of non-global imported library is only supported since cmake 3.18, hence the
IMPORTED_TARGET GLOBAL
inpkg_search_module
, because the build environment of yuzu is on cmake 3.16
Right, thanks, I forgot about this detail.
- The content of the first variable listed in REQUIRED_VARS will be displayed to the user, so if you use
xXx_FOUND
the message will be:Found lz4: 1 (found suitable version "1.9.3", minimum required is "1.8")
instead ofFound lz4: /usr/lib/liblz4.so (found suitable version "1.9.3", minimum required is "1.8")
for example withxXx_LINK_LIBRARIES
.
Oh, I missed it, as the documentation says this in the examples section... Thanks for letting me know, I was quite puzzled by that 1 :)
- The prefix used in
pkg_search_module
andfind_package_handle_standard_args
cannot be the same or some variables generated by cmake will conflict.
Could you be more specific?
- The prefix used in
pkg_search_module
andfind_package_handle_standard_args
cannot be the same or some variables generated by cmake will conflict.Could you be more specific?
If you use both pkg_search_module(lz4 ...)
and find_package_handle_standard_args(lz4 ...
(same lowercase prefix), pkg_search_module
will create the variable lz4_VERSION
even if the package is not found (with empty value in this case).
Then find_package_handle_standard_args
will produce this message:
Could NOT find lz4: Found unsuitable version "", but required is at least "1.8" (found )
instead of:
Could NOT find lz4 (missing: DIFFERENT_PREFIX_LINK_LIBRARIES) (Required is at least version "1.8")
.
Oh, nice to know, thanks. CMake really is quirky...
If you use both
pkg_search_module(lz4 ...)
andfind_package_handle_standard_args(lz4 ...
(same lowercase prefix),pkg_search_module
will create the variablelz4_VERSION
even if the package is not found (with empty value in this case). Thenfind_package_handle_standard_args
will produce this message:Could NOT find lz4: Found unsuitable version "", but required is at least "1.8" (found )
instead of:Could NOT find lz4 (missing: DIFFERENT_PREFIX_LINK_LIBRARIES) (Required is at least version "1.8")
.
Uhm... It seems that changing the pkg_search_module
prefix doesn't help with the Opus findmodule.
I have this in FindOpus.cmake
(yes, I've renamed it from Findopus.cmake
):
find_package(PkgConfig)
if (PKG_CONFIG_FOUND)
pkg_search_module(opus_pc IMPORTED_TARGET GLOBAL opus)
if (opus_pc_FOUND)
add_library(Opus::opus ALIAS PkgConfig::opus_pc)
endif()
endif()
message("opus_pc_FOUND: ${opus_pc_FOUND}")
message("opus_pc_LINK_LIBRARIES: ${opus_pc_LINK_LIBRARIES}")
message("opus_pc_VERSION: ${opus_pc_VERSION}")
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(Opus
REQUIRED_VARS
opus_pc_LINK_LIBRARIES
opus_pc_FOUND
VERSION_VAR opus_pc_VERSION
)
And here's the error message I get if opus.pc is not installed on my system:
-- Checking for one of the modules 'opus'
opus_pc_FOUND:
opus_pc_LINK_LIBRARIES:
opus_pc_VERSION:
CMake Error at /usr/share/cmake-3.23/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
Could NOT find Opus: Found unsuitable version "", but required is at least
"1.3" (found )
Call Stack (most recent call first):
/usr/share/cmake-3.23/Modules/FindPackageHandleStandardArgs.cmake:592 (_FPHSA_FAILURE_MESSAGE)
externals/find-modules/FindOpus.cmake:18 (find_package_handle_standard_args)
externals/CMakeLists.txt:131 (find_package)
Do you have an idea of what is going on?
I guess that the variable listed in VERSION_VAR
must be non existent if the library is not found. Here opus_pc_VERSION
is still created with empty value. You should create the variable Opus_VERSION
(with value of opus_pc_VERSION
) after the alias library and use this one in VERSION_VAR
. Every FindXxx.cmake
module should create a Xxx_VERSION
variable on success anyway.
I guess that the variable listed in
VERSION_VAR
must be non existent if the library is not found. Hereopus_pc_VERSION
is still created with empty value. You should create the variableOpus_VERSION
(with value ofopus_pc_VERSION
) after the alias library and use this one inVERSION_VAR
. EveryFindXxx.cmake
module should create aXxx_VERSION
variable on success anyway.
Uhm, this sounds like working around CMake's quirky behaviour instead of fixing it. I've submitted a bug report upstream, to see what they think about this. I might try to write a patch afterwards.
The bug report is here: https://gitlab.kitware.com/cmake/cmake/-/issues/23807
As this looks to me like a CMake quirk and since it doesn't create actual issues to the build process, I'll avoid making the code more confusing just to work around it (unless upstream CMake says that this is working as intended).
This is now ready to be merged :)
The CMake quick has been fixed: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/7527
Alias of non-global imported library is only supported since cmake 3.18, hence the IMPORTED_TARGET GLOBAL in pkg_search_module, because the build environment of yuzu is on cmake 3.16
Right, thanks, I forgot about this detail.
The content of the first variable listed in REQUIRED_VARS will be displayed to the user, so if you use xXx_FOUND the message will be: Found lz4: 1 (found suitable version "1.9.3", minimum required is "1.8") instead of Found lz4: /usr/lib/liblz4.so (found suitable version "1.9.3", minimum required is "1.8") for example with xXx_LINK_LIBRARIES.
Oh, I missed it, as the documentation says this in the examples section... Thanks for letting me know, I was quite puzzled by that 1 :)
The prefix used in pkg_search_module and find_package_handle_standard_args cannot be the same or some variables generated by cmake will conflict.
Could you be more specific?