[PATCH] configure.ac: fix pthreads detection on Mac OS X

[PATCH] configure.ac: fix pthreads detection on Mac OS X

[Max Horn]

The configure script checks whether certain flags are required to use
pthreads. But it did not consider that *none* might be needed (as is the
case on Mac OS X). This lead to configure adding "-mt" to the list of
flags (which does nothing on OS X except producing a warning). This in
turn triggered a compiler warning on every single file.

To solve this, we now first check if pthreads work without extra flags.
This means the check is now order dependant, hence a comment is added
explaining this, and the reasons for it.

Note that it might be possible to write an order independent test, but
it does not seem worth the extra effort required for implementing and
testing such a solution, when this simple solution exists and works.

Signed-off-by: Max Horn <max@quendi.de>
---

This is actually a revised version from my patch
 "Change configure to check if pthreads are usable without any extra flags"
from July. I simply had forgotten all about it :-(.

Chers,
Max

 configure.ac | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index ad215cc..41ac9a5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1021,7 +1021,17 @@ if test -n "$USER_NOPTHREAD"; then
 # -D_REENTRANT' or some such.
 elif test -z "$PTHREAD_CFLAGS"; then
   threads_found=no
-  for opt in -mt -pthread -lpthread; do
+  # Attempt to compile and link some code using pthreads to determine
+  # required linker flags. The order is somewhat important here: We
+  # first try it without any extra flags, to catch systems where
+  # pthreads are part of the C library, then go on testing various other
+  # flags. We do so to avoid false positives. For example, on Mac OS X
+  # pthreads are part of the C library; moreover, the compiler allows us
+  # to add "-mt" to the CFLAGS (although it will do nothing except
+  # trigger a warning about an unused flag). Hence if we checked for
+  # "-mt" before "" we would end up picking it. But unfortunately this
+  # would then trigger compiler warnings on every single file we compile.
+  for opt in "" -mt -pthread -lpthread; do
      old_CFLAGS="$CFLAGS"
      CFLAGS="$opt $CFLAGS"
      AC_MSG_CHECKING([for POSIX Threads with '$opt'])
-- 
1.8.0.393.gcc9701d

Re: [PATCH] configure.ac: fix pthreads detection on Mac OS X

[Junio C Hamano]

Max Horn <max@quendi.de> writes:

> The configure script checks whether certain flags are required to use
> pthreads. But it did not consider that *none* might be needed (as is the
> case on Mac OS X). This lead to configure adding "-mt" to the list of
> flags (which does nothing on OS X except producing a warning). This in
> turn triggered a compiler warning on every single file.
>
> To solve this, we now first check if pthreads work without extra flags.
> This means the check is now order dependant, hence a comment is added
> explaining this, and the reasons for it.
>
> Note that it might be possible to write an order independent test, but
> it does not seem worth the extra effort required for implementing and
> testing such a solution, when this simple solution exists and works.
>
> Signed-off-by: Max Horn <max@quendi.de>
> ---
>
> This is actually a revised version from my patch
>  "Change configure to check if pthreads are usable without any extra flags"
> from July. I simply had forgotten all about it :-(.

Will queue, but we would need wider testing to avoid "compiles well
without an option but fails to link" issues similar to cea13a8
(Improve test for pthreads flag, 2011-03-28) on other people's
platforms (I know you tested on Mac OS X and over there it compiles
and links well---I am worried about others).

Thanks.

> Chers,
> Max
>
>  configure.ac | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index ad215cc..41ac9a5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1021,7 +1021,17 @@ if test -n "$USER_NOPTHREAD"; then
>  # -D_REENTRANT' or some such.
>  elif test -z "$PTHREAD_CFLAGS"; then
>    threads_found=no
> -  for opt in -mt -pthread -lpthread; do
> +  # Attempt to compile and link some code using pthreads to determine
> +  # required linker flags. The order is somewhat important here: We
> +  # first try it without any extra flags, to catch systems where
> +  # pthreads are part of the C library, then go on testing various other
> +  # flags. We do so to avoid false positives. For example, on Mac OS X
> +  # pthreads are part of the C library; moreover, the compiler allows us
> +  # to add "-mt" to the CFLAGS (although it will do nothing except
> +  # trigger a warning about an unused flag). Hence if we checked for
> +  # "-mt" before "" we would end up picking it. But unfortunately this
> +  # would then trigger compiler warnings on every single file we compile.
> +  for opt in "" -mt -pthread -lpthread; do
>       old_CFLAGS="$CFLAGS"
>       CFLAGS="$opt $CFLAGS"
>       AC_MSG_CHECKING([for POSIX Threads with '$opt'])

Re: [PATCH] configure.ac: fix pthreads detection on Mac OS X

[Max Horn]

On 28.11.2012, at 07:38, Junio C Hamano wrote:

> Max Horn <max@quendi.de> writes:
> 
>> The configure script checks whether certain flags are required to use
>> pthreads. But it did not consider that *none* might be needed (as is the
>> case on Mac OS X). This lead to configure adding "-mt" to the list of
>> flags (which does nothing on OS X except producing a warning). This in
>> turn triggered a compiler warning on every single file.
>> 
>> To solve this, we now first check if pthreads work without extra flags.
>> This means the check is now order dependant, hence a comment is added
>> explaining this, and the reasons for it.
>> 
>> Note that it might be possible to write an order independent test, but
>> it does not seem worth the extra effort required for implementing and
>> testing such a solution, when this simple solution exists and works.
>> 
>> Signed-off-by: Max Horn <max@quendi.de>
>> ---
>> 
>> This is actually a revised version from my patch
>> "Change configure to check if pthreads are usable without any extra flags"
>> from July. I simply had forgotten all about it :-(.
> 
> Will queue,

OK

> but we would need wider testing to avoid "compiles well
> without an option but fails to link" issues similar to cea13a8
> (Improve test for pthreads flag, 2011-03-28) on other people's
> platforms (I know you tested on Mac OS X and over there it compiles
> and links well---I am worried about others).

Sure, understood. Though note that the test in question performs a compile & link test. So I have a hard time to see how this could break something. Then again, I dabbled in portable code long enough to never say never ;-).

BTW, is there such a thing as a build farm for git which automatically compiles and runs tests for pu / next / main, across a variety of platforms? Or does it all rely on devs test building everything regularly?


Cheers,
Max