cmake, Makefile.mk: use -isystem for dep headers, silence BearSSL issues

[vszakats]

Patch started out for working around compiler warnings in BearSSL latest
tarball release v0.6 (2018-08-14) and Apple clang 14 with CMake.

Then turned into patching CMake and Makefile.mk builds to use
-isystem instead -I when adding header directories for
dependencies. This avoids compiler warnings in dependency headers,
syncing behaviour with autotools.

Also:

• Makefile.mk: add support for BearSSL.
• delete warning suppression for mbedTLS headers. No longer necessary
  after this patch.
  Follow-up to 434db99 mbedtls: fix -Wnull-dereference and -Wredundant-decls #12720

Silenced BearSSL warnings:

In file included from curl/lib/vtls/bearssl.c:28:
In file included from bearssl/inc/bearssl.h:127:
bearssl/inc/bearssl_hash.h:727:5: warning: 'BR_DOXYGEN_IGNORE' is not defined, evaluates to 0 [-Wundef]
    ^
bearssl/inc/bearssl_hash.h:745:5: warning: 'BR_DOXYGEN_IGNORE' is not defined, evaluates to 0 [-Wundef]
    ^
In file included from curl/lib/vtls/bearssl.c:28:
In file included from bearssl/inc/bearssl.h:136:
bearssl/inc/bearssl_ssl.h:1253:20: warning: implicit conversion loses integer precision: 'unsigned int' to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
        cc->version_min = version_min;
                        ~ ^~~~~~~~~~~
bearssl/inc/bearssl_ssl.h:1254:20: warning: implicit conversion loses integer precision: 'unsigned int' to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
        cc->version_max = version_max;
                        ~ ^~~~~~~~~~~
bearssl/inc/bearssl_ssl.h:1327:28: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
        ctx->protocol_names_num = num;
                                ~ ^~~
5 warnings generated.

(These warnings were fixed in BearSSL Git master in 2019 via
2893441f2efd4603ddd6d7f49011bdda096a4a87 and
ecdf89770ee82dfea6186fb4369cff3d06cd852e.)

Also these two cases, which are caused by an unidentified component
(outside curl) cranking up MSVC warnings in external headers to /W4
when ZLIB is deselected:
#14859 (comment)

mbedTLS 3.6.1:

C:\vcpkg\installed\x64-windows\include\psa\crypto_struct.h(254,13): error C2220: the following warning is treated as an error [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
C:\vcpkg\installed\x64-windows\include\psa\crypto_struct.h(254,13): warning C4200: nonstandard extension used: zero-sized array in struct/union [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')

Ref: https://github.com/curl/curl/actions/runs/10842694205/job/30107466989?pr=14859#step:10:29

nghttp3 1.5.0:

C:\vcpkg\installed\x64-windows\include\nghttp3\nghttp3.h(2678,1): error C2220: the following warning is treated as an error [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
  C:\vcpkg\installed\x64-windows\include\nghttp3\nghttp3.h(2678,1): warning C4324: 'nghttp3_pri': structure was padded due to alignment specifier [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')

Ref: https://github.com/curl/curl/actions/runs/10871875297/job/30166233862?pr=14859#step:10:28

---

TODO:

• swap C-level fix with CMake's SYSTEM keyword.
• swap -I to -isystem in Makefile.mk for mbedTLS.
• delete BearSSL and mbedTLS C-level hacks for this.
• prepend SYSTEM to all include_directories() arg list.
• swap -I to -isystem in Makefile.mk for the rest of dependencies.

[bagder]

I'm curious: when do you see these warnings? Why do we get those warnings for bearssl headers? If we -isystem them doesn't that fix this?

[vszakats]

It's on macOS with CommandLineTools and CMake. (mbedTLS also needed a similar hack earlier.)

Yes, -isystem also silences it.

autotools has a separate manual step for that: convert -I options to -isystem.

CMake has a SYSTEM keyword for this, but it's not used in any include_directories() or target_include_directories() calls, yet:

https://cmake.org/cmake/help/latest/command/include_directories.html
https://cmake.org/cmake/help/latest/command/target_include_directories.html
https://stackoverflow.com/questions/3371127/use-isystem-instead-of-i-with-cmake

It seems like a case where the CMake-level fix would be better. But then it would be nice to apply it universally for all dependency header dirs, as done in autotools?

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -596,7 +596,7 @@ if(CURL_USE_BEARSSL)
   set(_ssl_enabled ON)
   set(USE_BEARSSL ON)
   list(APPEND CURL_LIBS ${BEARSSL_LIBRARIES})
-  include_directories(${BEARSSL_INCLUDE_DIRS})
+  include_directories(SYSTEM ${BEARSSL_INCLUDE_DIRS})
 
   if(CURL_DEFAULT_SSL_BACKEND AND CURL_DEFAULT_SSL_BACKEND STREQUAL "bearssl")
     set(_valid_default_ssl_backend TRUE)

[bagder]

then it would be nice to apply it universally for all dependency header dirs, as done in autotools?

I think so. Because compiler warnings in dependencies' header files are always just a nuisance so we are better off ignoring problems for those.

[vszakats]

I'm thinking to apply this for all dependencies.

Is there a risk applying this to all include_directories() and -I options in lib/Makefile.mk?

[bagder]

Is there a risk applying this to all include_directories() and -I options in lib/Makefile.mk?

The risk is that you modify the few -I instances we use to point out include dirs when building curl itself.

[vszakats]

Is there a risk applying this to all include_directories() and -I options in lib/Makefile.mk?

The risk is that you modify the few -I instances we use to point out include dirs when building curl itself.

This seems well-separated in CMake. I can't see how it could happen. (Though reviews are welcome! :)

(Unless a dependency header dir coincides with one of curl's own source dirs, e.g. by installing one straight into the curl sandbox, or mis-detected as installed there. But that's messed up, and likely risks autotools the same way.)