On Mon, Apr 14, 2014 at 4:22 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Dave Borowitz <dborowitz@xxxxxxxxxx> writes: > >> curl-config should always be 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. >> >> Use this only when CURLDIR is not explicitly specified, to continue >> supporting older builds. >> >> Signed-off-by: Dave Borowitz <dborowitz@xxxxxxxxxx> > > Sounds logically the right thing to do. Was there a particular > platform that needed this (i.e. cannot be made to work with the > existing CURLDIR and "guessing flags and dependeent libraries") > that may be worth mentioning in the log message? My end goal is to statically link git on Mac OS X (10.9) against a newer version of libcurl than ships with the OS. The normal CURLDIR approach should work with system libcurl: $ /usr/bin/curl-config --libs -lcurl But it gets a bit more complicated with a recent curl version. This likely has to do with the set of enabled options; I passed flags to ./configure based on the build script "MacOSX-Framework" included in the curl distribution: $ ~/d/curl-out-7.36.0/bin/curl-config --libs -L/Users/dborowitz/d/curl-out-7.36.0/lib -lcurl -lgssapi_krb5 -lresolv -lldap -lz And with --static-libs there's just that much more: $ ~/d/curl-out-7.36.0/bin/curl-config --static-libs /Users/dborowitz/d/curl-out-7.36.0/lib/libcurl.a -Wl,-syslibroot,/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk -arch x86_64 -Wl,-headerpad_max_install_names -framework CoreFoundation -framework Security -lgssapi_krb5 -lresolv -lldap -lz So I don't think specifying NEEDS_*_WITH_CURL scales to this use case. While writing this up I also noticed an issue with the second patch, namely that `curl-config --static-libs` is empty when curl is not built for static linking. I will reroll with a more detailed commit message and a fix to the second patch. >> --- >> Makefile | 35 +++++++++++++++++++++++------------ >> 1 file changed, 23 insertions(+), 12 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index 2128ce3..d6330bc 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -34,8 +34,12 @@ 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'. >> +# >> # 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 +147,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). >> # >> @@ -1121,18 +1127,23 @@ else >> # 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 >> + 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 >> - CURL_LIBCURL = -lcurl >> - endif >> - ifdef NEEDS_SSL_WITH_CURL >> - CURL_LIBCURL += -lssl >> - ifdef NEEDS_CRYPTO_WITH_SSL >> - CURL_LIBCURL += -lcrypto >> + CURL_CONFIG ?= curl-config >> + BASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags) >> + CURL_LIBCURL = $(shell $(CURL_CONFIG) --libs) >> + ifeq "$(CURL_LIBCURL)" "" >> + $(error curl not detected; try setting CURLDIR) >> endif >> endif >> - ifdef NEEDS_IDN_WITH_CURL >> - CURL_LIBCURL += -lidn >> - endif >> >> REMOTE_CURL_PRIMARY = git-remote-http$X >> REMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html