Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CMake Linux VCPKG build #79104

Merged
merged 6 commits into from
Jan 17, 2025
Merged

Conversation

alef
Copy link
Contributor

@alef alef commented Jan 12, 2025

Summary

Build "Fix CMake Linux VCPKG build"

Purpose of change

Fix CMake Linux VCPKG build

Describe the solution

There was a mismatch between VCPKG triplet and DYNAMIC_LINKING in the preset. This lead to a confusion of static linking targets used in dynamic.

Also add a CMake function to list the targets added by VCPKG.

Describe alternatives you've considered

Testing

To build, one has to remove pdcurses from msvc-full-features/vcpkg.json because it is a Windows-only package.

Run the executable.

Additional context

The system hardcodes the RPATH to point to the build directory.

@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. Code: Build Issues regarding different builds and build environments [JSON] Changes (can be) made in JSON [Markdown] Markdown issues and PRs json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jan 12, 2025
Copy link
Contributor

@moxian moxian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No opinion on the core static->dynamic switch (i have not tried building this on linux), but I have a couple of generic CMake nitpicks

src/CMakeLists.txt Show resolved Hide resolved
src/CMakeLists.txt Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
CMakeModules/ListImportedTargets.cmake Show resolved Hide resolved
Copy link
Contributor

@moxian moxian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just tested this and can confirm that MSVS VCPKG CMake build still works. So as long as it works for your Linux use case, it should be good to go.
LGTM

src/CMakeLists.txt Show resolved Hide resolved
@moxian
Copy link
Contributor

moxian commented Jan 12, 2025

Ah, CI complains about target_link_libraries(cataclysm-common PUBLIC ZLIB::ZLIB)

CMake Error at src/CMakeLists.txt:240 (target_link_libraries):
  Cannot specify link libraries for target "cataclysm-common" which is not
  built by this project.

I can confirm that bare mkdir -p build; cd build ; cmake -DTILES=1 .. gives this error on my WSL, but I'm frankly not sure how to interpet it :/

I should perhaps also have been more explicit in my above statement about this working on windows, I've only tested that windows-x64-msvc preset works. The windows-tiles-sounds-x64-msvc doesn't but it's also broken on master with some seemingly unrelated problem (Could NOT find libxmp (missing: libxmp_LIBRARY)) which i assumed is due to my active fiddling with my vcpkg setup..

@github-actions github-actions bot added BasicBuildPassed This PR builds correctly, label assigned by github actions and removed BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jan 12, 2025
@Maleclypse
Copy link
Member

Much clang failure. Is that related or just an artifact of previous build states?

@moxian
Copy link
Contributor

moxian commented Jan 15, 2025

It's definitely related. It's failing trying to process the newly-added cmake directives (although i have not looked any deeper into this)

@alef alef force-pushed the cmake-linux-vcpkg-fix branch from b2fced2 to 5e4d0bc Compare January 16, 2025 20:53
@Maleclypse Maleclypse merged commit acd0756 into CleverRaven:master Jan 17, 2025
28 of 29 checks passed
@alef alef deleted the cmake-linux-vcpkg-fix branch January 17, 2025 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Build Issues regarding different builds and build environments <Documentation> Design documents, internal info, guides and help. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants