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

Use vswhere to find Visual C++ in Windows #1057

Closed
wants to merge 1 commit into from

Conversation

tats-u
Copy link
Contributor

@tats-u tats-u commented Sep 5, 2024

Description

A clear and concise description of this pull request.

Visual Studio provides vswhere to make it easier to find the path of a file provided by Visual Studio.
We should use it to find vcvarsall.bat instaed of designating its path directly.

Issue IDs

Issue and/or discussion IDs related to this pull request.

Steps to test new behaviors (if any)

A clear and concise description about how to verify new behaviors (if any).

  • OS: Windows
  • Steps:
    1. cd src
    2. Run sub commands of python ./build_mozc.py

Additional context

Add any other context about the pull request here.

We won't need to make extra changes for newer Visual Studio in the future anymore.

@yukawa
Copy link
Collaborator

yukawa commented Sep 6, 2024

Thank you for your pull request. While I agree that vswhere is a nice tool we can rely on, I’m leaning towards revisiting this after we complete the migration to bazel as being tracked in #948. Feel free to resend the pull request once #948 is completed and build_mozc.py is completely removed.

We won't need to make extra changes for newer Visual Studio in the future anymore.

It’s highly unlikely that we can support any newer version of Visual Studio as long as we keep using GYP, which is no longer actively developed.

@yukawa yukawa closed this Sep 6, 2024

def find_vswhere() -> Optional[pathlib.Path]:
path = pathlib.Path(r'C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let’s use %ProgramFiles(x86)%.

return pathlib.Path(first)

def get_display_name_from_vswhere(vswhere_path: pathlib.Path) -> Optional[str]:
result = subprocess.run([str(vswhere_path), '-property', 'displayName', '-utf8'], stdout=subprocess.PIPE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless it’s absolutely necessary, I prefer to set shell=False to avoid any unnecessary complexities coming from cmd.exe.

@tats-u
Copy link
Contributor Author

tats-u commented Sep 7, 2024

build_mozc.py is completely removed.

vs_uti.py will be also unnecessary after that, I think.

hiroyuki-komatsu pushed a commit that referenced this pull request Oct 4, 2024
Unlike GYP build, we cannot simply pass '--vcvarsall_path' in Bazel
build for Windows (#948). To support environment where Visual C++ is
installed in a non standard directory, 'vs_util.py' starts using
'vswhere.exe' [1] with this commit.

See also #1057.

 [1]: https://github.com/microsoft/vswhere

PiperOrigin-RevId: 682300183
@tats-u tats-u deleted the vswhere branch October 12, 2024 09:23
yukawa added a commit to yukawa/mozc that referenced this pull request Jan 8, 2025
This is a follow up commit to my previous commit [1], which started
using 'vswhere.exe' to locate 'vcvarsall.bat' as discussed in google#1057.

One thing I overlooked is that 'vswhere.exe' might have returned an
empty result even when the exitcode is ERROR_SUCCESS. With this commit
such a case will be gracefully handled.

This is also a preparation to support ARM64 build on Windows (google#1130).

 [1]: ace3145
yukawa added a commit to yukawa/mozc that referenced this pull request Jan 8, 2025
This is a follow up commit to my previous commit [1], which started
using 'vswhere.exe' to locate 'vcvarsall.bat' as discussed in google#1057.

One thing I overlooked is that 'vswhere.exe' could return an empty
result even when the exitcode is ERROR_SUCCESS. With this commit such a
case will be gracefully handled.

This is also a preparation to support ARM64 build on Windows (google#1130).

 [1]: ace3145
yukawa added a commit to yukawa/mozc that referenced this pull request Jan 8, 2025
This is a follow up commit to my previous commit [1], which started
using 'vswhere.exe' to locate 'vcvarsall.bat' as discussed in google#1057.

One thing I overlooked is that 'vswhere.exe' could return an empty
result even when the exitcode is ERROR_SUCCESS. With this commit such a
case will be gracefully handled.

This is also a preparation to support ARM64 build on Windows (google#1130).

 [1]: ace3145
hiroyuki-komatsu pushed a commit that referenced this pull request Jan 9, 2025
This is a follow up commit to my previous commit [1], which started
using 'vswhere.exe' to locate 'vcvarsall.bat' as discussed in #1057.

One thing I overlooked is that 'vswhere.exe' could return an empty
result even when the exitcode is ERROR_SUCCESS. With this commit such a
case will be gracefully handled.

This is also a preparation to support ARM64 build on Windows (#1130).

 [1]: ace3145

PiperOrigin-RevId: 713563231
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants