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

Use /MTd and /MDd flags on Windows dev builds #1469

Closed
wants to merge 1 commit into from

Conversation

xissburg
Copy link

Select the debug runtime library for dev builds (i.e. when passing dev_build=yes to scons).

I had to do this change to build my extension successfully in debug mode because I link with other libraries which are built with this flag which is the default choice for debug builds. Otherwise, I get linker errors of the kind "error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MTd_StaticDebug' in <file>.obj"

Select the debug runtime library for dev builds.

I had to do this change to build my extension successfully in debug mode because I link with other libraries which are built with this flag which is the default choice for debug builds. Otherwise, I get linker errors of the kind `"error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MTd_StaticDebug' in <file>.obj"`
@xissburg xissburg requested a review from a team as a code owner May 19, 2024 19:29
@AThousandShips
Copy link
Member

This is controlled by the debug_crt option in the engine, and not dev_build, I think we should match here

@AThousandShips AThousandShips added platform:windows topic:buildsystem Related to the buildsystem or CI setup labels May 20, 2024
@bruvzg
Copy link
Member

bruvzg commented May 20, 2024

It's definitely should be separate debug_crt, using debug CRT is rarely useful and should not be a default for debug builds.

Also, /MTd is never used by the engine (due to broken thread_local at least in some versions), but it's irrelevant what's CRT is used in the engine (GDExtension API is pure C and is not passing CRT objects), so it's should be OK to support /MTd as well.

Engine use something like:

    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"])

@xissburg
Copy link
Author

Thank you for clarifying. I will then recompile my libraries with /MT or /MD instead.

@dsnopek
Copy link
Collaborator

dsnopek commented May 21, 2024

@xissburg Are you going to update this PR to match the engine (per the code @bruvzg shows above)? Or, are you simply going to switch to not using /MDd and so this PR can be closed?

@xissburg
Copy link
Author

@dsnopek I'll close since this does not seem to be an issue to nobody else since it hasn't been brought up before apparently. I'll adopt the same practice used by the engine.

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

Successfully merging this pull request may close these issues.

4 participants