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

Support multi configuration build tools (MSVC) with CMake #1229

Closed
Torets opened this issue Sep 2, 2023 · 8 comments
Closed

Support multi configuration build tools (MSVC) with CMake #1229

Torets opened this issue Sep 2, 2023 · 8 comments
Labels
cmake topic:buildsystem Related to the buildsystem or CI setup

Comments

@Torets
Copy link

Torets commented Sep 2, 2023

Godot version

4.1-dev

godot-cpp version

4.1

System information

Windows10

Issue description

When creating GDExtention C++ module using CMake, both Release and Debug configurations are linked against debug runtime libraries for MSVC, wich causes multiple errors like that:

LNK2038	mismatch detected for '_ITERATOR_DEBUG_LEVEL': value '2' doesn't match value '0' in ****.obj	
LNK2038	mismatch detected for 'RuntimeLibrary': value 'MDd_DynamicDebug' doesn't match value 'MD_DynamicRelease' in ****\.build\godot-cpp.windows.debug.64.lib(string.obj)

This happens because in CMakeLists.txt there is a static check for CMAKE_BUILD_TYPE, which is not used for multi-config build generators (MSVC, Ninja-MultiConfig, xcode) at this point of CMakeLists.txt

set(GODOT_COMPILE_FLAGS )

if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC")
	# using Visual Studio C++
	set(GODOT_COMPILE_FLAGS "/EHsc /utf-8") # /GF /MP

	if(CMAKE_BUILD_TYPE MATCHES Debug)
		set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} /MDd") # /Od /RTC1 /Zi
	else()
		set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} /MD /O2") # /Oy /GL /Gy
		STRING(REGEX REPLACE "/RTC(su|[1su])" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
		string(REPLACE "/RTC1" "" CMAKE_CXX_FLAGS_DEBUG ${CMAKE_CXX_FLAGS_DEBUG})
	endif(CMAKE_BUILD_TYPE MATCHES Debug)

	add_definitions(-DNOMINMAX)
else()  # GCC/Clang
	set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -g")

	if(CMAKE_BUILD_TYPE MATCHES Debug)
		set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -fno-omit-frame-pointer -O0")
	else()
		set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -O3")
	endif(CMAKE_BUILD_TYPE MATCHES Debug)
endif()

Its better to use CMAKE_CXX_FLAGS_DEBUG, CMAKE_CXX_FLAGS_RELEASE etc for Release|Debug configurations without any condition on CMAKE_BUILD_TYPE, because it will break any multi-config generators (while using CMAKE_CXX_FLAGS_ is universal). Using CMAKE_CXX_FLAGS should be reserved for common flags: in the example of MSCV: "/EHsc /utf-8".

here is how I propose to change this segment (removed project-specific options, because it might be better to have same compile options for godot-cpp and GDExtention project):

diff -r  
84,85d83
< 
< set(GODOT_COMPILE_FLAGS )
89,97c87,90
< 	set(GODOT_COMPILE_FLAGS "/EHsc /utf-8") # /GF /MP
< 
< 	if(CMAKE_BUILD_TYPE MATCHES Debug)
< 		set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} /MDd") # /Od /RTC1 /Zi
< 	else()
< 		set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} /MD /O2") # /Oy /GL /Gy
< 		STRING(REGEX REPLACE "/RTC(su|[1su])" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
< 		string(REPLACE "/RTC1" "" CMAKE_CXX_FLAGS_DEBUG ${CMAKE_CXX_FLAGS_DEBUG})
< 	endif(CMAKE_BUILD_TYPE MATCHES Debug)
---
> 	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc /utf-8") # /GF /MP
> 	set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /MDd") # /Od /RTC1 /Zi
> 	set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} /MD /O2") # /Oy /GL /Gy
> 	STRING(REGEX REPLACE "/RTC(su|[1su])" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
100,107c93,98
< else()  # GCC/Clang
< 	set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -g")
< 
< 	if(CMAKE_BUILD_TYPE MATCHES Debug)
< 		set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -fno-omit-frame-pointer -O0")
< 	else()
< 		set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -O3")
< 	endif(CMAKE_BUILD_TYPE MATCHES Debug)
---
> elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNUC" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")  # GCC/Clang
> 	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g")
> 	set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fno-omit-frame-pointer -O0")
> 	set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3")
> else()
> 	message(FATAL_ERROR "godot-cpp supports only MSVC, GCC and Clang")
108a100,106
> 
> # make sure only debug and release are used, otherwise 
> # CMAKE_CXX_FLAGS_RELWITHDEBINFO and CMAKE_CXX_FLAGS_MINSIZEREL
> # should be supported in code above as well
> if(GENERATOR_IS_MULTI_CONFIG)
> 	set(CMAKE_CONFIGURATION_TYPES Debug Release)
> endif()
188c186
< set_property(TARGET ${PROJECT_NAME} APPEND_STRING PROPERTY COMPILE_FLAGS ${GODOT_COMPILE_FLAGS})
---
> # set_property(TARGET ${PROJECT_NAME} APPEND_STRING PROPERTY COMPILE_FLAGS ${GODOT_COMPILE_FLAGS})

Steps to reproduce

  1. Create any GDExtetion project with CMAKE and default settings
  2. Attempt to compile release configuration

Minimal reproduction project

N/A

@AThousandShips AThousandShips changed the title Support multy configuration build tools (MSVC) with CMake Support multi configuration build tools (MSVC) with CMake Sep 4, 2023
@AThousandShips AThousandShips added the topic:buildsystem Related to the buildsystem or CI setup label Sep 4, 2023
@Torets
Copy link
Author

Torets commented Sep 12, 2023

pull request
https://github.com/godotengine/godot-cpp/pull/344/files
potentially solves this issue as well

@cjl3230
Copy link

cjl3230 commented May 30, 2024

I encountered the same problem on vs2022. I hope it can be merged soon.

@enetheru
Copy link
Contributor

I have a PR to modernise the current solution which no longer relies on CMAKE_BUILD_TYPE for exactly this reason. If merged I believe should solve this issue.

@Torets
Copy link
Author

Torets commented Nov 23, 2024

With PR Modernise existing cmake options by @enetheru being merged, I consider this issue being resolved.

Thank you a lot!! We wanted a candy and you baked us a master chief level cake!!!

I have some issues though, that are probably rise from legacy usage of godot-cpp and I don't know how to adapt to modernized approach. Can't find the exact guide I've followed, but here is a similar one: Godot4 GDExtension C++ Example
Here is what I do in my project's CMakeLists.txt (generating for VisualStudio2022):

cmake_minimum_required(VERSION 3.19)
project(MyGdExCppProject )
include(FetchContent)

FetchContent_Declare(
        godot-cpp
        GIT_REPOSITORY https://github.com/godotengine/godot-cpp.git
        GIT_TAG master  (the last tag, I used before was godot-4.1.1-stable)
)
FetchContent_MakeAvailable(godot-cpp)
add_library(MyGdExCppProject MODULE MyGdExCppProject .cpp) 
target_link_libraries(MyGdExCppProject PRIVATE godot::cpp )

What it used to do is automatically pull git repo, run CMake for it and link target godot::cpp to my target (MyGdExCppProject here). godot::cpp was a separate target for making a static library.

Now CMake gives error:

Target "SFNodes" links to:

    godot::cpp

 but the target was not found.

I've disabled target_link_libraries line and project generated, but now there are too many projects in solution:

  • MyGdExCppProject (my target/project)
  • editor (I assume its the whole Godot Project Editor?)
  • godot-cpp-test (good example, but I don't need it in my project)
  • template-debug
  • template-release

My main question is how to properly link against godot::cpp (in old terms) and how to have it and only it as extra project in my solution? If at all possible I'd rather preserve usage of FetchContent, because it simplifies the usage of the library.

Once again thank you for titanic work you've done!

@enetheru
Copy link
Contributor

The godot::cpp target has been split into three to follow the way the SCons build works. so you want to add to target link libraries one of the aliases

  • godot-cpp::template_release
  • godot-cpp::template_debug
  • godot-cpp::editor

I'll look into methods of hiding godot-cpp targets from consumers so they dont clutter up the list.
Cheers.

@Torets
Copy link
Author

Torets commented Nov 23, 2024

The godot::cpp target has been split into three to follow the way the SCons build works. so you want to add to target link libraries one of the aliases

* godot-cpp::template_release

* godot-cpp::template_debug

* godot-cpp::editor

I'll look into methods of hiding godot-cpp targets from consumers so they dont clutter up the list. Cheers.

Do I Get it right, that I should use something like this?

target_link_libraries(MyGdExCppProject PRIVATE godot-cpp::template_$<LOWER_CASE:$<CONFIG>> )

Will it be possible to add proxy (INTERFACE in CMake terms) target godot::cpp for backward compatibility? There are many guides that use that and it might cause confusion.
As for target clutter, usually in CMake configurations introduce options like USE_TEST, USE_EDITOR, etc with consumer oriented default values. If it'll fit your general concept, you can also decide on what target godot::cpp will link to, bases on that flags

@enetheru
Copy link
Contributor

Will it be possible to add proxy (INTERFACE in CMake terms) target godot::cpp for backward compatibility? There are many guides that use that and it might cause confusion.

are PUBLIC,PRIVATE,INTERFACE the thing you mean? I'm not sure.

As for target clutter, usually in CMake configurations introduce options like USE_TEST, USE_EDITOR, etc with consumer oriented default values. If it'll fit your general concept, you can also decide on what target godot::cpp will link to, bases on that flags

The reason they are all visible is because you can build all the targets at once, and they dont block each other, no re-configure required. so you can do something like cmake --build . --target my_release my_debug my_editor and have all your things.

I am on discord in the godot-cafe or on the dev rocket chat if you want more real-time chat.

Do I Get it right, that I should use something like this?

target_link_libraries(MyGdExCppProject PRIVATE godot-cpp::template_$<LOWER_CASE:$<CONFIG>> )

template_debug and template_release mean something different than your typical Release/Debug build configurations. so while you can do what you listed, unless you want debug symbols I would build only as release. a template_debug build is more about enabling godot debug features rather than enabling c++ debug symbols and optimisation flags. It's confusing I know.

@aaronfranke
Copy link
Member

Closing as resolved. If there is still some confusion, discussion can continue here (or elsewhere).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

No branches or pull requests

5 participants