search

conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.27k stars 981 forks source link

Incorrect handling of MSVC versions #3573

Closed DoDoENT closed 3 years ago

DoDoENT commented 6 years ago

I am using Conan v1.7.4.

I've created a static library Conan package using VS 2017 15.7 and updated my Visual Studio to 15.8. After the update, Conan failed to detect a change in the compiler version and did not perform recompilation of the static library. Instead, it just downloaded the same package that was built with VS 2017 15.7. However, when linking to that package from VS 2017 15.8, I am getting following linker error:

LINK : fatal error C1047: The object or library file 'C:\Users\dodo\.conan\data\LogAndTimer\1.0.1\microblink\stable\package\8b2772668ccbf879d34a70a9d54c78ce70f38a1b\lib\LogAndTimer.lib' was created with an older compiler than other objects; rebuild old objects and libraries
2>LINK : fatal error LNK1257: code generation failed

However, the debug build does work (details how to reproduce are below).

I think the problem is in how Conan handles MSVC versions - instead of using the actual compiler version (CMake detects MSVC 19.14.26431.0 in VS 2017 15.7 and MSVC 19.15.26729.0 in VS 2017 15.8), it uses the version of Visual Studio (VS 2017 is compiler version 15, according to Conan).

I suggest that Conan should use the MSVC compiler version under compiler.version setting and additionally have compiler.vs_version for Visual Studio version. The reason for that is that sometimes libraries built with one version of MSVC are not compatible with later versions of MSVC, especially if you use link time code generation flag (/LTCG link flag and /GL compile flag).

How to reproduce

Expected behaviour

Actual behaviour

Now, the main question is - could this be fixed without breaking existing conan VS packages, or we need to wait for Conan 2.0 (and disable LTCG until this issue gets fixed).

Also, as a side question, is there a possibility to configure Conan to trigger rebuild of libraries even when a minor/bugfix version of the compiler changes, regardless of the compiler being used (e.g. if we update GCC from 8.1.0 to 8.1.1, we would like to rebuild all our binaries, even though they are link-compatible). This would get us the benefits of always having binaries built with the latest compiler to get the latest bugfixes and performance improvements - this benefit is also mentioned in conan documentation:

What happens if we have a library that we can be built with GCC 4.8 and will preserve the ABI compatibility with GCC 4.9? (This kind of compatibility is easier to achieve for example for pure C libraries).

Although it could be argued that it is worth rebuilding with 4.9 too -to get fixes and performance improvements-. Let’s suppose that we don’t want to create 2 different binaries, but just a single built with GCC 4.8 which also needs to be compatible for GCC 4.9 installations.

DoDoENT commented 6 years ago

@memsharded, any hints on how this can be worked around?

lasote commented 6 years ago

Normally the base settings.yml file is designed as a minimum for the community and so far the visual studio segmentation by 14, 15 worked well.

If you need a more accurate model you could:

A) Introduce a subsetting. If the subsetting has a "None" value as a possible value it won't be required, and all the current conan center packages will be compatible. If you give a value to the subsetting, you will be generating a new package_id.

B) Change the list of versions including the minor (15.8) but keeping the "15" value. So the old packages will still be "usable" if a user specifies just "15" but you can generate more accurate binaries for the users specifying "15.8". But probably we would need to check the conan code to search all the Visual Studio version setting comparison to ensure "15.8" is processed correctly, the CMake checks etc.

I think the correct is the B) and might be the way to go, even if by default Conan detects "15" instead of "15.8".

BUT, of course, this is something the community can give feedback, I see many upvotes. Are they friends/colleagues of you? or is the whole community really demanding a better approach? ;)

@memsharded feedback is needed here too.

DoDoENT commented 6 years ago

Hi @lasote, thank you for the tip. I'll try that and see if it works for me.

However, in general, I think the approach of using VS version as MSVC version is fundamentally wrong. You can use VS with any compiler, MSVC being only one of the possibilities. Currently, this can be distinguished using toolsets, but this still lacks another configuration dimension, as a specific toolset can have their own versions. This is currently worked around by having "different" toolsets, such as v140 (indicating MSVC from VS 2015 used within VS 2017), v120, etc. However, this is not very practical, as you can have lots of toolsets with different compilers and compiler versions.

Therefore, I think it would be a better approach for Conan to treat MSVC in the same way as it treats GCC and Clang - as the compiler, not as the IDE. Its subsettings would be version and runtime and possibly other things that are specific to the MSVC compiler, not to the VS IDE. Such an approach would also cover combinations where MSVC is used in non-MSBuild build systems. For example, if you use VS 2017's feature open folder to open a folder containing CMakeLists.txt file, it will generate a build directory with MSVC and Ninja build system. Besides that, Conan also uses the same philosophy for Apple Clang, which it treats as a separate compiler, not as the "Xcode IDE" (which can use a non-clang compiler, just as VS can use non-MSVC compilers).

I think the correct is the B) and might be the way to go, even if by default Conan detects "15" instead of "15.8".

Although this may work (I still need to check that), what would then mean to have version "15.8" and toolset "vs140_xp"? Would that be different from version "15.7" and same toolset? It shouldn't be, but Conan would calculate different package ID for that combination.

... and so far the visual studio segmentation by 14, 15 worked well.

This segmentation works well until you enable the link time code generation (link time optimization). I guess that industry that uses that feature of the compiler/linker still hasn't made the transition to C++ package managers (I still need to check how vcpkg works with that) and that open source projects and teams that are using Conan either do not target Windows as their main platform or is not aware of the /LTCG optimization, so they still haven't stumbled upon that issue.

Also, I am not entirely sure that current visual studio segmentation works well in cases when you change the version of the Visual Studio (for example from VS 2015 to VS 2017), but keep using the same toolset (e.g. vs_140) - will Conan calculate new package ID in that case? It shouldn't because the compilers used for creating the binaries have not changed - only the code editor has.

I would really like to see what other people think about that, especially all those who gave a thumbs up.

Maybe for Conan v2.0 (breaking release), it would make sense for Conan to treat MSVC as the compiler, not as the IDE (i.e. as "Visual Studio").

memsharded commented 6 years ago

Yes, we could revise this for 2.0. However, one thing is that it could be changed, and the other is the massive impact that it could have on existing packages in bintray.

In any case, maybe by conan 2.0, Msft has estabilized a bit and we have better idea of their plans regarding compiler minor updates too.

memsharded commented 6 years ago

Actually.... this could and probably should be discussed earlier. If anything, it could make sense to introduce a new compiler in the setting, to replace the old one, but without replacing. Conan 2.0 would deprecate the previous one (without removing), maybe Conan 3.0 could remove the old one. Lets discuss.

psiha commented 6 years ago

In any case, maybe by conan 2.0, Msft has estabilized a bit and we have better idea of their plans regarding compiler minor updates too.

There really is nothing to stabilize here - Visual Studio and the Visual C++ (AKA MSVC) have always been separate products (one is an IDE and the other a compiler - e.g. you can get the compiler w/o the IDE through the Windows SDK) and their separate versioning schemas used currently have been in place for quite some time - it is just plain wrong to 'guess' or 'tie' the version of the compiler to the version of the IDE (not just because, as DoDoENT already pointed out, you can have multiple toolchains and compiler versions 'within' one IDE version). IOW&IMNHO there should be no question of whether this needs to be fixed ... only when and how...

With that said, given that it is already creating problems or even showstoppers for MSVC users, it would be nice if you could come up with some kind of a workaround until it gets 'officially fixed'...

DoDoENT commented 6 years ago

If anything, it could make sense to introduce a new compiler in the setting, to replace the old one, but without replacing.

I like the idea - introduce new compiler msvc in Conan 1.9.x, but keep Visual Studio a default choice for Windows with the same behaviour as current. The msvc would behave in the same way as gcc and clang. Then, in v2.0 switch the windows default from Visual Studio to msvc, but keep Visual Studio as a legacy option. Finally, remove Visual Studio in Conan v3.0 and keep it only as the generator, not as the "compiler".

I think this should be accompanied by appropriate updates in cmake-conan project, but I think this should not be difficult (such a change is even within my capabilities :P )

The introduction of msvc in v1.9.0 would enable those that have the same problem as we do to circumvent them almost immediately, without waiting for breaking change - although using prebuilt packages from bintray would require some additional code in consumer's conanfiles** (let's discuss that as well - I think that would also require some backwards-compatible additions to conanfiles).

Then, with v2.0 users that were already using msvc instead of Visual Studio compiler would not have a breaking change and v2.x series would give a plenty of time to everyone to switch their packages to msvc compiler. In v3.0 Visual Studio could be removed just to reduce the code bloat, if necessary. If not, it can remain eternally for backwards compatibility if Conan adopts policies in the way similar to how CMake does***).

** Here I generally think of a way for the consumer to "override" the calculation of the package ID of the package being consumed. The idea here is for the consumer to be able to "tell" Conan that if it uses, e.g. compiler msvc 19.15 that it can safely consume a specific package built for Visual Studio 15 (something like how you tell the versioning schema) - to avoid rebuilding the package for compiler the author of the package didn't anticipate. This is especially useful for binary-only packages built with earlier versions of Conan.

*** I think policies are a good way to keep backward compatibility while providing new features. Yes, it causes bloatware, but I think it is essential for package management software to be as backwards-compatible as possible. Also, as I already mentioned, a recipe should be able to specify the minimum required version of Conan for a recipe to be parsed. This would enable much easier development of packages that support wide range of Conan versions.

sigiesec commented 6 years ago

To my knowledge, MSVC relies much more on LTCG (and PGO) to generate optimal code than other compilers (in particular gcc and clang) do, even though they also provide comparable features. But as @DoDoENT said, maybe not everyone is aware of that. I have found build files of many open source libraries that were not enabled for LTCG.

MS never guaranteed binary compatibility for .lib files between different toolset versions, but in many cases it worked anyway (or at least it did not obviously not work). However, before Visual Studio 2017 there haven't been multiple toolset versions for the same major Visual Studio version, or at least they did not exhibit such obvious incompatibilities.

Also, not all users of Visual Studio will have already migrated to Visual Studio 2017. This is also true for us. We have prepared new versions of several components that are built with the VS2017 toolsets already, and during development we already stumbled upon this problem where a mismatch between the toolset used on the CI and locally caused problems. But the products using these components still use VS2015 as of now. I am somewhat scared of what will happen when they start to use VS2017, and get compilation failures because of mismatches in the toolset versions.

Just for information, I am not a coworker of @DoDoENT (since @lasote was asking).

lasote commented 6 years ago

Hi all! Thanks for the feedback. About the compatibility between the old Visual Studio setting and the new one, it is not that easy, we would need to think about it carefully but it is not going to be solved in the firsts versions. By the way, it cannot be in 1.8, it is already packaged and struggling to release it, so anyway this will wait for 1.9. Sorry.

DaniCybereason commented 6 years ago

any workarounds meanwhile? I run and it doesn't detect VS 2017 at all after the latest 15.8.7 update..

lasote commented 6 years ago

@DaniCybereason it seems that it is a different issue. Could you please open a new one specifying the conan version you are using?

claasd commented 6 years ago

How about introducing a new compiler in conan 1.9, maybe called MSVC. This could have the version 19.14, 19.15 and so on.

sigiesec commented 6 years ago

@claasd I think you are suggesting the same as @DoDoENT already did above (https://github.com/conan-io/conan/issues/3573#issuecomment-424000299)

claasd commented 6 years ago

@sigiesec True, i did not read carefully enough.

lasote commented 6 years ago

Hi all, I really would like to start with this because I agree it is something to fix and better to try it as you suggested before Conan 2.0 but we hadn't time enough this release either. :(

claasd commented 6 years ago

Hi, we discussed this internally, and found out that CMake adressed this issue in CMake 2.13: Some links: https://cmake.org/cmake/help/latest/variable/CMAKE_VS_PLATFORM_TOOLSET_VERSION.html https://gitlab.kitware.com/cmake/cmake/merge_requests/2093 https://gitlab.kitware.com/cmake/cmake/issues/17549 https://blogs.msdn.microsoft.com/vcblog/2017/11/15/side-by-side-minor-version-msvc-toolsets-in-visual-studio-2017/

So, with CMake supporting this (soon) as minor version of Visual Studio Compiler (beginning at 2017), I would suggest enhancing the current Visual Studio compiler with a minor_version subparam, that is only used for vs2017.

When installing VS2017 SideBySide, it will install multiple vcvars.bat, one for each installed compiler. This way, the minor version could also be implemented for the MsBuild Helper.

Of course, conan needs to check if the cmake-version supports this, if the vcvars are available and so on, but I think it is a better soulution that to introduce a fully new compiler.

DoDoENT commented 6 years ago

@claasd, I disagree with you. Your proposal is tightly coupled with specific cmake version and msbuild build system.

What about other generators (vs, autotools, custom tools)? Also, how would your proposal work with MSVC+Ninja combination, as used by visual studio code and visual studio's "open folder" feature?

Isn't it much simpler to simply introduce msvc as the new compiler to Conan and eventually stop treating a specific code editor (VS) as compiler?

lasote commented 5 years ago

We will look at this before 2.0 to see what can we introduce in 1.X. We understand this is important.

lasote commented 5 years ago

https://twitter.com/Donzanoid/status/1111682600215678976?s=09

lasote commented 5 years ago

So, from the twit joke above we have this table, do we all agree the first column should we use? @DoDoENT @sigiesec @psiha @memsharded @claasd image

We need to discuss what to do with the CMake generator argument if the IDE is not available as a setting, probably we should try to deduce (or try) from the compiler version but let the user override it with an environment variable/conan.conf?

sigiesec commented 5 years ago

By first column you mean the version numbers which include 14.16 for the most recent version listed, right? I think that's fine to use.

I don't understand what you mean by "what to do with the CMake generator argument if the IDE is not available as a setting". Can you give an example where this is the case?

lasote commented 5 years ago

Yes, to both questions. I meant, the CMake build helper calls Cmake, but it needs to specify a generator, and the generator contains the "Visual Studio 15 2017...etc". If now we don't have that as a setting, how we should deduce it?

DoDoENT commented 5 years ago

I meant, the CMake build helper calls Cmake, but it needs to specify a generator, and the generator contains the "Visual Studio 15 2017...etc". If now we don't have that as a setting, how we should deduce it?

Well, that actually depends on the selected cmake generator. For example, if ninja or make are used, then compiler can be given via CC/CXX environment variables. OTOH, if Visual Studio generator is used, then either some mapping is required or CMake Toolset needs to be set.

sigiesec commented 5 years ago

If a VS solution should be generated, the CMake generator to use is the one that can be read from the table. I don't see a use case where this mapping should be overridden by the user. This would mean that a different toolset would be used, and if the user wants that, they can and should specify a different compiler.version. The CMake generator does not really refer to the IDE to use. If you want to use a different IDE, you generate with the generator corresponding to the toolset to use and then open the solution with a newer IDE without migrating the project files.

jgsogo commented 5 years ago

We are going to make a proposal following the lines of all that has been discussed here: it should be a new compiler for Conan v1.x that will use the toolset version. As it is more common to hear about toolset v140, v142 IMO we should choose the toolset over the _MSC_VER variable that is available as a macro provided by the compiler, anyway there is bi-univocal correspondence between them.

The first approach (or the settings.yml provided by default by Conan) should consider only the first number in the minor version as it should be compatible and it is not so easy to have several minor versions installed at the same time (also this), nevertheless as there are bugs from time to time the implementation should allow an advanced user to use full version number (v14.11 -vs- v14.12) in their custom settings.yml. _Note.- There is a patch version for toolsets too (as it is _MSC_FULL_VER, but IMHO it is too much to consider at this moment, although if possible, the implementation should not forbid using it_.

The profile for this new compiler will contain the toolset version and Conan should be able to go from the toolset version to the compiler that is able to generate the binaries or even to the IDE version if we need to generate the solution using CMake for example. We need to write a POC about this first, but these are some insights to build a POC for this:

With this approach we should be able to go back to VS 2010, older versions could require other approaches, but I haven't looked for information about them, sorry.

Once we have the IDE (and the toolset version) we know the CMake generator name and we can use the cmake ... -G "Visual Studio Studio ..." -T <toolset> or we can call one of the vcvars scripts with the proper arguments,... or find the MSBuild...

Probably it is not as easy as it looks like, but I think it is feasible and there are good foundations to start building on top of it.

Thanks everyone for the feedback

There is still something else I would like to mention (and probably discard): what about the Windows SDK? is it something we want/need to consider? And what about the Framework .NET version, can it generate incompatible binaries? Probably these are just subsettings that the user building binaries that depend on them should take into account, but not for the default settings.yml

Ping @DoDoENT @sigiesec @claasd @psiha @DaniCybereason for a final round of feedback before we start walking this path. Thanks!

DoDoENT commented 5 years ago

This sounds reasonable for me as long as there will be a guarantee that each MSVC compiler version will have its own toolset, i.e. that there will be no MSVC compiler updates (at least incompatible ones, in terms of LTO) without changing the toolset version.

Also, we will also need to update cmake-conan to correctly infer toolset from MSVC version detected by the CMake.

SSE4 commented 5 years ago

some notes:

DoDoENT commented 4 years ago

We need to discuss what to do with the CMake generator argument if the IDE is not available as a setting, probably we should try to deduce (or try) from the compiler version but let the user override it with an environment variable/conan.conf?

Well, according to this, when conan invokes vcvars it should set the appropriate toolset with vcvars_ver.

As far as I see, conan already supports setting this parameter to vcvars, yet it sill does not support MSVC as the compiler.

Any update on this?

jgsogo commented 4 years ago

Hi! This is something we really want to implement, it will surely be a new compiler more than a refactor or evolution on top of current "Visual Studio". It has really high priority, but we don't have enough bandwidth to address all the issues, now we are more focused on breaking behaviors for the transition to version 2.0 and this should be (I hope) a new feature that won't break existing recipes.

DoDoENT commented 4 years ago

OK, I solved this problem for our use cases. The solution (ab)uses the fact that conan will not set vcvars if they are already set and assumes that developers will never manually invoke conan install or conan create on Windows systems and that all packages use CMake build system. This assumption is true for us, as we use cmake helper that automatically invokes conan install during cmake configuration.

Here is how I did it:

First, I've added a msvc compiler to my settings.yml. The entry looks like this:

    msvc:
        runtime: [MD, MT, MTd, MDd]
        version: ["19.25", "19.26", "19.27"]
        cppstd: [None, 14, 17, 20]

Then, I've created several conan profiles:

msvc-generic:

[settings]
os=Windows
os_build=Windows
arch_build=x86_64
arch=x86_64
compiler=msvc
build_type=Release

msvc-19.27:

include(msvc-generic)

[settings]
compiler.version=19.27

[env]
CONAN_CMAKE_GENERATOR = Ninja

vs-19.27:

include(msvc-generic)

[settings]
compiler.version=19.27

[env]
CONAN_CMAKE_GENERATOR = Visual Studio 16

and similar for every MSVC version (19.25, 19.26 and 19.27).

The real magic happens here. While configuring the project and deciding about command line parameters to be given to conan install, cmake script first obtains the version of MSVC compiler currently used and decides to use vs-<version> if Visual Studio generator is used and msvc-<version> if not (in our case it's Ninja, but it should also work with Makefiles).

The idea is the following:

Of course, this assumes that developers will not manually invoke conan install/create, as invoking conan install/create conanfile.py -pr vs-19.25 while having Visual Studio updated to v16.7 will create invalid packages (the package will be built with MSVC 19.27, but conan package will advertise 19.25, which is wrong).

Similar could happen (I didn't test) if invoking conan install/create conanfile.py -pr msvc-19.25 from the shell where vcvars are initialized to msvc 19.27 (although this line of code may catch that, but I'm not entirely sure).

Fortunately, the profiles I described above don't have compiler.runtime set, and conan automatically fills the runtime information only for Visual Studio and intel compilers, so if you accidentally invoke conan install/create manually, you will get an error that the compiler.runtime setting is missing 🎉 . This is hacky, but it works 😛

On the other hand, the Jenkins that builds our packages is configured to always use Ninja generator and set correct vcvars environment before invoking any Conan commands.

In general, I would suggest that conan gets native support for MSVC compiler. The implementation may be based on this solution:

puetzk commented 3 years ago

as there will be a guarantee that each MSVC compiler version will have its own toolset, i.e. that there will be no MSVC compiler updates (at least incompatible ones, in terms of LTO) without changing the toolset version

This is not the case: e.g. the v141 toolset encompasses all VS2017 versions, the v142 all VS2019 versions, and yet there are LTO-incompatible changes the minor versions (in a semver-conforming way; the newer linker can read old libraries, but not vice-versa)

https://devblogs.microsoft.com/cppblog/side-by-side-minor-version-msvc-toolsets-in-visual-studio-2019/

DoDoENT commented 3 years ago

@puetzk, if you refer to VS 16.7.0 vs 16.7.1 and so on - they are LTO link compatible - I've tested that.

There are incompatible changes between minor versions of v142 toolsets, as it always select the latest compiler from the v142 toolset branch. But you can select the exact compiler version with vcvarsall.bat. For example:

vcvarsall.bat amd64 -vcvars_ver=14.25

will select the MSVC 19.25 for current shell (to be used with ninja/make etc.), even if you have updated your VS up to 16.8 (you just need to install 14.25 build tools using VS installer tool). However, AFAIK, it's not possible to select that same compiler using VS project sheet in MSBuild project.

memsharded commented 3 years ago

I am proposing some preliminary investigation in https://github.com/conan-io/conan/pull/8201. Some ideas:

DoDoENT commented 3 years ago

Changing MT, MTd, MDd, MD for "static", "dynamic" for the runtime, to avoid incompatible combinations with build_type.

Actually, we are building our windows dev packages in release mode, but with MDd runtime. This enables both optimizations of the binary and the debug runtime (iterator checks and other). This is what we put in a "Debug" version of our windows conan package.

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 😛

Using the major toolset version "140", "141", "142" as the version, but adding a MSC_VER 19XX as an optional sub-setting for each one, for those that want to manage different binaries (LTO) for minor MSC_VER values.

Seems completely reasonable.

memsharded commented 3 years ago

Actually, we are building our windows dev packages in release mode, but with MDd runtime. This enables both optimizations of the binary and the debug runtime (iterator checks and other). This is what we put in a "Debug" version of our windows conan package.

Ok, this is very useful information. Some other users were reporting that they were totally incompatible, check https://github.com/conan-io/conan/issues/7019

memsharded commented 3 years ago

Actually, we are building our windows dev packages in release mode, but with MDd runtime. This enables both optimizations of the binary and the debug runtime (iterator checks and other). This is what we put in a "Debug" version of our windows conan package.

@DoDoENT Can you please clarify, the "Debug" thing?. You say it is "windows dev packages in release mode", but then you say that you put it in the "Debug" version.

SSE4 commented 3 years ago

Using the major toolset version "140", "141", "142" as the version, but adding a MSC_VER 19XX as an optional sub-setting for each one, for those that want to manage different binaries (LTO) for minor MSC_VER values.

is there any reason to complicate versioning so much? I mean getting one part of setting from the one place (toolset), and the second part of setting from the another (_MSC_VER but only two digits).

why not just use the compiler version like for all other compilers and be consistent?

also, this is a kinda time bomb, as soon as _MSC_VER runs out to 20XX or 21XX (it may happen eventually, and earlier than we can expect).

DoDoENT commented 3 years ago

@DoDoENT Can you please clarify, the "Debug" thing?. You say it is "windows dev packages in release mode", but then you say that you put it in the "Debug" version.

This is what conan thinks to be the debug, although it's actually built with optimizations.

So, if you issue this command for our internal package: conan create ./conanfile.py user/testing -pr msvc-19.27 -s compiler.runtime=MDd -s build_type=Debug, the cmake invoked by our recipe's build method is will actually build the binary in release mode, but with debug runtime - we call this "dev-release" internally, but it's similar to CMake's RelWithDebugInfo, except that besides Debug Info it also contains debug runtime checks. This gives us both debugability and performance and our CI only has to build two types of packages: Release (i.e. what Conan sees as "the release" and is built in release mode with full optimizations and LTO, disabled asserts and with release runtime library (MD)) and "Debug" (i.e. what conan sees as "the debug", but is actually built with full optimizations without LTO, enabled asserts and with debug runtime library (MDd)).

And yes, we know that conan supports the "RelWithDebugInfo" build type, but as I said, our "DevRelease" build type is something in between "RelWithDebugInfo" and "Debug".

Some other users were reporting that they were totally incompatible, check #7019

MD and MDd are just two different c++ runtime libraries - both dynamic, i.e. provided as DLLs (unlike MT and MTd, which are static, provided as .lib file and end-up in your app's binary). By default VS links your app to MDd when building in debug mode and to MD when building in release mode, but nothing prevents you to enable compiler optimizations and then link to MDd or vice versa. It's just important not to mix binaries built with MD and binaries built with MDd in the same app - this will result with linker error (some symbols will be duplicated, some will be missing).

The difference between MD and MDd is similar to libc++ vs libstdc++ in the Linux world - they both provide the C++ runtime, but have different implementations and cannot be mixed together in the same app. The difference is that libc++ and libstdc++ are both optimized versions implemented by different teams, while MD and MDd are just different builds of the same runtime, but ABI-incompatible (MDd STL containers contain additional fields to aid debugging).

is there any reason to complicate versioning so much? I mean getting one part of setting from the one place (toolset), and the second part of setting from the another (_MSC_VER but only two digits).

I guess it's due to the compatibility with current packages in conan-center that are not aware of the MSVC versions.

why not just use the compiler version like for all other compilers and be consistent?

I actually want that too. It's more clean and elegant.

Croydon commented 3 years ago

why not just use the compiler version like for all other compilers and be consistent?

I agree. MSVC presents itself with such a version number and CMake does too

> cl
Microsoft (R) C/C++ Optimizing Compiler Version 19.28.29335 for x86
-- Selecting Windows SDK version 10.0.18362.0 to target Windows 10.0.19042.
-- The C compiler identification is MSVC 19.28.29335.0
-- The CXX compiler identification is MSVC 19.28.29335.0

Though, CMake adds another .0 to it for some reason.

memsharded commented 3 years ago

Can you please clarify (@SSE4 , @DoDoENT , @Croydon ) with an example the actual settings.yml that you would use?. Here is the rational that I am using:

Maybe you mean just to change the names and use 140 => 19.0, 141 => 19.1, 142 => 19.2? And then leave the rest as a subsetting? If that is the case, yes, totally good for me.

SSE4 commented 3 years ago

Can you please clarify (@SSE4 , @DoDoENT , @Croydon ) with an example the actual settings.yml that you would use?

msvc: &msvc
runtime: [MD, MT, MTd, MDd]
version: ["14.00", "15.00", ...<many versions>, "19.26", "19.27", "19.28"]

the rational:

otherwise, if you still want to use the toolset version, you'd better change the setting name to the toolset.version, to avoid the source of confusion for users.

I am proposing the 140, 141, 142, as they are aligned with the default binary compatibility model: binaries are compatible by default by the toolset version, not the full compiler version.

I am not sure it's the default compatibility mode for MSVC, given the comment from STL. see also:

It is not possible (maybe it is, but seems challenging), to have more than 1 mscver installed at the same time in one machine.

it is actually very possible. I have all of the versions installed in parallel, starting from 2005 to 2019. plus, there are compilers installed as part of Windows SDK and other Microsoft SDKs.

jgsogo commented 3 years ago

Just pointing that toolsets are more like v14.00, v14.10, v14.11, v14.12, v14.20, v14.21 than v141, v142. Now you can have all of them side by side, although the IDE lets you select only one for the same minor or you need to modify props files: https://devblogs.microsoft.com/cppblog/side-by-side-minor-version-msvc-toolsets-in-visual-studio-2019/ and then you have all the numbers required to match a specific MSVC compiler.version as @SSE4 proposes.

From my experience, I was more used to toolset versions that to MSVC compiler version (IDE Visual Studio)... although it is true that the macro you get in sources is the compiler and not the toolset.

DoDoENT commented 3 years ago

Can you please clarify with an example the actual settings.yml that you would use?

I've actually described the settings.yml I use in this comment above.

I am proposing the 140, 141, 142, as they are aligned with the default binary compatibility model: binaries are compatible by default by the toolset version, not the full compiler version.

This seems reasonable for the default VS settings, where LTO is disabled. However, this is then not the msvc compiler - it's visual studio toolset - that should be different from msvc compiler.

Then, for users that want finer control of the binaries, they can use the mscver.

Here is where I would actually treat that as the separate compiler, for simplicity, not as the mscver subsetting. If you choose to use msvc compiler instead of visual studio toolset then you should probably have different binaries. Yes, sometimes they might match, but this is similar to how some GCC vs Clang binaries are compatible.

It is not possible (maybe it is, but seems challenging), to have more than 1 mscver installed at the same time in one machine. So this is not actionable at all, it is just information about the version, but nothing that Conan can use to select one or other.

Yes, it's possible and we have multiple msvc versions installed on our CI machines. I've even described in the comment linked above how you can force conan to select correct msvc version.

Maybe you mean just to change the names and use 140 => 19.0, 141 => 19.1, 142 => 19.2? And then leave the rest as a subsetting? If that is the case, yes, totally good for me.

For msvc compiler, the versions should definitely be in line with what compiler reports (19.25, 19.26, 19.27, ... ). The 140, 141, ... versions are in line with visual studio toolset and should not be used for msvc compiler.

So, to conclude:

If all that seems too complicated (I'd guess most people will get confused by having both msvc and visual studio "compilers"), and if we just want to have only one msvc compiler, then I would definitely vote for the first option (msvc, not visual studio). The versions should then be in sync with versions reported by cl.exe and for general case (when LTO is not needed), the version should be parsed from major number (i.e. 19.1, 19.2, ...) and if someone needs LTO and fine-grained control, they can easily add versions 19.16, 19.17, 19.25, 19.26, ...) to their settings.yml.

This is not without precedent: the default settings.yml, shipped by conan, already has such versioning scheme for GCC: there is version 9, but also versions 9.1, 9.2 and 9.3. Version 9 is OK for general case, but if you use LTO, you need to use 9.1, 9.2 and 9.3.

We use the same trick also for Apple Clang - without LTO all compilers shipped with Xcode 11 produce link-compatible binaries, but if you enable LTO, there are actually two different versions of clang in Xcode 11 series: 11.0.0 and 11.0.3, which are not LTO-link compatible.

So, why not doing the same with MSVC as well?

memsharded commented 3 years ago

I have updated https://github.com/conan-io/conan/pull/8201 to implement:

 msvc:
            version: ["19.0",
                      "19.1", "19.10", "19.11", "19.12", "19.13", "19.14", "19.15", "19.16",
                      "19.2", "19.20", "19.21", "19.22", "19.23", "19.24", "19.25", "19.26", "19.27", "19.28"]
            runtime: [static, dynamic]
            cppstd: [None, 14, 17, 20]

The mapping to default VS versions (to call cmake -G "Generator") is done dropping the latest digit if existing. This is basically the same approach done for gcc and clang. Auto-detect (when this happens, Conan 2.0) will resolve to 19.1, 19.2, assuming that binary compatibility, and letting users to opt-in into the full versions for LTO and other binary-incompatible issues.

I still need to figure out the runtime and work on the migration plan for package_id compatibility with existing Visual Studio compiler.

solvingj commented 3 years ago

Changing MT, MTd, MDd, MD for "static", "dynamic" for the runtime, to avoid incompatible combinations with build_type.

Actually, we are building our windows dev packages in release mode, but with MDd runtime. This enables both optimizations of the binary and the debug runtime (iterator checks and other). This is what we put in a "Debug" version of our windows conan package.

I don't think we can enforce the combination of runtime and build_type. Tons of companies build .dlls with MT. It works fine for so many people, they don't care that it's not safe or not recommended.

solvingj commented 3 years ago

The version and cppstd look good. I would revert to the previous build_type and runtime. I admit that as a user, it's painful and frustrating to manage and work with the microsoft runtime settings. Sadly, I don't think it's something we can ever abstract away.

memsharded commented 3 years ago

I am proposing this:

msvc:
            version: ["19.0",
                      "19.1", "19.10", "19.11", "19.12", "19.13", "19.14", "19.15", "19.16",
                      "19.2", "19.20", "19.21", "19.22", "19.23", "19.24", "19.25", "19.26", "19.27", "19.28"]
            runtime: [static, dynamic]
            runtime_type: [Debug, Release]
            cppstd: [None, 14, 17, 20]

The runtime_type will be filled automatically based on build_type, but the user always has the possibility of explicitly defining it. I am testing it for the CMake toolchain, seem to be a good approach so far.

solvingj commented 3 years ago

Ahhhh, ok i think i see it now. This might work.

memsharded commented 3 years ago

This has been implemented in https://github.com/conan-io/conan/pull/8201 and will be released as experimental in 1.33.

It is basic support:

More work will follow in next releases, specially the custom mapping to different Visual Studio versions with toolsets.

DoDoENT commented 3 years ago

Oops, Microsoft did it again 🤦‍♂️