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

Add support for vcvars when using Powershell #15461

Merged
merged 17 commits into from
Jan 29, 2024
Merged

Add support for vcvars when using Powershell #15461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8b1a1e9
add support for vcvars with powershell and add test for it
juansblanco Dec 21, 2023
5472ec7
conanbuild changes
juansblanco Jan 3, 2024
e2c9211
fixed duplicate env run
juansblanco Jan 10, 2024
bbd9ba3
parameter for vcvars exit
juansblanco Jan 10, 2024
435f30e
get env variables
juansblanco Jan 16, 2024
1d34797
fix test
juansblanco Jan 16, 2024
54c1921
change conanbuild.ps1 logic
juansblanco Jan 17, 2024
979f262
improve test
juansblanco Jan 17, 2024
ab184fc
change activated env check
juansblanco Jan 18, 2024
68e9fd0
add check for env actvated in test
juansblanco Jan 22, 2024
4f50919
Moved powershell env tests from integration to functional
juansblanco Jan 22, 2024
008eda7
remove commented out code
juansblanco Jan 22, 2024
af6408b
remove compiler version from test
juansblanco Jan 22, 2024
be0e62c
lowered minimum cmake version required in test
juansblanco Jan 22, 2024
dcd4fb4
change cmake version in test
juansblanco Jan 22, 2024
0475d83
use default files for tests
juansblanco Jan 23, 2024
62238be
Fix test import
juansblanco Jan 23, 2024
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
30 changes: 24 additions & 6 deletions conan/tools/microsoft/visual.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
from conan.tools.scm import Version
from conan.tools.intel.intel_cc import IntelCC

CONAN_VCVARS_FILE = "conanvcvars.bat"
CONAN_VCVARS_BAT = "conanvcvars.bat"
CONAN_VCVARS_PS1 = "conanvcvars.ps1"


def check_min_vs(conanfile, version, raise_invalid=True):
Expand Down Expand Up @@ -142,15 +143,32 @@ def generate(self, scope="build"):
winsdk_version=winsdk_version, vcvars_ver=vcvars_ver,
vs_install_path=vs_install_path)

content = textwrap.dedent("""\
content = textwrap.dedent(f"""\
@echo off
set __VSCMD_ARG_NO_LOGO=1
set VSCMD_SKIP_SENDTELEMETRY=1
echo conanvcvars.bat: Activating environment Visual Studio {} - {} - winsdk_version={} - vcvars_ver={}
{}
""".format(vs_version, vcvarsarch, winsdk_version, vcvars_ver, vcvars))
echo conanvcvars.bat: Activating environment Visual Studio {vs_version} - {vcvarsarch} - winsdk_version={winsdk_version} - vcvars_ver={vcvars_ver}
{vcvars}
""")
from conan.tools.env.environment import create_env_script
create_env_script(conanfile, content, CONAN_VCVARS_FILE, scope)
create_env_script(conanfile, content, CONAN_VCVARS_BAT, scope)

is_ps1 = conanfile.conf.get("tools.env.virtualenv:powershell", check_type=bool, default=False)
if is_ps1:
content_ps1 = textwrap.dedent(f"""\
if (-not $env:VSCMD_ARG_VCVARS_VER){{
Push-Location "$PSScriptRoot"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted this back to using "push/pop" as calling the path directly in the cmd command was causing trouble when path had spaces.

cmd /c "conanvcvars.bat&set" |
memsharded marked this conversation as resolved.
Show resolved Hide resolved
foreach {{
if ($_ -match "=") {{
$v = $_.split("=", 2); set-item -force -path "ENV:\$($v[0])" -value "$($v[1])"
}}
}}
Pop-Location
write-host conanvcvars.ps1: Activated environment}}
""")
Comment on lines +161 to +169
Copy link
Contributor

@jmarrec jmarrec Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I know no-one asked for my opinion ( :) ) but this makes me uneasy.

here's how you could do it similarly than the Launch-VsDevShell.ps1 ($vspath\Common7\Tools\Launch-VsDevShell.ps1)

$vspath=$(vswhere -products '*' -requires Microsoft.Component.MSBuild -property installationPath -latest)
$devShellModule = "$vspath\Common7\Tools\Microsoft.VisualStudio.DevShell.dll"
Import-Module $devShellModule
Enter-VsDevShell -VsInstallPath $vspath -SkipAutomaticLocation -Arch amd64

Copy link
Contributor

Choose a reason for hiding this comment

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

Get-Help Enter-VsDevShell

NAME
    Enter-VsDevShell

SYNTAX
    Enter-VsDevShell -VsInstallPath <string> [-SkipExistingEnvironmentVariables] [-StartInPath <string>] [-Arch {Default | x86 | amd64 | arm | arm64}] [-HostArch {Default | x86 | amd64}]
    [-DevCmdArguments <string>] [-DevCmdDebugLevel {None | Basic | Detailed | Trace}] [-SkipAutomaticLocation] [-SetDefaultWindowTitle] [-ReportNewInstanceType {PowerShell | Cmd |
    LaunchScript}] [<CommonParameters>]

    Enter-VsDevShell [-VsInstanceId] <string> [-SkipExistingEnvironmentVariables] [-StartInPath <string>] [-Arch {Default | x86 | amd64 | arm | arm64}] [-HostArch {Default | x86 |
    amd64}] [-DevCmdArguments <string>] [-DevCmdDebugLevel {None | Basic | Detailed | Trace}] [-SkipAutomaticLocation] [-SetDefaultWindowTitle] [-ReportNewInstanceType {PowerShell | Cmd
    | LaunchScript}] [<CommonParameters>]

    Enter-VsDevShell [-Arch {Default | x86 | amd64 | arm | arm64}] [-HostArch {Default | x86 | amd64}] [-Test] [-DevCmdDebugLevel {None | Basic | Detailed | Trace}] [<CommonParameters>]


ALIASES
    None


REMARKS
    None

Copy link
Contributor

@jcar87 jcar87 Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @jmarrec - thanks for your feedback.

This is the approach we considered initially, but discarded it for a couple of reasons:

  • Launch-VsDevShell.ps1 and the underlying implementation do not expose the same interface that vcvars provides, in particular the ability to specify the toolset with the -vcvars_ver, or using the Windows Store SDK, or specifying the WIndows SDK version. We do rely on mapping Conan settings/ conf to some of these - and the interface for the powershell ones do not expose these.
  • The documented way would be to invoke Launch-VsDevShell.ps1 once located - rather than importing DevShell.dll - this is evidenced:

Launch-VsDevShell.ps1 is the recommended way to initialize Developer PowerShell interactively or for scripting build automation.

we wouldn't want to use undocumented/subject to breakages features, and the Launch-VsDevShell.ps1 does not provide feature parity, so we relied on this fallback.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, thanks a lot for the detailed explanation!

create_env_script(conanfile, content_ps1, CONAN_VCVARS_PS1, scope)



def vs_ide_version(conanfile):
Expand Down
40 changes: 40 additions & 0 deletions ...lchains/env/test_virtualenv_powershell.py → ...lchains/env/test_virtualenv_powershell.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import pytest

from conans.test.assets.cmake import gen_cmakelists
from conans.test.assets.sources import gen_function_cpp
from conans.test.assets.genconanfile import GenConanfile
from conans.test.utils.test_files import temp_folder
from conans.test.utils.tools import TestClient
Expand Down Expand Up @@ -114,3 +116,41 @@ def test(self):
assert "MYPS1!!!!" in client.out
assert "MYVC_CUSTOMVAR1=PATATA1" in client.out
assert "MYVC_CUSTOMVAR2=PATATA2" in client.out

@pytest.mark.skipif(platform.system() != "Windows", reason="Requires Windows powershell")
def test_vcvars():
client = TestClient()
conanfile = textwrap.dedent(r"""
from conan import ConanFile
from conan.tools.cmake import CMake, cmake_layout, CMakeToolchain, CMakeDeps
from conan.tools.env import VirtualBuildEnv

class Conan(ConanFile):
settings = "os", "compiler", "build_type", "arch"

generators = 'CMakeDeps', 'CMakeToolchain'

def layout(self):
cmake_layout(self)

def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()
""")
hello_cpp = gen_function_cpp(name="main")
cmakelists = gen_cmakelists(appname="hello", appsources=["hello.cpp"])
client.save({"conanfile.py": conanfile, "hello.cpp": hello_cpp, "CMakeLists.txt": cmakelists})

client.run("build . -c tools.env.virtualenv:powershell=True -c tools.cmake.cmaketoolchain:generator=Ninja")
client.run_command(r'powershell.exe ".\build\Release\generators\conanbuild.ps1; dir env:"')
#check the conanbuid.ps1 activation message
assert "conanvcvars.ps1: Activated environment" in client.out
#check that the new env variables are set
assert "VSCMD_ARG_VCVARS_VER" in client.out
conanbuild = client.load(r".\build\Release\generators\conanbuild.ps1")
vcvars_ps1 = client.load(r".\build\Release\generators\conanvcvars.ps1")
#check that the conanvcvars.ps1 is being added to the conanbuild.ps1
assert "conanvcvars.ps1" in conanbuild
#check that the conanvcvars.ps1 is setting the environment
assert "conanvcvars.bat&set" in vcvars_ps1