Update windows build machines by agocke · Pull Request #65798 · dotnet/runtime

agocke · 2022-02-23T19:24:21Z

Two changes: removes vs2019 in favor of vs2022

Removes 19H1 in favor of 21H1, as 19H1 is being expired.

ghost · 2022-02-23T19:24:24Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

hoyosjs · 2022-02-23T21:34:59Z

These changes are in eng/common.

ghost · 2022-02-24T12:11:15Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

Two changes: removes vs2019 in favor of vs2022

Removes 19H1 in favor of 21H1, as 19H1 is being expired.

Author:	agocke
Assignees:	agocke
Labels:	`area-Infrastructure`
Milestone:	-

AndyAyersMS · 2022-02-24T16:51:40Z

Seems like updating to VS2022 wasn't sufficient? The build error is still there in the Build Cross OS Linux DAC for Windows task

** Visual Studio 2022 Developer Command Prompt v17.0.6
...
[1/441] Building C object pal\src\libunwind\src\CMakeFiles\libunwind_xdac.dir\aarch64\Gtrace.c.obj
FAILED: pal/src/libunwind/src/CMakeFiles/libunwind_xdac.dir/aarch64/Gtrace.c.obj C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1430~1.307\bin\Hostx86\x64\cl.exe /nologo -DBUILDENV_CHECKED=1 -DCROSS_COMPILE -DDEBUG -DDISABLE_CONTRACTS -DHAVE_CONFIG_H=1 -DHAVE_UNW_GET_ACCESSORS -DHAVE___THREAD=0 -DHOST_64BIT -DHOST_AMD64 -DHOST_WINDOWS -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_STRING=\"\" -DTARGET_64BIT -DTARGET_ARM64 -DTARGET_LINUX -DTARGET_UNIX -DUNW_REMOTE_ONLY -DURTBLDENV_FRIENDLY=Checked -D_CRT_SECURE_NO_WARNINGS -D_DBG -D_DEBUG -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D_Thread_local="" -D__aarch64__ -D__linux__ -Ipal\src\libunwind\src -ID:\a\_work\1\s\src\coreclr\pal\src\libunwind\src -ID:\a\_work\1\s\src\native -ID:\a\_work\1\s\src\coreclr\pal\src\libunwind\include\tdep -ID:\a\_work\1\s\src\coreclr\pal\src\libunwind\include -Ipal\src\libunwind\include\tdep -Ipal\src\libunwind\include -ID:\a\_work\1\s\src\coreclr\pal\src\libunwind\include\win /DWIN32 /D_WINDOWS /guard:cf /guard:ehcont -MTd /O2 /nologo /W3 /WX /Oi /Oy- /Gm- /Zp8 /Gy /GS /fp:precise /FC /MP /Zm200 /Zc:strictStrings /Zc:wchar_t /Zc:inline /Zc:forScope /wd4065 /wd4100 /wd4127 /wd4189 /wd4200 /wd4201 /wd4245 /wd4291 /wd4456 /wd4457 /wd4458 /wd4733 /wd4838 /wd4960 /wd4961 /wd5105 /we4007 /we4013 /we4102 /we4551 /we4700 /we4640 /we4806 /w34092 /w34121 /w34125 /w34130 /w34132 /w34212 /w34530 /w35038 /w44177 /Zi /ZH:SHA_256 /source-charset:utf-8 /TC /permissive- -wd4068 -wd4146 -wd4244 -wd4267 -wd4334 -wd4311 -wd4475 -wd4477 /showIncludes /Fopal\src\libunwind\src\CMakeFiles\libunwind_xdac.dir\aarch64\Gtrace.c.obj /Fdpal\src\libunwind\src\CMakeFiles\libunwind_xdac.dir\ /FS -c D:\a\_work\1\s\src\coreclr\pal\src\libunwind\src\aarch64\Gtrace.c
D:\a\_work\1\s\src\coreclr\pal\src\libunwind\include\libunwind-aarch64.h(198): error C2061: syntax error: identifier 'alignas'
D:\a\_work\1\s\src\coreclr\pal\src\libunwind\include\libunwind-aarch64.h(199): error C2059: syntax error: '}'
D:\a\_work\1\s\src\coreclr\pal\src\libunwind\include\libunwind-aarch64.h(215): error C2079: 'uc_mcontext' uses undefined struct 'unw_sigcontext'

AndyAyersMS · 2022-02-24T16:58:34Z

Problematic line is:

	alignas(16) uint8_t __reserved[(66 * 8)];

We're including the right header <stdalign.h> so not clear what is wrong yet. Will need to look at preprocessor output.

agocke · 2022-02-24T17:11:21Z

I tried to repro this failure locally and had no luck on my win10 machine with vs2022 P1. The compiler on my machine is older though, so it might just be in an even more recent preview.

jkotas · 2022-02-24T17:34:52Z

We're including the right header <stdalign.h> so not clear what is wrong yet.

stdalign.h comes from Windows SDK (e.g. C:\Program Files (x86)\Windows Kits\10\Include\10.0.22000.0\ucrt). Maybe we are using a bad Windows SDK version on CI machines?

AndyAyersMS · 2022-02-24T17:46:14Z

My local build is fine too.

CI log shows CMake was able to find <stdalign.h> but not clear which version.

-- Looking for include file stdalign.h
-- Looking for include file stdalign.h - found

AndyAyersMS · 2022-02-24T17:48:44Z

Also possibly relevant: #56811.

hoyosjs · 2022-02-24T18:56:46Z

We're including the right header <stdalign.h> so not clear what is wrong yet.
stdalign.h comes from Windows SDK (e.g. C:\Program Files (x86)\Windows Kits\10\Include\10.0.22000.0\ucrt). Maybe we are using a bad Windows SDK version on CI machines?

There was an image rollout yesterday that caused other issues with WinSDK, so entirely possible.

agocke · 2022-02-24T19:06:33Z

Looks like this is happening in main as well: https://dev.azure.com/dnceng/public/_build/results?buildId=1629951&view=results

Probably a Windows SDK update issue

janvorli · 2022-02-24T19:57:42Z

The stdalign.h that's included there should be one that we generate if we don't find one. I could see in a ci failure that we have not found one on the system. So we should have generated a fake one and included it. For some reason, the CI build didn't find it. My local build also doesn't find it, generates one and it works.

janvorli · 2022-02-24T20:09:33Z

Ah, I can see something - on my machine, the stdalign.h was not found. However, in the CI, the stdalign.h was found (contrary to what I've said before), however in both cases the "HAVE_STDALIGN_ALIGNAS - Failed". So what happens is that the stdalign.h that's new in the SDK doesn't have the alignas in it, so when we generate the stdalign.h, the one from the SDK gets included and the alignas is missing.

AndyAyersMS · 2022-02-24T20:11:45Z

You might take a look at #56811 because there is some cmake policy to ensure the cmake scouting and build use the same options and such.

agocke · 2022-02-24T20:12:18Z

@janvorli So you'd be comfortable with a fix where we add this to fakestdalign.h? It sounds like this might be failing in servicing as well, so we'd have to port that change back quite a ways.

hoyosjs · 2022-02-24T20:12:45Z

should the fake check be changed from header exists to a check source compiles?

janvorli · 2022-02-24T20:14:49Z

@janvorli So you'd be comfortable with a fix where we add this to fakestdalign.h?

I guess I've not explained it properly. The fakestdalign.h.in contains the alignas definition, we generate the stdalign.h from it, but since the stdalign.h is now also in the SDK, the SDK one is included first. And that one doesn't have the alignas in it (I guess, it seems to be the only explanation)

agocke · 2022-02-24T20:16:09Z

I see, so we won't generate it at all, if one is present already? Or at least we won't include the one we generate?

hoyosjs · 2022-02-24T20:16:49Z

I see, so we won't generate it at all, if one is present already?

We don't generate if there's an stdalign header, that doesn't guarantee the function exists.

agocke · 2022-02-24T20:17:25Z

Gotcha. Is there any way to not include the system one and use our own?

hoyosjs · 2022-02-24T20:18:32Z

My thinking is we should have a check_source_compiles instead of header exists. If it doesn't compile, then generate the header to have the def.

janvorli · 2022-02-24T20:19:13Z

I don't think any of these would work. Since the SDK one doesn't have the alignas defined and we need one.

hoyosjs · 2022-02-24T20:19:54Z

Haven't we been faking it this whole time though?

janvorli · 2022-02-24T20:19:58Z

So I think the way is to define the alignas macro in the CMakeLists.txt as compiler define in case it was not found.

hoyosjs · 2022-02-25T01:38:22Z

And even worse: CI is using the one dynamically obtainedL D:\a_work\1\s\native-tools\bin\cmake.exe -> the one in global.json

jkotas · 2022-02-25T01:43:41Z

Delete the global.json entry for cmake and just use the one that comes with VS? No workarounds for old cmake necessary.

agocke · 2022-02-25T01:47:19Z

Are the ARM64 machines running VS? Notably this is only happening on ARM, so I wonder if that's why they have a different CMake version.

hoyosjs · 2022-02-25T01:48:02Z

Yes - the global.json one was creating the issues and I just repro'd. The native-dependencies install feature in arcade is being deprecated anyway, so maybe we should just remove native deps from there?

agocke · 2022-02-25T01:49:29Z

Yes - the global.json one was creating the issues and I just repro'd. The native-dependencies install feature in arcade is being deprecated anyway, so maybe we should just remove native deps from there?

That sounds fine to me. We'll just have to confirm that the ARM machines have an up to date version ready to go.

hoyosjs · 2022-02-25T01:50:14Z

Delete the global.json entry for cmake and just use the one that comes with VS? No workarounds for old cmake necessary.

Looks like Jan got to it before I did :)

Are the ARM64 machines running VS? Notably this is only happening on ARM, so I wonder if that's why they have a different CMake version.

No, we always build on Windows x64, same hosts and image/tooling. It's more likely that there's plenty defines in the library that change the use of alignas depending on the architecture targeted.

am11 · 2022-02-25T02:13:13Z

Ah, it was not the issue. Now /std:c11 is showing up in that line from logs, but the compilation still failed in the CI.

AndyAyersMS · 2022-02-25T04:54:27Z

Seems like progress though -- now we have too many definitions instead of too few:

D:\a\_work\1\s\artifacts\obj\coreclr\Linux.arm64.Checked\crossgen\pal\src\libunwind\include\config.h(26): error C2220: the following warning is treated as an error
D:\a\_work\1\s\artifacts\obj\coreclr\Linux.arm64.Checked\crossgen\pal\src\libunwind\include\config.h(26): warning C4005: 'alignas': macro redefinition
C:\Program Files (x86)\Windows Kits\10\include\10.0.22000.0\ucrt\stdalign.h(21): note: see previous definition of 'alignas'

hoyosjs · 2022-02-25T17:30:38Z

So the builds passed with that, but there's a lot of tests failing. Not sure if it's the change of queue (so a compiler version change) or the change in cmake logic. I think it's best to split the two changes to unblock other PRs

am11 · 2022-02-25T18:26:39Z

For wasm build failure, I think it can be fixed by adding this line:

<_MonoCMakeBuildCommandCondition="'$(TargetsBrowser)' == 'true' and '$(HostOS)' == 'windows'">call &quot;$(RepositoryEngineeringDir)native\init-vs-env.cmd&quot;&amp;&amp; $(_MonoCMakeBuildCommand)</_MonoCMakeBuildCommand>

after this one:

runtime/src/mono/mono.proj

Line 493 in 95f7f7a

 <_MonoCMakeBuildCommandCondition="'$(TargetsBrowser)' != 'true' and '$(HostOS)' == 'windows'">call &quot;$(RepositoryEngineeringDir)native\init-vs-env.cmd&quot; $(_CompilerTargetArch) &amp;&amp; cd /D &quot;$(MonoObjDir)&quot;&amp;&amp; @(_MonoBuildEnv, ' ') $(_MonoCMakeBuildCommand)</_MonoCMakeBuildCommand> 

The effective _MonoCMakeBuildCommand on wasm was expecting that cmake is in PATH (global.json mechanism made sure of that which was deleted in last commit), but elsewhere in this file, we are not relying on that assumption and calling init-vs-env to ensure needed prerequisites are present.

cc @lambdageek

x64 failure is something to do with change in .yml files:

.packages\microsoft.dotnet.helix.sdk\7.0.0-beta.22122.3\tools\Microsoft.DotNet.Helix.Sdk.MonoQueue.targets(46,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) Helix API does not contain an entry for Windows.10.Amd64.Server21H1.ES.Open

Should it be Windows.Amd64.Server2022.Open? that one seems to be available.

agocke · 2022-02-25T18:39:29Z

Still working through queue updates -- looks like there's no replacement for the ES queues yet, so I'll just end up reverting those changes. I think the PR to take through first is #65901

akoeplinger · 2022-03-17T09:04:47Z

The Windows.10.Amd64.Server2022.ES.Open queue should be available now

This reverts commit 2ad8f63ff2eed6dcff6dfab3028516ef0053190d.

danmoseley · 2022-04-16T04:01:46Z

Rebased so that hopefully we can merge this.

danmoseley · 2022-04-16T04:16:03Z

@hoyosjs any reason we shouldn't merge this (if green)?

hoyosjs · 2022-04-16T04:51:50Z

It will conflict with the arcade update. I'm happy to say merge this and then we will resolve conflicts.

hoyosjs · 2022-04-16T04:54:31Z

Actually - this one needs to be in the monthly rollouts. It drops support for vs 2019.

danmoseley · 2022-04-16T07:50:28Z

Right, but my interpretation of comments on the other one is that VS2022 / Dev17.4 is required to change the CL errors to warnings.

hoyosjs · 2022-04-16T08:38:27Z

We will probably have to fold this one into that one, but yeah

hoyosjs · 2022-04-16T08:59:34Z

I've folded all these changes onto that one. I'll close this one.

ghost assigned agocke Feb 23, 2022

agocke requested a review from a teamFebruary 23, 2022 19:24

AndyAyersMS mentioned this pull request Feb 23, 2022
Enable QJFL and OSR by default for x64 and arm64 #65675
Merged

hoyosjs reviewed Feb 23, 2022
View reviewed changes

teo-tsirpanis added the area-Infrastructure label Feb 24, 2022

rzikm mentioned this pull request Feb 24, 2022
Assertion failed in System.Net.NetworkInformation.Functional.Tests #65841
Closed

AndyAyersMS mentioned this pull request Feb 24, 2022
Build Cross OS Linux DAC for Windows fails to build on CI #65818
Closed

tannergooding mentioned this pull request Apr 13, 2022
[main] Update dependencies from dotnet/arcade #67771
Merged

agockeand others added 7 commits April 15, 2022 21:57

Remove use of VS2019 machines from queues
d1e6e3e

Revert changes to common
aa51eac

Use correct queue names
1f670d1

Add c11 target_compile_feature spec for coreclrpal
572920d

Revert "Add c11 target_compile_feature spec for coreclrpal"
d394675
This reverts commit 2ad8f63ff2eed6dcff6dfab3028516ef0053190d.

Try a different fix
cd88260

Remove usage of native bootstrapping on Windows
c55c066

danmoseley force-pushed the update-windows-build-machine branch from be4345e to c55c066Compare April 16, 2022 04:00

hoyosjs closed this Apr 16, 2022

ghost locked as resolved and limited conversation to collaborators May 16, 2022

Uh oh!

Conversation

agocke commented Feb 23, 2022

Uh oh!

ghost commented Feb 23, 2022

Uh oh!

hoyosjsFeb 23, 2022

Choose a reason for hiding this comment

Uh oh!

agockeFeb 23, 2022

Choose a reason for hiding this comment

Uh oh!

ghost commented Feb 24, 2022

Uh oh!

AndyAyersMS commented Feb 24, 2022

Uh oh!

AndyAyersMS commented Feb 24, 2022

Uh oh!

agocke commented Feb 24, 2022

Uh oh!

jkotas commented Feb 24, 2022

Uh oh!

AndyAyersMS commented Feb 24, 2022

Uh oh!

AndyAyersMS commented Feb 24, 2022

Uh oh!

hoyosjs commented Feb 24, 2022

Uh oh!

agocke commented Feb 24, 2022

Uh oh!

janvorli commented Feb 24, 2022

Uh oh!

janvorli commented Feb 24, 2022

Uh oh!

AndyAyersMS commented Feb 24, 2022

Uh oh!

agocke commented Feb 24, 2022

Uh oh!

hoyosjs commented Feb 24, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

janvorli commented Feb 24, 2022

Uh oh!

agocke commented Feb 24, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hoyosjs commented Feb 24, 2022

Uh oh!

agocke commented Feb 24, 2022

Uh oh!

hoyosjs commented Feb 24, 2022

Uh oh!

janvorli commented Feb 24, 2022

Uh oh!

hoyosjs commented Feb 24, 2022

Uh oh!

janvorli commented Feb 24, 2022

Uh oh!

hoyosjs commented Feb 25, 2022

Uh oh!

jkotas commented Feb 25, 2022

Uh oh!

agocke commented Feb 25, 2022

Uh oh!

hoyosjs commented Feb 25, 2022

Uh oh!

agocke commented Feb 25, 2022

Uh oh!

hoyosjs commented Feb 25, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

am11 commented Feb 25, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndyAyersMS commented Feb 25, 2022

Uh oh!

hoyosjs commented Feb 25, 2022

Uh oh!

am11 commented Feb 25, 2022

Uh oh!

hoyosjs commented Feb 24, 2022•
edited
Loading

agocke commented Feb 24, 2022•
edited
Loading

hoyosjs commented Feb 25, 2022•
edited
Loading

am11 commented Feb 25, 2022•
edited
Loading