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
workflow to compare configure vs cmake outputs #11964
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
11950ca
to
cd20263
Compare
With new option `CURL_DISABLE_SRP=ON` to force-disable it. Also: - fix detecting GnuTLS. - comment typos, whitespace. Ref: curl#11964 Closes #xxxxx
With new option `CURL_DISABLE_SRP=ON` to force-disable it. To match existing similar option and detection logic in autotools. Also: - fix detecting GnuTLS. - comment typos, whitespace. Ref: curl#11964 Closes #xxxxx
Attempt to address TLS-SRP: #11967 [MERGED] |
6e8c397
to
e314193
Compare
With new option `CURL_DISABLE_SRP=ON` to force-disable it. To match existing similar option and detection logic in autotools. Also: - fix detecting GnuTLS. - comment typos, whitespace. Ref: curl#11964 Closes #xxxxx
With new option `CURL_DISABLE_SRP=ON` to force-disable it. To match existing option and detection logic in autotools. Also: - fix detecting GnuTLS. We assume `nettle` as a GnuTLS dependency. - add CMake GnuTLS CI job. - bump AppVeyor CMake OpenSSL MSVC job to OpenSSL 1.1.1 (from 1.0.2) TLS-SRP fails to detect with 1.0.2 due to an OpenSSL header bug. - fix compiler warning when building with GnuTLS and disabled TLS-SRP. - fix comment typos, whitespace. Ref: #11964 Closes #11967
- for sys/uio.h - for fork - for connect Ref: #11964
- check for arc4random. To make rand.c use it accordingly. - check for fcntl - fix fseek detection - add SIZEOF_CURL_SOCKET_T - fix USE_UNIX_SOCKETS - define HAVE_SNPRINTF to 1 - check for fnmatch - check for sched_yield - remove HAVE_GETPPID duplicate from curl_config.h - add HAVE_SENDMSG Ref: #11964
This comment was marked as resolved.
This comment was marked as resolved.
It is not used in any code anywhere. Ref: #11964
And fix the HAVE_LONGLONG define Ref: #11964
- check for arc4random. To make rand.c use it accordingly. - check for fcntl - fix fseek detection - add SIZEOF_CURL_SOCKET_T - fix USE_UNIX_SOCKETS - define HAVE_SNPRINTF to 1 - check for fnmatch - check for sched_yield - remove HAVE_GETPPID duplicate from curl_config.h - add HAVE_SENDMSG Ref: #11964 Co-authored-by: Viktor Szakats Closes #11973
cee33ea
to
ccc0515
Compare
Move detection before the creation of detection results in `curl_config.h`. Ref: curl#11964 Closes #xxxxx
Move detection before the creation of detection results in `curl_config.h`. Ref: curl#11964 Closes #xxxxx
This restores `CURL_CHECK_FUNC_IOCTL` detection. I deleted it in 4d73854 and c345665 (2022-08), because the `HAVE_IOCTL` result it generated was unused in the source. But, I did miss the fact that this had two dependent checks: `CURL_CHECK_FUNC_IOCTL_FIONBIO`, `CURL_CHECK_FUNC_IOCTL_SIOCGIFADDR` that we do actually need: `HAVE_IOCTL_FIONBIO`, `HAVE_IOCTL_SIOCGIFADDR`. Regression from 4d73854 Ref: #11964 (effort to sync cmake detections with autotools) Closes #12008
555506c
to
61d8e94
Compare
- set `HAVE_LDAP_URL_PARSE` if `ldap_url_parse` function exists. Before this patch we set it based it on the presence of `stricmp`, which correctly enabled it on e.g. Windows, but was inaccurate for other platforms. - always set `HAVE_LDAP_SSL` if an LDAP backend is detected and LDAPS is not explicitly disabled. This mimics autotools behaviour. Previously we set it only for Windows LDAP. After this fix, LDAPS is correctly enabled in default macOS builds. - enable LDAP[S] for a CMake macOS CI job. Target OS X 10.9 (Mavericks) to avoid deprecation warnings for LDAP API. - always detect `HAVE_LDAP_SSL_H`, even with LDAPS explicitly disabled. This doesn't make much sense, but let's do it to sync behaviour with autotools. - fix benign typo in variable name. Ref: #11964 (effort to sync cmake detections with autotools) Closes #12006
This is currently sorted out in the header file so it does not actually need to be fixed. |
61d8e94
to
ba485c5
Compare
Pushed #12015 that fixes There are more problems though: autotools detects LDAP and LDAPS (via libs?), but NOT the LDAP headers, which it checks separately. CMake only checks for headers and that fails. I'm leaving this to somebody else, as the last weeks were exhausting and I'm starting to make mistakes. I'm also not familiar with LDAP and its history. UPDATE: I tested this locally and turns out I misread the autotools output above. autotools is not detecting LDAP libs, only saying that LDAP/LDAPS is enabled as a feature. Later it reports that LDAPs was not found. Also in local tests the LDAP results match on a vanilla Debian (bullseye). |
@vszakats I also think that detection of individual third party libs are less important than getting "the generic" stuff more aligned and I think we have taken a huge step in that direction now. |
ba485c5
to
c30dc49
Compare
Agreed, we can tackle the LDAP details later. With some (wildcard?) allowlisting, we're almost there. It'd even be useful to include this check and ignore the diff exit code for now. Suggestion: I think results would be easier to read if named |
8317df4
to
b783445
Compare
Presumbly generated with configure and cmake. Display the differences. Filter out a lot of known lines we ignore.
c7babc1
to
d54713f
Compare
@vszakats I kept the exit code now but have 20-something lines filtered off and now it runs green. I figure we can go live with this and see how it runs over time and adjust it as we go forward. You good with that? The script outputs the filter lines that are not actually filtered out, so we can figure out if we want to clean the filter going forward. |
LGTM. Thanks for this! |
- cmake: detect OpenLDAP based on function `ldap_init_fd`. autotools does this. autotools also publishes this detection result in `HAVE_LDAP_INIT_FD`. We don't mimic that with CMake as the source doesn't use this value. (it might need to be remove-listed in `scripts/cmp-config.pl` for future OpenLDAP test builds.) This also deletes existing self-declaration method via the CMake-specific `CURL_USE_OPENLDAP` configuration. - cmake: define `LDAP_DEPRECATED=1` for OpenLDAP. Like autotools does. This fixes a long list of these warnings: ``` /usr/local/opt/openldap/include/ldap.h:1049:5: warning: 'LDAP_DEPRECATED' is not defined, evaluates to 0 [-Wundef] ``` - cmake: delete LDAP TODO comment no longer relevant. Also: - autotools: replace domain name `dummy` with `0.0.0.0` in LDAP feature detection functions. Ref: #11964 (effort to sync cmake detections with autotools) Closes #12024
Syncing this up with CMake. Source code uses the built-in `OPENSSL_IS_AWSLC` and `OPENSSL_IS_BORINSSL` macros to detect BoringSSL and AWS-LC. No help is necessary from the build tools. autotools detects this anyway for display purposes. CMake detects this to decide whether to use the BoringSSL-specific crypto lib with ngtcp2. It detects AWS-LC, but doesn't use the detection results just yet. Ref: curl#11964 Closes #xxxxx
Syncing this up with CMake. Source code uses the built-in `OPENSSL_IS_AWSLC` and `OPENSSL_IS_BORINSSL` macros to detect BoringSSL and AWS-LC. No help is necessary from the build tools. The one use of `HAVE_BORINGSSL` in the source turned out to be no longer necessary for warning-free BoringSSL + Schannel builds. Ref: #1610 #2634 autotools detects this anyway for display purposes. CMake detects this to decide whether to use the BoringSSL-specific crypto lib with ngtcp2. It detects AWS-LC, but doesn't use the detection result just yet (planned in #12066). Ref: #11964 Reviewed-by: Daniel Stenberg Reviewed-by: Jay Satiro Closes #12065
Fix `HAVE_H_ERRNO_ASSIGNABLE` to not run, only compile its test snippet, aligning this with autotools. This fixes an error when doing cross-builds and also actually detects this feature. It affected systems not allowlisted into this, e.g. SerenityOS. We used this detection result to enable `HAVE_GETADDRINFO_THREADSAFE`. Follow-up to 04a3a37 #11979 Ref: #12095 (closed in favour of this patch) Ref: #11964 (effort to sync cmake detections with autotools) Reported-by: Kartatz on Github Assisted-by: Kartatz on Github Fixes #12093 Closes #12094
Details pending to fix/address:
CURL_SA_FAMILY_T
is worked out bycurl_setup.h
and not purely in cmake but should be fineHAVE_WRITABLE_ARGV
detection #11978HAVE_GETADDRINFO_THREADSAFE
#11979HAVE_CLOCK_GETTIME_MONOTONIC_RAW
#11981sys/wait.h
andnetinet/udp.h
#11996CURL_CA_PATH
value to CMake #11997HAVE_IOCTL_*
detections #12008HAVE_LDAP_SSL
,HAVE_LDAP_URL_PARSE
on non-Windows #12006 cmake: delete oldHAVE_LDAP_URL_PARSE
logic #12015