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

Bazel C/C++ toolchain autodetection fails with newest version of Visual Studio on Windows (17.6.2) due to new directory layout #18592

Closed
redsun82 opened this issue Jun 6, 2023 · 9 comments
Assignees
Labels
area-Windows Windows-specific issues and feature requests P1 I'll work on this now. (Assignee required) team-Rules-CPP Issues for C++ rules type: bug

Comments

@redsun82
Copy link

redsun82 commented Jun 6, 2023

Description of the bug:

When running C/C++ builds on Windows with autodetection where the most recent version of Visual Studio Enterprise is installed (version 17.6.2), the build fails with

Auto-Configuration Error: Cannot find VCVARSALL.BAT script under C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

  • Install latest Visual Studio (I did not try with the build tools version though), at least 17.6.2
  • Compile any dummy C/C++ project not using any kind of custom toolchain (for example, //examples/cpp:hello-world in the bazel repo itself)

Which operating system are you running Bazel on?

Windows

What is the output of bazel info release?

release 6.2.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

By debugging I've traced back this behaviour to this _is_vs_2017_or_2019 function

Apart from the misname (it's supposed to actually check whether the release year is greater than 2017, including 2022), the problem is that a recent addition to VS has added a fourth vcpkg directory in the VC directory, that throws the check off.

The check could be updated to include the presence of this vcpkg directory, but I wonder whether there wouldn't be a more solid alternative robust to similar changes in the future. Maybe

def _is_vs_2017_or_later(repository_ctx, vc_path):
    # example path C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC
    release_year = int(repository_ctx.path(vc_path).dirname.dirname.basename)
    return release_year >= 2017

could work better? I can whip up a PR if this seems like a good idea.

@redsun82
Copy link
Author

redsun82 commented Jun 6, 2023

hmm, maybe relying on the year being in the path is not a good idea, seen that you can change the installation directory from the default, including that bit of the path.

@redsun82
Copy link
Author

redsun82 commented Jun 7, 2023

Here's my rough shot at a fix, though I'm not entirely sure how to add tests for it

#18608

@mkruskal-google
Copy link
Contributor

This seems to be affected in Bazel 5 as well, and has broken our CI (we need to support 5 and 6): actions/runner-images#7662

@redsun82
Copy link
Author

redsun82 commented Jun 14, 2023

I've noticed no prior changes to the autoconfiguration included any kind of tests, so I'm marking my PR as ready for review, after trying it out locally.

I've also noticed that all Windows CI checks currently use VS 2019. It might probably be a good idea to add or switch to VS 2022.

As a temporary workaround that I used to compile bazel itself, one can go as admin into the VC directory (for example C:\Program Files\Microsoft Visual Studio\2022\Community\VC, or Enterprise) and move vcpkg ... I did not notice anything amiss in compilation then, but YMMV. I've then undone the change with move ../vcpkg ..

@Mizux
Copy link

Mizux commented Jun 14, 2023

beware that Visual Studio 2022 Community and Enterprise are not the same (FYI github runner and this user is using Enterprise version)

@redsun82
Copy link
Author

Yes, but both actually exhibit the same behaviour and work with the same workaround of moving the vcpkg directory out of the way. I could see after dealing both with a virtual machine at work (with the Enterprise edition) and my personal machine at home (with the Community edition). I did not look at the Professional edition but I'd expect the same. BuildTools might not have that directory, or it's just that I did not have a recent version enough. In any case the proposed PR deals gracefully with the presence or absence of that directory in any case.

@redsun82
Copy link
Author

FYI, I think there is another workaround:

#18608 (comment)

@meteorcloudy meteorcloudy added the P1 I'll work on this now. (Assignee required) label Jun 28, 2023
@meteorcloudy meteorcloudy self-assigned this Jun 28, 2023
dmah42 added a commit to google/benchmark that referenced this issue Jul 11, 2023
Windows 2022 is not well supported by bazel yet:
bazelbuild/bazel#18592
dmah42 added a commit to google/benchmark that referenced this issue Jul 12, 2023
* Downgrade bazel to windows-2019

Windows 2022 is not well supported by bazel yet:
bazelbuild/bazel#18592

* no windows-latest, only windows-2019 (for bazel)
iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Jul 17, 2023
This is the same PR as bazelbuild#18608 but extended by the modification proposed by @meteorcloudy

This should fix bazelbuild#18592

Should also be picked to 6.3.0 -> bazelbuild#18799

Closes bazelbuild#18945.

PiperOrigin-RevId: 548725707
Change-Id: Iff0f972c9fc23491c8070ee2b12ec600a3d1ead9
iancha1992 added a commit that referenced this issue Jul 19, 2023
This is the same PR as #18608 but extended by the modification proposed by @meteorcloudy

This should fix #18592

Should also be picked to 6.3.0 -> #18799

Closes #18945.

PiperOrigin-RevId: 548725707
Change-Id: Iff0f972c9fc23491c8070ee2b12ec600a3d1ead9

Co-authored-by: Vertexwahn <julian.amann@tum.de>
avdv added a commit to tweag/rules_sh that referenced this issue Aug 16, 2023
The Bazel versions under test currently fail to detect Visual Studio 2022, see
bazelbuild/bazel#18592. This is only fixed in
Bazel 6.3.0.
phst added a commit to phst/rules_elisp that referenced this issue Jan 21, 2024
Since Bazel 6.3, Visual C++ 2022 should be supported.  See
bazelbuild/bazel#18592 and
bazelbuild/bazel#18960.
phst added a commit to phst/rules_elisp that referenced this issue Jan 21, 2024
Since Bazel 6.3, Visual C++ 2022 should be supported.  See
bazelbuild/bazel#18592 and
bazelbuild/bazel#18960.
copybara-service bot pushed a commit to google/jax that referenced this issue Apr 30, 2024
copybara-service bot pushed a commit to google/jax that referenced this issue Apr 30, 2024
copybara-service bot pushed a commit to google/jax that referenced this issue Apr 30, 2024
copybara-service bot pushed a commit to google/jax that referenced this issue Apr 30, 2024
copybara-service bot pushed a commit to google/jax that referenced this issue Apr 30, 2024
copybara-service bot pushed a commit to google/jax that referenced this issue Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Windows Windows-specific issues and feature requests P1 I'll work on this now. (Assignee required) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants