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

From: Benoit SIGOURE <tsuna@lrde.epita.fr>
Date: Oct 4, 2007, 15:59 UTC
Message-ID: <34DAC3CA-E226-4488-8B03-FC45A6A95F78@lrde.epita.fr>
In-Reply-To: <7vejgbdbyn.fsf@gitster.siamese.dyndns.org>

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

Show 31 quoted lines

> Benoit Sigoure <tsuna@lrde.epita.fr> writes:
>
>> 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;
>
>> @@ -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.

Show 45 quoted lines

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

Show 17 quoted lines

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