Skip to content

cmake: honor CMAKE_C_FLAGS in test 1119 and 1167#18307

Closed
vszakats wants to merge 1 commit intocurl:masterfrom
vszakats:cm-1119-1167
Closed

cmake: honor CMAKE_C_FLAGS in test 1119 and 1167#18307
vszakats wants to merge 1 commit intocurl:masterfrom
vszakats:cm-1119-1167

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Aug 18, 2025

CMAKE_C_FLAGS is not set by curl, but may contain custom options
required for a successful compiler run, when invoked by these tests.

One such case is when configuring Visual Studio or clang-cl via compiler
options, instead of envs.

Cherry-picked from #18301

`CMAKE_C_FLAGS` may contain options required for a successful compiler
invocation, done by these tests.

One such case is when configuring Visual Studio or clang-cl via compiler
options, instead of envs.

Cherry-picked from curl#18301
@vszakats vszakats added the cmake label Aug 18, 2025
@github-actions github-actions bot added the build label Aug 18, 2025
@vszakats vszakats added the tests label Aug 18, 2025
@vszakats vszakats closed this in a19bd43 Aug 18, 2025
@vszakats vszakats deleted the cm-1119-1167 branch August 18, 2025 09:09
vszakats added a commit that referenced this pull request Aug 18, 2025
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: #14854 (comment)
Ref: a19bd43 #18307
Follow-up to 9463769 #16583

Closes #18301
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant

Comments