tests/server: support bundle binary#15000
Closed
vszakats wants to merge 7 commits intocurl:masterfrom
Closed
Conversation
vszakats
force-pushed
the
server-bundle
branch
2 times, most recently
from
602937c to
5dd6751
Compare
vszakats
force-pushed
the
server-bundle
branch
2 times, most recently
from
d23bcb2 to
1f94bfa
Compare
vszakats
force-pushed
the
server-bundle
branch
4 times, most recently
from
4bd2924 to
fffba0b
Compare
vszakats
force-pushed
the
server-bundle
branch
from
4c0771f to
4eb986f
Compare
vszakats
force-pushed
the
server-bundle
branch
2 times, most recently
from
a380749 to
fb2905b
Compare
vszakats
force-pushed
the
server-bundle
branch
4 times, most recently
from
575bb18 to
46b8f5f
Compare
vszakats
force-pushed
the
server-bundle
branch
2 times, most recently
from
c7c9937 to
0d1e3a8
Compare
vszakats
force-pushed
the
server-bundle
branch
from
0d1e3a8 to
0dd5267
Compare
1 task
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Feb 24, 2025
Support multi-target cmake builds via `CURL_DIRSUFFIX` env. Drop using using `-DCMAKE_RUNTIME_OUTPUT_DIRECTORY_*=` as a workaround in CI. Also: - replace `resolve` references in tests with a new `%RESOLVE` variable. - fix `-c` option format in manual. - fix some whitespace. Ref: curl#15000 Cherry-picked from curl#16394
vszakats
force-pushed
the
server-bundle
branch
from
229a1fd to
a8cd21e
Compare
vszakats
added a commit
that referenced
this pull request
Feb 24, 2025
Support multi-target cmake builds via `CURL_DIRSUFFIX` env. For example:
`export CURL_DIRSUFFIX=Debug/`.
Multi-target generators place their output to `src/<subdir>/`,
`lib/<subdir>/`, `tests/server/<subdir>`, `tests/libtest/<subdir>` and
`tests/unit/<subdir>/` by default. Before this patch, `runtests.pl`
couldn't run on such builds because it expected the binaries under the
their `<subdir>`-less directories. This patch allows to set such subdir
and make `runtests.pl` find the binaries. In CI we use multi-target
builds with tests for MSVC. It also helps Xcode-generator builds, though
in CI we don't have such job running tests.
There may be better solutions to configure this, but passing a custom
value to `runtests.pl` including its subprocesses is somewhat tricky.
The reason the configuration value expects the slash at the end is
because MSYS is automagically expanding the env to a (wrong) absolute
path if the slash is in the front.
Also:
- drop the `-DCMAKE_RUNTIME_OUTPUT_DIRECTORY_*=` workaround from CI.
- replace `resolve` references in tests with a new `%RESOLVE` variable.
It didn't use a filename extension before. After this patch it uses
`exe_ext('TOOL')`. I'm not sure if this is the correct choice vs.
`exe_ext('SRV')`.
- fix `-c` option format in manual.
- fix some whitespace.
Note, in CI we still tweak `CMAKE_RUNTIME_OUTPUT_DIRECTORY_*` in jobs
which share steps between `./configure` and cmake. It's easier that way.
Ref: #15000
Cherry-picked from #16394
Closes #16452
vszakats
force-pushed
the
server-bundle
branch
8 times, most recently
from
922f3bc to
44fac15
Compare
Member
Author
|
\o/ first successful run with the bundled server binary: The, or at least the final, hurdle was runtime confusion (possibly server crash?) due to building servers with |
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Mar 13, 2025
Include two more sources in unity mode to optimize builds, syncing this pattern with `lib` and `src`. Follow-up to de0693f curl#16274 Follow-up to 3efba94 curl#14765 Cherry-picked from curl#15000
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Mar 13, 2025
Include two more sources in unity mode to optimize builds, syncing this pattern with `lib` and `src`. Follow-up to de0693f curl#16274 Follow-up to 3efba94 curl#14765 Cherry-picked from curl#15000
This was referenced Mar 13, 2025
mk-bundle.pl: add .c extension if missing fixup socket_domain differently, drop unused enum type' mk-bundle.pl: drop deduped macros/variables/functions mk-bundle.pl: improve readability of generated code move script to server mk-bundle.pl: add support for raw curl source (to support autotools "unity") mk-bundle: make p_mains static
reshuffle first.c use first: make p_mains static
When non-bundled, the server source is included last. This is important for unity builds because server global variables cannot shadow libcurl function parameters and local variables. Keep this order when doing bundles to avoid triggering more -Wshadow warnings than otherwise. Makefile.inc: move CURLDEBUG-dependent sources to MEMDEBUG Makefile.inc: fixup for non-bundle builds cm: move CURLDEBUG-dependent sources to MEMDEBUG cm: make p_mains static
add first.c, first.h to dist keep resolve tool out of servers Reason is that it's called from data/test* modules where conditionally executing either `server/servers disabled` or `server/disabled` depending on build settings would be problematic. do not checksrc generated bundle source am: keep the origin order of sources Makefile.am: do unity when doing bundle Makefile.am: move first.c last as in cmake am: make p_mains static Makefile.am: add FIRSTFILES to distro
Member
Author
|
I plan to merge this tomorrow if no objections (and if I finish in time It's extending the existing test bundle feature, that is disabled by |
Member
Author
|
Here are two CI run with and without test bundles enabled: without: https://github.com/curl/curl/actions/runs/13909934284 Some examples: Cygwin AM: 10m1 vs 1m32 (6.5x) For MSVC it's a 20x reduction, others are also pretty good. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Extend existing
--enable-test-bundlesand-DCURL_TEST_BUNDLES=ONoptions to also bundle test server programs into a single binary. With
autotools, also bundle auxiliary libcurl sources for a "unity"-style
build.
It saves almost 10 minutes per run, across all CI jobs.
On average it makes
build testssteps 25% faster.With CMake, it brings down
testdepsbuild steps to 32 to 37, from45 to 64 before this patch, with unity. Without unity it brings it down
from 400-420 to 280-300. For comparison, without unity and bundles,
the number of build steps is around 1850.
With autotools the gain is possibly larger because this patch does unity
and bundle for test servers.
The total reduction of build steps / log lines is 12000. It's 44% of
reduction on average across all CI jobs.
Follow-up to 77401af #16695
Follow-up to 71cf0d1 #14772
Comparison of 'build tests' targets in S(econds) and L(ines/steps),
between before and after this patch:
Before:
GHA/windows: https://github.com/curl/curl/actions/runs/13854983424
GHA/old-linux: https://github.com/curl/curl/actions/runs/13854983399
GHA/non-native: https://github.com/curl/curl/actions/runs/13854983427
GHA/linux-http3: https://github.com/curl/curl/actions/runs/13854983409
GHA/linux: https://github.com/curl/curl/actions/runs/13854983406
GHA/macos: https://github.com/curl/curl/actions/runs/13854983401
Appveyor: https://ci.appveyor.com/project/curlorg/curl/builds/51703551
After:
GHA/windows: https://github.com/curl/curl/actions/runs/13860433850?pr=15000
GHA/old-linux: https://github.com/curl/curl/actions/runs/13860433809?pr=15000
GHA/non-native: https://github.com/curl/curl/actions/runs/13860433828?pr=15000
GHA/linux-http3: https://github.com/curl/curl/actions/runs/13860433806?pr=15000
GHA/linux: https://github.com/curl/curl/actions/runs/13860433848?pr=15000
GHA/macos: https://github.com/curl/curl/actions/runs/13860433835?pr=15000
Appveyor: https://ci.appveyor.com/project/curlorg/curl/builds/51704222
MEMDEBUGsources in unity for non-CURLDEBUGbuilds. Ref: autotools: useCURLDEBUGto exclude TrackMemory code from unity #16723-bundleoption with something automatic? This would allow running it directly without having to add-bundlemanually to match such build. [SKIP for now] → runtests: auto-detect test bundle builds #16750mk-bundle.pl,mk-bundle-server.pl? and perhapsmk-unity.pl? [SKIP → tests: always make bundles, adapt build and tests #17590]mk-bundle.pland just make it a manualserver.cthat includes all sources? Thus fixing up unity by default, regardless of build-system? also simplifying combinations and ensuring no symbol collision regressions by default? [SKIP, this bundler also does "unity" for autotools builds, which would be impractical the manual way.]test/server cleanup plans:
Curl_wait_ms()directly instead of the current copy-paste dupe. → [FUTURE → lib: makecurlx_wait_ms()and use it #17641]sockdaemon()copies more. → [FUTURE → tests/server: de-dupe/merge threesockdaemon()clones into one #17719]sockfilt/juggle()on Windows, where the local override ofselect()may return WindowsGetLastError()viaerrno, which is unexpected and was never checked for. It should be a WSA code throughSOCKERRNO.tests/server: fix to check against winsock2 error codes on Windows #16612 (comment) → [FUTURE]
tvnow()inutil.cwith coreCurl_now()? (there is triplicate insrc/tool_util.c) → curltime: use libcurl time functions in src and tests/server #16653strtoul()withCurl_str_number()and ban it. → tests/server: usecurlx_str_numblanks()to avoiderrno#16671 (skip those not involved witherrno, they make the code more complicated)This also reduces
errnouse.wait_ms()with a better implementation from coreCurl_wait_ms().SOCKERRNOvserrnoconstant. → tests/server: fix to check against winsock2 error codes on Windows #16612