| Message ID | 20190516180521.1933-1-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Makefile: remove the NO_R_TO_GCC_LINKER flag | expand |
On Thu, May 16, 2019 at 08:05:21PM +0200, Ævar Arnfjörð Bjarmason wrote: > 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. So...does this mean with just your patch that the build is now broken on AIX if you use CURLDIR? 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. -Peff
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. Those who found having to give NO_R_TO_GCC_LINKER=NoThanks annoying could have passed CC_LD_DYNPATH=-Wl,-rpath instead, but that is not much shorter or sweeter X-<; with this change, they do not have to do anything, but those who are broken can pass CC_LD_DYNPATH=-R to unbreak it. So it may not be the end of the world, but this move certainly smells like robbing non-GCC users to pay GCC users to me.
On Fri, May 17, 2019 at 08:21:09AM +0900, 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. > > Those who found having to give NO_R_TO_GCC_LINKER=NoThanks annoying > could have passed CC_LD_DYNPATH=-Wl,-rpath instead, but that is not > much shorter or sweeter X-<; with this change, they do not have to > do anything, but those who are broken can pass CC_LD_DYNPATH=-R to > unbreak it. So it may not be the end of the world, but this move > certainly smells like robbing non-GCC users to pay GCC users to me. Thanks, I wasn't quite clear on whether there was an escape hatch for people who were broken (I took Ævar's "perhaps we should follow this up" meaning we needed to add new knobs, but it is really just "perhaps we should set this existing knob by default on AIX"). So that at least quells the worst of my worries. After that, I agree it's now just a question of the default. I don't have a sense whether the majority is helped or hurt here (I admit that I did not even know about these flags, despite compiling with gcc for the past 13 years, so possibly everybody was perfectly happy with the existing behavior). -Peff
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
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. How we invoke the linker to do this can still be overridden with CC_LD_DYNPATH, as seen in our configure.ac script. 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[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. Further reading and prior art can be found at [2][3][4][5]. 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 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 Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Looking at that HP/UX configure patch I was reminded of being annoyed by the NO_R_TO_GCC_LINKER flag. Makefile | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-)