threads / discuss / 40701

configure: -lpthread doesn't belong in CFLAGS

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

## tl;dr

7 messages between Nov 1, 2015 and Nov 20, 2015.

replies: 6people: 3as markdown or json

Rainer M. Canavan· Nov 1, 2015, 22:30 UTC · lore

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],

Matthieu Moy· Nov 2, 2015, 08:27 UTC · re: Rainer M. Canavan · lore

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

"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.

Show 5 quoted lines

>    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/

Rainer M. Canavan· Nov 6, 2015, 01:11 UTC · re: Matthieu Moy · lore

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

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

Matthieu Moy· Nov 6, 2015, 08:25 UTC · re: Rainer M. Canavan · lore

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

"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/

Rainer M. Canavan· Nov 8, 2015, 15:28 UTC · re: Matthieu Moy · lore

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

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

Matthieu Moy· Nov 8, 2015, 16:00 UTC · re: Rainer M. Canavan · lore

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

"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/

Jeff King· Nov 20, 2015, 11:42 UTC · re: Matthieu Moy · lore

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

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

Show 7 quoted lines

> "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