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 wrong build options for MSVC #1319

Closed
wants to merge 1 commit into from
Closed

Fix wrong build options for MSVC #1319

wants to merge 1 commit into from

Conversation

crashfort
Copy link

Building for MSVC does not work because there is a mismatch in the used CRT.

Building godot-cpp like this:

scons platform=windows -j15 custom_api_file="..\Godot_v4.2-rc2_win64\extension_api.json" target=template_release use_static_cpp=no optimize=speed debug_symbols=yes

and

scons platform=windows -j15 custom_api_file="..\Godot_v4.2-rc2_win64\extension_api.json" target=template_debug use_static_cpp=no optimize=debug debug_symbols=yes

On master this produces lib files that you can't actually use because they are linked to the wrong CRT.

@crashfort crashfort requested a review from a team as a code owner November 25, 2023 22:50
@AThousandShips AThousandShips added bug This has been identified as a bug topic:buildsystem Related to the buildsystem or CI setup labels Nov 26, 2023
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

For the reference: Godot itself is never using /MTd, and enabling /MDd is done with a separate flag. While GDExtension is a pure-C API and matching CRTs is not strictly necessary (official builds are done using MinGW), it's usually preferred to have the same build flags for both the engine and extensions.

    if env["debug_crt"]:
        # Always use dynamic runtime, static debug CRT breaks thread_local.
        env.AppendUnique(CCFLAGS=["/MDd"])
    else:
        if env["use_static_cpp"]:
            env.AppendUnique(CCFLAGS=["/MT"])
        else:
            env.AppendUnique(CCFLAGS=["/MD"])

@crashfort crashfort closed this by deleting the head repository Dec 4, 2023
@GreenCrowDev
Copy link

Is there a reason why /MTd and /MDd haven't been implemented ultimately?

Context:
I'm building two libraries: a .lib with all the logic, and then the gdextension .dll that will work as an adapter (the reason is eventually implement the library for other uses outside Godot).

From my understanding gdextension on Windows compiles with static release CRT with flat /MT

godot-cpp/tools/windows.py

Lines 120 to 123 in c20a84e

if env["use_static_cpp"]:
env.Append(CCFLAGS=["/MT"])
else:
env.Append(CCFLAGS=["/MD"])

Compiling my external lib with CMake in Debug mode, and then compiling the gdextension dll with: scons target=editor dev_mode=yes results in linker errors like [LNK2038: mismatch detected for '_ITERATOR_DEBUG_LEVEL'](https://stackoverflow.com/questions/7668200/error-lnk2038-mismatch-detected-for-iterator-debug-level-value-0-doesnt) and LNK2038: mismatch detected for 'RuntimeLibrary'.

For those who run in these kind of issues, i resolved with a workaround: change the global env flags in your SConstruct file with something like this:

# Function to filter out conflicting flags
def remove_runtime_flags(env):
    flags_to_remove = ["/MD", "/MDd", "/MT", "/MTd"]
    for flag in flags_to_remove:
        while flag in env["CCFLAGS"]:
            env["CCFLAGS"].remove(flag)

# Manually control the runtime library
if env["platform"] == "windows":
    remove_runtime_flags(env)
    if env["use_static_cpp"]:
        if env["target"] == "editor":
            env.Append(CCFLAGS=["/MTd"])
        else:
            env.Append(CCFLAGS=["/MT"])
    else:
        if env["target"] == "editor":
            env.Append(CCFLAGS=["/MDd"])
        else:
            env.Append(CCFLAGS=["/MD"])

@GreenCrowDev
Copy link

GreenCrowDev commented Nov 21, 2024

For the reference: Godot itself is never using /MTd, and enabling /MDd is done with a separate flag. While GDExtension is a pure-C API and matching CRTs is not strictly necessary (official builds are done using MinGW), it's usually preferred to have the same build flags for both the engine and extensions.

    if env["debug_crt"]:
        # Always use dynamic runtime, static debug CRT breaks thread_local.
        env.AppendUnique(CCFLAGS=["/MDd"])
    else:
        if env["use_static_cpp"]:
            env.AppendUnique(CCFLAGS=["/MT"])
        else:
            env.AppendUnique(CCFLAGS=["/MD"])

I now noticed this: # Always use dynamic runtime, static debug CRT breaks thread_local..
Can anyone explain the implications of this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archived bug This has been identified as a bug topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants