]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
GHA/windows: switch from MSBuild to Ninja for MSVC jobs running tests
authorViktor Szakats <commit@vsz.me>
Sun, 17 Aug 2025 08:46:22 +0000 (10:46 +0200)
committerViktor Szakats <commit@vsz.me>
Mon, 18 Aug 2025 11:56:35 +0000 (13:56 +0200)
This patch fixes flakiness caused by MSBuild scanning the runtests.pl
output for regex patterns. When finding a hit, it returns an error code
to cmake, making the build test CI step fail. This happens rarely after
an earlier mitigation tweaking outputs, but, as expected, it did not
resolve it completely.

MSBuild doesn't have an option to disable this behavior. To fix, this
patch migrates the two affected jobs from MSBuild to Ninja. To align
with existing multi-config logic, it uses the `Ninja Multi-Config`
generator, which hasn't been tested before in CI.

Switching to Ninja was not trivial. Visual Studio to this day relies on
an MS-DOS batch file stored at an unstable location (containing spaces
and parenthesis), to initialize its environment. Without this env,
`cl.exe` is unable to find its own components. GHA does not initialize
it (even if it did, it could only default to a single specific target).
CMake helps with this when using a Visual Studio generator, but doesn't
when using Ninja. (On local machines the VS installer adds a couple
of Start menu items for launching pre-configured command prompts.)

Ref: https://learn.microsoft.com/cpp/build/building-on-the-command-line

The MS-DOS batches don't integrate well with CI envs and even less so
with shell scripts. To avoid it, this patch uses manual configuration.
Also without using environment variables, to make it easy to use and
easy to debug and trace in logs. Configuring Visual Studio is relatively
stable across releases and hasn't changed a whole lot in the last 2
decades, but still may need more maintenance compared to llvm, or pretty
much any other toolchain out there. On the upside, it allows to manually
select compiler version, SDK version, cross-combinations, and allows
choosing clang-cl. The configuration aims to find the latest of these
automatically.

Some traps that had to be avoided:
- need to switch to MS-DOS short names to avoid spaces in the VS
  component paths.
- need to switch to forward slashes to avoid confusing downstream tools
  with backslashes.
- need to pass either MSYS2 for Windows-style path depending on setting.
- need to use a trick to retrieve the oddly named `ProgramFiles(x86)`
  Windows env from shell script.
- need to match VS version (2022) and edition (Enterprise), found on GHA
  runners.
- need to pass the CMake generator via env so that the space in the name
  doesn't trip the shell when passed via a variable.
- trash and unexpected dirs when detecting SDK/toolchain versions.
- need to pass `-external:W0` to the C compiler to avoid MSVC warning:
  `D9007: '/external:I' requires '/external:W'; option ignored`
- using cmake options only, to make it run without relying on envs and
  work out-of-the-box when running subsequent cmake sessions.
- some others discovered while making work clang-cl locally in
  cross-builds.

Ninja also improves performance in most cases (though wasn't a goal
here). After this patch configure is significantly faster (1.5-2x),
builds are a tiny bit faster, except examples which was twice as fast
with MSBuild. Disk space use is 10% lower.

MSBuild builds remain tested in AppVeyor CI and the UWP job.

Before: https://github.com/curl/curl/actions/runs/17025737223/job/48260856051
After: https://github.com/curl/curl/actions/runs/17027981486/job/48266133301

Fixes:
```
  === Start of file stderr1635
     % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                    Dload  Upload   Total   Spent    Left  Speed

     0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
   100     4  100     4    0     0    449      0 --:--:-- --:--:-- --:--:--   500
curl : (22) The requested URL returned error : 429 [D:\a\curl\curl\bld\tests\test-ci.vcxproj]
CUSTOMBUILD : warning : Problem : HTTP error. Will retry in 1 second. 1 retry left. [D:\a\curl\curl\bld\tests\test-ci.vcxproj]
[...]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'D:\a\curl\curl\bld\CMakeFiles\621f80ddbb0fa48179f056ca77842ff0\test-ci.rule;D:\a\curl\curl\tests\CMakeLists.txt' exited with code -1. [D:\a\curl\curl\bld\tests\test-ci.vcxproj]
Error: Process completed with exit code 1.
```
Ref: https://github.com/curl/curl/actions/runs/16966304797/job/48091058271?pr=18287#step:13:3471

Bug: https://github.com/curl/curl/discussions/14854#discussioncomment-14104166
Ref: a19bd4321030c6ea18e3bbec411dfd49961a1f71 #18307
Follow-up to 9463769f2e2dba9eeff554a88e5df5195d2c774b #16583

Closes #18301

.github/workflows/windows.yml

index 796534c0144ab0ca4a7e4e9be4316623f830f4ff..af05bb92c854faa3920bf37d5d3c8910f7f51b19 100644 (file)
@@ -821,16 +821,58 @@ jobs:
           [ -f "${MINGW_PREFIX}/include/zconf.h" ] && sed -i -E 's|(# +define +Z_HAVE_UNISTD_H)|/*\1*/|g' "${MINGW_PREFIX}/include/zconf.h"  # Patch MSYS2 zconf.h for MSVC
           for _chkprefill in '' ${MATRIX_CHKPREFILL}; do
             options=''
+            cflags=''
+            rcflags=''
+            ldflags=''
             if [ "${MATRIX_PLAT}" = 'uwp' ]; then
               options+=' -DCMAKE_SYSTEM_NAME=WindowsStore -DCMAKE_SYSTEM_VERSION=10.0'
-              cflags='-DWINAPI_FAMILY=WINAPI_FAMILY_PC_APP'
-              ldflags='-OPT:NOREF -OPT:NOICF -APPCONTAINER:NO'
+              cflags+=' -DWINAPI_FAMILY=WINAPI_FAMILY_PC_APP'
+              ldflags+=' -OPT:NOREF -OPT:NOICF -APPCONTAINER:NO'
               vsglobals=';AppxPackage=false;WindowsAppContainer=false'
             fi
-            [ "${MATRIX_ARCH}" = 'arm64' ] && options+=' -A ARM64'
-            [ "${MATRIX_ARCH}" = 'x64' ] && options+=' -A x64'
-            [ "${MATRIX_ARCH}" = 'x86' ] && options+=' -A Win32'
-            [ "${TFLAGS}" = 'skiprun' ] && options+=' -D_CURL_SKIP_BUILD_CERTS=ON'
+            if [ "${TFLAGS}" = 'skiprun' ]; then
+              [ "${MATRIX_ARCH}" = 'arm64' ] && options+=' -A ARM64'
+              [ "${MATRIX_ARCH}" = 'x64' ] && options+=' -A x64'
+              [ "${MATRIX_ARCH}" = 'x86' ] && options+=' -A Win32'
+              options+=" -DCMAKE_VS_GLOBALS=TrackFileAccess=false${vsglobals}"
+              options+=' -D_CURL_SKIP_BUILD_CERTS=ON'
+              unset CMAKE_GENERATOR
+            else
+              # Use Ninja when running tests to avoid MSBuild heuristics picking
+              # up "error messages" in the test log output and making the job fail.
+              # Officially this requires the vcvarsall.bat MS-DOS batch file (as of
+              # VS2022). Since it integrates badly with CI steps and shell scripts
+              # scripts, reproduce the necessary build configuration manually, and
+              # without envs.
+              [[ "$(uname -s)" = *'ARM64'* ]] && MSVC_HOST='arm64' || MSVC_HOST='x64'  # x86
+              MSVC_ROOTD="$(cygpath --mixed --short-name "$PROGRAMFILES/Microsoft Visual Studio")"  # to avoid spaces in directory names
+              MSVC_ROOTU="$(/usr/bin/find "$(cygpath --unix "$MSVC_ROOTD/2022/Enterprise/vc/tools/msvc")" -mindepth 1 -maxdepth 1 -type d -name '*.*' | sort | tail -n 1)"
+              MSVC_ROOTW="$(cygpath --mixed "$MSVC_ROOTU")"
+              MSVC_ROOTU="$(cygpath --unix "$MSVC_ROOTW")"
+              MSVC_BINU="$MSVC_ROOTU/bin/Host$MSVC_HOST/$MATRIX_ARCH"
+              MSDK_ROOTW="$(cygpath --mixed --short-name "$(printenv 'ProgramFiles(x86)')/Windows Kits")/10"
+              MSDK_ROOTU="$(cygpath --unix "$MSDK_ROOTW")"
+              MSDK_VER="$(basename "$(/usr/bin/find "$MSDK_ROOTU/lib" -mindepth 1 -maxdepth 1 -type d -name '*.*' | sort | tail -n 1)")"
+              MSDK_LIBW="$MSDK_ROOTW/lib/$MSDK_VER"
+              MSDK_INCW="$MSDK_ROOTW/include/$MSDK_VER"
+              MSDK_BINU="$MSDK_ROOTU/bin/$MSDK_VER/$MSVC_HOST"
+              cflags+=" -external:W0"
+              cflags+=" -external:I$MSVC_ROOTW/include"
+              cflags+=" -external:I$MSDK_INCW/shared"
+              cflags+=" -external:I$MSDK_INCW/ucrt"
+              cflags+=" -external:I$MSDK_INCW/um"
+              cflags+=" -external:I$MSDK_INCW/km"
+              rcflags+=" -I$MSDK_INCW/shared"
+              rcflags+=" -I$MSDK_INCW/um"
+              ldflags+=" -libpath:$MSVC_ROOTW/lib/$MATRIX_ARCH"
+              ldflags+=" -libpath:$MSDK_LIBW/ucrt/$MATRIX_ARCH"
+              ldflags+=" -libpath:$MSDK_LIBW/um/$MATRIX_ARCH"
+              ldflags+=" -libpath:$MSDK_LIBW/km/$MATRIX_ARCH"
+              options+=" -DCMAKE_RC_COMPILER=$MSDK_BINU/rc.exe"
+              options+=" -DCMAKE_MT=$MSDK_BINU/mt.exe"
+              options+=" -DCMAKE_C_COMPILER=$MSVC_BINU/cl.exe"
+              export CMAKE_GENERATOR='Ninja Multi-Config'  # pass it via env to avoid space issues
+            fi
             [ "${_chkprefill}" = '_chkprefill' ] && options+=' -D_CURL_PREFILL=OFF'
             if [ -n "${MATRIX_INSTALL_VCPKG}" ]; then
               options+=" -DCMAKE_TOOLCHAIN_FILE=$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake"
@@ -840,9 +882,9 @@ jobs:
             fi
             cmake -B "bld${_chkprefill}" ${options} \
               -DCMAKE_C_FLAGS="${cflags}" \
+              -DCMAKE_RC_FLAGS="${rcflags}" \
               -DCMAKE_EXE_LINKER_FLAGS="-INCREMENTAL:NO ${ldflags}" \
               -DCMAKE_SHARED_LINKER_FLAGS="-INCREMENTAL:NO ${ldflags}" \
-              -DCMAKE_VS_GLOBALS="TrackFileAccess=false${vsglobals}" \
               -DCMAKE_UNITY_BUILD=ON \
               -DCURL_WERROR=ON \
               -DLIBPSL_INCLUDE_DIR="${MINGW_PREFIX}/include" \