cmake: allow building tests in unity mode #14765
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
|
Analysis of PR #14765 at e7e164e8: Test 900 failed, which has NOT been flaky recently, so there could be a real issue in the PR. Test 940 failed, which has NOT been flaky recently, so there could be a real issue in the PR. Test 3005 failed, which has NOT been flaky recently, so there could be a real issue in the PR. Generated by Testclutch |
vszakats
force-pushed
the
cm-tests-server-unity
branch
2 times, most recently
from
f9ead44
to
474f743
Compare
vszakats
force-pushed
the
cm-tests-server-unity
branch
from
15a346c
to
276e686
Compare
19 tasks
vszakats
force-pushed
the
cm-tests-server-unity
branch
from
34d33a7
to
d810072
Compare
vszakats
force-pushed
the
cm-tests-server-unity
branch
from
d810072
to
4b2b42a
Compare
Except tests/http/clients which are always single sources.
``` C:\projects\curl\tests\libtest\first.c(123,36): warning C4459: declaration of 'buffer' hides global declaration [C:\projects\curl\_bld\tests\libtest\lib651.vcxproj] C:\projects\curl\tests\libtest\first.c(123,36): warning C4459: declaration of 'buffer' hides global declaration [C:\projects\curl\_bld\tests\libtest\lib652.vcxproj] C:\projects\curl\tests\libtest\first.c(123,36): warning C4459: declaration of 'buffer' hides global declaration [C:\projects\curl\_bld\tests\libtest\lib666.vcxproj] C:\projects\curl\tests\libtest\first.c(123,36): warning C4459: declaration of 'buffer' hides global declaration [C:\projects\curl\_bld\tests\libtest\lib1911.vcxproj] C:\projects\curl\tests\libtest\first.c(139,12): warning C4459: declaration of 'result' hides global declaration [C:\projects\curl\_bld\tests\unit\unit1652.vcxproj] ``` Ref: https://ci.appveyor.com/project/curlorg/curl/builds/50522821/job/8q3um37s94uudsen
```
In file included from /cygdrive/d/a/curl/curl/bld/tests/server/CMakeFiles/sws.dir/Unity/unity_0_c.c:37:
/cygdrive/d/a/curl/curl/tests/server/sws.c:1389: error: "CTRL" redefined
1389 | #define CTRL 0
[ 5%] Building C object tests/server/CMakeFiles/sws.dir/__/__/lib/memdebug.c.o
|
In file included from /usr/include/sys/ioctl.h:15,
from /cygdrive/d/a/curl/curl/lib/nonblock.c:28,
from /cygdrive/d/a/curl/curl/bld/tests/server/CMakeFiles/sws.dir/Unity/unity_0_c.c:7:
/usr/include/sys/termios.h:74: note: this is the location of the previous definition
74 | #define CTRL(ch) ((ch)&0x1F)
|
```
https://github.com/curl/curl/actions/runs/10674023027/job/29583891142?pr=14765 (cygwin)
macOS: SecureTransport: https://github.com/curl/curl/actions/runs/10674833829/job/29585938056?pr=14765 Linux: https://github.com/curl/curl/actions/runs/10674833840/job/29585935178?pr=14765 msys2, mingw ucrt, uwp: https://github.com/curl/curl/actions/runs/10674833839/job/29585940816?pr=14765 netbsd/openbsd: https://github.com/curl/curl/actions/runs/10674833831/job/29585934198?pr=14765 resolve.c is unique by using getaddrinfo/freeaddrinfo. In some build envs/config these functions are macros. lib/memdebug.h gets included via other sources before including system headers declaring (or rather: defining) these functions. This makes memdebug.h redefinitions applied to the system headers, breaking them, triggering bogus warnings, and missing to declare/define the functions when trying to call them in resolve.c. The workaround is to undefing them before the system headers (e.g. `netdb.h` on macOS), and forcing to re-apply `memdebug.h` on the next inclusion (which always comes _after_ system headers). This causes 'multiple definition' warnings for functions declared in `memdebug.h`. Fix those by guarding the declarations with an extra guard that is never undefined. This surely has a cleaner solution, but it'd need rethinking the way these overrides are done, or perhaps offer apply/deapply/reapply headers to make them compatible with sources with spots where the `memdebug.h` effect is undesired. Or, perhaps make sure to include all potentially affected system headers from `memdebug.h` itself, _before_ it applying any redefinitions. This ensures no interference. Or, drop memdebug.h from server builds. (Though this issue is also present in `src`, though in a more "lucky" constellation.) Or, turn off UNITY for resolve.c specifically.
```
D:/a/curl/curl/lib/warnless.c: In function 'curlx_read':
D:/a/curl/curl/lib/warnless.c:372:34: error: declaration of 'buf' shadows a global declaration [-Werror=shadow]
372 | ssize_t curlx_read(int fd, void *buf, size_t count)
| ~~~~~~^~~
In file included from D:/a/curl/curl/bld/tests/libtest/CMakeFiles/lib677.dir/Unity/unity_0_c.c:4:
D:/a/curl/curl/tests/libtest/lib677.c:31:13: note: shadowed declaration is here
31 | static char buf[1024];
| ^~~
D:/a/curl/curl/lib/warnless.c: In function 'curlx_write':
D:/a/curl/curl/lib/warnless.c:377:41: error: declaration of 'buf' shadows a global declaration [-Werror=shadow]
377 | ssize_t curlx_write(int fd, const void *buf, size_t count)
| ~~~~~~~~~~~~^~~
D:/a/curl/curl/tests/libtest/lib677.c:31:13: note: shadowed declaration is here
31 | static char buf[1024];
| ^~~
```
Ref: https://github.com/curl/curl/actions/runs/10682565270/job/29608750021?pr=14765
This reverts commit 23550c4.
This reverts commit d5174ac.
set it globally for autotools, to stay in sync with cmake.
vszakats
force-pushed
the
cm-tests-server-unity
branch
from
4b2b42a
to
1ca473f
Compare
moritzbuhl
pushed a commit
to moritzbuhl/curl
that referenced
this pull request
Sep 20, 2024
Makes building tests noticeably faster. Apply changes/fixes/workarounds to make Unity work: - rename test variables to avoid collisions or shadowing each other when combined into single units. - add workaround to avoid applying `lib/memdebug.h` overrides to system headers declaring/defining `getaddrinfo()`/`freeaddrinfo()` for `tests/server/resolve.c`. This replaces a previous workaround that worked for that specific source. - rename test macro `CTRL` clashing with Cygwin `sys/ioctl.h`. - add include guard to `test.h`. Also: - exclude `tests/http/clients` which are all single-source. (like `docs/examples`.) Build time improvements for tests: - AppVeyor CI: - MSVC 2008, 2010: 1 minute faster (4m8s -> 2m56s, 3m19s -> 2m24s) - MSVC 2022 arm64: 3.5 minutes faster (10m18s -> 6m48s) before: https://ci.appveyor.com/project/curlorg/curl/builds/50522785 after: https://ci.appveyor.com/project/curlorg/curl/builds/50522942 - GHA: - Cygwin: 1.5 minutes faster (3m13s -> 1m43s) before: https://github.com/curl/curl/actions/runs/10681535327/job/29605384398 after: https://github.com/curl/curl/actions/runs/10680818726/job/29603130637 - Windows: before: https://github.com/curl/curl/actions/runs/10680818713 after: https://github.com/curl/curl/actions/runs/10683850187 - MSYS2, mingw-w64: 1 minute faster - MSVC: 30 seconds faster (3m17s -> 2m48s) - macOS: double speed (39s -> 18s) before: https://github.com/curl/curl/actions/runs/10680818753/job/29603133447 after: https://github.com/curl/curl/actions/runs/10683850174/job/29612914515 - Linux: almost double speed (30/31s -> 18s) before: https://github.com/curl/curl/actions/runs/10681535311/job/29605387156 after: https://github.com/curl/curl/actions/runs/10680818721/job/29603133976 - non-native: no obvious effect. before: https://github.com/curl/curl/actions/runs/10680818722 after: https://github.com/curl/curl/actions/runs/10683850187 - Old Linux: Unity mode not supported by old CMake, no effect. Closes curl#14765
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.
Makes building tests noticeably faster.
Apply changes/fixes/workarounds to make Unity work:
combined into single units.
lib/memdebug.hoverrides to systemheaders declaring/defining
getaddrinfo()/freeaddrinfo()fortests/server/resolve.c. This replaces a previous workaround thatworked for that specific source.
CTRLclashing with Cygwinsys/ioctl.h.test.h.Also:
tests/http/clientswhich are all single-source. (likedocs/examples.)Build time improvements for tests:
before: https://ci.appveyor.com/project/curlorg/curl/builds/50522785
after: https://ci.appveyor.com/project/curlorg/curl/builds/50522942
before: https://github.com/curl/curl/actions/runs/10681535327/job/29605384398
after: https://github.com/curl/curl/actions/runs/10680818726/job/29603130637
before: https://github.com/curl/curl/actions/runs/10680818713
after: https://github.com/curl/curl/actions/runs/10683850187
before: https://github.com/curl/curl/actions/runs/10680818753/job/29603133447
after: https://github.com/curl/curl/actions/runs/10683850174/job/29612914515
before: https://github.com/curl/curl/actions/runs/10681535311/job/29605387156
after: https://github.com/curl/curl/actions/runs/10680818721/job/29603133976
before: https://github.com/curl/curl/actions/runs/10680818722
after: https://github.com/curl/curl/actions/runs/10683850187
#define CURL_NO_GETADDRINFO_OVERRIDEas an alternative hack forgetaddrinfo().