yuzu icon indicating copy to clipboard operation
yuzu copied to clipboard

cmake: allow unbundling some external libraries

Open abouvier opened this issue 2 years ago • 25 comments

Add support for linking against the following system libraries:

  • cubeb
  • inih
  • xbyak
  • dynarmic
  • httplib
  • discord-rpc

abouvier avatar Aug 07 '21 09:08 abouvier

I think this would be superseded by #7610

liushuyu avatar Dec 21 '21 06:12 liushuyu

I think this would be superseded by #7610

No longer. I hope this PR gets merged instead. My #7610 only covers Opus now.

Tatsh avatar Dec 21 '21 10:12 Tatsh

@abouvier can you rebase this? Sorry for the delay, but I think we can merge this now.

bunnei avatar Jan 05 '22 01:01 bunnei

(also, some of these maybe can use conan instead of externals?)

bunnei avatar Jan 05 '22 01:01 bunnei

I rebased and also added support for dynarmic and httplib. Sorry about conan but I don't use it and I'm not sure how it works. I'm only interested in using system libraries.

abouvier avatar Jan 06 '22 00:01 abouvier

Do we really want Dynarmic like this? It seems like between Citra and Yuzu, Dynarmic HEAD rarely works with both. Usually fails to compile with one.

Tatsh avatar Jan 06 '22 10:01 Tatsh

Until Dynarmic starts tagging releases more frequently (i.e. semantic versioning) it will be hard to reliably use a shared version of it

Tachi107 avatar Jan 06 '22 10:01 Tachi107

Until Dynarmic starts tagging releases more frequently (i.e. semantic versioning) it will be hard to reliably use a shared version of it

I'm all for that but I do not expect it right now due to it being heavily worked on. In my packaging, Yuzu and Citra both get their respective bundled versions of Dynarmic.

Tatsh avatar Jan 06 '22 10:01 Tatsh

Rebased again and also added support for discord-rpc.

abouvier avatar Mar 27 '22 23:03 abouvier

Hey @abouvier, could you please rebase this again? The patch can't be cleanly applied since #8134 was merged

Tachi107 avatar Apr 03 '22 14:04 Tachi107

Hey @abouvier, could you please rebase this again? The patch can't be cleanly applied since #8134 was merged

Done! See you in six months for the next rebase 😅

abouvier avatar Apr 03 '22 16:04 abouvier

Dynarmic changes frequently. Often a mismatch breaks the build. I don't think is good to not use the one in externals

german77 avatar Jul 15 '22 15:07 german77

That's true, but recently Dynarmic started tagging releases more often, thus making it easier to stick to a specific version.

Tachi107 avatar Jul 15 '22 15:07 Tachi107

~~Currently the builds are failing because of https://github.com/conan-io/conan-center-index/pull/11690 and https://github.com/conan-io/conan-center-index/issues/11718.~~

~~And now https://github.com/conan-io/conan-center-index/issues/11822.~~

abouvier avatar Jul 15 '22 16:07 abouvier

OK I think it's good enough to be merged now :p

abouvier avatar Jul 30 '22 16:07 abouvier

Why are you removing pkg_check_modules for zstd? My system does not have a CMake config for zstd 1.5.2.

CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find zstd: found neither zstdConfig.cmake nor zstd-config.cmake
  (Required is at least version "1.5")
Call Stack (most recent call first):
  /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:269 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:579 (_FPHSA_HANDLE_FAILURE_CONFIG_MODE)
  externals/find-modules/Findzstd.cmake:7 (find_package_handle_standard_args)
  CMakeLists.txt:193 (find_package)

Tatsh avatar Aug 07 '22 15:08 Tatsh

Why are you removing pkg_check_modules for zstd? My system does not have a CMake config for zstd 1.5.2.

Because upstream zstd provides a CMake config file since nearly 2 years, so I thought that every system will have one. But zstd can be compiled/installed without using CMake, so I was wrong :p I will update the find modules.

abouvier avatar Aug 07 '22 16:08 abouvier

Because upstream zstd provides a CMake config file since nearly 2 years, so I thought that every system will have one.

As far as I understand their "main" build system is Make, so Meson, CMake, and Visual Studio solutions are second class citizens

Tachi107 avatar Aug 07 '22 17:08 Tachi107

How about an update to allow system cpp-jwt?

Tatsh avatar Aug 07 '22 18:08 Tatsh

It's already supported :p There is no find module for cpp-jwt because it use CMake by default, so there is a system CMake config file installed.

abouvier avatar Aug 07 '22 18:08 abouvier

Fair enough :)

-- OpenPGP key: 66DE F152 8299 0C21 99EF A801 A8A1 28A8 AB1C EE49

Tachi107 avatar Aug 26 '22 20:08 Tachi107

Is there something blocking the merge of this PR? I'm ready to amend anything, just tell me!

abouvier avatar Sep 16 '22 23:09 abouvier

Is there something blocking the merge of this PR? I'm ready to amend anything, just tell me!

I'm still not convinced about adding a large number of options to toggle the use of bundled subprojects. It's more code, hence more fragile, and goes against what the rest of yuzu's build script does: using system deps when available, and falling back to vcpkg/externals/whatever otherwise.

I've also recently realised that explicitly defining options is not needed, because in CMake you can define CMAKE_DISABLE_FIND_PACKAGE_<PackageName> to make find_package() skip dependency lookups for a given package.

If I were to write patch with the same goal of this one, I would do something like this:

find_package(Opus 1.3)
if (NOT Opus_found)
    add_subdirectory(opus EXCLUDE_FROM_ALL)
endif()

This way yuzu would use the bundled Opus version only if not installed on the host, or if the user explicitly requires to use the bundled version by invoking cmake with the argument -DCMAKE_DISABLE_FIND_PACKAGE_Opus=true

Tachi107 avatar Sep 20 '22 17:09 Tachi107

OK you convinced me! I was just mimicking the behavior of the original file, with options for ffmpeg, opus, qt, etc.

abouvier avatar Sep 20 '22 20:09 abouvier

I removed the module FindVulkanHeaders because CMake already provides the module FindVulkan which exports many imported targets, including Vulkan::Headers.

abouvier avatar Oct 08 '22 02:10 abouvier