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.

Sign up for GitHub

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

adding tools.microsoft:winsdk_version conf #15272

Merged
merged 8 commits into from
Dec 15, 2023
Merged

adding tools.microsoft:winsdk_version conf #15272

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8f869ce
adding tools.microsoft:winsdk_version conf
memsharded Dec 13, 2023
085d756
Merge branch 'release/2.0' into feature/winsdk_version
memsharded Dec 14, 2023
88f39db
fix tests
memsharded Dec 14, 2023
680be06
wip
memsharded Dec 15, 2023
e8d14bd
Merge branch 'release/2.0' into feature/winsdk_version
memsharded Dec 15, 2023
8ebb938
fix
memsharded Dec 15, 2023
872a31b
Merge branch 'release/2.0' into feature/winsdk_version
memsharded Dec 15, 2023
0b7830f
review
memsharded Dec 15, 2023
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
4 changes: 3 additions & 1 deletion conan/tools/microsoft/visual.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,14 @@ def generate(self, scope="build"):
vcvars_ver = _vcvars_vers(conanfile, compiler, vs_version)
vcvarsarch = _vcvars_arch(conanfile)

winsdk_version = conanfile.conf.get("tools.microsoft:winsdk_version", check_type=str)
winsdk_version = winsdk_version or conanfile.setttings.get_safe("os.version")
Copy link
Contributor

@jcar87 jcar87 Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think this might cause issues if an SDK that matches the os version is not installed - but also, I would advise against making this the default.
Last time I checked, the default behaviour of vcvars was to pick up "the most recent" version, unless told otherwise - I'd try and keep that behaviour. We did have a a lot of issues in Conan Center when CMake was picking up a version that matches the os.version and not the latest. Recently CMake has added a policy to NOT behave like that: https://cmake.org/cmake/help/latest/policy/CMP0149.html#policy:CMP0149

Edit: nevermind, thought we were asking the current OS version, but this is the Conan profile!!

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 will only be used if:

  • Users adds version to Windows subsettings in settings.yml
  • Defines os.version in their profiles

I have kept os.version instead of sdk_version because other Windows oss, like CE in settings.yml already have a version subsetting that represents the sdk

Copy link
Member Author

Choose a reason for hiding this comment

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

Added integrations for VCVars, MSBuildToolchain (so opening directly from IDE works) and CMakeToolchain

# The vs_install_path is like
# C:\Program Files (x86)\Microsoft Visual Studio\2019\Community
# C:\Program Files (x86)\Microsoft Visual Studio\2017\Community
# C:\Program Files (x86)\Microsoft Visual Studio 14.0
vcvars = vcvars_command(vs_version, architecture=vcvarsarch, platform_type=None,
winsdk_version=None, vcvars_ver=vcvars_ver,
winsdk_version=winsdk_version, vcvars_ver=vcvars_ver,
vs_install_path=vs_install_path)

content = textwrap.dedent("""\
Expand Down
1 change: 1 addition & 0 deletions conans/model/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"tools.google.bazel:bazelrc_path": "List of paths to bazelrc files to be used as 'bazel --bazelrc=rcpath1 ... build'",
"tools.meson.mesontoolchain:backend": "Any Meson backend: ninja, vs, vs2010, vs2012, vs2013, vs2015, vs2017, vs2019, xcode",
"tools.meson.mesontoolchain:extra_machine_files": "List of paths for any additional native/cross file references to be appended to the existing Conan ones",
"tools.microsoft:winsdk_version": "Use this winsdk_version in vcvars",
"tools.microsoft.msbuild:vs_version": "Defines the IDE version (15, 16, 17) when using the msvc compiler. Necessary if compiler.version specifies a toolset that is not the IDE default",
"tools.microsoft.msbuild:max_cpu_count": "Argument for the /m when running msvc to build parallel projects",
"tools.microsoft.msbuild:installation_path": "VS install path, to avoid auto-detect via vswhere, like C:/Program Files (x86)/Microsoft Visual Studio/2019/Community. Use empty string to disable",
Expand Down
19 changes: 19 additions & 0 deletions conans/test/integration/toolchains/microsoft/vcvars_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,22 @@ class TestConan(ConanFile):
vcvars = client.load("conanvcvars.bat")
assert 'vcvarsall.bat" x86_amd64' in vcvars
assert "-vcvars_ver" not in vcvars


@pytest.mark.skipif(platform.system() != "Windows", reason="Requires Windows")
def test_vcvars_winsdk_version():
client = TestClient(path_with_spaces=False)

conanfile = textwrap.dedent("""
from conan import ConanFile
class TestConan(ConanFile):
generators = "VCVars"
settings = "os", "compiler", "arch", "build_type"
""")
client.save({"conanfile.py": conanfile})
client.run('install . -s os=Windows -s compiler=msvc -s compiler.version=193 '
'-s compiler.cppstd=14 -s compiler.runtime=static '
'-c tools.microsoft:winsdk_version=8.1')

vcvars = client.load("conanvcvars.bat")
assert 'vcvarsall.bat" amd64 8.1 -vcvars_ver=14.3' in vcvars