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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags
Dave Borowitz dborow...@google.com writes: 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. + 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags
On Mon, Apr 28, 2014 at 12:44 PM, Jonathan Nieder jrnie...@gmail.com wrote: 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags
Junio C Hamano gits...@pobox.com writes: 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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags
Junio C Hamano gits...@pobox.com writes: 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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
