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

Simplify pthreads library detection #1274

Merged
merged 1 commit into from
Sep 14, 2017
Merged

Conversation

jabl
Copy link
Contributor

@jabl jabl commented Sep 13, 2017

Use the cmake builtin Threads library detection, and bail out if it
wasn't pthreads. This allows deleting the cmake/FindPTHREAD.cmake file
which complains because pthreads doesn't provide a pkg-config file.

In newer versions of cmake the FindThreads module has the
THREADS_PREFER_PTHREAD_FLAG variable and creates a Threads::Threads
import target. However, in order to remain compatible with older
cmake versions this is not used.

Also, it seems pthreads is used unconditionally in the code, thus don't make it optional in cmakefiles.

Use the cmake builtin Threads library detection, and bail out if it
wasn't pthreads. This allows deleting the cmake/FindPTHREAD.cmake file
which complains because pthreads doesn't provide a pkg-config file.

In newer versions of cmake the FindThreads module has the
THREADS_PREFER_PTHREAD_FLAG variable and creates a Threads::Threads
import target. However, in order top remain compatible with older
cmake version this is not used.
@antenore
Copy link
Member

This is good! I had problems, from time to time, about this error and I was not sure about the root cause.

Let us test it on some different distros to be sure we can merge it (I'm quite sure it will be safe).

If travis doesn't complains it should be already good

@jabl
Copy link
Contributor Author

jabl commented Sep 14, 2017

Good. FWIW, I developed and tested this on Ubuntu 16.04 amd64.

@antenore
Copy link
Member

Tested, it's fine for me, I'm merging.

@antenore antenore merged commit 940b935 into FreeRDP:next Sep 14, 2017
@antenore antenore removed the request for review from giox069 September 14, 2017 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants