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.

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

cmake: add Linux CI job, fix pytest with cmake #14382

Closed
wants to merge 26 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Aug 4, 2024

  • extend existing Linux workflow with CMake support.
    Including running pytest the first time with CMake.

  • cmake: generate tests/config and tests/http/config.ini.
    Required for pytest tests.
    Uses basic detection logic. Feel free to take it from here.
    Also dump config files in a CI step for debugging purposes.

  • cmake: build tests/http/clients programs.

  • fix portability issues with tests/http/clients programs.
    Some of them use getopt(), which is not supported by MSVC.
    Fix the rest to compile in CI (old-mingw-w64, MSVC, Windows).

  • GHA/linux: add CMake job matching an existing autotools one.

  • GHA/linux: test -DCURL_LIBCURL_VERSIONED_SYMBOLS=ON
    in the new CMake job.

  • reorder testdeps to build server, client tests first and then
    libtests and units, to catch errors in the more complex/unique
    sources earlier.

  • sort list in tests/http/clients/Makefile.inc.

Closes #14382

@github-actions github-actions bot added the build label Aug 4, 2024
@vszakats vszakats marked this pull request as draft August 4, 2024 23:43
@github-actions github-actions bot added the CI Continuous Integration label Aug 5, 2024
@vszakats vszakats force-pushed the cm-build-http-clients branch 2 times, most recently from d4f8ca8 to 4629a81 Compare August 5, 2024 08:43
@vszakats vszakats changed the title cmake: build tests/http/clients cmake: add Linux CI job, with tests Aug 5, 2024
```
C:/Users/runneradmin/my-cache/mingw64/x86_64-w64-mingw32/include/inttypes.h:94:20: note: format string is defined here
 #define PRIu64 "I64u"
D:/a/curl/curl/tests/http/clients/h2-upgrade-extreme.c:184:44: error: ISO C does not support the 'I' printf flag [-Werror=format=]
       curl_msnprintf(range, sizeof(range), "%" PRIu64 "-%" PRIu64,
                                            ^~~
D:/a/curl/curl/tests/http/clients/h2-upgrade-extreme.c:184:44: error: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'long long unsigned int' [-Werror=format=]
```
Ref: https://github.com/curl/curl/actions/runs/10237291938/job/28320411708?pr=14382#step:10:479
test -DCURL_LIBCURL_VERSIONED_SYMBOLS=ON on Linux

use CMAKE_C_COMPILER_TARGET to match autotools -V output

enable static libcurl for cmake job

match autotools with brotli and zstd

Could not figure out why the difference. brotli and zstd are
auto-detected by autotools on Linux, while at least brotli isn't
on FreeBSD.

It's also suspect that autotools detections depend on each other,
e.g. LDAP fails to detect unless a `--with-brotli` precedes it?:
```
configure: found both libz and libz.h header
checking for BrotliDecoderDecompress in -lbrotlidec... no
checking for brotli/decode.h... no
checking for ZSTD_createDStream in -lzstd... no
checking for zstd.h... no
checking for lber.h... no
checking for ldap.h... no
checking for ldap_ssl.h... no
checking for LDAP libraries... cannot find LDAP libraries
configure: error: couldn't detect the LDAP libraries
```
https://github.com/curl/curl/actions/runs/10238274365/job/28322546461?pr=14378#step:3:657

With `--with-brotli` it looks like this:
```
configure: found both libz and libz.h header
checking for pkg-config... (cached) /usr/local/bin/pkg-config
checking for libbrotlidec options with pkg-config... found
checking for BrotliDecoderDecompress in -lbrotlidec... yes
checking for brotli/decode.h... yes
configure: Added /usr/local/lib to CURL_LIBRARY_PATH
checking for ZSTD_createDStream in -lzstd... yes
checking for zstd.h... yes
checking for lber.h... yes
checking for ldap.h... yes
checking for ldap_ssl.h... no
checking for LDAP libraries... -lldap -llber
checking for ldap_url_parse... yes
checking for ldap_init_fd... yes
```
https://github.com/curl/curl/actions/runs/10238164076/job/28322294717#step:3:650
```
curl\tests\http\clients\tls-session-reuse.c(276) : warning C4706: assignment within conditional expression
```
Ref: https://ci.appveyor.com/project/curlorg/curl/builds/50346041/job/7hnmtv31rsc9g0xa#L153
Three tests apps use getopt() which isn't supported by MSVC.
Start with the small number of unique targets before starting
the long process of building libtests and units.
for curl_setup.h mainly in trying to fix these:
````
/Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/sys/_types/_fd_def.h:54:60: error: 'introduced' undeclared here (not in a function)
   54 | int __darwin_check_fd_set_overflow(int, const void *, int) __API_AVAILABLE(macosx(11.0), ios(14.0), tvos(14.0), watchos(7.0));
      |                                                            ^~~~~~~~~~~~~~~
```
Ref: https://github.com/curl/curl/actions/runs/10249561761/job/28353255485?pr=14382#step:14:1232
@vszakats vszakats marked this pull request as ready for review August 5, 2024 15:19
@vszakats vszakats changed the title cmake: add Linux CI job, with tests cmake: add Linux CI job, fix pytest with cmake Aug 6, 2024
@vszakats vszakats closed this in 232302f Aug 6, 2024
@vszakats vszakats deleted the cm-build-http-clients branch August 6, 2024 00:43
vszakats added a commit to vszakats/curl that referenced this pull request Aug 9, 2024
To limit building them with the testdeps target, like it's done with
the rest of test programs.

Follow-up to 232302f curl#14382
Closes #xxxxx
vszakats added a commit that referenced this pull request Aug 10, 2024
To limit building them with the testdeps target, like it's done with
the rest of test programs.

Follow-up to 232302f #14382
Closes #14477
vszakats added a commit that referenced this pull request Aug 22, 2024
Add tweak for mingw-w64 when building tests/http/client programs to
avoid a bogus `-Wformat` warning when using mingw-w64 v7.0.0 or older.
The warning is bogus because these programs use curl's `printf()`
implementation that is guaranteed to support that format spec.

Add this for both CMake and autotools. (But only CMake is CI tested with
an old toolchain.)

Apply the workaround to `docs/examples`, and fix an example to use
curl's `printf()` with `CURL_FORMAT_CURL_OFF_T`.

Reintroduce curl `printf()` calls into `tests/http/client`, via #14625.
Also restore large number masks to a printf, changed earlier in #14382.

Follow-up to 232302f #14382
Ref: #14625 (comment)

Closes #14640
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build CI Continuous Integration cmake tests
Development

Successfully merging this pull request may close these issues.

1 participant