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 default library filename generation in CMakeLists.txt #687

Merged

Conversation

jtcooper10
Copy link
Contributor

The repository's CMakeLists.txt file was not reliably giving a library file of the name structure godot-cpp.<platform>.<build_type>.<system_bits>. Tested on Windows using MSVC and CMake v3.19, which gave me the mysterious file: godot-cpp.windows..32.lib, which is missing the build_type entirely and detected 32-bit despite being on a 64-bit machine.

While I'm unsure why CMAKE_SIZEOF_VOID_P was resolving as 4 on my 64-bit machine, I figured it would be nice to be able to explicitly specify the architecture like you can in the SConstruct file (with bits=64). Additionally, the statement CMAKE_BUILD_TYPE STREQUAL "" evaluates to false when CMAKE_BUILD_TYPE is not defined.

With both of these updates, you should be able to replicate (using an out-of-source build on Windows as an example):

mkdir build
# Default build: godot-cpp.windows.debug.32.lib
cmake build
# Explicitly set: godot-cpp.windows.release.64.lib
cmake -DCMAKE_BUILD_TYPE=Release -DBITS=64 build

@Calinou Calinou added the bug This has been identified as a bug label Feb 5, 2022
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Seems fine, but needs a rebase. I'll push one.

@akien-mga akien-mga force-pushed the cmake-default-build-type-fix branch from 236ae6f to 851c388 Compare July 18, 2022 10:52
@akien-mga akien-mga force-pushed the cmake-default-build-type-fix branch from 851c388 to 165ad14 Compare July 18, 2022 11:01
@akien-mga akien-mga merged commit 4bd0dab into godotengine:master Jul 18, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.5.

@akien-mga akien-mga added this to the 4.0 milestone Jul 18, 2022
@akien-mga akien-mga added cmake topic:buildsystem Related to the buildsystem or CI setup labels Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug cmake topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants