Re: [PATCH] Be nice with compilers that do not support runtime paths at all.

On Oct 4, 2007, at 1:18 AM, Junio C Hamano wrote:

Benoit Sigoure [off-list ref] writes:

quoted

diff --git a/Makefile b/Makefile
index a1fe443..7c6c453 100644
--- a/Makefile
+++ b/Makefile

@@ -100,6 +100,9 @@ all::
 # that tells runtime paths to dynamic libraries;
 # "-Wl,-rpath=/path/lib" is used instead.
 #
+# Define NO_RPATH if your dynamic loader doesn't support runtime  

paths at
+# all.
+#
 # Define USE_NSEC below if you want git to care about sub-second  
file mtimes
 # and ctimes. Note that you need recent glibc (at least 2.2.4)  
for this, and
 # it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it  
will likely

Thanks for this part;

quoted

@@ -507,6 +510,7 @@ ifeq ($(uname_S),Darwin)
 			BASIC_LDFLAGS += -L/opt/local/lib
 		endif
 	endif
+        NO_RPATH = YesPlease
 endif

I'll let Darwin users to fight the defaults for this part out.

No more replies on this thread, and the Apple documentation confirms  
that there is no rpath support in the dynamic loader of OSX 10.4 and  
before.  I don't know about the soon-to-be-released 10.5 aka Leopard.

quoted

@@ -521,7 +525,10 @@ ifndef NO_CURL
 	ifdef CURLDIR
 		# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
 		BASIC_CFLAGS += -I$(CURLDIR)/include
-		CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$ 

(lib) -lcurl
+		CURL_LIBCURL = -L$(CURLDIR)/$(lib) -lcurl
+ifndef NO_RPATH
+		CURL_LIBCURL += $(CC_LD_DYNPATH)$(CURLDIR)/$(lib)
+endif
 	else
 		CURL_LIBCURL = -lcurl
 	endif

quoted

@@ -539,7 +546,10 @@ endif

 ifdef ZLIB_PATH
 	BASIC_CFLAGS += -I$(ZLIB_PATH)/include
-	EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH)$(ZLIB_PATH)/$ 

(lib)
+	EXTLIBS += -L$(ZLIB_PATH)/$(lib)
+ifndef NO_RPATH
+	EXTLIBS += $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib)
+endif
 endif
 EXTLIBS += -lz

While these parts are ugly but correct, I think...

quoted

@@ -547,7 +557,10 @@ ifndef NO_OPENSSL
 	OPENSSL_LIBSSL = -lssl
 	ifdef OPENSSLDIR
 		BASIC_CFLAGS += -I$(OPENSSLDIR)/include
-		OPENSSL_LINK = -L$(OPENSSLDIR)/$(lib) $(CC_LD_DYNPATH)$ 

(OPENSSLDIR)/$(lib)
+		OPENSSL_LINK = -L$(OPENSSLDIR)/$(lib)
+ifndef NO_RPATH
+		OPENSSL_LINK = $(CC_LD_DYNPATH)$(OPENSSLDIR)/$(lib)
+endif
 	else
 		OPENSSL_LINK =
 	endif

this and the ICONV one are missing s/=/+=/.

You're right, sorry.

If we do not care about supporting too old GNU make, we can do
this by first adding this near the top:

        ifndef NO_RPATH
        LINKER_PATH = -L$(1) $(CC_LD_DYNPATH)$(1)
        else
        LINKER_PATH = -L$(1)
        endif

and then doing something like:

	CURL_LIBCURL = $(call LINKER_PATH,$(CURLDIR)/$(lib))
	OPENSSL_LINK = $(call LINKER_PATH,$(OPENSSLDIR)/$(lib))

to make it easier to read and less error prone.

Yes.  I can rework the patch, but the question is: do you care about  
old GNU make?  Can I rewrite the patch with this feature?

Thanks.

Cheers,

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory