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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 2, 2018, 15:10 UTC
Message-ID: <xmqq7ehvplak.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <SN4PR0701MB3679BA798ABD874D34A595DDA0CF0@SN4PR0701MB3679.namprd07.prod.outlook.com>

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

Show 17 quoted lines

>  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?

Show 11 quoted lines

> 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.