Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: do not pass linker flags to the static library tool #13697

Closed
wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented May 18, 2024

Do not add linker flags to the global CMake 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

Do not add linker flags to the global CMake static linker (e.g. `ar`)
flags list. These don't mix well.

```
-- |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 that
misses building unittests when not using either the `ENABLE_DEBUG=ON`
or `ENABLE_CURLDEBUG=ON` options (to set `-DCURLDEBUG`) when using
CMake. With that bug fixed, this problem surfaces:
```
test 1300 SKIPPED: curl lacks unittest support
```
Ref: https://github.com/curl/curl/actions/runs/9135571781/job/25123104557#step:9:2883

It's possible that this bug also requires an older CMake version and/or
a specific OS environment which uses linker flags (in GSS API) not
playing well with `ar` options, to reproduce.

Follow-up to 558814e (2014-09-25)
Closes #xxxxx
@vszakats vszakats added the cmake label May 18, 2024
@github-actions github-actions bot added the build label May 18, 2024
vszakats added a commit to vszakats/curl that referenced this pull request May 18, 2024
Before this patch, building unit tests required `CURLDEBUG` to 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
Ref: curl#13697
Follow-up to 39e7c22 curl#11446
Closes #xxxxx
@vszakats vszakats changed the title cmake: fix linker flags leaking into ar command-line cmake: fix linker flags passed to the static library tool May 18, 2024
@vszakats vszakats changed the title cmake: fix linker flags passed to the static library tool cmake: do not pass linker flags to the static library tool May 18, 2024
@vszakats vszakats closed this in 8f30c96 May 18, 2024
@vszakats vszakats deleted the cmake-ar-linker-flags branch May 18, 2024 18:55
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
vszakats added a commit to vszakats/curl that referenced this pull request May 27, 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
vszakats added a commit that referenced this pull request May 27, 2024
Before this patch, the `testdeps` build target required `-DCURLDEBUG`
be set either via `ENABLE_DEBUG=ON` or `ENABLE_CURLDEBUG=ON` to build
the 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

1 participant