Use vcpkg on all presets

[tophyr]

Pull Request Type

• GitHub Workflow changes
• Documentation or Wiki changes
• Build and Dependency changes
• Runtime changes
  • Render changes
  • Audio changes
  • Input changes
  • Network changes
  • Other changes

Description

Uses vcpkg for all platforms and removes platform-specific installation methods for ZLib, SDL and GTest. Still allows system-provided libraries.

Related Issues

Effectively a rebased and simplified reboot of @Arcnor's #242

Checklist

• I have tested my changes locally and verified that they work as intended.
• I have documented any new or modified functionality.
• I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
• I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

[tophyr]

I added it into the main CMakeLists right at the top. That accomplishes two things: it removes the burden of specifying it on the invocation (or on specifying a preset) and it lets us tie in to vcpkg's toolchain-chaining capability, which will be critical for Android.

…

On Wed, Jun 26, 2024 at 12:51 PM Louis Gombert ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In CMakePresets.json <#469 (comment)> : > @@ -39,8 +21,7 @@ "architecture": { "strategy": "external", "value": "x64" - }, - "toolchainFile": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" I think you still need that to load the toolchain, especially locally — Reply to this email directly, view it on GitHub <#469 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPB6RKC76GU7MYXKHKS5RLZJL5S7AVCNFSM6AAAAABJ5B7LOKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNBSGU4DKMZQHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[Lgt2x]

I added it into the main CMakeLists right at the top. That accomplishes two things: it removes the burden of specifying it on the invocation (or on specifying a preset) and it lets us tie in to vcpkg's toolchain-chaining capability, which will be critical for Android.
…
On Wed, Jun 26, 2024 at 12:51 PM Louis Gombert @.> wrote: @.* commented on this pull request. ------------------------------ In CMakePresets.json <#469 (comment)> : > @@ -39,8 +21,7 @@ "architecture": { "strategy": "external", "value": "x64" - }, - "toolchainFile": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" I think you still need that to load the toolchain, especially locally — Reply to this email directly, view it on GitHub <#469 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPB6RKC76GU7MYXKHKS5RLZJL5S7AVCNFSM6AAAAABJ5B7LOKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNBSGU4DKMZQHA . You are receiving this because you authored the thread.Message ID: @.***>

Oh right, I did not notice, that's fine

[winterheart]

This adds additional required dependency vcpkg for users on Linux and macOS, which on some cases undesirable (Linux distribution enforces using external dependencies from thier repositories for security reasons). We should keep ability to install external dependencies without using vcpkg.

[tophyr]

@winterheart I'd be happy with a solution that accepted either system-installed libraries or vcpkg ones. I would assume that a system-packaged library should take precedence over a vcpkg-provided one? Is there a pre-existing convention to use for buildscript parameters (-DCMAKE_FOO etc) to force using one or the other? (For example, in the case that I do have a system SDL installed but I want the build to use the vcpkg one anyway.)

[Jayman2000]

@winterheart I'd be happy with a solution that accepted either system-installed libraries or vcpkg ones.

What about this?

I would assume that a system-packaged library should take precedence over a vcpkg-provided one?

I don’t think that the system-packaged library should take precedence over a vcpkg-provided one. I want to be able to easily test two scenarios:

1. Compiling Descent 3 with all dependencies provided by vcpkg. Testing this scenario helps ensure that there are no undeclared dependencies.
2. Compiling Descent 3 with all dependencies provided by the system package manager. Testing this scenario helps ensure that packages for Descent 3 be built in a way that distributions will approve of.

If system packages took precedence over vcpkg packages, then it would be more difficult to test that first scenario.

Is there a pre-existing convention to use for buildscript parameters (-DCMAKE_FOO etc) to force using one or the other? (For example, in the case that I do have a system SDL installed but I want the build to use the vcpkg one anyway.)

I don’t know if there’s a pre-existing convention, but I do know that GZDoom has options like FORCE_INTERNAL_BZIP2 that you might want to take a look at.

[winterheart]

@winterheart I'd be happy with a solution that accepted either system-installed libraries or vcpkg ones. I would assume that a system-packaged library should take precedence over a vcpkg-provided one? Is there a pre-existing convention to use for buildscript parameters (-DCMAKE_FOO etc) to force using one or the other? (For example, in the case that I do have a system SDL installed but I want the build to use the vcpkg one anyway.)

I think -DUSE_VCPKG (with default OFF) option would be OK. I don't think, that we'll need to install different dependencies from different sources.

[Lgt2x]

Why not try to use VCPKG if available (ie VCPKG_ROOT env variable set) and fallback to system libs if not found? With a clear message during configuration that tells you what's being used. I agree though that mixing VCPKG and systems libs is a bad idea

[tophyr]

Went with a combination of @Jayman2000's and @Lgt2x's ideas: The system will use vcpkg by default if VCPKG_ROOT is set in the environment, and if USE_VCPKG is not set to OFF.

Something I've noticed is that if the build has already been configured, it will continue using that configuration's settings even if different ones are specified. Is that normal CMake behavior? This feels like either a bug or a misunderstanding on my part.

[Lgt2x]

Went with a combination of @Jayman2000's and @Lgt2x's ideas: The system will use vcpkg by default if VCPKG_ROOT is set in the environment, and if USE_VCPKG is not set to OFF.

Something I've noticed is that if the build has already been configured, it will continue using that configuration's settings even if different ones are specified. Is that normal CMake behavior? This feels like either a bug or a misunderstanding on my part.

Can you give an example of the commands you've tried?

Needs a warning or error message when USE_VCPKG is ON and VCPKG_ROOT cannot be found

[tophyr]

Can you give an example of the commands you've tried?

https://gist.github.com/tophyr/2f8df17c6a46c3a9157ac73f69de1939#file-gistfile1-txt-L104-L110

I think this is intended behavior due to CMake's CMakeCache.txt file. Doing this:

cmake --preset mac

ends up storing USE_VCPKG=ON and CMAKE_TOOLCHAIN_FILE=/Users/csarbora/Projects/vcpkg/ (for me) in the CMakeCache.txt file, and then those values are already set (as if they were passed in on the command line) for subsequent CMake runs. This is intentional, so that this works:

cmake -B builds/mac -DUSE_VCPKG=OFF -DSOME_OTHER_OPTION=FOO
cmake --build builds/mac # USE_VCPKG and SOME_OTHER_OPTION will be set even tho they are unspecified

.... in short, i hate cmake

[tophyr]

All @Jayman2000 feedback incorporated, and build instructions for both Ubuntu and Fedora, with and without vcpkg, tested from absolute fresh in VMs (Fedora 40 and Ubuntu 22.04 LTS).

Should be ready to merge!

[Jayman2000]

I tested this PR and got it to work in these situations:

• Windows 10 x86_64

• Ubuntu 24.04 x86_64 without vcpkgs

• Fedora 40 x86_64 without vcpkgs.

I was not able to get it to work in these situations:

• Ubuntu 24.04 x86_64 with vcpkgs. The game compiles fine, but when I run it, I get this error:

  SDL: SDL_Init() failed! rc == (-1).
  SDL_GetError() reports "dsp: No such audio device".
  

  Here’s a log of me building and running the game.

• Fedora 40 x86_64 with vcpkgs. When I run cmake --preset linux, it fails with several error messages. The most important one seems to be:

  error: building libsystemd:x64-linux failed with: BUILD_FAILED
  

  Here’s a log of me trying to build the game.

[tophyr]

Incorporated @Jayman2000 README feedback
Fixed audio runtime errors on Linux by adding ALSA and PulseAudio packages to requirements list
Fixed SystemD build error by updating vcpkg hash
Removed term.h reference and dropped ncurses dependency
Squashed commit stack

[tophyr]

One thing worth noting is that the mouse behavior is wonky, in both of my Linux builds. But, I only have Linux VMs - I'd appreciate anyone with a bare-metal Linux machine testing this and running the binary and checking if the mouse is usable (both in the GUI and in the game).

[Lgt2x]

One thing worth noting is that the mouse behavior is wonky, in both of my Linux builds. But, I only have Linux VMs - I'd appreciate anyone with a bare-metal Linux machine testing this and running the binary and checking if the mouse is usable (both in the GUI and in the game).

I don't have such a problem on my machine (not a VM)

The code looks mostly good to me now, very good job. After the last comment is addressed we're all set. I'll let @Jayman2000 approve additionally

[Lgt2x]

this should still be on top

[Jayman2000]

I tested this PR and got it to work in these situations:

• Windows 10 x86_64 using an “x64 Native Tools Command Prompt for VS 2022”

• Ubuntu 24.04 x86_64 with vcpkg.

• Ubuntu 24.04 x86_64 without vcpkg.

• Fedora 40 x86_64 with vcpkg.

• Fedora 40 x86_64 without vcpkg.

I was not able to get it to work in these situations:

• Windows 10 x86_64 using a “Developer Command Prompt for VS 2022”. When I try to run cmake --preset win, it fails with this error:

    Could not find a configuration file for package "SDL2" that is compatible
    with requested version "".
  

  Here’s a log of me trying to build the game.

Incorporated @Jayman2000 README feedback

I noticed that the README still mentions Android. Do you think that it would be better if the README mentioned Android even though we don’t support it yet?

One thing worth noting is that the mouse behavior is wonky, in both of my Linux builds. But, I only have Linux VMs - I'd appreciate anyone with a bare-metal Linux machine testing this and running the binary and checking if the mouse is usable (both in the GUI and in the game).

I get the same problem when I run the main branch in a VM, so I don’t think that it’s related to the changes in this PR.

[Jayman2000]

Why did we switch from telling people to use an x64 Native Tools Command Prompt to telling people to use any of the “-for VS 2022” command prompts?

[tophyr]

Hubris! I wanted to make sure it was clear that PowerShell would work as well, and overreached. I've switched the README back to saying

Open a "x64 Native Tools Command Prompt" or "x64 Native Tools PowerShell" and run:

[Jayman2000]

I don’t have an x64 Native Tool PowerShell shortcut in my Start menu:

How do I open an x64 Native Tools PowerShell? Do I need to do something in order to get a Start menu shortcut for it?

[tophyr]

Updated, hopefully for the final time, to address @Jayman2000's Developer Command Prompt for VS 2022 failure - README now excludes that configuration, like it probably should've from the start.

Also added SDL2, gtest and zlib back to Brewfile, to support non-vcpkg building there as well.

[Lgt2x]

Thanks for completing this :)