{"thread":{"id":"36525","subject":"[PATCH v3 2/2] Makefile: allow static linking against libcurl","startedAt":"2014-04-28T19:35:03Z","lastAt":"2014-04-28T21:13:38Z","messageCount":7,"participants":["Dave Borowitz","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"240010","messageId":"1398713704-15428-1-git-send-email-dborowitz@google.com","threadId":"36525","inReplyTo":null,"subject":"[PATCH v3 1/2] Makefile: use curl-config to determine curl flags","fromName":"Dave Borowitz","fromEmail":"dborowitz@google.com","sentAt":"2014-04-28T19:35:03Z","receivedAt":"2014-04-28T19:35:03Z","isPatch":true,"sender":{"key":"dborowitz@google.com","avatar":"https://avatars.githubusercontent.com/u/194927?v=4"},"body":"curl-config is usually installed alongside a curl distribution, and\nits purpose is to provide flags for building against libcurl, so use\nit instead of guessing flags and dependent libraries.\n\nAllow overriding CURL_CONFIG to a custom path to curl-config, to\ncompile against a curl installation other than the first in PATH.\n\nDepending on the set of features curl is compiled with, there may be\nmore libraries required than the previous two options of -lssl and\n-lidn. For example, with a vanilla build of libcurl-7.36.0 on Mac OS X\n10.9:\n\n$ ~/d/curl-out-7.36.0/lib/curl-config --libs\n-L/Users/dborowitz/d/curl-out-7.36.0/lib -lcurl -lgssapi_krb5 -lresolv -lldap -lz\n\nUse this only when CURLDIR is not explicitly specified, to continue\nsupporting older builds. Moreover, if CURL_CONFIG is unset or running\nit returns no results (e.g. because it is missing), default to the old\nbehavior of blindly setting -lcurl.\n\nSigned-off-by: Dave Borowitz <dborowitz@google.com>\n---\n Makefile | 51 +++++++++++++++++++++++++++++++++++++--------------\n 1 file changed, 37 insertions(+), 14 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 2128ce3..cb4ee37 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -34,8 +34,14 @@ all::\n # git-http-push are not built, and you cannot use http:// and https://\n # transports (neither smart nor dumb).\n #\n+# Define CURL_CONFIG to the path to a curl-config binary other than the\n+# default 'curl-config'. If CURL_CONFIG is unset or points to a binary that\n+# is not found, defaults to the CURLDIR behavior, or if CURLDIR is not set,\n+# uses -lcurl with no additional library detection.\n+#\n # Define CURLDIR=/foo/bar if your curl header and library files are in\n-# /foo/bar/include and /foo/bar/lib directories.\n+# /foo/bar/include and /foo/bar/lib directories.  This overrides CURL_CONFIG,\n+# but is less robust.\n #\n # Define NO_EXPAT if you do not have expat installed.  git-http-push is\n # not built, and you cannot push using http:// and https:// transports (dumb).\n@@ -143,9 +149,11 @@ all::\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n #\n-# Define NEEDS_SSL_WITH_CURL if you need -lssl with -lcurl (Minix).\n+# Define NEEDS_SSL_WITH_CURL if you need -lssl with -lcurl (Minix).  Only used\n+# if CURLDIR is set.\n #\n-# Define NEEDS_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix).\n+# Define NEEDS_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix).  Only\n+# used if CURLDIR is set.\n #\n # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin).\n #\n@@ -1118,20 +1126,35 @@ ifdef NO_CURL\n \tREMOTE_CURL_NAMES =\n else\n \tifdef CURLDIR\n-\t\t# Try \"-Wl,-rpath=$(CURLDIR)/$(lib)\" in such a case.\n-\t\tBASIC_CFLAGS += -I$(CURLDIR)/include\n-\t\tCURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) -lcurl\n+\t\tCURL_LIBCURL=\n \telse\n-\t\tCURL_LIBCURL = -lcurl\n-\tendif\n-\tifdef NEEDS_SSL_WITH_CURL\n-\t\tCURL_LIBCURL += -lssl\n-\t\tifdef NEEDS_CRYPTO_WITH_SSL\n-\t\t\tCURL_LIBCURL += -lcrypto\n+\t\tCURL_CONFIG ?= curl-config\n+\t\tifeq \"$(CURL_CONFIG)\" \"\"\n+\t\t\tCURL_LIBCURL =\n+\t\telse\n+\t\t\tCURL_LIBCURL := $(shell $(CURL_CONFIG) --libs)\n \t\tendif\n \tendif\n-\tifdef NEEDS_IDN_WITH_CURL\n-\t\tCURL_LIBCURL += -lidn\n+\n+\tifeq \"$(CURL_LIBCURL)\" \"\"\n+\t\tifdef CURLDIR\n+\t\t\t# Try \"-Wl,-rpath=$(CURLDIR)/$(lib)\" in such a case.\n+\t\t\tBASIC_CFLAGS += -I$(CURLDIR)/include\n+\t\t\tCURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) -lcurl\n+\t\telse\n+\t\t\tCURL_LIBCURL = -lcurl\n+\t\tendif\n+\t\tifdef NEEDS_SSL_WITH_CURL\n+\t\t\tCURL_LIBCURL += -lssl\n+\t\t\tifdef NEEDS_CRYPTO_WITH_SSL\n+\t\t\t\tCURL_LIBCURL += -lcrypto\n+\t\t\tendif\n+\t\tendif\n+\t\tifdef NEEDS_IDN_WITH_CURL\n+\t\t\tCURL_LIBCURL += -lidn\n+\t\tendif\n+\telse\n+\t\tBASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags)\n \tendif\n \n \tREMOTE_CURL_PRIMARY = git-remote-http$X\n-- \n1.9.1.423.g4596e3a\n"},{"id":"240009","messageId":"1398713704-15428-2-git-send-email-dborowitz@google.com","threadId":"36525","inReplyTo":"1398713704-15428-1-git-send-email-dborowitz@google.com","subject":"[PATCH v3 2/2] Makefile: allow static linking against libcurl","fromName":"Dave Borowitz","fromEmail":"dborowitz@google.com","sentAt":"2014-04-28T19:35:04Z","receivedAt":"2014-04-28T19:35:04Z","isPatch":true,"sender":{"key":"dborowitz@google.com","avatar":"https://avatars.githubusercontent.com/u/194927?v=4"},"body":"This requires more flags than can be guessed with the old-style\nCURLDIR and related options, so is only supported when curl-config is\npresent.\n\nSigned-off-by: Dave Borowitz <dborowitz@google.com>\n---\n Makefile | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex cb4ee37..360d427 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -39,6 +39,9 @@ all::\n # is not found, defaults to the CURLDIR behavior, or if CURLDIR is not set,\n # uses -lcurl with no additional library detection.\n #\n+# Define CURL_STATIC to statically link libcurl.  Only applies if\n+# CURL_CONFIG is used.\n+#\n # Define CURLDIR=/foo/bar if your curl header and library files are in\n # /foo/bar/include and /foo/bar/lib directories.  This overrides CURL_CONFIG,\n # but is less robust.\n@@ -1137,6 +1140,9 @@ else\n \tendif\n \n \tifeq \"$(CURL_LIBCURL)\" \"\"\n+\t\tifdef CURL_STATIC\n+                        $(error \"CURL_STATIC must be used with CURL_CONFIG\")\n+\t\tendif\n \t\tifdef CURLDIR\n \t\t\t# Try \"-Wl,-rpath=$(CURLDIR)/$(lib)\" in such a case.\n \t\t\tBASIC_CFLAGS += -I$(CURLDIR)/include\n@@ -1155,6 +1161,12 @@ else\n \t\tendif\n \telse\n \t\tBASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags)\n+\t\tifdef CURL_STATIC\n+\t\t\tCURL_LIBCURL = $(shell $(CURL_CONFIG) --static-libs)\n+\t\t\tifeq \"$(CURL_LIBCURL)\" \"\"\n+                                $(error libcurl not detected or not compiled with static support)\n+\t\t\tendif\n+\t\tendif\n \tendif\n \n \tREMOTE_CURL_PRIMARY = git-remote-http$X\n-- \n1.9.1.423.g4596e3a\n"},{"id":"240015","messageId":"20140428194449.GM9218@google.com","threadId":"36525","inReplyTo":"1398713704-15428-1-git-send-email-dborowitz@google.com","subject":"Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-04-28T19:44:49Z","receivedAt":"2014-04-28T19:44:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nDave Borowitz wrote:\n\n> curl-config is usually installed alongside a curl distribution, and\n> its purpose is to provide flags for building against libcurl, so use\n> it instead of guessing flags and dependent libraries.\n\nThe previous version of these two patches is already part of \"master\".\nCould you make an incremental patch?\n\nSorry for the fuss,\nJonathan\n"},{"id":"240020","messageId":"CAD0k6qS2n-DKcJpz+wjd1ZWLdAsvKMa_hAiu2D_BPKeWhv1jFA@mail.gmail.com","threadId":"36525","inReplyTo":"20140428194449.GM9218@google.com","subject":"Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags","fromName":"Dave Borowitz","fromEmail":"dborowitz@google.com","sentAt":"2014-04-28T19:51:40Z","receivedAt":"2014-04-28T19:51:40Z","isPatch":true,"sender":{"key":"dborowitz@google.com","avatar":"https://avatars.githubusercontent.com/u/194927?v=4"},"body":"On Mon, Apr 28, 2014 at 12:44 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Hi,\n>\n> Dave Borowitz wrote:\n>\n>> curl-config is usually installed alongside a curl distribution, and\n>> its purpose is to provide flags for building against libcurl, so use\n>> it instead of guessing flags and dependent libraries.\n>\n> The previous version of these two patches is already part of \"master\".\n> Could you make an incremental patch?\n\nDone. Thanks for pointing that out, and sorry for the noise.\n\n> Sorry for the fuss,\n> Jonathan\n"},{"id":"240024","messageId":"xmqqtx9dp6rd.fsf@gitster.dls.corp.google.com","threadId":"36525","inReplyTo":"1398713704-15428-1-git-send-email-dborowitz@google.com","subject":"Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-28T19:56:54Z","receivedAt":"2014-04-28T19:56:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Borowitz <dborowitz@google.com> writes:\n\n> Use this only when CURLDIR is not explicitly specified, to continue\n> supporting older builds. Moreover, if CURL_CONFIG is unset or running\n> it returns no results (e.g. because it is missing), default to the old\n> behavior of blindly setting -lcurl.\n>  \tifdef CURLDIR\n> +\t\tCURL_LIBCURL=\n>  \telse\n> +\t\tCURL_CONFIG ?= curl-config\n> +\t\tifeq \"$(CURL_CONFIG)\" \"\"\n> +\t\t\tCURL_LIBCURL =\n> +\t\telse\n> +\t\t\tCURL_LIBCURL := $(shell $(CURL_CONFIG) --libs)\n>  \t\tendif\n\nThis \"ifeq\" is redundant and will never set CURL_LIBCURL to empty\nwithout running the \"else\" part, I think.  In a Makefile, a variable\nexplicitly set to empty and a variable that is unset are treated the\nsame.\n\n\t$ cat >Makefile <<EOF\n\tCURL_CONFIG ?= curl-config\n\tifeq \"$(CURL_CONFIG)\" \"\"\n\t\tX=Empty\n\telse\n\t\tX=NotEmpty\n\tendif\n\n\tifdef \"$(CURL_CONFIG)\"\n\t\tZ=Defined\n\telse\n\t\tZ=Undefined\n\tendif\n\n\tall::\n\t\t@echo \"$(X) $(Z)\"\n\tEOF\n\t$ make -f Makefile CURL_CONFIG=\"\"\n\tEmpty Undefined\n\nThat does not mean the patch will give us a broken behaviour,\nthough.  It just means the ifeq/else part will be redundant.\n\n>  \tendif\n> +\n> +\tifeq \"$(CURL_LIBCURL)\" \"\"\n\nThis will catch the \"$(shell $(CURL_CONFIG) --libs) assigned an\nempty string to CURL_LIBCURL\" case, so the result is good.\n\nI haven't checked what it would look like if we turn this into an\nincremental patch to be applied on top of 'master' (which would give\nus a place to document better why we do not rely on the presense of\ncurl-config), but if we can do so, that would be more preferable\nthan having to revert the merge of the previous one and then\napplying these two patches anew.\n\nThanks.\n\n> +\t\tifdef CURLDIR\n> +\t\t\t# Try \"-Wl,-rpath=$(CURLDIR)/$(lib)\" in such a case.\n> +\t\t\tBASIC_CFLAGS += -I$(CURLDIR)/include\n> +\t\t\tCURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) -lcurl\n> +\t\telse\n> +\t\t\tCURL_LIBCURL = -lcurl\n> +\t\tendif\n> +\t\tifdef NEEDS_SSL_WITH_CURL\n> +\t\t\tCURL_LIBCURL += -lssl\n> +\t\t\tifdef NEEDS_CRYPTO_WITH_SSL\n> +\t\t\t\tCURL_LIBCURL += -lcrypto\n> +\t\t\tendif\n> +\t\tendif\n> +\t\tifdef NEEDS_IDN_WITH_CURL\n> +\t\t\tCURL_LIBCURL += -lidn\n> +\t\tendif\n> +\telse\n> +\t\tBASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags)\n>  \tendif\n>  \n>  \tREMOTE_CURL_PRIMARY = git-remote-http$X\n"},{"id":"240027","messageId":"xmqqppk1p6ly.fsf@gitster.dls.corp.google.com","threadId":"36525","inReplyTo":"xmqqtx9dp6rd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-28T20:00:09Z","receivedAt":"2014-04-28T20:00:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> That does not mean the patch will give us a broken behaviour,\n> though.  It just means the ifeq/else part will be redundant.\n>\n>>  \tendif\n>> +\n>> +\tifeq \"$(CURL_LIBCURL)\" \"\"\n>\n> This will catch the \"$(shell $(CURL_CONFIG) --libs) assigned an\n> empty string to CURL_LIBCURL\" case, so the result is good.\n>\n> I haven't checked what it would look like if we turn this into an\n> incremental patch to be applied on top of 'master' (which would give\n> us a place to document better why we do not rely on the presense of\n> curl-config), but if we can do so, that would be more preferable\n> than having to revert the merge of the previous one and then\n> applying these two patches anew.\n\nAnd I just checked; it is not very pretty to call it \"trivially\ncorrect\", and I would feel safer to revert the merge for 2.0, and\nqueue the new one for the next cycle, cooking it in 'pu' and then\n'next' in the meantime.\n"},{"id":"240048","messageId":"xmqqppk1non1.fsf@gitster.dls.corp.google.com","threadId":"36525","inReplyTo":"xmqqtx9dp6rd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-28T21:13:38Z","receivedAt":"2014-04-28T21:13:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> This \"ifeq\" is redundant and will never set CURL_LIBCURL to empty\n> without running the \"else\" part, I think.  In a Makefile, a variable\n> explicitly set to empty and a variable that is unset are treated the\n> same....\n> \t$ make -f Makefile CURL_CONFIG=\"\"\n> \tEmpty Undefined\n\nOh, I was blind.  Please ignore this noise.\n\nThanks.\n"}]}