workflow to compare configure vs cmake outputs #11964
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
vszakats
reviewed
Sep 27, 2023
vszakats
reviewed
Sep 27, 2023
bagder
force-pushed
the
bagder/cmp-config
branch
from
11950ca
to
cd20263
Compare
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Sep 27, 2023
With new option `CURL_DISABLE_SRP=ON` to force-disable it. Also: - fix detecting GnuTLS. - comment typos, whitespace. Ref: curl#11964 Closes #xxxxx
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Sep 27, 2023
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] |
bagder
force-pushed
the
bagder/cmp-config
branch
from
6e8c397
to
e314193
Compare
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Sep 27, 2023
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
vszakats
added a commit
that referenced
this pull request
Sep 28, 2023
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
bagder
added a commit
that referenced
this pull request
Sep 28, 2023
- for sys/uio.h - for fork - for connect Ref: #11964
bagder
added a commit
that referenced
this pull request
Sep 28, 2023
- 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.
bagder
added a commit
that referenced
this pull request
Sep 28, 2023
It is not used in any code anywhere. Ref: #11964
bagder
added a commit
that referenced
this pull request
Sep 28, 2023
And fix the HAVE_LONGLONG define Ref: #11964
bagder
added a commit
that referenced
this pull request
Sep 28, 2023
- 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
bagder
force-pushed
the
bagder/cmp-config
branch
from
cee33ea
to
ccc0515
Compare
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Sep 28, 2023
Move detection before the creation of detection results in `curl_config.h`. Ref: curl#11964 Closes #xxxxx
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Sep 28, 2023
Move detection before the creation of detection results in `curl_config.h`. Ref: curl#11964 Closes #xxxxx
vszakats
added a commit
that referenced
this pull request
Oct 4, 2023
vszakats
added a commit
that referenced
this pull request
Oct 4, 2023
- 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
18 tasks
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Oct 8, 2023
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
vszakats
added a commit
that referenced
this pull request
Oct 8, 2023
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
vszakats
added a commit
that referenced
this pull request
Oct 12, 2023
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
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
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: curl#11964 Closes curl#11967
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
And fix the HAVE_LONGLONG define Ref: curl#11964 Closes curl#11977
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
It is not used in any code anywhere. Ref: curl#11964 Closes curl#11975
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
- for sys/uio.h - for fork - for connect Ref: curl#11964 Closes curl#11973
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
- 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: curl#11964 Co-authored-by: Viktor Szakats Closes curl#11973
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
Move detection before the creation of detection results in `curl_config.h`. Ref: curl#11964 (effort to sync cmake detections with autotools) Closes curl#11978
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
Based on existing autotools logic. autotools checks for old versions of the allowlisted target OSes and disables this feature when seeing them. In CMake we assume we're running on newer systems and enable regardless of OS version. autotools always runs all 3 probes for non-fast-tracked systems and enables this feature if any one of them was successful. To save configuration time, CMake stops at the first successful check. OpenBSD is not fast-tracked and then gets blocklisted as a generic BSD system. I haven't double-checked if this is correct, but looks odd. Ref: curl#11964 (effort to sync cmake detections with autotools) Closes curl#11979
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
Based on existing autotools logic. Ref: curl#11964 (effort to sync cmake detections with autotools) Closes curl#11981
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
Ref: curl#11964 (effort to sync cmake detections with autotools) Closes curl#11996
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
autotools was using the same value as CMake, but with an ending slash. Delete the ending slash to match configurations. Ref: curl#11964 (effort to sync cmake detections with autotools) Closes curl#11997
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
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: curl#11964 (effort to sync cmake detections with autotools) Closes curl#12008
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
- 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: curl#11964 (effort to sync cmake detections with autotools) Closes curl#12006
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
Left there by accident after adding proper detection for this. Follow-up to 772f0d8 curl#12006 Ref: curl#11964 (effort to sync cmake detections with autotools) Closes curl#12015
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
Uses scripts/cmp-config.pl two compare two curl_config.h files, presumbly generated with configure and cmake. It displays the differences and filters out a lot of known lines we ignore. The script also shows the matches that were *not* used. Possibly subjects for removal. Closes curl#11964
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
Follow-up to 2e0fa50 curl#11964 Follow-up to c39585d curl#12000 Closes curl#12023
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
- 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: curl#11964 (effort to sync cmake detections with autotools) Closes curl#12024
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
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: curl#1610 curl#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 curl#12066). Ref: curl#11964 Reviewed-by: Daniel Stenberg Reviewed-by: Jay Satiro Closes curl#12065
zuoxiaofeng
pushed a commit
to zuoxiaofeng/curl
that referenced
this pull request
Nov 28, 2023
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 curl#11979 Ref: curl#12095 (closed in favour of this patch) Ref: curl#11964 (effort to sync cmake detections with autotools) Reported-by: Kartatz on Github Assisted-by: Kartatz on Github Fixes curl#12093 Closes curl#12094
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details pending to fix/address:
CURL_SA_FAMILY_Tis worked out bycurl_setup.hand not purely in cmake but should be fineHAVE_WRITABLE_ARGVdetection #11978HAVE_GETADDRINFO_THREADSAFE#11979HAVE_CLOCK_GETTIME_MONOTONIC_RAW#11981sys/wait.handnetinet/udp.h#11996CURL_CA_PATHvalue to CMake #11997HAVE_IOCTL_*detections #12008HAVE_LDAP_SSL,HAVE_LDAP_URL_PARSEon non-Windows #12006 cmake: delete oldHAVE_LDAP_URL_PARSElogic #12015