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

Jump to bottom

[cmake] Workaround missing stdalign.h in windows portability test #32764

Merged
merged 1 commit into from
Mar 31, 2023
Merged

[cmake] Workaround missing stdalign.h in windows portability test #32764

merged 1 commit into from
Mar 31, 2023

Conversation

jtattermusch
Copy link
Contributor

@jtattermusch jtattermusch commented Mar 31, 2023 •
edited
Loading

Tentative fix for b/275694647. Also see #32662

Adhoc windows/grpc_portability run: http://sponge/bb75e14c-fc77-45bb-89bd-4c4685746490

Once merged, I'll backport to 1.54.x and 1.53.x (these have switched to VS2019)

@jtattermusch jtattermusch added the release notes: no Indicates if PR should not be in release notes label Mar 31, 2023
@jtattermusch jtattermusch changed the title Workaround missing stdalign.h in windows portability test [cmake] Workaround missing stdalign.h in windows portability test Mar 31, 2023
@jtattermusch jtattermusch marked this pull request as ready for review March 31, 2023 08:10
@jtattermusch
Copy link
Contributor Author

The adhoc windows/grpc_portability run: http://sponge/bb75e14c-fc77-45bb-89bd-4c4685746490 is green so this is ready for review&merge.

@jtattermusch
Copy link
Contributor Author

FYI @gnossen

@jtattermusch jtattermusch enabled auto-merge (squash) March 31, 2023 10:48
@davidben
Copy link
Contributor

Oh, that's fun. :-/ Wonder how MSVC or CMake picks the default.

@davidben
Copy link
Contributor

davidben commented Mar 31, 2023 •
edited
Loading

Oh, that's fun. :-/ Wonder how MSVC or CMake picks the default.

I think I found it!
https://github.com/Kitware/CMake/blob/f52a2fd26d13fa33d34dd1320f82d3f8aa49f3c3/Source/cmGlobalVisualStudio14Generator.cxx#L301

Though it's odd. The very last bit grabs the latest version, so it seems like we're not hitting that case. But otherwise it seems to just look for a match to SystemVersion, which I presume is CMAKE_SYSTEM_VERSION. I don't suppose it's possible your CI has set CMAKE_SYSTEM_VERSION elsewhere??

@davidben
Copy link
Contributor

davidben commented Mar 31, 2023 •
edited
Loading

Ugh. Looks like CMake defaults to the Windows SDK version that matches the host Windows version, rather than the latest one:

https://github.com/Kitware/CMake/blob/f52a2fd26d13fa33d34dd1320f82d3f8aa49f3c3/Source/cmGlobalGenerator.cxx#L624
https://github.com/Kitware/CMake/blob/f52a2fd26d13fa33d34dd1320f82d3f8aa49f3c3/Modules/CMakeDetermineSystem.cmake#L176

See also this discussion, and this comment in particular:
https://gitlab.kitware.com/cmake/cmake/-/issues/16202#note_140259

So I guess that means -DCMAKE_SYSTEM_VERSION=10.0 may be cleaner way to force CMake to pick the latest one. But this whole situation is quite a mess. :-) Sounds like CMake has intentionally chosen the wrong behavior and we need to work around it.

@jtattermusch jtattermusch merged commit 569d007 into grpc:master Mar 31, 2023
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Mar 31, 2023
This was referenced Apr 3, 2023
Merged
Merged
jtattermusch added a commit that referenced this pull request Apr 5, 2023
@jtattermusch
[cmake] Improved stdalign.h workaround for windows build (#32777)
b9f0c01 
Improved the workaround from #32764 as
suggested in
#32764 (comment).

Once merged, I'll backport to 1.54.x and 1.53.x together with
#32764 (these have switched to VS2019)
jtattermusch added a commit to jtattermusch/grpc that referenced this pull request Apr 5, 2023
@jtattermusch
[cmake] Workaround missing stdalign.h in windows portability test (gr…
ebd64c1 
…pc#32764)

Tentative fix for b/275694647. Also see
grpc#32662

Adhoc windows/grpc_portability run:
http://sponge/bb75e14c-fc77-45bb-89bd-4c4685746490

Once merged, I'll backport to 1.54.x and 1.53.x (these have switched to
VS2019)
jtattermusch added a commit to jtattermusch/grpc that referenced this pull request Apr 5, 2023
@jtattermusch
[cmake] Improved stdalign.h workaround for windows build (grpc#32777)
bdf8aad 
Improved the workaround from grpc#32764 as
suggested in
grpc#32764 (comment).

Once merged, I'll backport to 1.54.x and 1.53.x together with
grpc#32764 (these have switched to VS2019)
@jtattermusch jtattermusch mentioned this pull request Apr 5, 2023
Merged
jtattermusch added a commit to jtattermusch/grpc that referenced this pull request Apr 5, 2023
@jtattermusch
[cmake] Workaround missing stdalign.h in windows portability test (gr…
d249dce 
…pc#32764)

Tentative fix for b/275694647. Also see
grpc#32662

Adhoc windows/grpc_portability run:
http://sponge/bb75e14c-fc77-45bb-89bd-4c4685746490

Once merged, I'll backport to 1.54.x and 1.53.x (these have switched to
VS2019)
jtattermusch added a commit to jtattermusch/grpc that referenced this pull request Apr 5, 2023
@jtattermusch
[cmake] Improved stdalign.h workaround for windows build (grpc#32777)
6617c9c 
Improved the workaround from grpc#32764 as
suggested in
grpc#32764 (comment).

Once merged, I'll backport to 1.54.x and 1.53.x together with
grpc#32764 (these have switched to VS2019)
@jtattermusch jtattermusch mentioned this pull request Apr 5, 2023
Merged
jtattermusch added a commit that referenced this pull request Apr 6, 2023
@jtattermusch
[cmake] Backport VS2019 fixes to 1.54.x branch (#32804)
7b177e8 
Backports #32764,
#32777 and
#32658
veblush pushed a commit that referenced this pull request Apr 13, 2023
@jtattermusch
[cmake] Backport VS2019 fixes to 1.53.x branch (#32805)
3c16bf1 
Backports #32764,
#32777 and
#32658
XuanWang-Amos pushed a commit to XuanWang-Amos/grpc that referenced this pull request May 1, 2023
@jtattermusch @XuanWang-Amos
[cmake] Workaround missing stdalign.h in windows portability test (gr…
1e3f215 
…pc#32764)

Tentative fix for b/275694647. Also see
grpc#32662

Adhoc windows/grpc_portability run:
http://sponge/bb75e14c-fc77-45bb-89bd-4c4685746490

Once merged, I'll backport to 1.54.x and 1.53.x (these have switched to
VS2019)
XuanWang-Amos pushed a commit to XuanWang-Amos/grpc that referenced this pull request May 1, 2023
@jtattermusch @XuanWang-Amos
[cmake] Improved stdalign.h workaround for windows build (grpc#32777)
4b93986 
Improved the workaround from grpc#32764 as
suggested in
grpc#32764 (comment).

Once merged, I'll backport to 1.54.x and 1.53.x together with
grpc#32764 (these have switched to VS2019)
wanlin31 pushed a commit that referenced this pull request May 18, 2023
@jtattermusch @wanlin31
[cmake] Workaround missing stdalign.h in windows portability test (#3…
6949f70 
…2764)

Tentative fix for b/275694647. Also see
#32662

Adhoc windows/grpc_portability run:
http://sponge/bb75e14c-fc77-45bb-89bd-4c4685746490

Once merged, I'll backport to 1.54.x and 1.53.x (these have switched to
VS2019)
wanlin31 pushed a commit that referenced this pull request May 18, 2023
@jtattermusch @wanlin31
[cmake] Improved stdalign.h workaround for windows build (#32777)
9b6ea2c 
Improved the workaround from #32764 as
suggested in
#32764 (comment).

Once merged, I'll backport to 1.54.x and 1.53.x together with
#32764 (these have switched to VS2019)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veblush veblush veblush approved these changes

Assignees

@veblush veblush

Labels
bloat/none imported Specifies if the PR has been imported to the internal repository per-call-memory/neutral per-channel-memory/neutral release notes: no Indicates if PR should not be in release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jtattermusch @davidben @veblush