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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:06:36

Remi Pommarel [off-list ref] 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.

quoted hunk
Signed-off-by: Remi Pommarel <redacted>
---
 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 $@ $^

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

From: Remi Pommarel <hidden>
Date: 2016-06-15 23:06:36

On Thu, Sep 17, 2015 at 12:51:18PM -0700, Junio C Hamano wrote:
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.
I think it is correct as anywhere else in Makefile the order is ssl
first then crypto (from Makefile:1047):

        ifdef NEEDS_SSL_WITH_CURL
                CURL_LIBCURL += -lssl
                ifdef NEEDS_CRYPTO_WITH_SSL
                        CURL_LIBCURL += -lcrypto
                endif
        endif

Other changes in this patch looks sensible, though.
If you agree with the NEED_SSL_WITH_CURL modification, I'll resend a
patch with the typos corrected.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help