Avoid setting CMake configuration types as a subproject

[jdumas]

Hi. Trying to include Jolt in my project causes CMake to crash between the configure & generate steps on my macOS machine.

Steps To Reproduce

git clone git@github.com:jrouwe/JoltPhysicsHelloWorld.git
cd JoltPhysicsHelloWorld/Build
mkdir build
cmake .. -G "Ninja Multi-Config"

Results

JoltPhysicsHelloWorld/Build/build on main
❯ cmake .. -G "Ninja Multi-Config"
-- The CXX compiler identification is AppleClang 15.0.0.15000309
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode15.3.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done (8.4s)
libc++abi: terminating due to uncaught exception of type std::out_of_range: map::at:  key not found
fish: Job 1, 'cmake .. -G "Ninja Multi-Config"' terminated by signal SIGABRT (Abort)

I'm on a Apple M2 machine with Sonoma 14.4 and CMake 3.28.3. The crash happens no matter which compiler I try to use (Xcode 13, 14 15 or LLVM-Clang). Looking at the CMakeLists.txt, it appears that this line is responsible for the problem: https://github.com/jrouwe/JoltPhysics/blob/0191c72d5f6a66217df02c0f6ad5d1c63134b4d4/Build/CMakeLists.txt#L71-L73

Honestly I would suggest to only set this variable when Jolt is built as a toplevel project. When included from another project you probably don't want to mess around with global CXX and linker flags, or change existing CMAKE_CONFIGURATION_TYPES.

[MrOnlineCoder]

Did I miss something, but you are building the Jolt itself, not including it in the project?

I managed to get it running on Apple M1 machine by using FetchContent module

[jdumas]

The issue I'm reporting appears when building Jolt as a subproject. In my example above, I'm using the JoltPhysicsHelloWorld project, which pulls Jolt via FetchContent. When building Jolt itself with the Ninja Multi-Config CMake generator, the project configures fine by itself. I can only assumes that the problem arises when trying to set CMAKE_CONFIGURATION_TYPES from a subproject that is not the root CMakeLists.txt.

[jrouwe]

Hello, I don't own a mac so it's hard for me to investigate this. Also, I don't really feel responsible for crashes in CMake, it is probably best to report the crash here. Perhaps you can narrow down the problem by forking Jolt and the HelloWorld app and pointing the HelloWorld app to your fork before changing CMakeLists.txt until it doesn't crash anymore. At this moment it is only speculation that it is caused by CMAKE_CONFIGURATION_TYPES, and I have had no other complaints about it so far.

[jdumas]

If I comment out these lines the crash goes away. I'll report the bug to CMake, but I'm pretty sure it is caused by the non-standard configuration types set by Jolt. Interestingly, I cannot reproduce the error on Windows. Note that the crash only happens when I try to build JoltPhysicsHelloWorld with the Ninja Multi-Config generator, not when building Jolt itself. This indicates an incompatibility between the subproject's config types and the root ones.

In any case I don't think it is a good practice to set non-standard build configuration when building Jolt as a subproject anyway (i.e. when included in a larger codebase via FetchContent), this would create incompatible configuration between the root project and Jolt. Simply checking whether get_directory_property(HAS_PARENT PARENT_DIRECTORY) in Jolt's CMakeLists.txt and not set CMAKE_CONFIGURATION_TYPES would help solve this issue.

[jdumas]

Ok found a CMake bug report about this exact issue. While the CMake developers recognize that CMake should error out more gracefully, they also recommend against setting CMAKE_CONFIGURATION_TYPES differently for different directories in a project.

[jrouwe]

Can you try out this PR? (you should be able to have FetchContent target GIT_TAG "7e50ddbcf101ab0ca73da76519e1a8ff8eab786f")

[jdumas]

Thanks for the quick fix! I tried this version on the JoltPhysicsHelloWorld project and it seems to work well now.

[jrouwe]

Thanks for testing!

@mihe do you see any issues for godot-jolt with this change?

[mihe]

Not that I can see from just testing on Windows at least.

I go through quite a lot of trouble to build all my dependencies as top-level, in large part to avoid issues like this.