Conversation
julianbrost
left a comment
julianbrost
left a comment
There was a problem hiding this comment.
I guess we won't finish #10878 today given the open questions and the cross-dependency to our Gitlab build infrastructure. Even though we will want to switch to Visual Studio 2026 eventually, unless we want to live with always-failing pipelines until then, the general approach of this PR sounds like the lowest friction workaround for the meantime.
| # GitHub Actions uses an image that comes with most dependencies preinstalled. Don't install them twice. | ||
| if (-not $Env:GITHUB_ACTIONS) { | ||
| choco install -y ` | ||
| "visualstudio${VsVersion}community" ` | ||
| "visualstudio${VsVersion}-workload-vctools" ` | ||
| "visualstudio${VsVersion}-workload-manageddesktop" ` | ||
| "visualstudio${VsVersion}-workload-nativedesktop" ` | ||
| "visualstudio${VsVersion}-workload-universal" ` | ||
| "visualstudio${VsVersion}buildtools" ` | ||
| git ` | ||
| cmake ` | ||
| winflexbison3 ` | ||
| netfx-4.6-devpack ` | ||
| windows-sdk-8.1 ` | ||
| wixtoolset | ||
| ThrowOnNativeFailure | ||
| } else { | ||
| choco install -y winflexbison3 | ||
| ThrowOnNativeFailure | ||
| } | ||
|
|
||
| choco install -y "visualstudio${VsVersion}community" | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y "visualstudio${VsVersion}-workload-vctools" | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y "visualstudio${VsVersion}-workload-manageddesktop" | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y "visualstudio${VsVersion}-workload-nativedesktop" | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y "visualstudio${VsVersion}-workload-universal" | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y "visualstudio${VsVersion}buildtools" | ||
| ThrowOnNativeFailure | ||
|
|
||
|
|
||
| choco install -y git | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y cmake | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y netfx-4.6-devpack | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y winflexbison3 | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y windows-sdk-8.1 | ||
| ThrowOnNativeFailure | ||
|
|
||
| choco install -y wixtoolset | ||
| ThrowOnNativeFailure |
There was a problem hiding this comment.
However, can we please not just blindly revert that commit bringing back this monstrosity and also installing more than what would be needed. Or is there a particular reason why we would also need to install our own version of Git and so on?
I'd suggest to just adapt the else branch by adding the visualstudio... entries there with a "Install an older version of VS until our build infrastructure is migrated to VS 2026, see [issue link]" comment and an issue for tracking that (issue just in case #10878 might be closed in favor of another PR that tries a different approach).
| $vcvars_locations = @( | ||
| "${VSBASE}\BuildTools\VC\Auxiliary\Build\vcvars${bits}.bat" | ||
| "${VSBASE}\BuildTools\VC\Auxiliary\Build\vcvars${bits}.bat", | ||
| "${VSBASE}\Community\VC\Auxiliary\Build\vcvars${bits}.bat" | ||
| "${VSBASE}\Enterprise\VC\Auxiliary\Build\vcvars${bits}.bat" | ||
| ) |
There was a problem hiding this comment.
And if we're not blindly reverting, there shouldn't be harm in also keeping this as-is.
The `windows-2025` image upgraded from VS 2022 to VS 2026, so VS 2022 isn't pre-installed anymore.
Al2Klimov
changed the title
2 participants
This reverts #9172, to fix our Windows GitHub actions.
This is meant as a bridge technology until we've done #10878 the way we want to.
With all respect for build times, they don't matter in Windows case, as they're shadowed by the much longer Linux build times.
closes #10884