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

Fix vs autodetection #18945

Closed
wants to merge 5 commits into from
Closed

Fix vs autodetection #18945

wants to merge 5 commits into from

Conversation

Vertexwahn
Copy link
Contributor

@Vertexwahn Vertexwahn commented Jul 14, 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

@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Rules-CPP Issues for C++ rules labels Jul 14, 2023
redsun82 and others added 5 commits July 14, 2023 21:02
Windows VS 2022 version 17.6 introduced a new `vspkg` directory
underneath `VC` that is throwing off the toolchain autodetection code.

The checks now got renamed to a more approriate `_is_vs_2017_or_newer`
and takes into account the possible presence of this `vspkg` directory.
Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

Thanks!

@meteorcloudy
Copy link
Member

@bazel-io fork 6.3.0

@meteorcloudy
Copy link
Member

@Wyverald @keertk I know we already have rc1 created for 6.3.0, but this is an important compatibility fix for Windows, so let's cherry pick it for 6.3.0 rc2.


# The layout of VC folder in VS 2017 and 2019 is different from that in VS 2015 and older versions.
# In VS 2017 and 2019, it contains only three directories:
# The layout of VC folder in VS 2017 and newer versions is different from that in VS 2015 and older versions.
Copy link
Member

Choose a reason for hiding this comment

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

Can you update the comment here as well?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe just:

# For VS 2017 and later, a `Tools` directory should exist under BAZEL_VC

Copy link
Member

Choose a reason for hiding this comment

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

I can also do this while importing the PR!

@sluongng
Copy link
Contributor

Reading through the smaller commits in this PR (the first one was very similar to mine in #18847), I'm stoked to find others who care about Bazel support for Windows 🤗

Thanks for contributing!

@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Jul 17, 2023
iancha1992 pushed a commit to iancha1992/bazel that referenced this pull request 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 pull request 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
4 participants