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

[Jonathan Nieder <jrnieder@gmail.com>]

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?

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

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