-
Notifications
You must be signed in to change notification settings - Fork 990
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 for msvc toolset version overflow #15588
fix for msvc toolset version overflow #15588
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the Microsoft blog post, I interpret a new compiler version that would be "194" ? https://devblogs.microsoft.com/cppblog/msvc-toolset-minor-version-number-14-40-in-vs-2022-v17-10/
If that is the case, there would be further implications, e.g.:
-
https://github.com/conan-io/conan/blob/develop2/conan/tools/microsoft/visual.py#L62 - here
194
should also map to '17` ? -
But here: https://github.com/conan-io/conan/blob/develop2/conan/tools/microsoft/visual.py#L78 -
194
is actually part ofv143
-
May need to revisit the code here as well:
conan/conan/tools/microsoft/visual.py
Line 315 in 748b417
def _vcvars_vers(conanfile, compiler, vs_version): 14.40
to vcvars
Note that if I read the following right, conan profile detect
will detect it as compiler.version=194
:
conan/conan/internal/api/detect_api.py
Line 434 in 748b417
version = f"{version_regex.group('major')}{version_regex.group('minor')[0]}" |
@jcar87 I have proposed a full |
I've tried this branch locally with VS2022 Update 10 Preview 6 + CMake 3.29.2 and it doesn't work
What exactly should be used in profile?
|
if my understanding is correct, there are some scenarios A user that has side-by-side installations of VS2022 and VS2022.10 preview:A) User runs
|
What do you mean by this? I just checked without conan
|
Hi @Nekto89 thanks for following up. Could you paste the bit of the CMake configuration log where it prints the location of cl.exe? On my machine in that directory |
I don't have 14.39 on my machine. It was automatically updated to 14.40. I've also explicitly added 14.40 in VS Installer. |
Some more info:
|
Thanks @Nekto89! I've been doing more troubleshooting using the Windows Sandbox
@memsharded - I think this was just a fluke with my installation and a manifestation of that visual studio installer bug - I think we can assume that both a clean install of the new version, or an upgrade, will be correctly configured to use 14.40 as the "msvc toolset" version. This is really only an issue of users rely on As for the |
I have just updated to VS 17.10 (official release) and got hit by this bug in conan 1.64.1. Following the discussion here I would like to add few comments regardless: In the release version there is no reference to v14.40 "tools version" in @echo off
set "VSCMD_START_DIR=%CD%" && call "C:\Program Files\Microsoft Visual Studio\2022\Community\VC/Auxiliary/Build/vcvarsall.bat" amd64 -vcvars_ver=14.3 with Which apparently identifies MSVC Toolset version and apparently is an abbreviation from v14.40 (as per the comment here https://devblogs.microsoft.com/cppblog/msvc-toolset-minor-version-number-14-40-in-vs-2022-v17-10/) which is extremely confusing as the version should be then 14.40 (as 4 != 40)! Now even more confusing (and where conan enters the picture) is that I am using
but it seems that
and technically should be identified as Now the problem is that despite the fact that this is apparently a minor compiler update from In fact I do not see any reason why the current toolset (v14.40) in platform toolchain (v143) should not be able to equally process the code emitted by both The distinctions conan chooses for maintaining the package binary compatibility seems to be wrong. |
It is likely that it might not be possible. The changes are not that trivial, and it might be too complex or even breaking. Conan 2 was released in February 2023, after 2.5 years in alpha and beta releases, Conan 1 is no longer getting significant new features and updates, specifically if they have risk. I'd say that what you are reporting in the rest of your comment is all related to Conan 1 behavior? Is this the case? If not it would be good to clarify it, and open a new ticket against latest Conan 2.3.1. |
@memsharded |
Conan 2 added specific treatment of the That change implements the correct mappings for 194 <-> v143 toolset, the mapping from VS IDE 17 <-> 193, 194 compiler versions, and it also includes a binary compatibility rule to fall back from 194->193 so users don't need to create new binaries when updating to VS 17.10 |
I guess my point was that the attributes conan uses (to determine the binary compatibility) no longer correspond to the factual state and the approach should be revised. I have been dealing with binary packages built as far as with VS 2015 and usually face binaries from VS 2019 while having the major part from up-to-date VS 2022. According to the conan logic a new VS is automatically a breaking change and packages built for example with This has been just an anecdotal experience which however corresponds to what Microsoft say on the topic https://learn.microsoft.com/en-us/cpp/porting/binary-compat-2015-2017?view=msvc-170 In a wiki table you may also notice that the compiler version is actually made out of four digits and corresponds to major+minor version number (of the That conan declares that versions From the info cited above one way to define binary compatibility would be either use Microsoft own definiton and declare the ranges of compiler versions and map those ranges to an "abstract" compatibility version, or go with an observation that the braking changes are typically happening when compiler major version changes (so anything from I was only using compiler version in the above argument as it seems to be the most suitable candidate for the task. There seem to be somewhat one-to-one mapping to MSVC toolset version (although there is a gap in major versions between VS 2013 and VS 2015, while compiler version is consistent). But if conan used MSVC toolset version (the one with four digits - not the Platform toolchain version - the one with three digits) basically the same argument as above would apply. I understand that 10 years ago the idea might have looked sound and might have even be the only one that worked, but this has diverged over the years to the point where it no longer reflects the reality, nor helps the user. The fact that you had to write "a non trivial" patch to fix may also suggest that the original scheme no longer serves its purpose. |
This was not 10 years ago, but a discussion in the tribe for 2.0, pushed by Conan users, that wanted to use the compiler version and not the IDE version as Conan 1.X did in the past. Regarding the default binary compatibility, in theory the binaries are compatible, but in practice we have learned from thousands of users and from the experience of ConanCenter that this doesn't hold in practice that well. Many libraries and C++ code will have pre-processor definitions based on the compiler version that affects their functionality, ABI and even APIs. We know that in practice developers have added these conditional preprocessor definitions mostly based on the compiler version tied to their IDE version, and jumping when jumping to a new IDE version, that means 190 for VS 2015, 191 for VS2017, 192 for VS2019 and 193 for VS2022. This has proved quite solid binary compatibility default in practice for a few years at large scale, and the recent change in MSVC versioning does not justify changing this default yet. And in any case, this is just a default, we understand that some users might want to use other behavior, so It is easy with Conan, and specifically with the new Microsoft changed their compiler versioning approach, and a solid proof is that for the first time it uses a toolset |
Do not take me wrong. I am not saying (I hope) that using compiler versions was a bad idea, quite contrary. I am saying that the logic built on top of that no longer works. By artificially narrowing the spread (of differences) by tenths of minor version number you can also reduce the chances for potential conflicts by simple statistics. But there you are trying solving a problem which is not yours. Because if my code does not link (or build) with a third party package, because the third party package uses (or defines) some incompatible macros, this is a problem with the third party code (or mine), not with the toolchain or its binary compatibility. I can use (or rather misuse) some built-in macros in a way that even the same toolchain will produce inconsistent code when linking two modules where each used different compiler options. And these modules will be perfectly binary compatible according to conan rules. But this does not mean that the toolchain is bad. It just means that there is always a way how to break things if someone does not care (or does not know what he is doing). |
I understand your point, but unfortunately this is for an ideal world and not how it works in real life. When creating binaries for ConanCenter +1500 packages, we cannot check each project, go to those open source projects and tell them: "hey, you are doing it wrong, you need to change the way you use these macros". So building maybe a few more binaries that would be strictly necessary for some libraries is a much better default that the opposite, because the opposite will result in ConanCenter users having very obscure errors at link time or even worse, at runtime that will be very complicated to debug. Building one binary per 191, 192, 193 compiler version is easy and not that expensive, but the damage caused to users that hit binary compatibility problems is simply much worse. Furthermore, even in the case there could be some binary compatibility accross compiler versions, it is also a possibility that binaries built with more modern compilers have performance optimization or even fixes, so building new binaries when a new compiler 193 comes out even if the binary for 192 was already existing still has benefits for users. There is simply no perfect rule for binary compatibility, and this is the reason Conan allows users to define their own, but our experience is that being overly optimistic has a net bad effect on users, so we prefer a possibly less optimistic scenario (a binary per 191, 192, 193 compiler version) than the opposite as default. As always in computer science, this is just a trade-off, but we are happy so far with this decision. |
@memsharded |
Changelog: Fix: Fix for future
msvc
toolset version overflowDocs: Omit
After #15947
Close #15583
Close #16239