-
Notifications
You must be signed in to change notification settings - Fork 475
Fix vswhere.exe discovery for non-default Visual Studio installations #1248
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -56,15 +56,17 @@ def run_cmd(cmd): | |
| output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, shell=True) | ||
| # Print output even on success to show warnings | ||
| if output: | ||
| decoded_output = output.decode() | ||
| decoded_output = output.decode("utf-8", errors="replace") | ||
| if decoded_output.strip(): # Only print if not just whitespace | ||
| # In parallel builds, associate output with its command for clarity | ||
| # Use single print to avoid interleaving with other processes | ||
| print(f"Output from: {cmd}\n{decoded_output}") | ||
| return output | ||
| except subprocess.CalledProcessError as e: | ||
| # Single print to avoid interleaving in parallel builds | ||
| print(f"Command failed with exit code {e.returncode}: {cmd}\nCommand output was:\n{e.output.decode()}") | ||
| print( | ||
| f"Command failed with exit code {e.returncode}: {cmd}\nCommand output was:\n{e.output.decode('utf-8', errors='replace')}" | ||
| ) | ||
| raise e | ||
|
|
||
|
|
||
|
|
@@ -96,6 +98,38 @@ def set_msvc_env(msvc_path, sdk_path): | |
| return os.path.join(msvc_path, "bin", "HostX64", "x64", "cl.exe") | ||
|
|
||
|
|
||
| def _find_vswhere() -> str: | ||
| """Locate vswhere.exe using multiple discovery strategies. | ||
|
|
||
| Search order: | ||
| 1. ``PATH`` (covers Chocolatey, Scoop, winget, or manual installs) | ||
| 2. Default VS Installer location under ``%ProgramFiles(x86)%`` | ||
| 3. Fallback under ``%ProgramFiles%`` (32-bit Windows or older setups) | ||
|
|
||
| Returns: | ||
| Absolute path to vswhere.exe, or empty string if not found. | ||
| """ | ||
| path_result = shutil.which("vswhere.exe") | ||
| if path_result: | ||
| if verbose_cmd: | ||
| print(f"Found vswhere.exe in PATH: {path_result}") | ||
| return path_result | ||
|
|
||
| candidates = [ | ||
| os.path.expandvars(r"%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe"), | ||
| os.path.expandvars(r"%ProgramFiles%\Microsoft Visual Studio\Installer\vswhere.exe"), | ||
| ] | ||
| for candidate in candidates: | ||
| if os.path.isfile(candidate): | ||
| if verbose_cmd: | ||
| print(f"Found vswhere.exe at: {candidate}") | ||
| return candidate | ||
|
|
||
| if verbose_cmd: | ||
| print("Warning: Could not locate vswhere.exe") | ||
| return "" | ||
|
|
||
|
|
||
| def find_host_compiler() -> str: | ||
| """Find the host C++ compiler. | ||
|
|
||
|
|
@@ -124,26 +158,52 @@ def find_host_compiler() -> str: | |
| if verbose_cmd: | ||
| print("Warning: VS environment variables set but cl.exe not found, attempting auto-configuration") | ||
|
|
||
| vswhere_path = r"%ProgramFiles(x86)%/Microsoft Visual Studio/Installer/vswhere.exe" | ||
| vswhere_path = os.path.expandvars(vswhere_path) | ||
| if not os.path.exists(vswhere_path): | ||
| return "" # Signal to caller that VS not found | ||
| vswhere_path = _find_vswhere() | ||
| if not vswhere_path: | ||
| return "" # Signal to caller that vswhere.exe not found | ||
|
|
||
| vs_path = run_cmd(f'"{vswhere_path}" -latest -property installationPath').decode().rstrip() | ||
| try: | ||
| vs_path = ( | ||
| run_cmd(f'"{vswhere_path}" -latest -property installationPath') | ||
| .decode("utf-8", errors="replace") | ||
| .rstrip() | ||
| ) | ||
| except subprocess.CalledProcessError: | ||
| return "" # Signal to caller that vswhere command failed | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if not vs_path: | ||
| if verbose_cmd: | ||
| print("Warning: vswhere.exe found no Visual Studio installation") | ||
| return "" | ||
| vsvars_path = os.path.join(vs_path, "VC\\Auxiliary\\Build\\vcvars64.bat") | ||
|
|
||
| if not os.path.exists(vsvars_path): | ||
| return "" # Signal to caller that VS environment script not found | ||
|
|
||
| output = run_cmd(f'"{vsvars_path}" && set').decode() | ||
| try: | ||
|
Contributor
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. I don't think this is necessary. Trying to call the batch script will already produce an error if it's not found or it failed. |
||
| output = run_cmd(f'"{vsvars_path}" && set').decode("utf-8", errors="replace") | ||
| except subprocess.CalledProcessError: | ||
| return "" # Signal to caller that VS environment script failed | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| for line in output.splitlines(): | ||
| pair = line.split("=", 1) | ||
| if len(pair) >= 2: | ||
| os.environ[pair[0]] = pair[1] | ||
|
|
||
| cl_path = shutil.which("cl.exe") | ||
| cl_version = os.environ["VCToolsVersion"].split(".") | ||
| if not cl_path: | ||
|
Contributor
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. This also seems unnecessary. If it's not found in the environment something went seriously wrong and printing nicely formatted error messages isn't any more useful than just the Python exception that gets raised. |
||
| if verbose_cmd: | ||
| print("Warning: cl.exe not found in PATH after running vcvars64.bat") | ||
| return "" | ||
| vc_tools_version = os.environ.get("VCToolsVersion", "") | ||
| if not vc_tools_version: | ||
| if verbose_cmd: | ||
| print("Warning: VCToolsVersion not set after running vcvars64.bat") | ||
| return "" | ||
| cl_version = vc_tools_version.split(".") | ||
Adityakk9031 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if len(cl_version) < 2: | ||
| if verbose_cmd: | ||
| print(f"Warning: Invalid VCToolsVersion format: {vc_tools_version}") | ||
| return "" | ||
|
|
||
| # ensure at least VS2019 version, see list of MSVC versions here https://en.wikipedia.org/wiki/Microsoft_Visual_C%2B%2B | ||
| cl_required_major = 14 | ||
|
|
||
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.
Under what conditions do we actually end up here having to try other candidate paths? As noted on line 105, accessing vswhere from PATH should already cover any known installation method.