cmake: always build unit tests with the testdeps target
#13698
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
vszakats
added a commit
that referenced
this pull request
May 18, 2024
Do not add linker flags to the global CMake static library tool (aka "static linker") (e.g. `ar`) flags list. They don't mix well. This was only done after successfully detecting GSSAPI. Linker flags seen on Old Linux CI: ``` -- |GSS_LINKER_FLAGS|-Wl,--enable-new-dtags -Wl,-rpath -Wl,/usr/lib/x86_64-linux-gnu/heimdal| -- |CMAKE_STATIC_LINKER_FLAGS| -Wl,--enable-new-dtags -Wl,-rpath -Wl,/usr/lib/x86_64-linux-gnu/heimdal| ``` Ref: https://github.com/curl/curl/actions/runs/9138988036/job/25130791712#step:6:85 Causing: ``` /usr/bin/ar qc libcurltool.a -Wl,--enable-new-dtags -Wl,-rpath -Wl,/usr/lib/x86_64-linux-gnu/heimdal CMakeFiles/curltool.dir/slist_wc.c.o CMakeFiles/curltool.dir/tool_binmode.c.o CMakeFiles/curltool.dir/tool_bname.c.o [...] CMakeFiles/curltool.dir/tool_writeout_json.c.o CMakeFiles/curltool.dir/tool_xattr.c.o CMakeFiles/curltool.dir/var.c.o CMakeFiles/curltool.dir/__/lib/base64.c.o CMakeFiles/curltool.dir/__/lib/dynbuf.c.o /usr/bin/ar: invalid option -- 'W' Usage: /usr/bin/ar [emulation options] [-]{dmpqrstx}[abcDfilMNoPsSTuvV] [--plugin <name>] [member-name] [count] archive-file file... /usr/bin/ar -M [<mri-script] ``` Ref: https://github.com/curl/curl/actions/runs/9138988036/job/25130791712#step:9:125 This problem is invisible at the moment because of another bug (#13698) that misses building unit tests when not using either the `ENABLE_DEBUG=ON` or `ENABLE_CURLDEBUG=ON` options (to set `-DCURLDEBUG`): ``` test 1300 SKIPPED: curl lacks unittest support ``` Ref: https://github.com/curl/curl/actions/runs/9135571781/job/25123104557#step:9:2883 With that fixed, this becomes the next issue. It's possible this bug also required an older CMake version and/or a specific OS environment which uses linker flags in GSSAPI that are not playing well with `ar` options, to reproduce. Follow-up to 558814e (2014-09-25) Ref: #13698 Closes #13697
vszakats
force-pushed
the
cmake-unlock-unittests
branch
from
7c5afb1
to
57b04ed
Compare
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
May 18, 2024
Before this patch, building unit tests (= the `testdeps` target) required `-DCURLDEBUG` be set either via `ENABLE_DEBUG=ON` or `ENABLE_CURLDEBUG=ON`. After fixing the build issues in curl#13694, the above requirement is no longer necessary as a workaround. This patch makes unit tests build unconditionally. Depends-on: curl#13694 (fix build issues) Depends-on: curl#13697 (fix unit test issue revealed by Old Linux CI job) Follow-up to 39e7c22 curl#11446 Closes curl#13698
This was referenced May 20, 2024
vszakats
changed the title
testdeps target
Before this patch, building unit tests (= the `testdeps` target) required `-DCURLDEBUG` be set either via `ENABLE_DEBUG=ON` or `ENABLE_CURLDEBUG=ON`. After fixing the build issues in curl#13694, the above requirement is no longer necessary as a workaround. This patch makes unit tests build unconditionally. Depends-on: curl#13694 (fix build issues) Depends-on: curl#13697 (fix unit test issue revealed by Old Linux CI job) Follow-up to 39e7c22 curl#11446 Closes curl#13698
vszakats
force-pushed
the
cmake-unlock-unittests
branch
from
57b04ed
to
b6c934e
Compare
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.
Before this patch, the
testdepsbuild target required-DCURLDEBUGbe set either via
ENABLE_DEBUG=ONorENABLE_CURLDEBUG=ONto buildthe curl unit tests.
After fixing build issues in #13694, we can drop this requirement and
build unit tests unconditionally.
Depends-on: #13694
Depends-on: #13697 (fix unit test issue revealed by Old Linux CI job)
Follow-up to 39e7c22 #11446
Closes #13698
Depends-on (to fix the failing Old Linux build):
UNITTESTSandDEBUGBUILDmacros #13694