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: clang-cl improvements #15478

Closed
wants to merge 5 commits into from
Closed

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Nov 2, 2024

  • drop /clang: prefix for -W options for clang-cl.
    Except for -Wall which gets interpreted as MSVC /Wall
    and translated to -Weverything, which is undesired.
    Related: Clang-cl should map /Wall to -Wall, not -Weverything llvm/llvm-project#102982

  • include MSVC_VERSION in target flags.
    Useful for clang-cl builds where this information doesn't appear
    elsewhere in the cmake configure log.

  • suppress -Wlanguage-extension-token more for clang-cl.
    This fixes clang-cl builds with default CURL_WERROR=OFF and
    PICKY_COMPILER=ON.
    This warning is enabled by -pedantic as a warning and by
    -pedantic-errors as an error. Verifiable using llvm's
    diagtool show-enabled -pedantic test.c.
    Follow-up to fb711b5 build: fix clang-cl builds, add CI job #15449


  • fix to always set -Wno-language-extension-token with clang-cl. (It's unclear which option enables it)

w/o whitespace: https://github.com/curl/curl/pull/15478/files?w=1

@vszakats vszakats added cmake Windows Windows-specific labels Nov 2, 2024
@vszakats vszakats marked this pull request as draft November 2, 2024 20:13
@github-actions github-actions bot added the build label Nov 2, 2024
@vszakats vszakats marked this pull request as ready for review November 4, 2024 22:12
@vszakats
Copy link
Member Author

vszakats commented Nov 4, 2024

An interesting benefit of clang-cl is that it allows testing MSVC builds
from non-Windows machines.

This needs obtaining MSVC runtime and headers. Here's a script that
allows doing just that:
https://github.com/mstorsjo/msvc-wine/blob/master/vsdownload.py

(I haven't tried that yet.)

Another potential benefit is with projects with incomplete or broken
support for the mingw-w64 runtime/devenv and requiring MSVC to
work best. Such projects are BoringSSL and AWC-LC.

BoringSSL builds have been broken with mingw-w64 for a while due
to: google/boringssl#32

Another long-term issue is that it relies on a MSVC-compiler-specifc
code to use native Windows threading, and requires pthreads with
mingw-w64. (I have a lightly tested local patch for mingw-w64,
but it's unlikely Google would want that, given their focus on clang-cl
and the MSVC runtime, for Chrome.). pthreads has been subject to
a weird link-time issue for a good while with strandard mingw-w64
toolchains and static linking. This is now fixed in my env, though I'm
not sure when or where the fix happened and if any of the distros are
still affected. (Possibly mingw-w64 12 brought the fix, maybe via this?:
mirror/mingw-w64@ad2b46c.)

AWS-LC could not be built with mingw-w64, and there was/is apparently
little focus on supporting it. It received fixes since then (I haven't
tried those yet). AWC-LC also relies on pthreads for mingw-w64, just like
BoringSSL.

Useful for clang-cl builds where this information doesn't appear
elsewhere in the cmake configure log.
@vszakats
Copy link
Member Author

vszakats commented Nov 5, 2024

The msvc-wine repo did work nicely on the first try. It doesn't need
wine when used with clang-cl.

Managed to do an actual MSVC-like cross-build for the first time.

In theory this also allows creating MSVC builds via ./configure, by
using the normal clang command-line-style while targeting
x86_64-windows-msvc.

The necessary download is 2GB. 5.5GB unpacked, for 4 archs.
Likely possible to trim by limiting to x64 and/or arm64 and
dropping unused Windows-native compiler tools.

Not necessarily for CI, but for local cross-tests this is neat:

curl 8.11.0-DEV (x86_64-windows-msvc) libcurl/8.11.0-DEV Schannel
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ipfs ipns ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp ws wss
Features: alt-svc AsynchDNS HSTS HTTPS-proxy IPv6 Kerberos Largefile NTLM SPNEGO SSL SSPI threadsafe UnixSockets

(the curl CMake configuration is very experimental yet.)

It's enabled by -pedantic as a warning and by -pedantic-errors as an error.
Before this patch this was done only for `CURL_WERROR=ON` builds.

(Verifiable using llvm's `diagtool show-enabled -pedantic test.c`)

Follow-up to fb711b5 curl#15449
@vszakats vszakats closed this in fa676a6 Dec 16, 2024
@vszakats vszakats deleted the cm-clang-cl branch December 16, 2024 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

1 participant