-
Notifications
You must be signed in to change notification settings - Fork 985
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
fixing msvc version model (breaking) #10057
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,14 @@ | |
CONAN_VCVARS_FILE = "conanvcvars.bat" | ||
|
||
|
||
def msvc_version_to_vs_ide_version(version): | ||
_visuals = {'190': '14', | ||
'191': '15', | ||
'192': '16', | ||
'193': '17'} | ||
return _visuals[str(version)] | ||
|
||
|
||
class VCVars: | ||
def __init__(self, conanfile): | ||
self._conanfile = conanfile | ||
|
@@ -54,12 +62,7 @@ def vs_ide_version(conanfile): | |
if toolset_override: | ||
visual_version = toolset_override | ||
else: | ||
version = compiler_version[:4] # Remove the latest version number 19.1X if existing | ||
_visuals = {'19.0': '14', # TODO: This is common to CMake, refactor | ||
'19.1': '15', | ||
'19.2': '16', | ||
'19.3': '17'} | ||
visual_version = _visuals[version] | ||
visual_version = msvc_version_to_vs_ide_version(compiler_version) | ||
else: | ||
visual_version = compiler_version | ||
return visual_version | ||
|
@@ -166,9 +169,6 @@ def _vcvars_vers(conanfile, compiler, vs_version): | |
assert compiler == "msvc" | ||
# Code similar to CMakeToolchain toolset one | ||
compiler_version = str(conanfile.settings.compiler.version) | ||
version_components = compiler_version.split(".") | ||
assert len(version_components) >= 2 # there is a 19.XX | ||
minor = version_components[1] | ||
# The equivalent of compiler 19.26 is toolset 14.26 | ||
vcvars_ver = "14.{}".format(minor[0]) | ||
# The equivalent of compiler 192 is toolset 14.2 | ||
vcvars_ver = "14.{}".format(compiler_version[-1]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above. Need to take care of special cases for VS 16.9 and VS 16.11. |
||
return vcvars_ver |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,10 +92,8 @@ | |
llvm, ClangCL, v143] | ||
cppstd: [None, 14, 17, 20, 23] | ||
msvc: | ||
version: ["19.0", | ||
"19.1X", "19.10", "19.11", "19.12", "19.13", "19.14", "19.15", "19.16", | ||
"19.2X", "19.20", "19.21", "19.22", "19.23", "19.24", "19.25", "19.26", "19.27", "19.28", "19.29", | ||
"19.3X", "19.30"] | ||
version: [190, 191, 192, 193] | ||
update: [None, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above comment. VS 16.8 and 16.9 both have "update 8", yet those are link-incompatible. Same stands for VS 16.10 and 16.11 (both are "update 9"). You need to handle that case as well. |
||
runtime: [static, dynamic] | ||
runtime_type: [Debug, Release] | ||
cppstd: [14, 17, 20, 23] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2906,10 +2906,8 @@ | |
llvm, ClangCL, v143] | ||
cppstd: [None, 14, 17, 20, 23] | ||
msvc: | ||
version: ["19.0", | ||
"19.1X", "19.10", "19.11", "19.12", "19.13", "19.14", "19.15", "19.16", | ||
"19.2X", "19.20", "19.21", "19.22", "19.23", "19.24", "19.25", "19.26", "19.27", "19.28", "19.29", | ||
"19.3X", "19.30"] | ||
version: [190, 191, 192, 193] | ||
update: [None, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my comment above. |
||
runtime: [static, dynamic] | ||
runtime_type: [Debug, Release] | ||
cppstd: [14, 17, 20, 23] | ||
|
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.
This does not correctly handle special cases for VS 16.9 and VS 16.11, which need 3-part version number to be selected correctly. See this thread for more information.
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.
This piece of code will handle whatever you have defined in your
msvc
compiler.update
. If you define your updates as9.XX
and9.YY
(which you can easily add to the defaultsettings.yml
) to differentiate those different cases, this would work wouldn't it?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.
🤔 , it should work. Excellent idea! So, for VS 16.8 I would need to add "8.29333" and for VS 16.10 "9.30037".
Seems elegant enough to even be part of the default
settings.yml
😊