fixing msvc version model (breaking)

[memsharded]

Changelog: Fix: Fix the msvc version model, which is comparison broken with the "main" 19.3 version < 19.22.
Docs: conan-io/docs#2314

cc @mpusz, @DoDoENT

[DoDoENT]

All in all - if all this is just to fix the version comparison, then I think it will introduce more problems than it will solve (see my comments regarding the special cases for VS 16.9 and VS 16.11).

Wouldn't be much simpler to perform version comparison by removing all dots from the version string and replacing all X with 0 and then converting that to an integer? Thus, 19.3X would be converted to 1930 and would be larger than 19.22 which would be treated as 1922.

Note that even the current model didn't correctly handle the VS 16.9 and 16.11, as I warned about back in March

[DoDoENT]

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.

[memsharded]

This piece of code will handle whatever you have defined in your msvc compiler.update. If you define your updates as 9.XX and 9.YY (which you can easily add to the default settings.yml) to differentiate those different cases, this would work wouldn't it?

[DoDoENT]

🤔 , 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 😊

[DoDoENT]

Same comment as above. Need to take care of special cases for VS 16.9 and VS 16.11.

[DoDoENT]

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.

[DoDoENT]

See my comment above.

[memsharded]

Wouldn't be much simpler to perform version comparison by removing all dots from the version string and replacing all X with 0 and then converting that to an integer? Thus, 19.3X would be converted to 1930 a

We already have this merged in develop branch, and this PR is actually removing this because it doesn't work. The issue is that we are not the ones doing the version comparisons, but the user (@mpusz reported this). So whatever versions we use in the settings should be comparable and correctly sortable by default, without any manipulation, because we will not be in control of this comparison and we will not be able to replace anything.

[memsharded]

I still don't fully see a complete solution, the issue is indeed complex and information from Msft in threads like https://developercommunity.visualstudio.com/t/How-to-select-VS-168-compiler-with-vcva/1359197 are not fully clarifying.

Lets summarize the overall requirements to define our settings.yml for msvc:

• The versions used should be naturally comparable, at all levels, with not replacements or tricks
• We need a "general" binary compatibility approach, that we know will work for 95% of the cases, and that allows a reasonable default binary creation in ConanCenter binaries. Something that matches more or less the VS IDE version, or the compiler main lines, at the moment it seems 190, 191, 192, 193 are those families
• We need an opt-in, detailed compatibility model that allows to fine tune and detail at any desired level, to account for all possible minor binary incompatibilities.

What would be then @DoDoENT your proposal in settings.yml?

[SSE4]

I am missing some context here. you already proposed another fix a few days ago: #10028
what exactly didn't work? any example?
I personally don't like these model with version 190 + update 8, it's too synthetic IMO.
why can't we use 19.3X? or why can't we just use 19.30 without X and always specify a precise version number for better reproducibility?
as I understand, we also can always declare 19.31 and 19.32 to be compatible packages, as we did for MSVC toolset or Intel compiler in the past, previously.

[memsharded]

@SSE4

Yes, 19.3X does not compare correctly with other versions, e.g "19.3X" < "19.21" => True. yes, I know this sounds like a bug, difficult to change in Conan 1.X. But even if we change it, having "19.3X" > "19.31" => True will still be incorrect, and would lead to incorrect user conditions in recipes.

Problem with 19.30 is that it also matches one specific "update", the first one. If we use that identifier to represent the whole "family" of "19.3X" updates, then it is broken for that specific updates.

Regarding compatible_packages, we are even considering proposing to remove them in Conan 2.0. They are very complicated to use, and we know very few cases of people successfully using them (there are some, but still a bit challenging feature). And what we definitely don't want to do is to add our own binary compatibility rules (beyond the basic settings.yml natural definition), because that is almost impossible to get it right for most cases, and in any case very challenging to maintain without breaking users.

Some of the main users of the intel compiler have explicitly requested me to remove the "base" compatible_packages mechanism for intel, as it was more confusing than helping (this is the reason why new IntelOneApi intel-cc compiler doesn't have any kind of "base" compatibility with other compilers. One of the main reasons that the new msvc compiler is pushed is because it removes the need of defining any toolset compatibility, necessary with the Visual Studio setting.

[SSE4]

Yes, 19.3X does not compare correctly with other versions, e.g "19.3X" < "19.21" => True. yes, I know this sounds like a bug, difficult to change in Conan 1.X. But even if we change it, having "19.3X" > "19.31" => True will still be incorrect, and would lead to incorrect user conditions in recipes.

that's basically a set that couldn't be ordered and comparison operators don't apply to (all of) its elements. you can compare 19.21 and 19.31, but you can't compare 19.3X and 19.31 or 19.32, it doesn't make any sense to say if 19.3X eqauls to 19.31, and it breaks transitivity and other properties. that might be probably just good enough to say you can't compare that versions at all, and it doesn't apply.

I would probably just have two distinct sets of versions: 19.1X, 19.2X, etc is the first set, and 19.10, 19.21, etc. is a second set. you can compare within the same set, but you can't within another set, as it's not defined operation (the same way as you can't say if vector is comparable to matrix, or whatever).
so people who want to operate with specific precise versions will operate with one set, and people who want to operate with "families" of updates (or how do we call it?) operate on another.

just a raw idea. maybe it should be a (sub) different setting, or different compiler.

[DoDoENT]

What would be then @DoDoENT your proposal in settings.yml?

This is how it currently looks like in my settings.yml:

    msvc:
        runtime: [MD, MT, MTd, MDd]
        version: ["19.16",
          "19.25", "19.26", "19.27", "19.28", "19.28-16.9", "19.29", "19.29-16.11",
          "19.30", "19.31"]
        cppstd: [None, 14, 17, 20]

Note that we still haven't migrated to using new static/dynamic naming. We don't use compiler version comparison in our codebase (we strive to recompile everything with every compiler update), but if I needed to use it, I would probably write my own custom version comparator and use it via the python_requires facility.

• The versions used should be naturally comparable, at all levels, with not replacements or tricks
• We need a "general" binary compatibility approach, that we know will work for 95% of the cases, and that allows a reasonable default binary creation in ConanCenter binaries. Something that matches more or less the VS IDE version, or the compiler main lines, at the moment it seems 190, 191, 192, 193 are those families
• We need an opt-in, detailed compatibility model that allows to fine tune and detail at any desired level, to account for all possible minor binary incompatibilities.

In general, your proposed model should work and cover all those cases, except special cases for VS 16.9 and VS 16.11. However, those could be added as special update versions (e.g. 8, 8-16.9, 9 and 9-16.11 for specific cases). In general, those special versions don't need to be part of the default settings.yml, however, it will need to be possible to be able to disable Conan's smart vcvars discovery process (i.e. detecting if correct vcvars are set and set them if they are not).

I've added comments about that in the above PR review because I have the fear of the following scenario:

• Conan is not aware that there are two different incompatible versions of msvc 19.29, but I have them in my settings.yml
• I initialize my shell with vcvars for msvc 19.29.30037 (vcvarsall.bat amd64 -vcvars_ver=14.29.30037)
• I run conan install ... --build missing -s compiler=msvc -s compiler.version=193 -s compiler.update=9-16.10
• some package needs to be built
• the Conan's build process determines that vcvars is not set correctly (this is my largest fear - if that doesn't happen, I'm OK with your proposal)
• Conan sets up its own vcvars by calling vcvarsall.bat amd64 -vcvars_ver=14..29, which selects the msvc 19.29.30129 or later (VS 16.11, incompatible with VS 16.10 binaries in terms of LTO)
• wrong binary gets built in the Conan package

Currently, with the visual studio "compiler", the Conan is stupid enough to simply check if vcvars are set and not if the correct vcvars are set. This way I could simply ensure that correct vcvars are initialized before calling any conan operations knowing that Conan will not mess anything up.

However, with msvc being now the first-class citizen in the Conan world, Conan is now able to detect if wrong vcvars are set and automatically correct that. For example, if my shell is initialized for VS 16.9 and I'm trying to install packages for msvc 19.29. Now, if the conan will just throw an error in that case, we're good, but if it tries to be smart and automatically set the correct vcvars for 19.29, then it needs to be aware of differences between VS 16.10 and 16.11 (and similarly, between VS 16.8 and 16.9). This is no easy task because getting a VS 16.10 vcvars can be done by either calling vcvarsall.bat amd64 -vcvars_ver=14.29.30037 if VS 16.11 is already installed and VS 16.10 side-by-side toolchain is installed, or by calling vcvarsall.bat amd64 -vcvars_ver=14.29 if VS 16.10 is installed and 16.11 update is still missing. However, calling vcvarsall.bat amd64 -vcvars_ver=14.29 with VS 16.11 installed will select msvc from VS 16.11 - you see the problem, right? The thing gets even more complicated as there is no easy way to detect if the 16.11 update is installed or not.

Since we don't know what else will Microsoft throw at us in their future updates, maybe it would be best to provide a conan hook for setting up the vcvars? If the hook is set, use it, if it's not, fall back to the heuristics that you've already implemented. Then, each organization can define its own hooks that will best suit its build environments.

[lasote]

I don't think we have to cover these cases necessarily. If you are concerned about these incompatibilities and you have generated binaries with both Visual Studio versions you can always alter the settings as you did. The default settings file is for the majority of the people/community at conan center. I think it would be better to have a correct version model that we can compare and not hard the model for these corner cases.

[DoDoENT]

If you are concerned about these incompatibilities and you have generated binaries with both Visual Studio versions you can always alter the settings as you did.

I agree - but this means that I then need to have the ability to prevent conan from calling vcvars batch script (because it will do it incorrectly). That is all I need and this is also the current behavior (Conan detects the vcvars are set and does nothing).