-
Notifications
You must be signed in to change notification settings - Fork 986
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
Incorrect handling of MSVC versions #3573
Comments
@memsharded, any hints on how this can be worked around? |
Normally the base 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. |
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
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.
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 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"). |
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. |
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. |
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). 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'... |
I like the idea - introduce new 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 Then, with v2.0 users that were already using ** 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 *** 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. |
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). |
Hi all! Thanks for the feedback. |
any workarounds meanwhile? I run and it doesn't detect VS 2017 at all after the latest 15.8.7 update.. |
@DaniCybereason it seems that it is a different issue. Could you please open a new one specifying the conan version you are using? |
How about introducing a new compiler in conan 1.9, maybe called MSVC.
|
@claasd I think you are suggesting the same as @DoDoENT already did above (#3573 (comment)) |
@sigiesec True, i did not read carefully enough. |
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. :( |
Hi, 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. |
@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? |
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) |
@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
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. |
I am proposing some preliminary investigation in #8201. Some ideas:
|
Actually, we are building our windows dev packages in release mode, but with 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 😛
Seems completely reasonable. |
Ok, this is very useful information. Some other users were reporting that they were totally incompatible, check #7019 |
@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. |
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). |
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: 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".
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).
I guess it's due to the compatibility with current packages in conan-center that are not aware of the MSVC versions.
I actually want that too. It's more clean and elegant. |
I agree. MSVC presents itself with such a version number and CMake does too
Though, CMake adds another |
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. |
the rational:
otherwise, if you still want to use the toolset version, you'd better change the setting name to the
I am not sure it's the default compatibility mode for MSVC, given the comment from STL. see also:
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. |
Just pointing that toolsets are more like 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. |
I've actually described the
This seems reasonable for the default VS settings, where LTO is disabled. However, this is then not the
Here is where I would actually treat that as the separate compiler, for simplicity, not as the
Yes, it's possible and we have multiple
For So, to conclude:
If all that seems too complicated (I'd guess most people will get confused by having both This is not without precedent: the default 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? |
I have updated #8201 to implement:
The mapping to default VS versions (to call cmake -G "Generator") is done dropping the latest digit if existing. I still need to figure out the |
I don't think we can enforce the combination of |
The |
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 |
Ahhhh, ok i think i see it now. This might work. |
This has been implemented in #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. |
@memsharded I am not sure whether I got you correct. You are planning to implement support for compiler minor versions in a way that the user will be able to choose which compiler to use during the build? Not sure about the others. But what we need is a way to detect the compiler minor version during build time and place it somewhere into the settings or options in order to ensure that one cannot reuse pre-built binaries which are not compatible. Something similar to my workaround would be highly appreciated for Conan 2.0. Will you think about implementing something like this in Conan natively? def configure(self)
if recipe.settings.os == "Windows" and recipe.settings.build_type == "Release":
vcvars = tools.vcvars_command(recipe)
output = StringIO()
recipe.run(f"{vcvars} && cl.exe", output=output)
regex = re.compile(r'Compiler Version [0-9]*\.(?P<minor>[0-9]*)\.[0-9]*')
compiler_minor = regex.search(output.getvalue()).group("minor")
recipe.output.info(f"Found compiler minor version {compiler_minor}. Updating ABI compatibility to ensure -GL / -LTCO builds will work.")
recipe.options.compiler_minor = compiler_minor |
@schwaerz, No, the applied profile should declare which compiler are you using. If the |
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:
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 andMSVC 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 havecompiler.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
Release
build type of that package is compiled with/GL
compile flag.Release
build type (the app should be linked with/LTCG
linker flag).Expected behaviour
conan install
after updating the VS should calculate different hash for the library and should rebuild it, thus making sure application linking will succeedActual 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:
The text was updated successfully, but these errors were encountered: