threads / patch / 36525

v3, 2 partsMakefile: allow static linking against libcurl

Subject: [PATCH v3 2/2] Makefile: allow static linking against libcurl

## tl;dr

7 messages between Apr 28, 2014 and Apr 28, 2014. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Dave Borowitz· Apr 28, 2014, 19:35 UTC · lore

[PATCH v3 1/2] Makefile: use curl-config to determine curl flags

curl-config is usually installed alongside a curl distribution, and its purpose is to provide flags for building against libcurl, so use it instead of guessing flags and dependent libraries.

Allow overriding CURL_CONFIG to a custom path to curl-config, to compile against a curl installation other than the first in PATH.

Depending on the set of features curl is compiled with, there may be more libraries required than the previous two options of -lssl and -lidn. For example, with a vanilla build of libcurl-7.36.0 on Mac OS X 10.9:

$ ~/d/curl-out-7.36.0/lib/curl-config --libs -L/Users/dborowitz/d/curl-out-7.36.0/lib -lcurl -lgssapi_krb5 -lresolv -lldap -lz

Use this only when CURLDIR is not explicitly specified, to continue supporting older builds. Moreover, if CURL_CONFIG is unset or running it returns no results (e.g. because it is missing), default to the old behavior of blindly setting -lcurl.

Signed-off-by: Dave Borowitz <dborowitz@google.com>
---
 Makefile | 51 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 14 deletions(-)
Show changes to Makefile +37 −14
diff --git a/Makefile b/Makefile
index 2128ce3..cb4ee37 100644
--- a/Makefile
+++ b/Makefile
@@ -34,8 +34,14 @@ all::
 # git-http-push are not built, and you cannot use http:// and https://
 # transports (neither smart nor dumb).
 #
+# Define CURL_CONFIG to the path to a curl-config binary other than the
+# default 'curl-config'. If CURL_CONFIG is unset or points to a binary that
+# is not found, defaults to the CURLDIR behavior, or if CURLDIR is not set,
+# uses -lcurl with no additional library detection.
+#
 # Define CURLDIR=/foo/bar if your curl header and library files are in
-# /foo/bar/include and /foo/bar/lib directories.
+# /foo/bar/include and /foo/bar/lib directories.  This overrides CURL_CONFIG,
+# but is less robust.
 #
 # Define NO_EXPAT if you do not have expat installed.  git-http-push is
 # not built, and you cannot push using http:// and https:// transports (dumb).
@@ -143,9 +149,11 @@ all::
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
 #
-# Define NEEDS_SSL_WITH_CURL if you need -lssl with -lcurl (Minix).
+# Define NEEDS_SSL_WITH_CURL if you need -lssl with -lcurl (Minix).  Only used
+# if CURLDIR is set.
 #
-# Define NEEDS_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix).
+# Define NEEDS_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix).  Only
+# used if CURLDIR is set.
 #
 # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin).
 #
@@ -1118,20 +1126,35 @@ ifdef NO_CURL
 	REMOTE_CURL_NAMES =
 else
 	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=
 	else
-		CURL_LIBCURL = -lcurl
-	endif
-	ifdef NEEDS_SSL_WITH_CURL
-		CURL_LIBCURL += -lssl
-		ifdef NEEDS_CRYPTO_WITH_SSL
-			CURL_LIBCURL += -lcrypto
+		CURL_CONFIG ?= curl-config
+		ifeq "$(CURL_CONFIG)" ""
+			CURL_LIBCURL =
+		else
+			CURL_LIBCURL := $(shell $(CURL_CONFIG) --libs)
 		endif
 	endif
-	ifdef NEEDS_IDN_WITH_CURL
-		CURL_LIBCURL += -lidn
+
+	ifeq "$(CURL_LIBCURL)" ""
+		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
+		else
+			CURL_LIBCURL = -lcurl
+		endif
+		ifdef NEEDS_SSL_WITH_CURL
+			CURL_LIBCURL += -lssl
+			ifdef NEEDS_CRYPTO_WITH_SSL
+				CURL_LIBCURL += -lcrypto
+			endif
+		endif
+		ifdef NEEDS_IDN_WITH_CURL
+			CURL_LIBCURL += -lidn
+		endif
+	else
+		BASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags)
 	endif
 
 	REMOTE_CURL_PRIMARY = git-remote-http$X
-- 
1.9.1.423.g4596e3a
Dave Borowitz· Apr 28, 2014, 19:35 UTC · re: Dave Borowitz · lore

This requires more flags than can be guessed with the old-style CURLDIR and related options, so is only supported when curl-config is present.

Signed-off-by: Dave Borowitz <dborowitz@google.com>
---
 Makefile | 12 ++++++++++++
 1 file changed, 12 insertions(+)
Show changes to Makefile +12 −0
diff --git a/Makefile b/Makefile
index cb4ee37..360d427 100644
--- a/Makefile
+++ b/Makefile
@@ -39,6 +39,9 @@ all::
 # is not found, defaults to the CURLDIR behavior, or if CURLDIR is not set,
 # uses -lcurl with no additional library detection.
 #
+# Define CURL_STATIC to statically link libcurl.  Only applies if
+# CURL_CONFIG is used.
+#
 # Define CURLDIR=/foo/bar if your curl header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.  This overrides CURL_CONFIG,
 # but is less robust.
@@ -1137,6 +1140,9 @@ else
 	endif
 
 	ifeq "$(CURL_LIBCURL)" ""
+		ifdef CURL_STATIC
+                        $(error "CURL_STATIC must be used with CURL_CONFIG")
+		endif
 		ifdef CURLDIR
 			# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
 			BASIC_CFLAGS += -I$(CURLDIR)/include
@@ -1155,6 +1161,12 @@ else
 		endif
 	else
 		BASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags)
+		ifdef CURL_STATIC
+			CURL_LIBCURL = $(shell $(CURL_CONFIG) --static-libs)
+			ifeq "$(CURL_LIBCURL)" ""
+                                $(error libcurl not detected or not compiled with static support)
+			endif
+		endif
 	endif
 
 	REMOTE_CURL_PRIMARY = git-remote-http$X
-- 
1.9.1.423.g4596e3a
Jonathan Nieder· Apr 28, 2014, 19:44 UTC · re: Dave Borowitz · lore

Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags

Hi,
Dave Borowitz wrote:
> curl-config is usually installed alongside a curl distribution, and
> its purpose is to provide flags for building against libcurl, so use
> it instead of guessing flags and dependent libraries.

The previous version of these two patches is already part of "master". Could you make an incremental patch?

Sorry for the fuss, Jonathan

Dave Borowitz· Apr 28, 2014, 19:51 UTC · re: Jonathan Nieder · lore

Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags

On Mon, Apr 28, 2014 at 12:44 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 10 quoted lines
> Hi,
>
> Dave Borowitz wrote:
>
>> curl-config is usually installed alongside a curl distribution, and
>> its purpose is to provide flags for building against libcurl, so use
>> it instead of guessing flags and dependent libraries.
>
> The previous version of these two patches is already part of "master".
> Could you make an incremental patch?
Done. Thanks for pointing that out, and sorry for the noise.
> Sorry for the fuss,
> Jonathan
Junio C Hamano· Apr 28, 2014, 19:56 UTC · re: Dave Borowitz · lore

Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags

Dave Borowitz <dborowitz@google.com> writes:
Show 13 quoted lines
> Use this only when CURLDIR is not explicitly specified, to continue
> supporting older builds. Moreover, if CURL_CONFIG is unset or running
> it returns no results (e.g. because it is missing), default to the old
> behavior of blindly setting -lcurl.
>  	ifdef CURLDIR
> +		CURL_LIBCURL=
>  	else
> +		CURL_CONFIG ?= curl-config
> +		ifeq "$(CURL_CONFIG)" ""
> +			CURL_LIBCURL =
> +		else
> +			CURL_LIBCURL := $(shell $(CURL_CONFIG) --libs)
>  		endif

This "ifeq" is redundant and will never set CURL_LIBCURL to empty without running the "else" part, I think. In a Makefile, a variable explicitly set to empty and a variable that is unset are treated the same.

	$ cat >Makefile <<EOF
	CURL_CONFIG ?= curl-config
	ifeq "$(CURL_CONFIG)" ""
		X=Empty
	else
		X=NotEmpty
	endif
	ifdef "$(CURL_CONFIG)"
		Z=Defined
	else
		Z=Undefined
	endif
	all::
		@echo "$(X) $(Z)"
	EOF
	$ make -f Makefile CURL_CONFIG=""
	Empty Undefined

That does not mean the patch will give us a broken behaviour, though. It just means the ifeq/else part will be redundant.

>  	endif
> +
> +	ifeq "$(CURL_LIBCURL)" ""

This will catch the "$(shell $(CURL_CONFIG) --libs) assigned an empty string to CURL_LIBCURL" case, so the result is good.

I haven't checked what it would look like if we turn this into an incremental patch to be applied on top of 'master' (which would give us a place to document better why we do not rely on the presense of curl-config), but if we can do so, that would be more preferable than having to revert the merge of the previous one and then applying these two patches anew.

Thanks.
Show 21 quoted lines
> +		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
> +		else
> +			CURL_LIBCURL = -lcurl
> +		endif
> +		ifdef NEEDS_SSL_WITH_CURL
> +			CURL_LIBCURL += -lssl
> +			ifdef NEEDS_CRYPTO_WITH_SSL
> +				CURL_LIBCURL += -lcrypto
> +			endif
> +		endif
> +		ifdef NEEDS_IDN_WITH_CURL
> +			CURL_LIBCURL += -lidn
> +		endif
> +	else
> +		BASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags)
>  	endif
>  
>  	REMOTE_CURL_PRIMARY = git-remote-http$X
Junio C Hamano· Apr 28, 2014, 20:00 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags

Junio C Hamano <gitster@pobox.com> writes:
Show 16 quoted lines
> That does not mean the patch will give us a broken behaviour,
> though.  It just means the ifeq/else part will be redundant.
>
>>  	endif
>> +
>> +	ifeq "$(CURL_LIBCURL)" ""
>
> This will catch the "$(shell $(CURL_CONFIG) --libs) assigned an
> empty string to CURL_LIBCURL" case, so the result is good.
>
> I haven't checked what it would look like if we turn this into an
> incremental patch to be applied on top of 'master' (which would give
> us a place to document better why we do not rely on the presense of
> curl-config), but if we can do so, that would be more preferable
> than having to revert the merge of the previous one and then
> applying these two patches anew.

And I just checked; it is not very pretty to call it "trivially correct", and I would feel safer to revert the merge for 2.0, and queue the new one for the next cycle, cooking it in 'pu' and then 'next' in the meantime.

Junio C Hamano· Apr 28, 2014, 21:13 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags

Junio C Hamano <gitster@pobox.com> writes:
Show 6 quoted lines
> This "ifeq" is redundant and will never set CURL_LIBCURL to empty
> without running the "else" part, I think.  In a Makefile, a variable
> explicitly set to empty and a variable that is unset are treated the
> same....
> 	$ make -f Makefile CURL_CONFIG=""
> 	Empty Undefined
Oh, I was blind.  Please ignore this noise.
Thanks.

← back to recent threads