[PATCH v2] Makefile: remove the NO_R_TO_GCC_LINKER flag

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

Change our default CC_LD_DYNPATH invocation to something GCC likes
these days. Since the GCC 4.6 release unknown flags haven't been
passed through to ld(1). Thus our previous default of CC_LD_DYNPATH=-R
would cause an error on modern GCC unless NO_R_TO_GCC_LINKER was set.

This CC_LD_DYNPATH flag is really obscure, and I don't expect anyone
except those working on git development ever use this.

It's not needed to simply link to libraries like say libpcre,
but *only* for those cases where we're linking to such a library not
present in the OS's library directories. See e.g. ldconfig(8) on Linux
for more details.

I use this to compile my git with a LIBPCREDIR=$HOME/g/pcre2/inst as
I'm building that from source, but someone maintaining an OS package
is almost certainly not going to use this. They're just going to set
USE_LIBPCRE=YesPlease after installing the libpcre dependency,
which'll point to OS libraries which ld(1) will find without the help
of CC_LD_DYNPATH.

Another thing that helps mitigate any potential breakage is that we
detect the right type of invocation in configure.ac, which e.g. HP/UX
uses[1], as does IBM's AIX package[2]. From what I can tell both AIX
and Solaris packagers are building git with GCC, so I'm not adding a
corresponding config.mak.uname default to cater to their OS-native
linkers.

Now for an overview of past development in this area:

Our use of "-R" dates back to 455a7f3275 ("More portability.",
2005-09-30). Soon after that in bbfc63dd78 ("gcc does not necessarily
pass runtime libpath with -R", 2006-12-27) the NO_R_TO_GCC flag was
added, allowing optional use of "-Wl,-rpath=".

Then in f5b904db6b ("Makefile: Allow CC_LD_DYNPATH to be overriden",
2008-08-16) the ability to override this flag to something else
entirely was added, as some linkers use neither "-Wl,-rpath," nor
"-R".

From what I can tell we should, with the benefit of hindsight, have
made this change back in 2006. GCC & ld supported this type of
invocation back then, or since at least binutils-gdb.git's[3]
a1ad915dc4 ("[...]Add support for -rpath[...]", 1994-07-20).

Further reading and prior art can be found at [4][5][6][7]. Making a
plain "-R" an error seems from reading those reports to have been
introduced in GCC 4.6 released on March 25, 2011[8], but I couldn't
confirm this with absolute certainty, its release notes are ambiguous
on the subject, and I couldn't be bothered to try to build & bisect it
against GCC 4.5.

1. https://public-inbox.org/git/20190516093412.14795-1-avarab@gmail.com/
2. https://www.ibm.com/developerworks/aix/library/aix-toolbox/alpha.html
3. git://sourceware.org/git/binutils-gdb.git
4. https://github.com/tsuna/boost.m4/issues/15
5. https://bugzilla.gnome.org/show_bug.cgi?id=641416
6. https://stackoverflow.com/questions/12629042/g-4-6-real-error-unrecognized-option-r
7. https://curl.haxx.se/mail/archive-2014-11/0005.html
8. https://gcc.gnu.org/gcc-4.6/changes.html

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Fri, May 17 2019, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
>
>> Far be it from me to care about AIX, but it seems like this is ripe for
>> regressions, because we don't know which platforms were relying on "-R"
>> instead of "-Wl,-rpath", and now everybody will be using the latter by
>> default.
>
> I do not have a stake in AIX, either, but I had the same reaction.

I did a bad job of summarizing why this change makes sense. Here's a
v2 with a changed commit message. The first 4 pargaraphs are most
relevant.

Range-diff:
1:  bd9558b1cf ! 1:  56abcd0fae Makefile: remove the NO_R_TO_GCC_LINKER flag
    @@ -2,13 +2,34 @@
     
         Makefile: remove the NO_R_TO_GCC_LINKER flag
     
    -    Remove the NO_R_TO_GCC_LINKER flag, thus switching the default to
    -    "-Wl,-rpath,$LIBPATH" instead of our current "-R$LIBPATH". This is a
    -    relatively obscure thing that only kicks in when using one of the
    -    LIBDIR flags, e.g. LIBPCREDIR or CURLDIR.
    +    Change our default CC_LD_DYNPATH invocation to something GCC likes
    +    these days. Since the GCC 4.6 release unknown flags haven't been
    +    passed through to ld(1). Thus our previous default of CC_LD_DYNPATH=-R
    +    would cause an error on modern GCC unless NO_R_TO_GCC_LINKER was set.
     
    -    How we invoke the linker to do this can still be overridden with
    -    CC_LD_DYNPATH, as seen in our configure.ac script.
    +    This CC_LD_DYNPATH flag is really obscure, and I don't expect anyone
    +    except those working on git development ever use this.
    +
    +    It's not needed to simply link to libraries like say libpcre,
    +    but *only* for those cases where we're linking to such a library not
    +    present in the OS's library directories. See e.g. ldconfig(8) on Linux
    +    for more details.
    +
    +    I use this to compile my git with a LIBPCREDIR=$HOME/g/pcre2/inst as
    +    I'm building that from source, but someone maintaining an OS package
    +    is almost certainly not going to use this. They're just going to set
    +    USE_LIBPCRE=YesPlease after installing the libpcre dependency,
    +    which'll point to OS libraries which ld(1) will find without the help
    +    of CC_LD_DYNPATH.
    +
    +    Another thing that helps mitigate any potential breakage is that we
    +    detect the right type of invocation in configure.ac, which e.g. HP/UX
    +    uses[1], as does IBM's AIX package[2]. From what I can tell both AIX
    +    and Solaris packagers are building git with GCC, so I'm not adding a
    +    corresponding config.mak.uname default to cater to their OS-native
    +    linkers.
    +
    +    Now for an overview of past development in this area:
     
         Our use of "-R" dates back to 455a7f3275 ("More portability.",
         2005-09-30). Soon after that in bbfc63dd78 ("gcc does not necessarily
    @@ -22,32 +43,24 @@
     
         From what I can tell we should, with the benefit of hindsight, have
         made this change back in 2006. GCC & ld supported this type of
    -    invocation back then, or since at least binutils-gdb.git's[1]
    -    a1ad915dc4 ("[...]Add support for -rpath[...]", 1994-07-20). Most
    -    people compiling git with a custom LIBDIR are going to be on a GNU-ish
    -    system, and having to provide this NO_R_TO_GCC_LINKER flag on top of a
    -    custom LIBDIR is annoying.
    -
    -    There are some OS's that don't support -rpath, e.g. AIX ld just
    -    supports "-R". Perhaps we should follow this up with some
    -    config.mak.uname changes, but as noted it's quite possible that nobody
    -    on these platforms uses this (instead libraries in the system's search
    -    path). We *could* also use "-Wl,-R", but let's not introduce something
    -    new.
    +    invocation back then, or since at least binutils-gdb.git's[3]
    +    a1ad915dc4 ("[...]Add support for -rpath[...]", 1994-07-20).
     
    -    Further reading and prior art can be found at [2][3][4][5]. Making a
    +    Further reading and prior art can be found at [4][5][6][7]. Making a
         plain "-R" an error seems from reading those reports to have been
    -    introduced in GCC 4.6 released on March 25, 2011, but I couldn't
    +    introduced in GCC 4.6 released on March 25, 2011[8], but I couldn't
         confirm this with absolute certainty, its release notes are ambiguous
         on the subject, and I couldn't be bothered to try to build & bisect it
         against GCC 4.5.
     
    -    1. git://sourceware.org/git/binutils-gdb.git
    -    2. https://github.com/tsuna/boost.m4/issues/15
    -    3. https://bugzilla.gnome.org/show_bug.cgi?id=641416
    -    4. https://stackoverflow.com/questions/12629042/g-4-6-real-error-unrecognized-option-r
    -    5. https://curl.haxx.se/mail/archive-2014-11/0005.html
    -    6. https://gcc.gnu.org/gcc-4.6/changes.html
    +    1. https://public-inbox.org/git/20190516093412.14795-1-avarab@gmail.com/
    +    2. https://www.ibm.com/developerworks/aix/library/aix-toolbox/alpha.html
    +    3. git://sourceware.org/git/binutils-gdb.git
    +    4. https://github.com/tsuna/boost.m4/issues/15
    +    5. https://bugzilla.gnome.org/show_bug.cgi?id=641416
    +    6. https://stackoverflow.com/questions/12629042/g-4-6-real-error-unrecognized-option-r
    +    7. https://curl.haxx.se/mail/archive-2014-11/0005.html
    +    8. https://gcc.gnu.org/gcc-4.6/changes.html
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     

 Makefile | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/Makefile b/Makefile
index f965509b3c..ce7a489d64 100644
--- a/Makefile
+++ b/Makefile
@@ -265,10 +265,6 @@ all::
 #
 # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.
 #
-# Define NO_R_TO_GCC_LINKER if your gcc does not like "-R/path/lib"
-# that tells runtime paths to dynamic libraries;
-# "-Wl,-rpath=/path/lib" is used instead.
-#
 # Define NO_NORETURN if using buggy versions of gcc 4.6+ and profile feedback,
 # as the compiler can crash (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49299)
 #
@@ -1160,6 +1156,7 @@ endif
 # which'll override these defaults.
 CFLAGS = -g -O2 -Wall
 LDFLAGS =
+CC_LD_DYNPATH = -Wl,-rpath,
 BASIC_CFLAGS = -I.
 BASIC_LDFLAGS =
 
@@ -1287,16 +1284,6 @@ ifeq ($(uname_S),Darwin)
 	PTHREAD_LIBS =
 endif
 
-ifndef CC_LD_DYNPATH
-	ifdef NO_R_TO_GCC_LINKER
-		# Some gcc does not accept and pass -R to the linker to specify
-		# the runtime dynamic library path.
-		CC_LD_DYNPATH = -Wl,-rpath,
-	else
-		CC_LD_DYNPATH = -R
-	endif
-endif
-
 ifdef NO_LIBGEN_H
 	COMPAT_CFLAGS += -DNO_LIBGEN_H
 	COMPAT_OBJS += compat/basename.o
-- 
2.21.0.1020.gf2820cf01a