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: fix curl-config --static-libs #852

Closed
wants to merge 1 commit into from

Conversation

Lekensteyn
Copy link
Contributor

Libraries for curl-config --static-libs are required to have the form
-ldl. Assume library paths like /usr/lib/libssl.so and map it to "ssl"
for curl-config.

This removes the confusing messag "Static linking is broken" which was
printed because curl-config --static-libs was disfunctional while the
static libcurl.a is fine.

Fixes #841

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @billhoffman, @jzakrzewski and @Sukender to be potential reviewers

@bagder
Copy link
Member

bagder commented Jun 27, 2016

I'd like a second cmake fluent person to +1 this PR before I consider merging it. @snikulov perhaps?

@snikulov
Copy link
Contributor

@bagder Will check it soon.

@snikulov
Copy link
Contributor

@bradking Could you also take a look on this PR?

@snikulov
Copy link
Contributor

snikulov commented Jun 28, 2016

Not really useful for MS platform, because of sh script.
Changes targeted to curl-config generation which is, I think, should be reworked per #885
If anyone use it, they should check this change also.

For me only one line is fine, where the message is removed.
👍 for that.

@bradking
Copy link
Contributor

Does curl-config --static-libs have to print -lfoo instead of /path/to/libfoo.so? Clients should link just fine with the absolute paths. IIUC the --static-libs option is about linking to libcurl.a plus whatever dependencies it has, whether or not they are shared or static libs. One could construct LIBCURL_LIBS by checking for / in library names to decide whether to use -l as a prefix.

@Lekensteyn
Copy link
Contributor Author

@bradking Oh, passing /usr/lib/libfoo.so rather than -lfoo seems to work (well, checked it with gcc -v test.c -lcurl vs. the absolute path). Is this a universal thing? It sounds like a good idea to prepend -l only if there is no / in the name.

@bradking
Copy link
Contributor

@Lekensteyn yes, passing the absolute path is universally supported. CMake tries to do this whenever possible because it ensures that the desired library file is used instead of just hoping that -lfoo finds the right one.

The `curl-config --static-libs` command should not output paths like
-l/usr/lib/libssl.so, instead print the absolute path without `-l`.

This also removes the confusing message "Static linking is broken" which
was printed because curl-config --static-libs was disfunctional even
though the static libcurl.a library works properly.

Fixes curl#841
@Lekensteyn
Copy link
Contributor Author

Sorry for the delay, I have updated the patch according to @bradking's suggestion and now curl-config shows:

/usr/local/lib/libcurl.a -lc -lidn -llber -lldap -ldl /usr/lib64/libssl.so /usr/lib64/libcrypto.so /usr/lib64/libz.so /usr/lib64/libssh2.so

This should work better.

@bradking
Copy link
Contributor

bradking commented Aug 9, 2016

LGTM.

@Lekensteyn
Copy link
Contributor Author

@snikulov Is this patch OK to merge? You referenced #885, but that is about a new cmake file for use with find_package. This change just provided compatibility for the curl-config tool on *nix.

@snikulov
Copy link
Contributor

@Lekensteyn Yes. LGTM.

@Lekensteyn
Copy link
Contributor Author

Applied in 2f3feda

@Lekensteyn Lekensteyn closed this Sep 11, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

5 participants