Re: [PATCH] Makefile: Change library order for static linking

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 17, 2015, 19:51 UTC
Message-ID: <xmqqfv2cj03t.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1442524332-10180-1-git-send-email-repk@triplefau.lt>

Remi Pommarel <repk@triplefau.lt> writes:

> Subject: Re: [PATCH] Makefile: Change library order for static linking

s/Change/change/;

But more importantly, I think you can do much better with the same space. Perhaps like this.

    Makefile: link libcurl before openssl and crypto

> For static linking especially library order while linking is important. For
> example libssl contains symbol from libcrypto so the farmer should be linked
> before the latter. The global link order should be libcurl then libssl then
> libcrypto then libintl and finally zlib.

IIRC historically the division between ssl and crypto was messy. Especially, I am not sure if the change to NEEDS_SSL_WITH_CRYPTO in this patch is correct for platforms that require that macro defined.

Other changes in this patch looks sensible, though.

Show 57 quoted lines

>
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
>  Makefile | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 81ac5bb..26a483a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1030,7 +1030,6 @@ ifdef HAVE_ALLOCA_H
>  endif
>  
>  IMAP_SEND_BUILDDEPS =
> -IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
>  
>  ifdef NO_CURL
>  	BASIC_CFLAGS += -DNO_CURL
> @@ -1087,6 +1086,7 @@ else
>  		endif
>  	endif
>  endif
> +IMAP_SEND_LDFLAGS += $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
>  
>  ifdef ZLIB_PATH
>  	BASIC_CFLAGS += -I$(ZLIB_PATH)/include
> @@ -1117,7 +1117,7 @@ ifdef NO_OPENSSL
>  	LIB_4_CRYPTO =
>  else
>  ifdef NEEDS_SSL_WITH_CRYPTO
> -	LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto -lssl
> +	LIB_4_CRYPTO = $(OPENSSL_LINK) -lssl -lcrypto
>  else
>  	LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto
>  endif
> @@ -1972,10 +1972,10 @@ git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
>  
>  git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> -		$(LIBS) $(CURL_LIBCURL)
> +		$(CURL_LIBCURL) $(LIBS)
>  git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> -		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
> +		$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
>  
>  git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \
> @@ -1989,7 +1989,7 @@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
>  
>  $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> -		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
> +		$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
>  
>  $(LIB_FILE): $(LIB_OBJS)
>  	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $^