cmake: always build unit tests with the testdeps target#13698
Closed
vszakats wants to merge 1 commit intocurl:masterfrom
Closed
cmake: always build unit tests with the testdeps target#13698vszakats wants to merge 1 commit intocurl:masterfrom
testdeps target#13698vszakats wants to merge 1 commit intocurl:masterfrom
Conversation
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
7c5afb1 to
57b04ed
Compare
This was referenced May 20, 2024
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
57b04ed to
b6c934e
Compare
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.
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