Re: [PATCH 1/2] Makefile: use curl-config to determine curl flags
On Mon, Apr 14, 2014 at 4:22 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com 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 dborow...@google.com 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 majord...@vger.kernel.org
Re: [PATCH 1/2] Makefile: use curl-config to determine curl flags
Dave Borowitz dborow...@google.com writes: 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. Thanks. As I said, the kosher way to learn how to link with libcURL is by asking curl-config about the details of options to give to the compiler and the linker, so I am all for this change. 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. I love it whenever I see the contents of the patch improved after spending a bit more time trying to describe the problem and the solution (which is time worth spending). 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
Re: [PATCH 1/2] Makefile: use curl-config to determine curl flags
Dave Borowitz dborow...@google.com 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 dborow...@google.com 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? --- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
