[pull] master from curl:master #314
Merged
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
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
- add hex and octal parsers to the Curl_str_* family - make curlx_strtoofft use these parsers - remove all use of strtol() and strtoul() in library code - generally use Curl_str_* more than strtoofft, for stricter parsing - supports 64-bit universally, instead of 'long' which differs in size between platforms Extended the unit test 1664 to verify hex and octal parsing. Closes #16336
Just make it a part of the thread_sync_data struct. Closes #16323
- Remove the workaround that disabled peer verification in DEBUGBUILDs when CA certs were provided. The workaround was part of a TODO that disabled verification in DEBUGBUILDs with a CAfile/path because apparently there's no way to set those options in msh3 and that caused some tests to fail. Instead the tests should fail and this problem should not be covered up. Ref: #16327 (comment) Closes #16342
TL;DR: Save 10 minutes of CI time for GHA/macos jobs using pre-fills and add pre-fill verification for Apple and Windows. Also restores Xcode job and saves 1.5-10 minutes configuring iOS jobs. Pre-filling feature detection results can bring down the CMake configure step to ~5 seconds on most GHA runners, ~10 seconds in slow envs like Cygwin/MSYS2. The potential savings per job are: - 5-40 (average 19) seconds on GHA/macos (33 jobs) - ~10 seconds on GHA for iOS GNU Makefile (1 job) - 1.5-10 minutes on GHA for iOS Xcode generator (1 job) - 10 seconds on GHA/linux with native Ubuntu (12 jobs) - 40 seconds for Cygwin/MSYS2 (2 jobs) - 5-10 seconds for virtualized BSDs, native CPU (3 jobs) - ~60 seconds for virtualized BSDs, emulated CPU (1 job) On native Windows pre-filling has been in place for a long time and saving 8 minutes (VS2019-VS2015) to 1.5-2 minutes (VS2022), 3 minutes (VS2022 UWP), and 30-60 seconds (MinGW), per CI job. The downside is that detection results need to be manually collected and filtered to those that universally apply to all platforms that they are enabled on. Another downside is that by using a cache, we're not running the actual detections, and thus won't catch regressions in them. It means we must make sure that the cache is solid and matches with actual detections results. An upside is that it gives a rough overview of which features are available on which platforms. Another upside is pre-filled values do work for feature detections skipped for cross-builds, e.g. `HAVE_WRITABLE_ARGV`. This PR adds a pre-fill cache that supports all Unixes (except OmniOS) used in CI, and makes it usable with an internal option. It also enables it for GHA/macos CI jobs, where the maximum savings are. And also for the two iOS [1] and two Cygwin/MSYS2 jobs. The latters don't have pre-fill checks and we can drop them if they turn into a hassle. Saving: - 10 minutes of CI time per GHA/macos workflow run. [2] - ~80 seconds per GHA/windows workflow run with Cygwin/MSYS2. (offsetting the cost of pre-fill verifications) - 1.5-10 minutes per GHA/non-native runs with iOS jobs. [3] You can enable pre-fill locally with `-D_CURL_PREFILL=ON`. It's experimental, and if you experience a problem, file a PR or an Issue. This PR also adds a pre-fill checker for macOS and MinGW/MSVC Windows GHA jobs to catch if the cache diverges from real detections. It also adds this logic to AppVeyor, but doesn't enable it due to the perf penalty of 2 minutes mininum. The pre-fill checker works by configuring out-of-tree with and without pre-fill, then diffing their `lib/curl_config.h` outputs. Exceptions are 3 detection results exposed indirectly [4], and missing to expose 2, of which one is the C89 header `stddef.h`. While we assume the C99 `stdint.h` available outside iOS. We can expose them in the future, if necessary. The pre-fill checks cost in total: - ~20 seconds for macOS - ~40 seconds for MinGW on GHA - ~80 seconds for MSVC on GHA (UWP would be 2x this) An extra time saving potential is caching type sizes. They are well-known, and seldom change, esp. in CI. GHA/Windows jobs spend 8-17 seconds per job on these ~12 feature checks. ~5s on Cygwin/MSYS2. Couple of seconds on other platforms. (This PR doesn't make this optimization.) Another opportunity is doing the same for autotools, which typically spends more time in the configuration step than cmake. [1] Xcode job restored as a follow-up to be5f202 #16302 [2] GHA/macos cmake configure times in seconds: Job | Bef. | After | Gain :----------------------------------------------- | ----: | ----: | ----: CM clang GnuTLS !ldap krb5 | 21.2 | 4.5 | 16.7 CM clang LibreSSL !ldap heimdal c-ares +examples | 13.3 | 3.9 | 9.4 CM clang OpenSSL +static libssh +examples | 20.0 | 4.6 | 15.4 CM clang OpenSSL IDN clang-tidy~ (w/chkprefill) | 15.7 | 18.6 | -2.9 CM clang OpenSSL gsasl rtmp AppleIDN | 25.0 | 4.7 | 20.3 CM clang OpenSSL torture !FTP | 15.3 | 4.5 | 10.8 CM clang OpenSSL torture FTP | 25.0 | 5.9 | 19.1 CM clang SecureTransport debug | 18.0 | 3.8 | 14.2 CM clang macos-13 SecureTransport | 45.8 | 12.4 | 33.4 CM clang macos-14 SecureTransport | 15.8 | 4.6 | 11.2 CM clang macos-15 SecureTransport | 26.8 | 6.1 | 20.7 CM clang mbedTLS openldap brotli zstd | 15.1 | 6.5 | 8.6 CM clang wolfSSL !ldap brotli zstd | 27.0 | 4.4 | 22.6 CM gcc-12 GnuTLS !ldap krb5 | 39.1 | 8.7 | 30.4 CM gcc-12 LibreSSL !ldap heimdal c-ares +examples| 23.8 | 7.2 | 16.6 CM gcc-12 OpenSSL +static libssh +examples | 20.7 | 8.5 | 12.2 CM gcc-12 OpenSSL gsasl rtmp AppleIDN | 23.1 | 10.1 | 13.0 CM gcc-12 SecureTransport debug | 21.1 | 4.8 | 16.3 CM gcc-12 mbedTLS openldap brotli zstd | 21.4 | 5.8 | 15.6 CM gcc-12 wolfSSL !ldap brotli zstd | 21.1 | 6.9 | 14.2 CM gcc-14 macos-13 SecureTransport | 61.9 | 18.7 | 43.2 CM gcc-14 macos-14 SecureTransport | 30.5 | 6.4 | 24.1 CM gcc-14 macos-15 SecureTransport | 32.7 | 8.4 | 24.3 CM llvm@15 GnuTLS !ldap krb5 | 21.1 | 7.5 | 13.6 CM llvm@15 LibreSSL !ldap heimdal c-ares +exampl~| 24.6 | 6.8 | 17.8 CM llvm@15 OpenSSL +static libssh +examples | 19.0 | 6.4 | 12.6 CM llvm@15 OpenSSL gsasl rtmp AppleIDN | 19.0 | 8.2 | 10.8 CM llvm@15 SecureTransport debug | 18.0 | 5.4 | 12.6 CM llvm@15 macos-13 SecureTransport | 66.2 | 25.7 | 40.5 CM llvm@15 macos-14 SecureTransport | 31.9 | 6.1 | 25.8 CM llvm@15 mbedTLS openldap brotli zstd | 19.5 | 8.9 | 10.6 CM llvm@15 wolfSSL !ldap brotli zstd | 24.3 | 5.9 | 18.4 CM llvm@18 macos-15 SecureTransport | 33.8 | 6.4 | 27.4 Total | 856.8 | 257.3 | 599.5 Before: https://github.com/curl/curl/actions/runs/13311042735/job/37173478424 After: https://github.com/curl/curl/actions/runs/13313927119/job/37183206426?pr=15841 [3] iOS: Before: https://github.com/curl/curl/actions/runs/13326401704?pr=15841 After: https://github.com/curl/curl/actions/runs/13332177764?pr=15841 [4] detection results exposed indirectly in `curl_config.h`: - `HAVE_FILE_OFFSET_BITS` via `_FILE_OFFSET_BITS` - `HAVE_GETHOSTBYNAME_R_*_REENTRANT` via `NEED_REENTRANT` - `HAVE_SOCKADDR_IN6_SIN6_ADDR` via `USE_IPV6` Closes #15841
…etection Allow overriding the `IMPORT_LIB_SUFFIX` default with an empty value. Also: - add a fatal error if the implib and static lib filename are identical. - clarify `IMPORT_LIB_SUFFIX` default value in the documentation. Reported-by: RubisetCie on Github Fixes #16324 Ref: 1199308 #11505 Closes #16332
Fix or silence compiler warnings happening in feature detections to reduce log noise. Warnings may also get promoted to errors in certain cases, causing missed detections. It reduces the number of warnings by 4500+ across the linux, linux-old, macos, non-native and windows GHA workflows (~142 jobs). Also move picky warning logic for MSVC/Borland to `CMake/PickyWarnings.cmake. To make them listed in the picky-warnings log output, and to also apply to feature detections to make them compile under the same conditions as source code. The hope is to help catching issues faster. It also improves code quality of feature tests. Fixed/silenced: ``` warning #177: variable "dummy" was declared but never referenced warning #177: variable "flag" was declared but never referenced warning #177: variable "res" was declared but never referenced warning #592: variable "s" is used before its value is set warning #1011: missing return statement at end of non-void function "main" warning #1786: function "SSL_CTX_set_srp_password" (declared at line 1888 of "/usr/include/openssl/ssl.h") was declared deprecated ("Since OpenSSL 3.0") warning #1786: function "SSL_CTX_set_srp_username" (declared at line 1887 of "/usr/include/openssl/ssl.h") was declared deprecated ("Since OpenSSL 3.0") warning #2332: a value of type "const char *" cannot be assigned to an entity of type "char *" (dropping qualifiers) warning: 'SSL_CTX_set_srp_password' is deprecated [-Wdeprecated-declarations] warning: 'SSL_CTX_set_srp_password' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations] warning: 'SSL_CTX_set_srp_username' is deprecated [-Wdeprecated-declarations] warning: 'SSL_CTX_set_srp_username' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations] warning: 'b' is used uninitialized [-Wuninitialized] warning: 'gethostname' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes] warning: Undefined or garbage value returned to caller [core.uninitialized.UndefReturn] warning: Value stored to 'i' is never read [deadcode.DeadStores] warning: assigning to 'char *' from 'const char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers] warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] warning: control reaches end of non-void function [-Wreturn-type] warning: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Wextra-semi-stmt] warning: excess elements in struct initializer warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] warning: macro "_FILE_OFFSET_BITS" is not used [-Wunused-macros] warning: macro "_REENTRANT" is not used [-Wunused-macros] warning: missing braces around initializer [-Wmissing-braces] warning: no previous extern declaration for non-static variable 'off_t_is_large' [-Wmissing-variable-declarations] warning: no previous prototype for 'check' [-Wmissing-prototypes] warning: no previous prototype for function 'check' [-Wmissing-prototypes] warning: null argument where non-null required (argument 2) [-Wnonnull] warning: passing 'const char[1]' to parameter of type 'char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers] warning: passing argument 2 of 'SSL_CTX_set_srp_password' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] warning: passing argument 2 of 'SSL_CTX_set_srp_username' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] warning: unused parameter 'c' [-Wunused-parameter] warning: unused parameter 'f' [-Wunused-parameter] warning: unused variable 'data' [-Wunused-variable] warning: unused variable 'dummy' [-Wunused-variable] warning: unused variable 'flag' [-Wunused-variable] warning: unused variable 'res' [-Wunused-variable] warning: unused variable 's' [-Wunused-variable] warning: variable 's' set but not used [-Wunused-but-set-variable] warning: variable 'ts' set but not used [-Wunused-but-set-variable] ``` Closes #16287
- replace `add_compile_options()`, `add_definitions()` with directory properties. To harmonize this across all scripts. The new commands are verbose, but describe better how they work. The syntax is also closer to setting target properties, helps grepping. - prefer `CMAKE_INSTALL_PREFIX` over `--prefix` (in tests, CI). - tidy up cmake invocations. - formatting. Closes #16238
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.1)
Can you help keep this open source service alive? 💖 Please sponsor : )