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

The pthread linker flag already contains "-l" #1552

Closed
wants to merge 1 commit into from
Closed

Conversation

bjori
Copy link
Contributor

@bjori bjori commented Jun 7, 2017

The pkgconfig created by cmake adds "-l" to the pthread linked flag, producing -l-lpthread.
This comes from ${CURL_LIBS} which retrieves the ${CMAKE_THREAD_LIBS_INIT} value which already contains -l

This is reproducable on Ubuntu like so:

$ rm -rf cbuild && mkdir cbuild && cd cbuild && cmake .. -DENABLE_THREADED_RESOLVER=1 && cat *.pc

Before:

Libs.private:  -lc -ldl -l-lpthread /usr/lib/x86_64-linux-gnu/libssl.so /usr/lib/x86_64-linux-gnu/libcrypto.so /usr/lib/x86_64-linux-gnu/libz.so

After:

Libs.private:  -lc -ldl -lpthread /usr/lib/x86_64-linux-gnu/libssl.so /usr/lib/x86_64-linux-gnu/libcrypto.so /usr/lib/x86_64-linux-gnu/libz.so

Note that with ./configure --enable-threaded-resolver pthread isn't added to the .pc at all, as it is instead added to the CFLAGS as -pthread rather then linker flags as -lpthread

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 73.213% when pulling 44f6906 on bjori:master into f7ee701 on curl:master.

@bagder bagder added the cmake label Jun 7, 2017
@bagder
Copy link
Member

bagder commented Jun 7, 2017

Yes indeed, -lpthread is not added there by configure. I suppose that tells us a little how rarely that data is actually used, since I believe most linux distros ship with threaded-resolver enabled libcurls these days...

@bagder
Copy link
Member

bagder commented Jun 8, 2017

ping @Lekensteyn, this changes code you brought!

@bagder
Copy link
Member

bagder commented Jul 6, 2017

No response from any cmake hackers. It seems fine to me so I'll go ahead and merge anyway.

@bagder bagder closed this in 75c3596 Jul 6, 2017
@bagder
Copy link
Member

bagder commented Jul 6, 2017

Thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 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.

None yet

3 participants