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

[bug] 'Visual Studio' settings are incorrect. #7019

Closed
DmitrySokolov opened this issue May 14, 2020 · 19 comments
Closed

[bug] 'Visual Studio' settings are incorrect. #7019

DmitrySokolov opened this issue May 14, 2020 · 19 comments
Milestone

Comments

@DmitrySokolov
Copy link

To date there are following settings for 'Visual Studio' compiler:

compiler:
    Visual Studio: &visual_studio
        runtime: [MD, MT, MTd, MDd]
        version: ["8", "9", "10", "11", "12", "14", "15", "16"]
        toolset: [None, v90, v100, v110, v110_xp, v120, v120_xp,
                  v140, v140_xp, v140_clang_c2, LLVM-vs2012, LLVM-vs2012_xp,
                  LLVM-vs2013, LLVM-vs2013_xp, LLVM-vs2014, LLVM-vs2014_xp,
                  LLVM-vs2017, LLVM-vs2017_xp, v141, v141_xp, v141_clang_c2, v142]
        cppstd: [None, 14, 17, 20]

The item runtime: should have the following values: [dynamic, static]

The reasons:

You can not use flags MDd, MTd in release builds (and vice versa):

  1. There is the special warning for this case - https://docs.microsoft.com/en-us/cpp/error-messages/tool-errors/linker-tools-warning-lnk4098?view=vs-2019
  2. The docs clearly stated

https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-library-features?view=vs-2019
To build a debug version of your application, the _DEBUG flag must be defined and the application must be linked with a debug version of one of these libraries.

  1. Debug libraries use special version of memory allocation functions, and mixing them with release version is not a good idea - https://docs.microsoft.com/en-us/visualstudio/debugger/debug-versions-of-heap-allocation-functions?view=vs-2019
@jgsogo
Copy link
Contributor

jgsogo commented May 14, 2020

Hi! Yes, this is something that we should probably revisit for Conan v2.0, without breaking current implementation it is not possible to change these default settings. I'm going to add it to the corresponding milestone.

Thanks for the suggestion

@memsharded
Copy link
Member

Hi @DmitrySokolov

We are trying to move forward the new msvc compiler settings, check #8201

Please read the comment from @DoDoENT in #3573 (comment). It is not possible to define only "static" "dynamic" runtimes, they are using combinations, building in release, but linking with MDd runtime.

@DoDoENT
Copy link
Contributor

DoDoENT commented Dec 14, 2020

@DmitrySokolov , nothing prevents you to compile your code in release mode and link it with MDd or MTd. You just need to make sure to define _DEBUG flag so that correct runtime functions will be called. However, you are free to compile your code with optimizations (/Ox, /Oy, /Os and /Ot flags). We are actually regularly doing so in our development (we call that a dev-release build, as it contains all runtime checks as the debug build, but has optimizations of our code enabled, making it much faster).

Of course, you must not mix the object files/static libraries built against MD and MDd (or MT and MTd) in the same final binary (DLL or exe), as those two are not compatible.

@DmitrySokolov
Copy link
Author

DmitrySokolov commented Dec 14, 2020

@DoDoENT
Then it will be the debug configuration, since you

  • define _DEBUG
  • link with debug libraries

Why are you calling it release? It will be the debug configuration with some optimizations switched on.

@DmitrySokolov
Copy link
Author

@DoDoENT
I think more accurately it should be called "optimized-debug" instead of "dev-release".

@DoDoENT
Copy link
Contributor

DoDoENT commented Dec 14, 2020

Why are you calling it release? It will be the debug configuration with some optimizations switched on.

Actually, I am calling this "Dev-Release", as it's not "release" (because of _DEBUG, enabled assertions and MDd) nor "debug" (because of optimizations - you can't step the code in the way it's written). Note that this is different from RelWithDebugInfo.

I think more accurately it should be called "optimized-debug" instead of "dev-release".

I guess you're right 😄 . However, keep in mind that this configuration is different from what VS treats as "Debug".

Also, we use the same name on other platforms (Linux, Mac, Android, iOS), where there are no concepts of "debug" and "release".

@DmitrySokolov
Copy link
Author

DmitrySokolov commented Dec 14, 2020

OK, let's see: you are building a DLL in "dev-release" configuration, can it be used in an app, that uses MD runtime?

@DoDoENT
Copy link
Contributor

DoDoENT commented Dec 14, 2020

OK, lets see: you are building a DLL in "dev-release" configuration, can it be used in an app, that uses MD runtime?

No, you can't - you need to use it in an app that uses MDd runtime (you cannot link together binaries built against MD and MDd runtimes).

It's similar to how you can't link together binaries on Linux built against libc++ and libstdc++ runtimes.

@DmitrySokolov
Copy link
Author

No. OK. So, why it is different from what VS treats as "Debug". Just because you've switched on some optimizations?

@DoDoENT
Copy link
Contributor

DoDoENT commented Dec 14, 2020

Just because you've switched on some optimizations?

Yes - if you step through the code with the debugger, due to reordering of the code caused by the optimizations, the steps won't follow the code as written. Also, some variables may not be visible and some functions could not be stepped-into due to inlining. It's hardly a "Debug" version 😛

@DmitrySokolov
Copy link
Author

I think it will be completely confusing for everyone to use "release" libraries that require "MDd" debug runtime.

@DmitrySokolov
Copy link
Author

DmitrySokolov commented Dec 14, 2020

I see no problem to build optimized debug libraries, and add some conan option to packages, that will distinguish them from ordinary debug packages.

@memsharded
My vote is for removing runtime: [MD ...] from conan settings and adding [dynamic, staitic]. From my point of view, what DoDoENT is actually do is building optimized debug libraries. And they actually do not mix release/debug versions.

@DoDoENT
Copy link
Contributor

DoDoENT commented Dec 14, 2020

Don't get me wrong. I am not against your proposal - I've even written that in my original comment in other thread:

Although I completely agree with your statement for the general conan default, please don't prevent advanced users to still use the "unnatural combinations" as described above 😛

I just don't want to be prevented from doing my "optimized debug builds", as you call it. I'm perfectly fine with having [dynamic, static] option instead of [MD, MDd, MT, MTd], but I do have some questions about those:

  • eventually, msvc compiler needs to get one of the /MD /MDd /MT /MTd flags - who and where will make the conversion from static/dynamic to those flags?
  • what rules will it follow? I guess it's easy to use MDd/MTd for Debug and MD/MT for Release, but what about RelWithDebugInfo?
  • what about custom build types that people add to their settings.yml?
  • should we add some kind of callback to let people choose if their custom build type should use debug or non-debug version of MSVC runtime library?
  • or is that something that new toolchains should handle?
  • should conan probably drop support for custom build types other than Debug, Release, RelWithDebugInfo and MinSizeRelease? I would be definitely against such a proposal

In general, I like the simplicity of static/dynamic very much - it's much more clear, especially to beginners. But, eventually, someone needs to answer the questions above.

I'm OK with having some custom sub-settings, like current MD/MDd/MT/MTd in my own version of settings.yml if I add custom build types to the very same settings.yml - I just don't want conan to prevent me from doing that, while still being simple, as you suggested, for most people out there that don't need such fine-grained control.

@memsharded
Copy link
Member

Ok, I think this discussion makes sense.

I will try to go with static, dynamic for the runtime, but leave the possibility of forcing the "unnatural" (opposite to the defined build_type), for advanced custom configuration.

should conan probably drop support for custom build types other than Debug, Release, RelWithDebugInfo and MinSizeRelease? I would be definitely against such a proposal

Actually this is the opposite. We are right now improving support for custom configurations in CMake and MSBuild

@DmitrySokolov
Copy link
Author

/MD /MDd /MT /MTd flags - who and where will make the conversion from static/dynamic to those flags?

For what purpose?

You build a package, assign a meta (conaninfo.txt) for it, like

[settings]
    arch=x86_64
    compiler=Visual Studio
    compiler.version=15
    os=Windows

When you install a package, you provide settings, like conan install -s os=Window -s arch=x86_64 .... Conan will match this provided settings against packages meta in the cache.

If your package requires some flags, you provide them in package_info().

but what about RelWithDebugInfo?

For VS it's just the flag /Zi, the underlying configuration is still Release

what about custom build types that people add to their settings.yml?

What do mean? If someone adds "build_type=MyConf" in settings.yml how should conan treats it? It's easy - -s build_type=MyConf should be specified both when building packages and when consuming packages.

should we add some kind of callback to let people choose if their custom build type should use debug or non-debug version of MSVC runtime library?

build_type member of ConanFile is already available.

...

I do not actually understand, what kind of a support is required. As for me, I use conan for building (and consuming) packages using QMake, CMake, VS. And I use build configuration names like "Release Pro with someth. turned on" without any troubles.

@DoDoENT
Copy link
Contributor

DoDoENT commented Dec 14, 2020

/MD /MDd /MT /MTd flags - who and where will make the conversion from static/dynamic to those flags?

For what purpose?

You need to provide one of these flags to cl.exe invocation - either via ninja or MSBuild. Which flag would you provide to cl.exe for MyConf build type?

@DmitrySokolov
Copy link
Author

It should be handled by the conan recipe:

class MyPackage(ConanFile):
    package_info(self):
        if self.settings.build_type == "MyConf" and self.settings.compiler == "Visual Studio":
            self.cpp_info.cxxflags = ["/MDd"]

@DoDoENT
Copy link
Contributor

DoDoENT commented Dec 15, 2020

It should be handled by the conan recipe:

But this can be done even now, and does not require neither static/dynamic nor MT/MTd/MD/MDd compiler sub-settings.

However, conan currently automatically appends /MD cxx flag if settings.compiler.runtime == 'MD', regardless of build type - and you cannot remove this - you can only override it later with custom cxx flag, yielding e.g. /MD /MDd in cl.exe invocation.

My question is what should conan automatically append to cl.exe invocation if settings.compiler.runtime == 'dynamic' and settings.build_type == 'MyCoolBuildType'?

@memsharded
Copy link
Member

the new settings model for msvc in 2.0 has solved it:

  • Using compiler versions instead of IDE versions
  • Using "static", "dynamic" for runtime
  • Using "Debug", "Release" for runtime_type (that will be automatically filled with build_type if not explicitly specified

Conan 2.0-beta is out there, feedback welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants