[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH 1/2] Makefile: use curl-config to determine curl flags
From:       Dave Borowitz <dborowitz () google ! com>
Date:       2014-04-15 10:20:52
Message-ID: CAD0k6qQJE6a5DmRnpiFpOc_MFjSn9eFyiJ2nFaL63rctvxwt5Q () mail ! gmail ! com
[Download RAW message or body]

On Mon, Apr 14, 2014 at 4:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Dave Borowitz <dborowitz@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 <dborowitz@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[prev in list] [next in list] [prev in thread] [next in thread]