Update CMake threads library detection

[mosullivan93]

When glibc < 2.34, -lpthread or -pthread needs to be present in order for this project to be linked successfully. CMake provides the @CMAKE_THREAD_LIBS_INIT@ variable that will expand to the necessary flag. This will typically expand to -lpthreads, but users can override this behaviour if they wish by setting the THREADS_PREFER_PTHREAD_FLAG variable to TRUE/ON/etc.

This patch ensures the thread library is always present in the linker options and restores the default behaviour.

[dmah42]

we need to keep this setting to Libs.private though please. pthread is only required to link benchmark, not to link against benchmark.

[mosullivan93]

As far as I can tell (if I understand you correctly), -lpthread is required when linking against benchmark. Based off of that conclusion it should be redundant to be in both Libs.private and Libs.

I was only using pkg-config inside of a Makefile and my system (glibc 2.31) would not link the code that depends on -lbenchmark unless I added in -lpthreads myself. For example, when I build the demonstration file from the README using g++ demo.cpp `pkg-config --cflags --libs benchmark` -o demo with benchmarks v1.8.0 I get

/usr/bin/ld: /usr/local/lib/libbenchmark.a(benchmark_runner.cc.o): in function `benchmark::internal::BenchmarkRunner::DoNIterations()':
benchmark_runner.cc:(.text+0x1418): undefined reference to `pthread_create'
collect2: error: ld returned 1 exit status

Edit: Anyone in future getting here from Google... add --static to pkg-config as described below or use a smarter build system 😜

[LebedevRI]

libbenchmark.a is a static library, it itself isn't linked to the -lpthread (or to anything, really),
so naturally, when linking to the static library, you need to also link to all the libraries
said library would have linked against. This isn't the case for shared libraries.

[mosullivan93]

Well that might explain why pkg-config --static --libs brings out -lpthread... Please hold.

[LebedevRI]

Why is this being removed?

[mosullivan93]

Whether -pthread or -lpthread is the right choice is not something I am knowledgeable enough to weigh in on. It's been discussed here a few times (#919 #771) and at googletest/#2482), too. I wanted to make as few decisions as possible, so I removed this to allow the default behaviour (@CMAKE_THREAD_LIBS_INIT@ will give -lpthread).

If these lines remained, I don't think there would be a way for someone to prefer -lpthread as THREADS_PREFER_PTHREAD_FLAG ON would always override it.

[LebedevRI]

I'm 99.99% sure this line should remain.

[mosullivan93]

@LebedevRI @dmah42 I'm happy to close this PR. I've learned that if using pkg-config on its own you need to include the --static flag to resolve the correct linker arguments. That fixes my original problem: broken in glibc 2.31, working in glibc 2.35.

I get the feeling it's best to keep things as they are (where you've had no complaints for quite some time) rather than change these flags in order to allow it to accidentally work correctly across both libc environments.

[LebedevRI]

@LebedevRI @dmah42 I'm happy to close this PR. I've learned that if using pkg-config on its own you need to include the --static flag to resolve the correct linker arguments. That fixes my original problem: broken in glibc 2.31, working in glibc 2.35.

Nice!

I get the feeling it's best to keep things as they are (where you've had no complaints for quite some time) rather than change these flags in order to allow it to accidentally work correctly across both libc environments.

Yeah, i think the LHS of the diff isn't incorrect.