Skip to content
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

Feature/msvc #8201

Merged
merged 35 commits into from
Jan 14, 2021
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
0a6f828
working in msvc new settings
memsharded Dec 10, 2020
27c6fc4
merged develop
memsharded Dec 12, 2020
72f6bee
preliminary support for msvc compiler
memsharded Dec 12, 2020
369143b
fix migration
memsharded Dec 12, 2020
fb110ba
changing to version 19.1, 19.11, etc
memsharded Dec 15, 2020
ee99a73
merged develop
memsharded Dec 15, 2020
cfc28fe
working on static/dynamic runtime
memsharded Dec 16, 2020
c55072f
Merge branch 'develop' into feature/msvc
memsharded Dec 16, 2020
b245c43
extracting toolchain checkers to use in CMake
memsharded Dec 16, 2020
c363312
removed unused import
memsharded Dec 16, 2020
fb6b57b
adding checks
memsharded Dec 16, 2020
e98d911
adding check
memsharded Dec 16, 2020
ae91af5
Merge branch 'feature/toolchain_checkers' into feature/msvc
memsharded Dec 16, 2020
3c3a7b3
first proposal
memsharded Dec 16, 2020
3bb3e4d
msvc proposal for runtime
memsharded Dec 16, 2020
ef36fd2
fix migration test
memsharded Dec 16, 2020
2000c85
fixing tests
memsharded Dec 16, 2020
640b134
binary compatibility
memsharded Dec 16, 2020
fca3c79
merged develop
memsharded Dec 17, 2020
1110c80
Merge branch 'develop' into feature/msvc
memsharded Dec 20, 2020
e9df343
Merge branch 'develop' into feature/msvc
memsharded Dec 22, 2020
c78bad2
merged develop
memsharded Dec 23, 2020
59c2ab2
fix test
memsharded Dec 23, 2020
0766d14
opt-out compatibility and helper for runtime flag
memsharded Dec 27, 2020
40fb8f0
merged develop
memsharded Dec 27, 2020
315de05
review
memsharded Dec 28, 2020
0c7935f
Merge branch 'develop' into feature/msvc
memsharded Jan 7, 2021
9bdeca1
Merge branch 'develop' into feature/msvc
memsharded Jan 11, 2021
72adaf3
protection
memsharded Jan 13, 2021
8cce2e1
merged develop
memsharded Jan 13, 2021
b252e83
adding msvc cppstd flag
memsharded Jan 13, 2021
637a9e7
fix migration test
memsharded Jan 13, 2021
3983c43
Merge branch 'develop' into feature/msvc
memsharded Jan 14, 2021
2738424
requiring compiler.cppstd to be defined
memsharded Jan 14, 2021
47c339b
removing cppstd=None
memsharded Jan 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion conan/tools/cmake/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def get_generator_platform(settings, generator):
if settings.get_safe("os") == "WindowsCE":
return settings.get_safe("os.platform")

if (compiler == "Visual Studio" or compiler_base == "Visual Studio") and \
if (compiler in ("Visual Studio", "msvc") or compiler_base == "Visual Studio") and \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (compiler in ("Visual Studio", "msvc") or compiler_base == "Visual Studio") and \
if (compiler in ("Visual Studio", "msvc") or compiler_base in ("Visual Studio", "msvc")) and \

generator and "Visual" in generator:
return {"x86": "Win32",
"x86_64": "x64",
Expand Down Expand Up @@ -213,6 +213,12 @@ def _runtimes(self):
"MTd": "MultiThreadedDebug",
"MD": "MultiThreadedDLL",
"MDd": "MultiThreadedDebugDLL"}[runtime]
if compiler == "msvc":
runtime_type = settings.get_safe("compiler.runtime_type")
rt = "MultiThreadedDebug" if runtime_type == "Debug" else "MultiThreaded"
if runtime != "static":
rt += "DLL"
config_dict[build_type] = rt
return config_dict

def _get_libcxx(self):
Expand Down
9 changes: 9 additions & 0 deletions conan/tools/cmake/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ def get_generator(conanfile):

compiler = conanfile.settings.get_safe("compiler")
compiler_version = conanfile.settings.get_safe("compiler.version")

if compiler == "msvc":
version = compiler_version[:4] # Remove the latest version number 19.1X if existing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line (and the line below) may raise, need sanity checks (and maybe tests)

_visuals = {'19.0': '14 2015',
'19.1': '15 2017',
'19.2': '16 2019'}[version]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is where we need vswhere, right? We need it to tell us which is the VS version that provides that msvc version. Maybe we can move these lines to some conan.tools.microsoft, add the comments (FIXME/TODO) there and call it from here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, probably we need not only vswhere, but also some easy way to configure it, because if vswhere finds VS2017 and VS2019, and both have the v140 toolset for older versions, which one will it pick? Seems a task for the config feature, where this can be explicit. And if not explicit, then vswhere, but as the explicit configuration is necessary anyway, I would start with it, then follow with vswhere one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make any difference? The tools (compiler, linker,...) will be exactly the same, won't they? Can you have more than one instance of the same MSVC installed and detected by vswhere? Maybe CMake generates something different (VS migrations), but under the hood everything should be exactly the same... I'd say that in this case the generated binary will be exactly the same. Maybe the config is not needed, we can use it if present, otherwise we can choose any at random (or the latest one). cc/ @SSE4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that might be different instances of compilers for VS2019 and VS2017 with the same toolset. they probably might have the same version, but I think they installed and updated intependently.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the approach of choosing one at random is a good one. Besides, asking information about the installed toolsets is not possible in vswhere: microsoft/vswhere#151

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what vswhere does (from that same link): "[vswhere] finds the installation path (and other basic information) of a version of Visual Studio that meets basic requirements" (#3573 (comment)):

vswhere [-latest] -requires <component-toolset>

The toolset is a requirement, vswhere returns the VS version that satisfies that requirement (VS 2017, VS 2019,...) which is the input we need for the CMake generator.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I have tried that. The toolset component name that you pass to the --requires argument is the same (Microsoft.VisualStudio.Component.VC.Tools.x86.x64) , and it will map to the default toolset of the different VS versions, not the specific one that you want to discover. So far I have not been able to know which toolsets contain each VS versions, I don't know if it is possible at all.

Copy link
Contributor

@jgsogo jgsogo Dec 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each toolset has a different component name (#3573 (comment)):

  • MSVC v141: Microsoft.VisualStudio.Component.VC.v141.x86.x64
  • MSVC v142 (14.20): Microsoft.VisualStudio.Component.VC.14.20.x86.x64
  • MSVC v142 (14.21): Microsoft.VisualStudio.Component.VC.14.21.x86.x64
  • v140: Microsoft.VisualStudio.Component.VC.140
  • ...

It could be easier, but at least it is documented.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried them too, but don't seem to work as expected, or at least they are trickier than they look. I didn't succeed in identifying the toolsets I have installed in my different VS versions. I will try again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Results differ from documented (or at least documentation is not very clear).
As in any case, it is necessary to allow configuring the VS/MSBuild instance that is being used to build with a compiler version, let's do the configuration part first, and then we will try to automate to define it automatically.

INSTALLED:
    VS 2019: v14.28, v14.16
    VS 2017: v14.16, v14.00
DETECTED
    Microsoft.VisualStudio.Component.VC.Tools.x86.x64 => [VisualStudio.15, VisualStudio.16]
    
    Microsoft.VisualStudio.Component.VC.140 => VisualStudio.15
    Microsoft.VisualStudio.Component.VC.141 => []
    Microsoft.VisualStudio.Component.VC.142 => []

    Microsoft.VisualStudio.Component.VC.14.20 => []
    Microsoft.VisualStudio.Component.VC.14.28 => []
    Microsoft.VisualStudio.Component.VC.14.16 => []

    Microsoft.VisualStudio.Component.VC.14.20.x86.x64 => []
    Microsoft.VisualStudio.Component.VC.14.28.x86.x64 => []
    Microsoft.VisualStudio.Component.VC.14.16.x86.x64 => []

    Microsoft.VisualStudio.Component.VC.v140.x86.x64 => []
    Microsoft.VisualStudio.Component.VC.v141.x86.x64 => VisualStudio.16
    Microsoft.VisualStudio.Component.VC.v142.x86.x64 => []

base = "Visual Studio %s" % _visuals
return base

compiler_base = conanfile.settings.get_safe("compiler.base")
arch = conanfile.settings.get_safe("arch")

Expand Down
14 changes: 14 additions & 0 deletions conan/tools/microsoft/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,17 @@
from .toolchain import MSBuildToolchain
from .msbuild import MSBuild
from .msbuilddeps import MSBuildDeps


def msvc_runtime_flag(conanfile):
settings = conanfile.settings
compiler = settings.get_safe("compiler")
Copy link
Contributor

@SSE4 SSE4 Dec 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
compiler = settings.get_safe("compiler")
compiler = settings.get_safe("compiler.base") or settings.get_safe("compiler")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to condition it to intel compiler explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

up to you :)

runtime = settings.get_safe("compiler.runtime")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
runtime = settings.get_safe("compiler.runtime")
runtime = settings.get_safe("compiler.base.runtime") or settings.get_safe("compiler.runtime")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is for the intel compiler, I prefer to wait until the next iteration. For a start, the settings.yml needs to be changed too, this will not work without changing the settings. And we need to understand how the binary model for intel will work, now that we are defining a fallback from msvc => Visual Studio, because intel compiler was relying on that too, and it might have some collisions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, that might not be not just Intel compiler, but any custom compiler with custom settings using msvc as a compiler.base.

if compiler == "Visual Studio":
return runtime
if compiler == "msvc":
runtime_type = settings.get_safe("compiler.runtime_type")
if runtime == "static":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a better implementation (from below):

        runtime = "MT" if runtime == "static" else "MD"
        if  runtime_type == "Debug":
            runtime = "{}d".format(runtime)

also, why not reuse?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

Premature reuse is like premature optimization. It leads to wrong and obscure abstractions. I prefer stronger encapsulation, and knowing msvc_runtime_flag() is fully self-contained. Anyway, which other piece of code is mapping the new msvc settings to the runtime flag? I see it nowhere else, it is indeed a new functionality necessary for the new model.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code was from def msvc_compatible(self): (info.py).

return "MTd" if runtime_type == "Debug" else "MT"
else:
return "MDd" if runtime_type == "Debug" else "MD"
15 changes: 15 additions & 0 deletions conans/client/conf/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@
threads: [None, posix, win32] # Windows MinGW
exception: [None, dwarf2, sjlj, seh] # Windows MinGW
cppstd: [None, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17, 20, gnu20]
msvc:
version: ["19.0",
memsharded marked this conversation as resolved.
Show resolved Hide resolved
"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]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this requires a None value, meaning that the value will follow the one defined by build_type? Otherwise it is explicit, and running conan install ... -s build_type=Debug will require explicit conan install -s build_type=Debug -s runtime_type=Debug which is what we are trying to avoid.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was automated. If runtime_type values is not defined, it will get the build_type one, which is the expected behavior in most cases.

cppstd: [None, 14, 17, 20]
Visual Studio: &visual_studio
runtime: [MD, MT, MTd, MDd]
version: ["8", "9", "10", "11", "12", "14", "15", "16"]
Expand Down Expand Up @@ -485,6 +492,14 @@ def full_transitive_package_id(self):
except ConanException:
return None

@property
def msvc_visual_incompatible(self):
try:
visual_comp = self.get_item("general.msvc_visual_incompatible")
return visual_comp.lower() in ("1", "true")
except ConanException:
return None
memsharded marked this conversation as resolved.
Show resolved Hide resolved

@property
def short_paths_home(self):
short_paths_home = get_env("CONAN_USER_HOME_SHORT")
Expand Down
4 changes: 4 additions & 0 deletions conans/client/graph/graph_binaries.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,10 @@ def _compute_package_id(self, node, default_package_id_mode, default_python_requ
python_requires=python_requires,
default_python_requires_id_mode=
default_python_requires_id_mode)
if not self._cache.config.msvc_visual_incompatible:
msvc_compatible = conanfile.info.msvc_compatible()
if msvc_compatible:
conanfile.compatible_packages.append(msvc_compatible)

# Once we are done, call package_id() to narrow and change possible values
with conanfile_exception_formatter(str(conanfile), "package_id"):
Expand Down
107 changes: 106 additions & 1 deletion conans/client/migrations_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -1820,4 +1820,109 @@
settings_1_32_0 = settings_1_31_4
settings_1_32_1 = settings_1_32_0

settings_1_33_0 = settings_1_32_1
settings_1_33_0 = """
# Only for cross building, 'os_build/arch_build' is the system that runs Conan
os_build: [Windows, WindowsStore, Linux, Macos, FreeBSD, SunOS, AIX]
arch_build: [x86, x86_64, ppc32be, ppc32, ppc64le, ppc64, armv5el, armv5hf, armv6, armv7, armv7hf, armv7s, armv7k, armv8, armv8_32, armv8.3, sparc, sparcv9, mips, mips64, avr, s390, s390x, sh4le]

# Only for building cross compilation tools, 'os_target/arch_target' is the system for
# which the tools generate code
os_target: [Windows, Linux, Macos, Android, iOS, watchOS, tvOS, FreeBSD, SunOS, AIX, Arduino, Neutrino]
arch_target: [x86, x86_64, ppc32be, ppc32, ppc64le, ppc64, armv5el, armv5hf, armv6, armv7, armv7hf, armv7s, armv7k, armv8, armv8_32, armv8.3, sparc, sparcv9, mips, mips64, avr, s390, s390x, asm.js, wasm, sh4le]

# Rest of the settings are "host" settings:
# - For native building/cross building: Where the library/program will run.
# - For building cross compilation tools: Where the cross compiler will run.
os:
Windows:
subsystem: [None, cygwin, msys, msys2, wsl]
WindowsStore:
version: ["8.1", "10.0"]
WindowsCE:
platform: ANY
version: ["5.0", "6.0", "7.0", "8.0"]
Linux:
Macos:
version: [None, "10.6", "10.7", "10.8", "10.9", "10.10", "10.11", "10.12", "10.13", "10.14", "10.15", "11.0"]
Android:
api_level: ANY
iOS:
version: ["7.0", "7.1", "8.0", "8.1", "8.2", "8.3", "9.0", "9.1", "9.2", "9.3", "10.0", "10.1", "10.2", "10.3", "11.0", "11.1", "11.2", "11.3", "11.4", "12.0", "12.1", "12.2", "12.3", "12.4", "13.0", "13.1", "13.2", "13.3", "13.4", "13.5", "13.6"]
watchOS:
version: ["4.0", "4.1", "4.2", "4.3", "5.0", "5.1", "5.2", "5.3", "6.0", "6.1"]
tvOS:
version: ["11.0", "11.1", "11.2", "11.3", "11.4", "12.0", "12.1", "12.2", "12.3", "12.4", "13.0"]
FreeBSD:
SunOS:
AIX:
Arduino:
board: ANY
Emscripten:
Neutrino:
version: ["6.4", "6.5", "6.6", "7.0", "7.1"]
arch: [x86, x86_64, ppc32be, ppc32, ppc64le, ppc64, armv4, armv4i, armv5el, armv5hf, armv6, armv7, armv7hf, armv7s, armv7k, armv8, armv8_32, armv8.3, sparc, sparcv9, mips, mips64, avr, s390, s390x, asm.js, wasm, sh4le]
compiler:
sun-cc:
version: ["5.10", "5.11", "5.12", "5.13", "5.14", "5.15"]
threads: [None, posix]
libcxx: [libCstd, libstdcxx, libstlport, libstdc++]
gcc: &gcc
version: ["4.1", "4.4", "4.5", "4.6", "4.7", "4.8", "4.9",
"5", "5.1", "5.2", "5.3", "5.4", "5.5",
"6", "6.1", "6.2", "6.3", "6.4", "6.5",
"7", "7.1", "7.2", "7.3", "7.4", "7.5",
"8", "8.1", "8.2", "8.3", "8.4",
"9", "9.1", "9.2", "9.3",
"10", "10.1"]
libcxx: [libstdc++, libstdc++11]
threads: [None, posix, win32] # Windows MinGW
exception: [None, dwarf2, sjlj, seh] # Windows MinGW
cppstd: [None, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17, 20, gnu20]
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]
Visual Studio: &visual_studio
runtime: [MD, MT, MTd, MDd]
version: ["8", "9", "10", "11", "12", "14", "15", "16"]
toolset: [None, v90, v100, v110, v110_xp, v120, v120_xp,
v140, v140_xp, v140_clang_c2, LLVM-vs2012, LLVM-vs2012_xp,
LLVM-vs2013, LLVM-vs2013_xp, LLVM-vs2014, LLVM-vs2014_xp,
LLVM-vs2017, LLVM-vs2017_xp, v141, v141_xp, v141_clang_c2, v142,
llvm, ClangCL]
cppstd: [None, 14, 17, 20]
clang:
version: ["3.3", "3.4", "3.5", "3.6", "3.7", "3.8", "3.9", "4.0",
"5.0", "6.0", "7.0", "7.1",
"8", "9", "10", "11"]
libcxx: [None, libstdc++, libstdc++11, libc++, c++_shared, c++_static]
cppstd: [None, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17, 20, gnu20]
runtime: [None, MD, MT, MTd, MDd]
apple-clang: &apple_clang
version: ["5.0", "5.1", "6.0", "6.1", "7.0", "7.3", "8.0", "8.1", "9.0", "9.1", "10.0", "11.0", "12.0"]
libcxx: [libstdc++, libc++]
cppstd: [None, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17, 20, gnu20]
intel:
version: ["11", "12", "13", "14", "15", "16", "17", "18", "19", "19.1"]
base:
gcc:
<<: *gcc
threads: [None]
exception: [None]
Visual Studio:
<<: *visual_studio
apple-clang:
<<: *apple_clang
qcc:
version: ["4.4", "5.4", "8.3"]
libcxx: [cxx, gpp, cpp, cpp-ne, accp, acpp-ne, ecpp, ecpp-ne]
cppstd: [None, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17]

build_type: [None, Debug, Release, RelWithDebInfo, MinSizeRel]


cppstd: [None, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17, 20, gnu20] # Deprecated, use compiler.cppstd
"""
4 changes: 4 additions & 0 deletions conans/client/settings_preprocessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ def _fill_runtime(settings):
settings.compiler.base.runtime = runtime
msg = "Setting 'compiler.base.runtime' not declared, automatically adjusted to '%s'"
logger.info(msg % runtime)
elif settings.compiler == "msvc":
if settings.get_safe("compiler.runtime_type") is None:
runtime = "Debug" if settings.get_safe("build_type") == "Debug" else "Release"
settings.compiler.runtime_type = runtime
except Exception: # If the settings structure doesn't match these general
# asumptions, like unexistant runtime
pass
21 changes: 21 additions & 0 deletions conans/model/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,27 @@ def header_only(self):
self.options.clear()
self.requires.clear()

def msvc_compatible(self):
if self.settings.compiler != "msvc":
return

compatible = self.clone()
version = compatible.settings.compiler.version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version = compatible.settings.compiler.version
version = compatible.settings.get_safe("compiler.version")

runtime = compatible.settings.compiler.runtime
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
runtime = compatible.settings.compiler.runtime
runtime = compatible.settings.get_safe("compiler.runtime")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nop, at this point, using the msvc compiler, the compiler.runtime must be defined, otherwise I want it to raise, not to continue silently. Why would you want/allow this to be undefined here? It is not possible in the settings (it doesn't have a None value)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's very possible settings.yml might be modified by the user, either manually, or by running conan config install. user may add None value or remove compiler.runtime sub-setting completely. IMO code should be reliable for custom settings, as we're allowing them and promote it as a first-class feature (especially for Enterprise users).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far this would be an unsupported case, the msvc compiler always has a runtime, it is not possible to compile without it. What does a None value means? What is the use case? It is important to code new features initially more restrictive/limited scope, and learn from use cases, better than be relaxed, allow use cases that we completely ignore and that might have other implications. I strongly prefer to evolve from the basic "canonical" use case, and account for variants and special use cases later.

runtime_type = compatible.settings.compiler.runtime_type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
runtime_type = compatible.settings.compiler.runtime_type
runtime_type = compatible.settings.get_safe("compiler.runtime_type")


compatible.settings.compiler = "Visual Studio"
version = str(version)[:4]
Copy link
Contributor

@SSE4 SSE4 Dec 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may raise, need a sanity check (maybe deserves a test)

_visuals = {'19.0': '14',
'19.1': '15',
'19.2': '16'}
compatible.settings.compiler.version = _visuals[version]
Copy link
Contributor

@SSE4 SSE4 Dec 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may raise, need a sanity check (maybe deserves a test)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, this is a preliminary implementation, this needs to be improved.

runtime = "MT" if runtime == "static" else "MD"
if runtime_type == "Debug":
runtime = "{}d".format(runtime)
compatible.settings.compiler.runtime = runtime
return compatible

def vs_toolset_compatible(self):
"""Default behaviour, same package for toolset v140 with compiler=Visual Studio 15 than
using Visual Studio 14"""
Expand Down
6 changes: 4 additions & 2 deletions conans/test/functional/toolchains/test_cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ def _run_app(self, build_type, bin_folder=False, msg="App", dyld_path=None):
@pytest.mark.skipif(platform.system() != "Windows", reason="Only for windows")
class WinTest(Base):
@parameterized.expand([("Visual Studio", "Debug", "MTd", "15", "14", "x86", "v140", True),
("Visual Studio", "Release", "MD", "15", "17", "x86_64", "", False)]
("Visual Studio", "Release", "MD", "15", "17", "x86_64", "", False),
("msvc", "Debug", "static", "19.1", "14", "x86", None, True),
("msvc", "Release", "dynamic", "19.11", "17", "x86_64", None, False)]
)
def test_toolchain_win(self, compiler, build_type, runtime, version, cppstd, arch, toolset,
shared):
Expand Down Expand Up @@ -230,7 +232,7 @@ def _verify_out(marker=">>"):
self._run_app("Debug", bin_folder=True)
check_msc_ver(toolset or "v141", self.client.out)
self.assertIn("main _MSVC_LANG20{}".format(cppstd), self.client.out)
static = "MT" in runtime
static = (runtime == "static" or "MT" in runtime)
check_vs_runtime("build/Release/app.exe", self.client, "15", build_type="Release",
static=static)
check_vs_runtime("build/Debug/app.exe", self.client, "15", build_type="Debug",
Expand Down
22 changes: 22 additions & 0 deletions conans/test/functional/toolchains/test_msbuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,28 @@
"""


@pytest.mark.tool_visual_studio
def test_msvc_runtime_flag():
client = TestClient()
conanfile = textwrap.dedent("""
from conans import ConanFile
from conan.tools.microsoft import msvc_runtime_flag
class App(ConanFile):
settings = "os", "arch", "compiler", "build_type"

def generate(self):
self.output.info("MSVC FLAG={}!!".format(msvc_runtime_flag(self)))
""")
client.save({"conanfile.py": conanfile})
client.run('install . -s compiler="Visual Studio" -s compiler.version=15 -s compiler.runtime=MD')
assert "MSVC FLAG=MD!!" in client.out
client.run('install . -s compiler=msvc -s compiler.version=19.1 -s compiler.runtime=static '
'-s compiler.runtime_type=Debug')
assert "MSVC FLAG=MTd!!" in client.out
client.run('install . -s compiler=msvc -s compiler.version=19.1 -s compiler.runtime=dynamic')
assert "MSVC FLAG=MD!!" in client.out


@pytest.mark.skipif(platform.system() != "Windows", reason="Only for windows")
@pytest.mark.tool_visual_studio
class WinTest(unittest.TestCase):
Expand Down
15 changes: 15 additions & 0 deletions conans/test/integration/package_id/compatible_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -492,3 +492,18 @@ def package_id(self):
self.assertIn("pkg/0.1@user/testing:1ebf4db7209535776307f9cd06e00d5a8034bc84 - Cache",
client.out)
self.assertIn("pkg/0.1@user/testing: Already installed!", client.out)


def test_msvc_visual_incompatible():
conanfile = GenConanfile().with_settings("os", "compiler", "build_type", "arch")
client = TestClient()
client.save({"conanfile.py": conanfile})
client.run('create . pkg/0.1@ -s os=Windows -s compiler="Visual Studio" -s compiler.version=15 '
'-s compiler.runtime=MD -s build_type=Release -s arch=x86_64')
client.run("install pkg/0.1@ -s os=Windows -s compiler=msvc -s compiler.version=19.1 "
"-s compiler.runtime=dynamic -s build_type=Release -s arch=x86_64")
assert "Using compatible package" in client.out
client.run("config set general.msvc_visual_incompatible=1")
client.run("install pkg/0.1@ -s os=Windows -s compiler=msvc -s compiler.version=19.1 "
"-s compiler.runtime=dynamic -s build_type=Release -s arch=x86_64", assert_error=True)
assert "ERROR: Missing prebuilt package for 'pkg/0.1'" in client.out