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
base: develop2
Are you sure you want to change the base?
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 |
@@ -101,7 +101,7 @@ | |||
exception: [null, dwarf2, sjlj, seh] # Windows MinGW | |||
cppstd: [null, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17, 20, gnu20, 23, gnu23] | |||
msvc: | |||
version: [170, 180, 190, 191, 192, 193] | |||
version: [170, 180, 190, 191, 192, 193, 194] | |||
update: [null, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9] |
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.
Are we not missing update values then if it can go higher than 10? Or am I not seeing something?
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.
Conan msvc.update
is the compiler update, not the IDE update. In that sense it is just and added version number to 193+update
, 194+update
, and it cannot be 10. So IDE 17.10 (ide version 10, update 10) => msvc 1940 (version=194, update=0)
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.
Ohh, it's not the IDE one, makes sense now :) ty!
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.
Yep, it seems the best we can do with the MSVC change of versioning approach.
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?
|
I am not sure what would be the issue, Conan seems to be doing the right thing, and using
Yes, this is expected, 193+10 doesn't exist, it is 194+0 because Conan models the compiler version, not the IDE update version |
in CMake you need to still use v143 for new update |
But that doesn't make sense? Microsoft documented the right toolset is v144, isn't it? |
Oh, you are right, thanks very much for pointing this! I have just uploaded a fix, could you please re-try? |
With fix conan has successfully built sqlite3 with proper toolset
|
Thanks for testing @Nekto89 Indeed the compiler with "MSVC toolset 19.40.xxx" is still part of the @memsharded I think it looks okay now, but I will double check the activation of vcvars, and the ability to force the 193 compiler version when the 19.4 is also available (and would otherwise be picked up by default) There is another place that is worth looking into: conan/conan/tools/microsoft/visual.py Lines 132 to 135 in a56c639
The code there parses the |
new_combinations.append(comb) | ||
combinations = new_combinations | ||
|
||
conanfile.output.info(f"COMbINATIONS {combinations}") |
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.
COMbINATIONS
typo here, but maybe consider having both FACTORS
and COMBINATIONS
not being all caps? Also found it a little confusing, from a user perspective I dont know what factors or combinations are.
Tested:
Here there seem to be multiple factors at play:
Only when using
A meson-toolchain recipe seems fine, which also instantiates vcvars:
|
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 |
Changelog: Fix: Fix for future
msvc
toolset version overflowDocs: Omit
After #15947
Close #15583