cmake: add support for "unity" builds #11095

vszakats · 2023-05-09T10:18:30Z

Aka "jumbo" or "amalgamation" builds. It means to compile all sources
per target as a single C source. This is experimental.

You can enable it by passing -DCMAKE_UNITY_BUILD=ON to cmake.
It requires CMake 3.16 or newer.

It makes builds (much) faster, allows for better optimizations and tends
to promote less ambiguous code.

Also add a new AppVeyor CI job and convert an existing one to use
"unity" mode (one MSVC, one MinGW), and enable it for one macOS CI job.

Fix related issues:

add missing include guard to easy_lock.h.
rename static variables and functions (and a macro) with names reused
across sources, or shadowed by local variables.
add an #undef after use.
add a missing #undef before use.
move internal definitions from ftp.h to ftp.c.
curl_memory.h fixes to make it work when included repeatedly.
stop building/linking curlx bits twice for a static-mode curl tool.
These caused doubly defined symbols in unity builds.
silence missing extern declarations compiler warning for _CRT_glob.
fix extern declarations for tool_freq and tool_isVistaOrGreater.
fix colliding static symbols in debug mode: debugtime() and
statename.
rename ssl_backend_data structure to unique names for each
TLS-backend, along with the ssl_connect_data struct member
referencing them. This required adding casts for each access.
add workaround for missing [P]UNICODE_STRING types in certain Windows
builds when compiling lib/ldap.c. To support "unity" builds, we had
to enable SCHANNEL_USE_BLACKLISTS for Schannel (a Windows
schannel.h option) globally. This caused an indirect inclusion of
Windows schannel.h from ldap.c via winldap.h to have it enabled
as well. This requires [P]UNICODE_STRING types, which is apperantly
not defined automatically (as seen with both MSVS and mingw-w64).
This patch includes <subauth.h> to fix it.
Ref: https://github.com/curl/curl/runs/13987772013
Ref: https://dev.azure.com/daniel0244/curl/_build/results?buildId=15827&view=logs&jobId=2c9f582d-e278-56b6-4354-f38a4d851906&j=2c9f582d-e278-56b6-4354-f38a4d851906&t=90509b00-34fa-5a81-35d7-5ed9569d331c
tweak unity builds to compile lib/memdebug.c separately in memory
trace builds to avoid PP confusion.
force-disable unity for test programs.
do not compile and link libcurl sources to libtests twice when libcurl
is built in static mode.

Ref: libssh2/libssh2#1034
Ref: https://cmake.org/cmake/help/latest/prop_tgt/UNITY_BUILD.html
Ref: https://en.wikipedia.org/wiki/Unity_build

Closes #11095

vszakats · 2023-05-09T22:42:56Z

Difficulties with H2, H2-proxy and H1-proxy name collisions, and CF_CTX_CALL_DATA vs. CF_DATA_CURRENT in particular.

The bigger hurdle is MultiSSL, especially ssl_backend_data, which is a name used for both type and variable and some other things I've yet to understand.

("unity" builds require unique symbol names across sources, including statics, types, and macros.)

jay · 2023-05-10T06:21:31Z

Is this worth the added maintenance and couldn't it introduce bugs? If we redefine a macro in one unit we don't then expect it to be redefined in another. Same thing for includes maybe one include is preferred over another in one unit and maybe the opposite in another unit. Unity might be faster for total rebuilds or clean builds but for every day development the compiler can usually just recompile affected units.

vszakats · 2023-05-10T07:47:53Z

I think the practice of using unique identifiers across the whole project has its own benefits and generally improves code quality. This applies to curl as well for the necessary updates so far. As for is it worth it, like everything, this has also opponents. I'm personally in favour. curl is built more often as a whole, than incrementally. The optimization benefits can also be considerable, though I'm not yet at the point to make measurements. For sure it makes full builds almost instantaneous (except the still lenghty configuration phase.)

Regarding bugs introduced: CI tests will tell us more once I got around adding one. But first this needs to work in more build combos than a default TLS one.

per target as a single C source. Can be enabled by passing `-DCMAKE_UNITY_BUILD=ON` to cmake. It makes builds (much) faster and allows for better optimization. Fix related issues: - add missing include guard to `easy_lock.h`. - rename static variables and functions (and a macro) with names reused across sources, or shadowed by local variables. - add an `#undef` after use. - add a missing `#undef` before use. - move internal definitions from `ftp.h` to `ftp.c`. Ref: libssh2/libssh2#1034 Ref: https://cmake.org/cmake/help/latest/prop_tgt/UNITY_BUILD.html Ref: https://en.wikipedia.org/wiki/Unity_build Closes curl#11095

MarcelRaad · 2023-05-10T16:47:41Z

FWIW, looking at the first commit, I like the fixes independently of the question if we want to maintain this option or not. Maybe extract them to a separate PR, or at least commit, once they're done.

I find some of the changes in the second commit harder to understand when mixed with other things, like the change from struct ssl_backend_data *backend to void *backend. Maybe even make multiple commits for them to explain them in the commit message and to make them easier to review.

vszakats · 2023-05-10T19:38:16Z

@MarcelRaad: The only "unity"-specific change is the single new line in CMakeLists.txt. Even that is optional, and only used when unity mode is explicitly enabled with cmake. Rest of the changes is all about avoiding conflicts between source files, and managing PP tricks. (There is probably further opportunities to make names more consistent after these changes in the H1-H2-proxy sources. Changes here so far are limited to what was strictly necessary.)

ssl_backend_data is the name of the struct used in each TLS backend module for the TLS context. These need to be renamed to be unique otherwise they conflict. After renaming them (as done already for OpenSSL and Schannel), struct ssl_backend_data *backend no longer finds this struct under this shared name. I changed that to a void * and added casts for accesses. It's possible there is a superior/simpler/shorter solution. On the first round my goal is to make it build at all. I'm still learning how all these bits are fitting together (esp. in MultiSSL mode).

The next riddle is [UPDATE: might be the case of selective declarations inside warnless.h, depending on BUILDING_WARNLESS_C)]:

lib/warnless.c:370:1: warning: all paths through this function will call itself [-Winfinite-recursion]
{
^
lib/warnless.c:375:1: warning: all paths through this function will call itself [-Winfinite-recursion]
{
^
2 warnings generated.

eli-schwartz · 2023-05-14T20:37:23Z

The optimization benefits can also be considerable, though I'm not yet at the point to make measurements.

This is usually not meaningful compared to simply using LTO.

bagder · 2023-05-16T06:40:18Z

This is usually not meaningful compared to simply using LTO.

I suppose this PR can be used to test that exact claim...

per target as a single C source. Can be enabled by passing `-DCMAKE_UNITY_BUILD=ON` to cmake. It makes builds (much) faster and allows for better optimization. Fix related issues: - add missing include guard to `easy_lock.h`. - rename static variables and functions (and a macro) with names reused across sources, or shadowed by local variables. - add an `#undef` after use. - add a missing `#undef` before use. - move internal definitions from `ftp.h` to `ftp.c`. Ref: libssh2/libssh2#1034 Ref: https://cmake.org/cmake/help/latest/prop_tgt/UNITY_BUILD.html Ref: https://en.wikipedia.org/wiki/Unity_build Closes curl#11095

vszakats · 2023-05-19T02:43:15Z

LTO: IME it makes builds slower and also fragile, especially with static builds (where any dependency issue will also be hit).

Remaining issues:

write():

/my/curl/lib/vquic/curl_ngtcp2.c:292:43: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long long') to 'unsigned int' [-Wshorten-64-to-32]
    ssize_t rc = write(ctx->qlogfd, data, datalen);
                 ~~~~~                    ^~~~~~~
1 warning generated.

A possibly odd regression when not using "unity" mode?: https://cirrus-ci.com/task/4638440571011072?logs=compile#L321 [FIXED with an hack. Maybe there is a more elegant one.]

Fixed two unrelated Windows issues with missing/misplaced externs.

Also had to add a tweak to CMake, to avoid compiling and linking CURLX sources twice when building a static curl tool.

per target as a single C source. Can be enabled by passing `-DCMAKE_UNITY_BUILD=ON` to cmake. It makes builds (much) faster and allows for better optimization. Fix related issues: - add missing include guard to `easy_lock.h`. - rename static variables and functions (and a macro) with names reused across sources, or shadowed by local variables. - add an `#undef` after use. - add a missing `#undef` before use. - move internal definitions from `ftp.h` to `ftp.c`. Ref: libssh2/libssh2#1034 Ref: https://cmake.org/cmake/help/latest/prop_tgt/UNITY_BUILD.html Ref: https://en.wikipedia.org/wiki/Unity_build Closes curl#11095

Sources did not use it. Autotools used it when checking for the `winldap` library, which is redundant. With CMake, detection was broken: ``` Run Build Command(s):/usr/local/Cellar/cmake/3.26.3/bin/cmake -E env VERBOSE=1 /usr/bin/make -f Makefile cmTC_2d8fe/fast && /Library/Developer/CommandLineTools/usr/bin/make -f CMakeFiles/cmTC_2d8fe.dir/build.make CMakeFiles/cmTC_2d8fe.dir/build Building C object CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj /usr/local/opt/llvm/bin/clang --target=x86_64-w64-mingw32 --sysroot=/usr/local/opt/mingw-w64/toolchain-x86_64 -D_WINSOCKAPI_="" -I/my/quictls/x64-ucrt/usr/include -I/my/zlib/x64-ucrt/usr/include -I/my/brotli/x64-ucrt/usr/include -Wno-unused-command-line-argument -D_UCRT -DCURL_HIDDEN_SYMBOLS -DHAVE_SSL_SET0_WBIO -DHAS_ALPN -DNGHTTP2_STATICLIB -DNGHTTP3_STATICLIB -DNGTCP2_STATICLIB -DUSE_MANUAL=1 -fuse-ld=lld -Wl,-s -static-libgcc -lucrt -Wextra -Wall -pedantic -Wbad-function-cast -Wconversion -Winline -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-long-long -Wno-multichar -Wpointer-arith -Wshadow -Wsign-compare -Wundef -Wunused -Wwrite-strings -Wcast-align -Wdeclaration-after-statement -Wempty-body -Wendif-labels -Wfloat-equal -Wignored-qualifiers -Wno-format-nonliteral -Wno-sign-conversion -Wno-system-headers -Wstrict-prototypes -Wtype-limits -Wvla -Wshift-sign-overflow -Wshorten-64-to-32 -Wdouble-promotion -Wenum-conversion -Wunused-const-variable -Wcomma -Wmissing-variable-declarations -Wassign-enum -Wextra-semi-stmt -MD -MT CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj -MF CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj.d -o CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj -c /my/curl/bld-cmake-llvm-x64-shared/CMakeFiles/CMakeScratch/TryCompile-3JP6dR/HAVE_WINLDAP_H.c In file included from /my/curl/bld-cmake-llvm-x64-shared/CMakeFiles/CMakeScratch/TryCompile-3JP6dR/HAVE_WINLDAP_H.c:2: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/winldap.h:17: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/schnlsp.h:9: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/schannel.h:10: /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/wincrypt.h:5041:254: error: unknown type name 'PSYSTEMTIME' WINIMPM PCCERT_CONTEXT WINAPI CertCreateSelfSignCertificate (HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey, PCERT_NAME_BLOB pSubjectIssuerBlob, DWORD dwFlags, PCRYPT_KEY_PROV_INFO pKeyProvInfo, PCRYPT_ALGORITHM_IDENTIFIER pSignatureAlgorithm, PSYSTEMTIME pStartTime, PSYSTEMTIME pEndTime, PCERT_EXTENSIONS pExtensions); ^ /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/wincrypt.h:5041:278: error: unknown type name 'PSYSTEMTIME' WINIMPM PCCERT_CONTEXT WINAPI CertCreateSelfSignCertificate (HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey, PCERT_NAME_BLOB pSubjectIssuerBlob, DWORD dwFlags, PCRYPT_KEY_PROV_INFO pKeyProvInfo, PCRYPT_ALGORITHM_IDENTIFIER pSignatureAlgorithm, PSYSTEMTIME pStartTime, PSYSTEMTIME pEndTime, PCERT_EXTENSIONS pExtensions); ^ 2 errors generated. make[1]: *** [CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj] Error 1 make: *** [cmTC_2d8fe/fast] Error 2 exitCode: 2 ``` Cherry-picked from curl#11095 Closes #xxxxx

Sources did not use it. Autotools used it when checking for the `winldap` library, which is redundant. With CMake, detection was broken: ``` Run Build Command(s):/usr/local/Cellar/cmake/3.26.3/bin/cmake -E env VERBOSE=1 /usr/bin/make -f Makefile cmTC_2d8fe/fast && /Library/Developer/CommandLineTools/usr/bin/make -f CMakeFiles/cmTC_2d8fe.dir/build.make CMakeFiles/cmTC_2d8fe.dir/build Building C object CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj /usr/local/opt/llvm/bin/clang --target=x86_64-w64-mingw32 --sysroot=/usr/local/opt/mingw-w64/toolchain-x86_64 -D_WINSOCKAPI_="" -I/my/quictls/x64-ucrt/usr/include -I/my/zlib/x64-ucrt/usr/include -I/my/brotli/x64-ucrt/usr/include -Wno-unused-command-line-argument -D_UCRT -DCURL_HIDDEN_SYMBOLS -DHAVE_SSL_SET0_WBIO -DHAS_ALPN -DNGHTTP2_STATICLIB -DNGHTTP3_STATICLIB -DNGTCP2_STATICLIB -DUSE_MANUAL=1 -fuse-ld=lld -Wl,-s -static-libgcc -lucrt -Wextra -Wall -pedantic -Wbad-function-cast -Wconversion -Winline -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-long-long -Wno-multichar -Wpointer-arith -Wshadow -Wsign-compare -Wundef -Wunused -Wwrite-strings -Wcast-align -Wdeclaration-after-statement -Wempty-body -Wendif-labels -Wfloat-equal -Wignored-qualifiers -Wno-format-nonliteral -Wno-sign-conversion -Wno-system-headers -Wstrict-prototypes -Wtype-limits -Wvla -Wshift-sign-overflow -Wshorten-64-to-32 -Wdouble-promotion -Wenum-conversion -Wunused-const-variable -Wcomma -Wmissing-variable-declarations -Wassign-enum -Wextra-semi-stmt -MD -MT CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj -MF CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj.d -o CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj -c /my/curl/bld-cmake-llvm-x64-shared/CMakeFiles/CMakeScratch/TryCompile-3JP6dR/HAVE_WINLDAP_H.c In file included from /my/curl/bld-cmake-llvm-x64-shared/CMakeFiles/CMakeScratch/TryCompile-3JP6dR/HAVE_WINLDAP_H.c:2: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/winldap.h:17: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/schnlsp.h:9: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/schannel.h:10: /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/wincrypt.h:5041:254: error: unknown type name 'PSYSTEMTIME' WINIMPM PCCERT_CONTEXT WINAPI CertCreateSelfSignCertificate (HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey, PCERT_NAME_BLOB pSubjectIssuerBlob, DWORD dwFlags, PCRYPT_KEY_PROV_INFO pKeyProvInfo, PCRYPT_ALGORITHM_IDENTIFIER pSignatureAlgorithm, PSYSTEMTIME pStartTime, PSYSTEMTIME pEndTime, PCERT_EXTENSIONS pExtensions); ^ /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/wincrypt.h:5041:278: error: unknown type name 'PSYSTEMTIME' WINIMPM PCCERT_CONTEXT WINAPI CertCreateSelfSignCertificate (HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey, PCERT_NAME_BLOB pSubjectIssuerBlob, DWORD dwFlags, PCRYPT_KEY_PROV_INFO pKeyProvInfo, PCRYPT_ALGORITHM_IDENTIFIER pSignatureAlgorithm, PSYSTEMTIME pStartTime, PSYSTEMTIME pEndTime, PCERT_EXTENSIONS pExtensions); ^ 2 errors generated. make[1]: *** [CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj] Error 1 make: *** [cmTC_2d8fe/fast] Error 2 exitCode: 2 ``` Cherry-picked from curl#11095 88e4a21 Closes #xxxxx

Move memdebug.c into a separate unity group, forcing it to compile separately and without going into recursive PP rules when debug mode is enabled.

A unity fallout happens with openssl + schannel builds. disabling openssl fixes it: ``` C:/projects/curl/lib/vtls/schannel.c: In function 'schannel_connect_step1': C:/projects/curl/lib/vtls/schannel.c:1217:7: error: 'SecApplicationProtocolNegotiationExt_ALPN' undeclared (first use in this function) SecApplicationProtocolNegotiationExt_ALPN; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C:/projects/curl/lib/vtls/schannel.c:1217:7: note: each undeclared identifier is reported only once for each function it appears in C:/projects/curl/lib/vtls/schannel.c:1240:27: error: 'SECBUFFER_APPLICATION_PROTOCOLS' undeclared (first use in this function); did you mean 'SECBUFFER_NEGOTIATION_INFO'? InitSecBuffer(&inbuf, SECBUFFER_APPLICATION_PROTOCOLS, alpn_buffer, cur); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ SECBUFFER_NEGOTIATION_INFO C:/projects/curl/lib/vtls/schannel.c: In function 'schannel_connect_step3': C:/projects/curl/lib/vtls/schannel.c:1698:3: error: unknown type name 'SecPkgContext_ApplicationProtocol'; did you mean 'SecPkgContext_ConnectionInfo'? SecPkgContext_ApplicationProtocol alpn_result; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ SecPkgContext_ConnectionInfo C:/projects/curl/lib/vtls/schannel.c:1730:40: error: 'SECPKG_ATTR_APPLICATION_PROTOCOL' undeclared (first use in this function); did you mean 'SECPKG_ATTR_SUPPORTED_PROTOCOLS'? SECPKG_ATTR_APPLICATION_PROTOCOL, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ SECPKG_ATTR_SUPPORTED_PROTOCOLS C:/projects/curl/lib/vtls/schannel.c:1738:19: error: request for member 'ProtoNegoStatus' in something not a structure or union if(alpn_result.ProtoNegoStatus == ^ C:/projects/curl/lib/vtls/schannel.c:1739:8: error: 'SecApplicationProtocolNegotiationStatus_Success' undeclared (first use in this function) SecApplicationProtocolNegotiationStatus_Success) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C:/projects/curl/lib/vtls/schannel.c:1742:53: error: request for member 'ProtocolId' in something not a structure or union Curl_alpn_set_negotiated(cf, data, alpn_result.ProtocolId, ^ C:/projects/curl/lib/vtls/schannel.c:1743:43: error: request for member 'ProtocolIdSize' in something not a structure or union alpn_result.ProtocolIdSize); ^ make[2]: *** [lib/CMakeFiles/libcurl.dir/build.make:64: lib/CMakeFiles/libcurl.dir/Unity/unity_0.c.obj] Error 1 make[2]: Target 'lib/CMakeFiles/libcurl.dir/build' not remade because of errors ``` Ref: https://ci.appveyor.com/project/curlorg/curl/builds/47230972/job/51wfesgnfuauwl8q#L250 Bad attempts: https://ci.appveyor.com/project/curlorg/curl/builds/47230369 https://ci.appveyor.com/project/curlorg/curl/builds/47230369/job/cpr9b67a2al293qj `tests\unit\unit3200.exe` -> hung https://ci.appveyor.com/project/curlorg/curl/builds/47230369/job/6usv7td7sfb6f4bm `could not get curl version! at C:/projects/curl/tests/runtests.pl line 2360.` Also: - appveyor.yml: add 'dll' to libcurl artifact name

vszakats · 2023-06-06T17:03:35Z

One AppVeyor CI job converted to unity, one other added. Both Windows,
the notorious problem-platform, so it will be enough for a start.

Caveats:

Running tests doesn't work in all cases. This will be an exercise for the future.
One random build combo (the new AppVeyor CI one with OpenSSL enabled
alongside Schannel) produced yet another bizarre Windows Schannel PP
fallout. There is probably more of these, especially on Windows, which is the
hot mess it usually is. This can be addressed as they come along in real life
or as interest / free time arises.

At this point I deem this mergable.

eli-schwartz · 2023-06-07T13:23:12Z

I remain generally skeptical of the values of unity builds, and maintain that if you want better optimization you should be building with LTO.

LTO: IME it makes builds slower and also fragile, especially with static builds (where any dependency issue will also be hit).

So LTO is the fragile thing, despite that this PR introduces "bizarre" fallout and generally has issues running tests? It seems less than ideal to be introducing a compile mode that isn't being thoroughly tested.

I'm curious what the apparent fragility of LTO is. I mean, yes, LTO tends to produce issues where existing UB gets miscompiled with LTO and has benevolent effects without LTO, but that's really no different in concept from building with higher -O settings, and the solution is to always fix the codebase in question to stop doing UB.

I'm also not sure what static builds or dependencies has to do with anything. No one suggested doing a unity build that gathers all dependencies into a single translation unit together with the curl sources -- by the same extension no one is under any obligation to use LTO across both curl and its static dependencies. Although the good news is that unlike unity builds (which will definitely not work with completely external project sources!) LTO is capable of doing that kind of optimization.

Be that as it may, I can happily report that curl builds fine with LTO on my machine (and passes all tests). And it doesn't even need a few thousand lines of changes that incur an ongoing maintenance cost which isn't revealed in the default build mode. :)

thesamesam · 2023-06-07T13:31:14Z

In addition to what you've said (and I wholeheartedly agree), unity builds in this form introduce variance across the build systems. And even with all of this, tests had to be skipped and weren't solved yet.

Meanwhile, many Linux distributions are building with LTO by default - including building curl, with no complaints.

vszakats · 2023-06-07T13:53:39Z

This PR doesn't disable the option to build curl with LTO. Nor is this against LTO, or in fact much if anything to do with LTO. It adds unity as a build option for those who might find it useful. It also unearthed some code smells.

[ On Windows (and MinGW), LTO makes build times considerably higher. When combined with static linking you're facing all the LTO bugs of all dependencies, which is sometimes more than one is willing (or able) to fix. LTO objects were (likely still are) compiler version specific with GCC, and had to deal with the fallout after each compiler update. It's a useful feature, but not necessarily for every use-case. ]

Bizarre fallouts are pretty common on Windows, unity builds and LTO aside.

eli-schwartz · 2023-06-07T14:38:44Z

I'm still confused by this "all the LTO bugs of all dependencies" thing, unless you just mean "C projects always have UB"?

I also don't think you really have to worry about compiler versions. Distros tend to support only a single version, and rebuild their entire stack as needed to ensure their packages are compatible (this is really a static libraries concern though), and outside of distros, people tend to stick to a single compiler version in their project anyway -- because of concerns about C++ compatibility for example.

...

While it's true that LTO can result in increased build times, I think it is generally meritorious. Someone who is seeking ways to increase runtime performance I would assume considers that to be a valuable priority, well worth spending a bit more time compiling releases. For optimizing the development cycle, one would simply turn off LTO because speed of testing loses out to speed of iterative testing... and for the same reason, one would turn off unity during development, because unity builds have a significant negative effect on your incremental build times. Even with a relatively low chunk size, the more you iterate the more you pay the cost for compiling multiple times the amount of code you modified.

So honestly, the motivation for adding support seems weak. And yes, I do understand that this doesn't prevent using LTO anyway. That was never my concern and if I accidentally gave that impression I apologize (although I'm not actually sure how I would have given that impression).

Again: my concerns are that

it produces cases that are known to be "bizarre", and claiming that it's common to have bizarre issues on Windows seems a poor excuse for merging what is apparently unfinished work,
it's acknowledged that the result of this is builds that can't be tested, and is thus apparently unfinished work, and
unity builds in general mean that one is committing to an ongoing maintenance burden of keeping it working, adding additional testing matrixes and then dealing with the fallout of people who successfully tested their code contributions the normal way and then had it fail in CI with obscure errors, because it's now necessary to avoid perfectly reasonable and idiomatic, correct code, because it misbehaves when scoping rules are violated and different translation units see each others' headers and macros and trample all over private local symbols

Maybe I'd feel less weird about it if it was obvious to me that the curl maintainers in general feel it's worth the maintenance burden. That concern was raised above -- it was the first response -- and then not brought up again.

Maybe I'd feel less weird about it if it didn't include big disclaimers about how the PR is incomplete and on the single biggest platform, it has "bizarre" issues -- and those issues appear to be exactly an example of my concern that unity is in fact fragile, flaky and buggy. It gives the impression that curl is okay merging and officially supporting buggy configurations, including configurations that build with a passing return code but are suspect of incorrect runtime behavior.

And this PR is a lot of work to do for something that has such a glaring issue.

vszakats · 2023-06-07T15:09:03Z

I believe one main difference is that I tried making LTO work on Windows (in the distant past), where things were/are much bumpier than in a Linux env, whereas you seem to have done the same on Linux, where it's most probably just a linker switch away.

This PR is already done, out of my free time. As I said bizarre issues are business as usual on Windows, and all of them here were Windows related. You're welcome to pick up any of the issues and reflect to that, but skimming through weeks of work, picking out 2-3 uncomfortable words in comments, don't seem to be a productive take on this.

Anyhow, I suggest opening a discussion of the merits of LTO. I'm not personally interested in it, and this PR has nothing to do with it.

Sources did not use it. Autotools used it when checking for the `winldap` library, which is redundant. With CMake, detection was broken: ``` Run Build Command(s):/usr/local/Cellar/cmake/3.26.3/bin/cmake -E env VERBOSE=1 /usr/bin/make -f Makefile cmTC_2d8fe/fast && /Library/Developer/CommandLineTools/usr/bin/make -f CMakeFiles/cmTC_2d8fe.dir/build.make CMakeFiles/cmTC_2d8fe.dir/build Building C object CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj /usr/local/opt/llvm/bin/clang --target=x86_64-w64-mingw32 --sysroot=/usr/local/opt/mingw-w64/toolchain-x86_64 -D_WINSOCKAPI_="" -I/my/quictls/x64-ucrt/usr/include -I/my/zlib/x64-ucrt/usr/include -I/my/brotli/x64-ucrt/usr/include -Wno-unused-command-line-argument -D_UCRT -DCURL_HIDDEN_SYMBOLS -DHAVE_SSL_SET0_WBIO -DHAS_ALPN -DNGHTTP2_STATICLIB -DNGHTTP3_STATICLIB -DNGTCP2_STATICLIB -DUSE_MANUAL=1 -fuse-ld=lld -Wl,-s -static-libgcc -lucrt -Wextra -Wall -pedantic -Wbad-function-cast -Wconversion -Winline -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-long-long -Wno-multichar -Wpointer-arith -Wshadow -Wsign-compare -Wundef -Wunused -Wwrite-strings -Wcast-align -Wdeclaration-after-statement -Wempty-body -Wendif-labels -Wfloat-equal -Wignored-qualifiers -Wno-format-nonliteral -Wno-sign-conversion -Wno-system-headers -Wstrict-prototypes -Wtype-limits -Wvla -Wshift-sign-overflow -Wshorten-64-to-32 -Wdouble-promotion -Wenum-conversion -Wunused-const-variable -Wcomma -Wmissing-variable-declarations -Wassign-enum -Wextra-semi-stmt -MD -MT CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj -MF CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj.d -o CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj -c /my/curl/bld-cmake-llvm-x64-shared/CMakeFiles/CMakeScratch/TryCompile-3JP6dR/HAVE_WINLDAP_H.c In file included from /my/curl/bld-cmake-llvm-x64-shared/CMakeFiles/CMakeScratch/TryCompile-3JP6dR/HAVE_WINLDAP_H.c:2: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/winldap.h:17: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/schnlsp.h:9: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/schannel.h:10: /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/wincrypt.h:5041:254: error: unknown type name 'PSYSTEMTIME' WINIMPM PCCERT_CONTEXT WINAPI CertCreateSelfSignCertificate (HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey, PCERT_NAME_BLOB pSubjectIssuerBlob, DWORD dwFlags, PCRYPT_KEY_PROV_INFO pKeyProvInfo, PCRYPT_ALGORITHM_IDENTIFIER pSignatureAlgorithm, PSYSTEMTIME pStartTime, PSYSTEMTIME pEndTime, PCERT_EXTENSIONS pExtensions); ^ /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/wincrypt.h:5041:278: error: unknown type name 'PSYSTEMTIME' WINIMPM PCCERT_CONTEXT WINAPI CertCreateSelfSignCertificate (HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey, PCERT_NAME_BLOB pSubjectIssuerBlob, DWORD dwFlags, PCRYPT_KEY_PROV_INFO pKeyProvInfo, PCRYPT_ALGORITHM_IDENTIFIER pSignatureAlgorithm, PSYSTEMTIME pStartTime, PSYSTEMTIME pEndTime, PCERT_EXTENSIONS pExtensions); ^ 2 errors generated. make[1]: *** [CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj] Error 1 make: *** [cmTC_2d8fe/fast] Error 2 exitCode: 2 ``` Cherry-picked from curl#11095 88e4a21 Reviewed-by: Daniel Stenberg Closes curl#11245

Aka "jumbo" or "amalgamation" builds. It means to compile all sources per target as a single C source. This is experimental. You can enable it by passing `-DCMAKE_UNITY_BUILD=ON` to cmake. It requires CMake 3.16 or newer. It makes builds (much) faster, allows for better optimizations and tends to promote less ambiguous code. Also add a new AppVeyor CI job and convert an existing one to use "unity" mode (one MSVC, one MinGW), and enable it for one macOS CI job. Fix related issues: - add missing include guard to `easy_lock.h`. - rename static variables and functions (and a macro) with names reused across sources, or shadowed by local variables. - add an `#undef` after use. - add a missing `#undef` before use. - move internal definitions from `ftp.h` to `ftp.c`. - `curl_memory.h` fixes to make it work when included repeatedly. - stop building/linking curlx bits twice for a static-mode curl tool. These caused doubly defined symbols in unity builds. - silence missing extern declarations compiler warning for ` _CRT_glob`. - fix extern declarations for `tool_freq` and `tool_isVistaOrGreater`. - fix colliding static symbols in debug mode: `debugtime()` and `statename`. - rename `ssl_backend_data` structure to unique names for each TLS-backend, along with the `ssl_connect_data` struct member referencing them. This required adding casts for each access. - add workaround for missing `[P]UNICODE_STRING` types in certain Windows builds when compiling `lib/ldap.c`. To support "unity" builds, we had to enable `SCHANNEL_USE_BLACKLISTS` for Schannel (a Windows `schannel.h` option) _globally_. This caused an indirect inclusion of Windows `schannel.h` from `ldap.c` via `winldap.h` to have it enabled as well. This requires `[P]UNICODE_STRING` types, which is apperantly not defined automatically (as seen with both MSVS and mingw-w64). This patch includes `<subauth.h>` to fix it. Ref: https://github.com/curl/curl/runs/13987772013 Ref: https://dev.azure.com/daniel0244/curl/_build/results?buildId=15827&view=logs&jobId=2c9f582d-e278-56b6-4354-f38a4d851906&j=2c9f582d-e278-56b6-4354-f38a4d851906&t=90509b00-34fa-5a81-35d7-5ed9569d331c - tweak unity builds to compile `lib/memdebug.c` separately in memory trace builds to avoid PP confusion. - force-disable unity for test programs. - do not compile and link libcurl sources to libtests _twice_ when libcurl is built in static mode. KNOWN ISSUES: - running tests with unity builds may fail in cases. - some build configurations/env may not compile in unity mode. E.g.: https://ci.appveyor.com/project/curlorg/curl/builds/47230972/job/51wfesgnfuauwl8q#L250 Ref: libssh2/libssh2#1034 Ref: https://cmake.org/cmake/help/latest/prop_tgt/UNITY_BUILD.html Ref: https://en.wikipedia.org/wiki/Unity_build Closes curl#11095

AndrewJDR · 2023-08-23T08:55:20Z

@vszakats Thanks for all the effort on this. I'm very sad that this is closed!

For me, what is truly exciting about a working unity / amalgamated libcurl is that it gets libcurl one step closer to this (from a twitter thread):

preconfigured and amalgamated source packages (e.g. "here's a single-source-libcurl preconfigured for Win32 and HTTP to drop right into your project")

@vszakats Do you think this sort of thing would be possible? Basically, if you're trying to ship a project that includes libcurl as source with minimal dependencies -- i.e. without requiring those adopting it to have cmake, etc), one could include a few pre-generated .c files, one for each platform that your project wants to support.

For example, someone supporting windows and linux might have these two files in in their project's repository:
deps/libcurl_win32_schannel.c
deps/libcurl_linux_mbedtls.c

Then, a simple build.bat and build.sh file could be used for the project, rather than burdening the developer with dealing with complex build systems like cmake. This would be a huge simplification for projects where libcurl is the only "complex" dependency that requires a build system.

* Obviously, for the deps/libcurl_linux_mbedtls.c example, mbedtls would need to be included in the repository as well, but mbedtls is actually fairly trivial to ship along with the repo without a build system. Plus that's outside the scope here, which is just getting libcurl to support unity builds.

vszakats · 2023-08-23T09:24:14Z

@AndrewJDR We always merge manually in curl, leaving PRs show up as "closed". I did merge this, so the feature is live. It's not as extensively tested by the CI as I'd want to though, and recently fixed (#11095) a regression in H2 builds.

What you say is technically possible after the efforts made in this PR, but curl for now leans on CMake to do the "amalgamation" part dynamically, as part of its build. Meaning we have no separate script that converts libcurl sources to a single .c file. CMake internally creates a wrapper .c file that #includes libcurl C sources, so it's not as easy to copy off an amalgamated .c from its build tree as-is either. But, certainly doable with some script foo. Haven't tried, but even a cat command might actually work.

AndrewJDR · 2023-08-23T09:27:34Z

@vszakats Wow that's great! I will definitely give it a try some time.

Sources did not use it. Autotools used it when checking for the `winldap` library, which is redundant. With CMake, detection was broken: ``` Run Build Command(s):/usr/local/Cellar/cmake/3.26.3/bin/cmake -E env VERBOSE=1 /usr/bin/make -f Makefile cmTC_2d8fe/fast && /Library/Developer/CommandLineTools/usr/bin/make -f CMakeFiles/cmTC_2d8fe.dir/build.make CMakeFiles/cmTC_2d8fe.dir/build Building C object CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj /usr/local/opt/llvm/bin/clang --target=x86_64-w64-mingw32 --sysroot=/usr/local/opt/mingw-w64/toolchain-x86_64 -D_WINSOCKAPI_="" -I/my/quictls/x64-ucrt/usr/include -I/my/zlib/x64-ucrt/usr/include -I/my/brotli/x64-ucrt/usr/include -Wno-unused-command-line-argument -D_UCRT -DCURL_HIDDEN_SYMBOLS -DHAVE_SSL_SET0_WBIO -DHAS_ALPN -DNGHTTP2_STATICLIB -DNGHTTP3_STATICLIB -DNGTCP2_STATICLIB -DUSE_MANUAL=1 -fuse-ld=lld -Wl,-s -static-libgcc -lucrt -Wextra -Wall -pedantic -Wbad-function-cast -Wconversion -Winline -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-long-long -Wno-multichar -Wpointer-arith -Wshadow -Wsign-compare -Wundef -Wunused -Wwrite-strings -Wcast-align -Wdeclaration-after-statement -Wempty-body -Wendif-labels -Wfloat-equal -Wignored-qualifiers -Wno-format-nonliteral -Wno-sign-conversion -Wno-system-headers -Wstrict-prototypes -Wtype-limits -Wvla -Wshift-sign-overflow -Wshorten-64-to-32 -Wdouble-promotion -Wenum-conversion -Wunused-const-variable -Wcomma -Wmissing-variable-declarations -Wassign-enum -Wextra-semi-stmt -MD -MT CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj -MF CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj.d -o CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj -c /my/curl/bld-cmake-llvm-x64-shared/CMakeFiles/CMakeScratch/TryCompile-3JP6dR/HAVE_WINLDAP_H.c In file included from /my/curl/bld-cmake-llvm-x64-shared/CMakeFiles/CMakeScratch/TryCompile-3JP6dR/HAVE_WINLDAP_H.c:2: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/winldap.h:17: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/schnlsp.h:9: In file included from /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/schannel.h:10: /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/wincrypt.h:5041:254: error: unknown type name 'PSYSTEMTIME' WINIMPM PCCERT_CONTEXT WINAPI CertCreateSelfSignCertificate (HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey, PCERT_NAME_BLOB pSubjectIssuerBlob, DWORD dwFlags, PCRYPT_KEY_PROV_INFO pKeyProvInfo, PCRYPT_ALGORITHM_IDENTIFIER pSignatureAlgorithm, PSYSTEMTIME pStartTime, PSYSTEMTIME pEndTime, PCERT_EXTENSIONS pExtensions); ^ /usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include/wincrypt.h:5041:278: error: unknown type name 'PSYSTEMTIME' WINIMPM PCCERT_CONTEXT WINAPI CertCreateSelfSignCertificate (HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey, PCERT_NAME_BLOB pSubjectIssuerBlob, DWORD dwFlags, PCRYPT_KEY_PROV_INFO pKeyProvInfo, PCRYPT_ALGORITHM_IDENTIFIER pSignatureAlgorithm, PSYSTEMTIME pStartTime, PSYSTEMTIME pEndTime, PCERT_EXTENSIONS pExtensions); ^ 2 errors generated. make[1]: *** [CMakeFiles/cmTC_2d8fe.dir/HAVE_WINLDAP_H.c.obj] Error 1 make: *** [cmTC_2d8fe/fast] Error 2 exitCode: 2 ``` Cherry-picked from curl#11095 88e4a21 Reviewed-by: Daniel Stenberg Closes curl#11245

Aka "jumbo" or "amalgamation" builds. It means to compile all sources per target as a single C source. This is experimental. You can enable it by passing `-DCMAKE_UNITY_BUILD=ON` to cmake. It requires CMake 3.16 or newer. It makes builds (much) faster, allows for better optimizations and tends to promote less ambiguous code. Also add a new AppVeyor CI job and convert an existing one to use "unity" mode (one MSVC, one MinGW), and enable it for one macOS CI job. Fix related issues: - add missing include guard to `easy_lock.h`. - rename static variables and functions (and a macro) with names reused across sources, or shadowed by local variables. - add an `#undef` after use. - add a missing `#undef` before use. - move internal definitions from `ftp.h` to `ftp.c`. - `curl_memory.h` fixes to make it work when included repeatedly. - stop building/linking curlx bits twice for a static-mode curl tool. These caused doubly defined symbols in unity builds. - silence missing extern declarations compiler warning for ` _CRT_glob`. - fix extern declarations for `tool_freq` and `tool_isVistaOrGreater`. - fix colliding static symbols in debug mode: `debugtime()` and `statename`. - rename `ssl_backend_data` structure to unique names for each TLS-backend, along with the `ssl_connect_data` struct member referencing them. This required adding casts for each access. - add workaround for missing `[P]UNICODE_STRING` types in certain Windows builds when compiling `lib/ldap.c`. To support "unity" builds, we had to enable `SCHANNEL_USE_BLACKLISTS` for Schannel (a Windows `schannel.h` option) _globally_. This caused an indirect inclusion of Windows `schannel.h` from `ldap.c` via `winldap.h` to have it enabled as well. This requires `[P]UNICODE_STRING` types, which is apperantly not defined automatically (as seen with both MSVS and mingw-w64). This patch includes `<subauth.h>` to fix it. Ref: https://github.com/curl/curl/runs/13987772013 Ref: https://dev.azure.com/daniel0244/curl/_build/results?buildId=15827&view=logs&jobId=2c9f582d-e278-56b6-4354-f38a4d851906&j=2c9f582d-e278-56b6-4354-f38a4d851906&t=90509b00-34fa-5a81-35d7-5ed9569d331c - tweak unity builds to compile `lib/memdebug.c` separately in memory trace builds to avoid PP confusion. - force-disable unity for test programs. - do not compile and link libcurl sources to libtests _twice_ when libcurl is built in static mode. KNOWN ISSUES: - running tests with unity builds may fail in cases. - some build configurations/env may not compile in unity mode. E.g.: https://ci.appveyor.com/project/curlorg/curl/builds/47230972/job/51wfesgnfuauwl8q#L250 Ref: libssh2/libssh2#1034 Ref: https://cmake.org/cmake/help/latest/prop_tgt/UNITY_BUILD.html Ref: https://en.wikipedia.org/wiki/Unity_build Closes curl#11095

"TrackMemory" is `ENABLE_DEBUG=ON` (aka `ENABLE_CURLDEBUG=ON`, aka `-DCURLDEBUG`). There is an issue with memory tracking and Unicode when build in "unity" mode, which results in the curl tool crashing right on startup, even without any command-line option. Interestingly this doesn't happen under WINE (at least on the system I tested this on), but consistenly happens on real Windows machines. Crash is 0xC0000374 heap corruption. Both shared and static curl executables are affected. This limitation probably won't hit too many people, but it remains a TODO to find and fix the root cause and drop this workaround. Example builds and runs: https://ci.appveyor.com/project/curlorg/curl/builds/48169111/job/17cptxhtpubd7iwj#L313 (static) https://ci.appveyor.com/project/curlorg/curl/builds/48169111/job/76e1ge758tbyqu9c#L317 (shared) Follow-up to 3f8fc25 curl#11095 Ref curl#11928 Closes #xxxxx

"TrackMemory" is `ENABLE_DEBUG=ON` (aka `ENABLE_CURLDEBUG=ON`, aka `-DCURLDEBUG`). There is an issue with memory tracking and Unicode when built in "unity" mode, which results in the curl tool crashing right on startup, even without any command-line option. Interestingly this doesn't happen under WINE (at least on the system I tested this on), but consistenly happens on real Windows machines. Crash is 0xC0000374 heap corruption. Both shared and static curl executables are affected. This limitation probably won't hit too many people, but it remains a TODO to find and fix the root cause and drop this workaround. Example builds and runs: https://ci.appveyor.com/project/curlorg/curl/builds/48169111/job/17cptxhtpubd7iwj#L313 (static) https://ci.appveyor.com/project/curlorg/curl/builds/48169111/job/76e1ge758tbyqu9c#L317 (shared) Follow-up to 3f8fc25 #11095 Ref: #11928 Closes #12005

Found the root cause of the startup crash with unity builds with Unicode and TrackMemory enabled at the same time. We must make sure that the `memdebug.h` header doesn't apply to `lib/curl_multibyte.c` (as even noted in a comment there.). In unity builds all headers apply to all sources, including `curl_multibyte.c`. This probably resulted in an infinite loop on startup. This patch excludes this source from unity compilation with TrackMemory enabled, in both libcurl and curl tool. Also delete the earlier workaround that fully disabled unity for affected builds. Also enable unity mode for a debug unicode CI job. Follow-up to d82b080 curl#12005 Follow-up to 3f8fc25 curl#11095 Closes curl#11928

Found the root cause of the startup crash in unity builds with Unicode and TrackMemory enabled at the same time. We must make sure that the `memdebug.h` header doesn't apply to `lib/curl_multibyte.c` (as even noted in a comment there.) In unity builds all headers apply to all sources, including `curl_multibyte.c`. This probably resulted in an infinite loop on startup. Exclude this source from unity compilation with TrackMemory enabled, in both libcurl and curl tool. Enable unity mode for a debug Unicode CI job to keep it tested. Also delete the earlier workaround that fully disabled unity for affected builds. Follow-up to d82b080 #12005 Follow-up to 3f8fc25 #11095 Closes #11928

Switch from `curl-gnumake.sh` to `curl-cmake.sh` with upcoming curl release v8.5.0. cmake builds are now _faster_ for Windows builds than raw gnumake (aka `Makefile.mk`). They also use 'unity' mode, which makes builds run fast with the side-effect of also creating potentially more efficient binaries by allowing better compiler optimizations. This also makes curl-for-win use the same build system for all target platforms (`Makefile.mk` is not available for *nix platforms). Initially on 2022-07-04 cmake was 25% slower than gnumake. By 2022-09-26 this reduced to 20%, by 2023-07-29 to 18% and after the latest round of updates gnumake came out 7% slower than cmake. This is for Windows, for a triple x64, arm64 and x86 build. In absolute times this is 27m22s for cmake and 29m11s for gnumake. (FWIW autotools builds are 52% slower than cmake unity builds now.) Making cmake builds fast was a multi-year effort with these major steps: 1. add support for cmake builds in curl-for-win. 420e73c 2. bring it in-line with gnumake and autotools builds and tidy-up as much as possible. Scattered to a many commits. 3. delete a whole bunch of unused feature detections. curl/curl@4d73854 curl/curl#9044 (and a lot more smaller commits) 4. speed up picky warning option initialization by avoiding brute-force all options. (first in libssh2, then in curl, then applied also ngtcp2, nghttp3, nghttp2) curl/curl@9c543de curl/curl#10973 5. implement single build run for shared and static libcurl + tool (first in libssh2) curl/curl@1199308 curl/curl#11505 53dcd49 6. implement single build pass for shared and static libcurl (for Windows initially) curl/curl@2ebc74c curl/curl#11546 7. extend above to non-Windows (e.g. macOS) curl/curl@fc9bfb1 curl/curl#11627 bafa77d (mac) 1b27304 (linux) 8. implement unity builds: single compiler invocation for libcurl + tool curl/curl@3f8fc25 curl/curl#11095 curl/curl@f42a279 curl/curl#11928 67d7fd3 9. speed up 4x the cmake initialization phase (for Windows) curl/curl@2100d9f curl/curl#12044 10. speed up cmake initialization even more by pre-filling detection results for our well-known mingw-w64 environments. 74dd967 5a43c61 +1: speed up autotools initialization by mitigating a brute-force feature detection on Windows. This reduced total build times by 5 minutes at the time, for the 3 Windows targets in total. curl/curl@6adcff6 curl/curl#9591 Also update build times for cmake-unity and gnumake, based on runs: cmake-unity: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/48357875 gnumake: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/48358005

vszakats added cmake feature-window A merge of this requires an open feature window performance labels May 9, 2023

github-actions bot added the cmdline tool label May 9, 2023

vszakats removed the cmdline tool label May 9, 2023

vszakats marked this pull request as draft May 10, 2023 11:50

vszakats force-pushed the cmake-unity branch from ed0b999 to 3f988d3 Compare May 10, 2023 16:25

github-actions bot added the cmdline tool label May 10, 2023

vszakats force-pushed the cmake-unity branch from 81129f4 to ab0fdde Compare May 19, 2023 02:28

vszakats force-pushed the cmake-unity branch 2 times, most recently from 370b5cc to 6bb24dd Compare May 25, 2023 00:26

vszakats force-pushed the cmake-unity branch from 6bb24dd to af8662f Compare May 25, 2023 15:10

vszakats force-pushed the cmake-unity branch from dd2bb05 to 1887c15 Compare May 26, 2023 11:41

vszakats marked this pull request as ready for review May 27, 2023 20:21

vszakats added 4 commits June 6, 2023 14:31

fix memdebug.c PP confusion with CMake tweak

2517511

Move memdebug.c into a separate unity group, forcing it to compile separately and without going into recursive PP rules when debug mode is enabled.

disable unity builds for test programs

ebfbfdb

avoid duplicate compile/link in libtest

07f0fc8

vszakats force-pushed the cmake-unity branch from b0e931f to 116e88a Compare June 6, 2023 14:33

ftp.c: use distinct name for state()

6bf9321

vszakats closed this in 3f8fc25 Jun 7, 2023

vszakats deleted the cmake-unity branch June 7, 2023 13:09

vszakats mentioned this pull request Oct 2, 2023

cmake: disable unity mode with Windows Unicode + TrackMemory #12005

Closed

vszakats mentioned this pull request Oct 3, 2023

cmake: fix unity with Windows Unicode + TrackMemory #11928

Closed

jan2000 mentioned this pull request Apr 23, 2024

mbedTLS: Implemented CURLOPT_SSL_CIPHER_LIST option #13442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmake: add support for "unity" builds #11095

cmake: add support for "unity" builds #11095

vszakats commented May 9, 2023 •

edited

vszakats commented May 9, 2023

jay commented May 10, 2023

vszakats commented May 10, 2023 •

edited

MarcelRaad commented May 10, 2023

vszakats commented May 10, 2023 •

edited

eli-schwartz commented May 14, 2023

bagder commented May 16, 2023

vszakats commented May 19, 2023 •

edited

vszakats commented Jun 6, 2023 •

edited

eli-schwartz commented Jun 7, 2023

thesamesam commented Jun 7, 2023

vszakats commented Jun 7, 2023

eli-schwartz commented Jun 7, 2023

vszakats commented Jun 7, 2023 •

edited

AndrewJDR commented Aug 23, 2023 •

edited

vszakats commented Aug 23, 2023

AndrewJDR commented Aug 23, 2023

cmake: add support for "unity" builds #11095

cmake: add support for "unity" builds #11095

Conversation

vszakats commented May 9, 2023 • edited

vszakats commented May 9, 2023

jay commented May 10, 2023

vszakats commented May 10, 2023 • edited

MarcelRaad commented May 10, 2023

vszakats commented May 10, 2023 • edited

eli-schwartz commented May 14, 2023

bagder commented May 16, 2023

vszakats commented May 19, 2023 • edited

vszakats commented Jun 6, 2023 • edited

eli-schwartz commented Jun 7, 2023

thesamesam commented Jun 7, 2023

vszakats commented Jun 7, 2023

eli-schwartz commented Jun 7, 2023

vszakats commented Jun 7, 2023 • edited

AndrewJDR commented Aug 23, 2023 • edited

vszakats commented Aug 23, 2023

AndrewJDR commented Aug 23, 2023

vszakats commented May 9, 2023 •

edited

vszakats commented May 10, 2023 •

edited

vszakats commented May 10, 2023 •

edited

vszakats commented May 19, 2023 •

edited

vszakats commented Jun 6, 2023 •

edited

vszakats commented Jun 7, 2023 •

edited

AndrewJDR commented Aug 23, 2023 •

edited