configure: -lpthread doesn't belong in CFLAGS

configure: -lpthread doesn't belong in CFLAGS

[Rainer M. Canavan]

Hi,

some linkers, namely the one on IRIX are rather strict concerning the 
order or arguments for symbol resolution, i.e. no libraries listed
before objects or other libraries on the command line are considered
for symbol resolution. That means that -lpthread can't work if it's 
put in CFLAGS, because it will not be considered for resolving 
pthread_key_create in conftest.o. Apparently only $LIBS goes
after conftest.o when the linker is called.

Without the patch below, the POSIX Threads with '-lpthread' fails, 
with the patch it succeeds. I haven't checked if that is relevant
at all, since that check is immediately followed by 
checking for pthread_create in -lpthread... yes


regards,


rainer


--- ../src/git-2.6.2/configure.ac	Fri Oct 16 23:58:26 CEST 2015
+++ configure.ac	Sun Nov 01 23:19:41 CET 2015
@@ -1126,7 +1126,13 @@
   # would then trigger compiler warnings on every single file we compile.
   for opt in "" -mt -pthread -lpthread; do
      old_CFLAGS="$CFLAGS"
-     CFLAGS="$opt $CFLAGS"
+     old_LIBS="$LIBS"
+     if test "$(echo $opt | cut -b 1-2)" = -l ; then
+        LIBS="$opt $LIBS"
+     else
+        CFLAGS="$opt $CFLAGS"
+     fi
+
      AC_MSG_CHECKING([for POSIX Threads with '$opt'])
      AC_LINK_IFELSE([PTHREADTEST_SRC],
 	[AC_MSG_RESULT([yes])
@@ -1138,6 +1144,7 @@
 	],
 	[AC_MSG_RESULT([no])])
       CFLAGS="$old_CFLAGS"
+      LIBS="$old_LIBS"
   done
   if test $threads_found != yes; then
     AC_CHECK_LIB([pthread], [pthread_create],

Re: configure: -lpthread doesn't belong in CFLAGS

[Matthieu Moy]

"Rainer M. Canavan" <git@canavan.de> writes:

> Hi,

Hi,

Thanks for the patch. However, it will need a bit more work to be
integrated into git.git.

Please, read
https://github.com/git/git/blob/master/Documentation/SubmittingPatches

The body of your email should end up being the commit message. It is not
(yet) properly written as such. Also, read about Developer's Certificate
of Origin in the document above.

>    for opt in "" -mt -pthread -lpthread; do
>       old_CFLAGS="$CFLAGS"
> -     CFLAGS="$opt $CFLAGS"
> +     old_LIBS="$LIBS"
> +     if test "$(echo $opt | cut -b 1-2)" = -l ; then

Don't use "echo" on string that may begin with - (different versions of
echo will have different behaviors). One option is to use "printf" which
is more robust. Another one, more elegant IMHO in this case is to use a
case:

case "$opt" in
   -l*)
      # ...
      ;;
   *)
      # ...
      ;;
esac

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCH] In configure.ac, try -lpthread in $LIBS instead of $CFLAGS to make picky linkers happy

[Rainer M. Canavan]

Some linkers, namely the one on IRIX are rather strict concerning the order or
arguments for symbol resolution, i.e. no libraries listed before objects or
other libraries on the command line are considered for symbol resolution. 
Therefore, -lpthread can't work if it's put in CFLAGS, because it will not be
considered for resolving pthread_key_create in conftest.o. Use $LIBS instead.

Signed-off-by: Rainer Canavan <git@canavan.de>
---
 configure.ac | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index fd22d41..1f55009 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1149,7 +1149,12 @@ elif test -z "$PTHREAD_CFLAGS"; then
   # would then trigger compiler warnings on every single file we compile.
   for opt in "" -mt -pthread -lpthread; do
      old_CFLAGS="$CFLAGS"
-     CFLAGS="$opt $CFLAGS"
+     old_LIBS="$LIBS"
+     case "$opt" in
+        -l*)  LIBS="$opt $LIBS" ;;
+        *)    CFLAGS="$opt $CFLAGS" ;;
+     esac
+
      AC_MSG_CHECKING([for POSIX Threads with '$opt'])
      AC_LINK_IFELSE([PTHREADTEST_SRC],
 	[AC_MSG_RESULT([yes])
@@ -1161,6 +1166,7 @@ elif test -z "$PTHREAD_CFLAGS"; then
 	],
 	[AC_MSG_RESULT([no])])
       CFLAGS="$old_CFLAGS"
+      LIBS="$old_LIBS"
   done
   if test $threads_found != yes; then
     AC_CHECK_LIB([pthread], [pthread_create],
-- 
2.6.2

Re: [PATCH] In configure.ac, try -lpthread in $LIBS instead of $CFLAGS to make picky linkers happy

[Matthieu Moy]

"Rainer M. Canavan" <git@canavan.de> writes:

> Subject: Re: [PATCH] In configure.ac, try -lpthread in $LIBS instead of $CFLAGS to make picky linkers happy

The patch looks good, but the subject should be improved. We normally
try to stick to 50 characters (to let "git log --oneline" fit in a 80
columns terminal). 50 is a bit strict and not always followed, but yours
is definitely too long.

Also, the convention is "subsystem: what you did". I'd suggest this:

configure.ac: try -lpthread in $LIBS instead of $CFLAGS

The "to make picky linkers happy" is important, but redundant with the
body of the message.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCH] configure.ac: try -lpthread in $LIBS instead of $CFLAGS

[Rainer M. Canavan]

Some linkers, namely the one on IRIX are rather strict concerning the order or
arguments for symbol resolution, i.e. no libraries listed before objects or
other libraries on the command line are considered for symbol resolution. That
means that -lpthread can't work if it's put in CFLAGS, because it will not be
considered for resolving pthread_key_create in conftest.o. Use $LIBS instead.

Signed-off-by: Rainer Canavan <git@canavan.de>
---
 configure.ac | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index fd22d41..1f55009 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1149,7 +1149,12 @@ elif test -z "$PTHREAD_CFLAGS"; then
   # would then trigger compiler warnings on every single file we compile.
   for opt in "" -mt -pthread -lpthread; do
      old_CFLAGS="$CFLAGS"
-     CFLAGS="$opt $CFLAGS"
+     old_LIBS="$LIBS"
+     case "$opt" in
+        -l*)  LIBS="$opt $LIBS" ;;
+        *)    CFLAGS="$opt $CFLAGS" ;;
+     esac
+
      AC_MSG_CHECKING([for POSIX Threads with '$opt'])
      AC_LINK_IFELSE([PTHREADTEST_SRC],
 	[AC_MSG_RESULT([yes])
@@ -1161,6 +1166,7 @@ elif test -z "$PTHREAD_CFLAGS"; then
 	],
 	[AC_MSG_RESULT([no])])
       CFLAGS="$old_CFLAGS"
+      LIBS="$old_LIBS"
   done
   if test $threads_found != yes; then
     AC_CHECK_LIB([pthread], [pthread_create],
-- 
2.6.2

Re: [PATCH] configure.ac: try -lpthread in $LIBS instead of $CFLAGS

[Matthieu Moy]

"Rainer M. Canavan" <git@canavan.de> writes:

>  configure.ac | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Looks all right to me. Thanks for you contribution and for your
patience!

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] configure.ac: try -lpthread in $LIBS instead of $CFLAGS

[Jeff King]

On Sun, Nov 08, 2015 at 05:00:00PM +0100, Matthieu Moy wrote:

> "Rainer M. Canavan" <git@canavan.de> writes:
> 
> >  configure.ac | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> Looks all right to me. Thanks for you contribution and for your
> patience!

Thanks, both. I've queued this to eventually hit "maint".

-Peff