Re: [PATCH v3 1/1] Makefile: link libcurl before libssl

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 5, 2015, 19:41 UTC
Message-ID: <20151005194134.GD11993@google.com>
In-Reply-To: <1444073043-24618-1-git-send-email-repk@triplefau.lt>

Remi Pommarel wrote:

> For static linking especially library order while linking is important. For

Please be more explicit for the uninitiated: during static linking, the library that needs symbols comes before the library providing those symbols.

> example, libcurl wants symbols for libssl when building http-push, http-fetch
> and remote-curl. So for these programs libcurl has to be linked before libssl.

Based on experience with NEEDS_CRYPTO_WITH_SSL vs NEEDS_SSL_WITH_CRYPTO, these kinds of dependencies can be platform-specific. But a little quick thought reveals that libssl is never going to depend on libcurl and libcurl always needs libssl. Good.

Based on this dependency, shouldn't CURL_LIBCURL always include -lssl when statically linking? How does this relate to NEEDS_SSL_WITH_CURL?

Show 7 quoted lines

> --- a/Makefile
> +++ b/Makefile
> @@ -1029,7 +1029,6 @@ ifdef HAVE_ALLOCA_H
>  endif
>  
>  IMAP_SEND_BUILDDEPS =
> -IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)

To protect against a value that might leak in from the environment, this should say

	IMAP_SEND_LDFLAGS =

[...]

Show 6 quoted lines

> @@ -1971,10 +1971,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)

What happens in the NEEDS_SSL_WITH_CURL=Yes case?

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

Same question.

I guess the general principle is that $(LIBS) should always go at the end. (That would make sense to me.)

Ideally this would be two patches:

 - one putting $(LIBS) at the end everywhere, which is the simple part of the change
 - a second doing some appropriate thing to turn on NEEDS_SSL_WITH_CURL when appropriate
   or something

Sensible?

Thanks, Jonathan