[PATCH] build: link with curl-defined linker flags

[PATCH] build: link with curl-defined linker flags

[James Knight]

Adjust the autotools configuration to populate libcurl-related linker
flags from curl-config instead of manually populating flags based off
detected features.

Originally, the configuration would check for SSL-support in the target
curl library. If enabled, NEEDS_SSL_WITH_CURL would be set and used in
the Makefile to append additional libraries to link against. Since the
process is already depending on a curl-config utility to provide
curl-related build information, adjusting the build to track the linker
flags in CURL_LIBCURL and pass the configuration option into the
Makefile.

Signed-off-by: James Knight <james.d.knight@live.com>
---
 Makefile         | 21 +++++++--------------
 config.mak.uname |  5 ++---
 configure.ac     | 17 +++++++----------
 3 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/Makefile b/Makefile
index b08d5ea25..c3be87b0e 100644
--- a/Makefile
+++ b/Makefile
@@ -183,10 +183,6 @@ 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_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix).
-#
 # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin).
 #
 # Define NEEDS_LIBINTL_BEFORE_LIBICONV if you need libintl before libiconv.
@@ -1305,18 +1301,15 @@ else
 	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
+		CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib)
 	else
-		CURL_LIBCURL = -lcurl
-	endif
-	ifdef NEEDS_SSL_WITH_CURL
-		CURL_LIBCURL += -lssl
-		ifdef NEEDS_CRYPTO_WITH_SSL
-			CURL_LIBCURL += -lcrypto
-		endif
+		CURL_LIBCURL =
 	endif
-	ifdef NEEDS_IDN_WITH_CURL
-		CURL_LIBCURL += -lidn
+
+	ifdef CURL_LDFLAGS
+		CURL_LIBCURL += $(CURL_LDFLAGS)
+	else
+		CURL_LIBCURL += -lcurl
 	endif
 
 	REMOTE_CURL_PRIMARY = git-remote-http$X
diff --git a/config.mak.uname b/config.mak.uname
index 8acdeb71f..923b8fa09 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -431,8 +431,7 @@ ifeq ($(uname_S),Minix)
 	NO_NSEC = YesPlease
 	NEEDS_LIBGEN =
 	NEEDS_CRYPTO_WITH_SSL = YesPlease
-	NEEDS_IDN_WITH_CURL = YesPlease
-	NEEDS_SSL_WITH_CURL = YesPlease
+	CURL_LDFLAGS = -lssl -lcrypto -lidn
 	NEEDS_RESOLV =
 	NO_HSTRERROR = YesPlease
 	NO_MMAP = YesPlease
@@ -458,7 +457,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	# Missdetected, hence commented out, see below.
 	#NO_CURL = YesPlease
 	# Added manually, see above.
-	NEEDS_SSL_WITH_CURL = YesPlease
+	CURL_LDFLAGS = -lssl -lcrypto
 	HAVE_LIBCHARSET_H = YesPlease
 	HAVE_STRINGS_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
diff --git a/configure.ac b/configure.ac
index e11b7976a..44e8c036b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -600,17 +600,14 @@ AC_CHECK_PROG([CURL_CONFIG], [curl-config],
 
 if test $CURL_CONFIG != no; then
     GIT_CONF_SUBST([CURL_CONFIG])
-    if test -z "${NO_OPENSSL}"; then
-      AC_MSG_CHECKING([if Curl supports SSL])
-      if test $(curl-config --features|grep SSL) = SSL; then
-         NEEDS_SSL_WITH_CURL=YesPlease
-         AC_MSG_RESULT([yes])
-      else
-         NEEDS_SSL_WITH_CURL=
-         AC_MSG_RESULT([no])
-      fi
-      GIT_CONF_SUBST([NEEDS_SSL_WITH_CURL])
+
+    if test -z "$CURL_CONFIG_OPTS"; then
+        CURL_CONFIG_OPTS="--libs"
     fi
+
+    CURL_LDFLAGS=$($CURL_CONFIG $CURL_CONFIG_OPTS)
+    AC_MSG_NOTICE([Setting CURL_LDFLAGS to '$CURL_LDFLAGS'])
+    GIT_CONF_SUBST([CURL_LDFLAGS], [$CURL_LDFLAGS])
 fi
 
 fi
-- 
2.19.1

Re: [PATCH] build: link with curl-defined linker flags

[Junio C Hamano]

James Knight <james.d.knight@live.com> writes:

>  Makefile         | 21 +++++++--------------
>  config.mak.uname |  5 ++---
>  configure.ac     | 17 +++++++----------
>  3 files changed, 16 insertions(+), 27 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b08d5ea25..c3be87b0e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -183,10 +183,6 @@ 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_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix).
> -#

Hmm, because the use of autoconf -> ./configure in our build
procedure is optional, wouldn't this change give regression to those
of us who use these Makefile variables to configure their build,
instead of relying on autoconf?

> diff --git a/config.mak.uname b/config.mak.uname
> index 8acdeb71f..923b8fa09 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -431,8 +431,7 @@ ifeq ($(uname_S),Minix)
>  	NO_NSEC = YesPlease
>  	NEEDS_LIBGEN =
>  	NEEDS_CRYPTO_WITH_SSL = YesPlease
> -	NEEDS_IDN_WITH_CURL = YesPlease
> -	NEEDS_SSL_WITH_CURL = YesPlease
> +	CURL_LDFLAGS = -lssl -lcrypto -lidn

OK, as long as we describe how to update their config.mak to adjust
to the new world order, the "regression" I noticed earlier is not
too bad, and the way the new CURL_LDFLAGS variable drives the build
is much more direct and transparent than via the old way to specify
NEEDS_*_WITH_CURL to affect the build indirectly.

I think such "describe how to configure in the new world order"
should go to where NEEDS_*_WITH_CURL used to be, e.g. "Define
CURL_LDFLAGS to specify flags that you need to link using -lcurl;
see config.mak.uname and look for the variable for examples"
or something like that, perhaps.

Thanks.