Re: [PATCH/RFC] Makefile: do not depend on curl-config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 30, 2014 at 5:27 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Erik Faye-Lund <kusmabite@xxxxxxxxx> writes:
>
>> MinGW builds of cURL does not ship with curl-config unless built
>> with the autoconf based build system, which is not the practice
>> recommended by the documentation. MsysGit has had issues with
>> binaries of that sort, so it has switched away from autoconf-based
>> cURL-builds.
>>
>> Unfortunately, broke pushing over WebDAV on Windows, because
>> http-push.c depends on cURL's multi-threaded API, which we could
>> not determine the presence of any more.
>>
>> Since troublesome curl-versions are ancient, and not even present
>> in RedHat 5, let's just assume cURL is capable instead of doing a
>> non-robust check.
>>
>> Instead, add a check for curl_multi_init to our configure-script,
>> for those on ancient system. They probably already need to do the
>> configure-dance anyway.
>>
>> Signed-off-by: Erik Faye-Lund <kusmabite@xxxxxxxxx>
>> ---
>>
>> OK, here's a proper patch. I've even tested it! ;)
>>
>>
>>  Makefile     |  8 +++-----
>>  configure.ac | 11 +++++++++++
>>  2 files changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index e90f57e..f6b5847 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1133,13 +1133,11 @@ else
>>       REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)
>>       PROGRAM_OBJS += http-fetch.o
>>       PROGRAMS += $(REMOTE_CURL_NAMES)
>> -     curl_check := $(shell (echo 070908; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)
>> -     ifeq "$(curl_check)" "070908"
>> -             ifndef NO_EXPAT
>> +     ifndef NO_EXPAT
>> +             ifndef NO_CURL_MULTI
>>                       PROGRAM_OBJS += http-push.o
>>               endif
>
> Dave's "ask curl-config about proper compilation options if
> available" and this change does *not* semantically conflict, as the
> former should stress "if available" part (but note that the key word
> is "should").
>
> How old/battle tested is this change?

Not very. I've successfully tested it on two systems, msysGit and RedHat 5.

> My inclination at this point
> is to revert the merge of Dave's series from 2.0 (yes, I know we
> have been looking at fixing it and I _think_ the issue of unpleasant
> error message you reported can be fixed, but at this rate we would
> not know if we eradicated all the issues for platforms and distros
> with confidence by the final), kick it back to 'next'/'pu', and
> queue this change also in 'next'/'pu' and cook them so that we can
> merge them early in the 2.1 cycle.

Sounds like a sensible plan to me. We can keep this patch in the
msysGit repo for the 2.0 release.

> This new makefile variable needs a comment at the top, no?
>
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index f7d33da..3164b62 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -34,6 +34,10 @@ all::
>  # git-http-push are not built, and you cannot use http:// and https://
>  # transports (neither smart nor dumb).
>  #
> +# Define NO_CURL_MULTI if your libcurl is sufficiently old and lacks
> +# curl_multi_init ("git http-push" to run the deprecated dumb push over
> +# http/webdab will not be built).
> +#
>  # Define CURL_CONFIG to the path to a curl-config binary other than the
>  # default 'curl-config'.  If CURL_CONFIG is unset or points to a binary that
>  # is not found, defaults to the CURLDIR behavior.

Ah yes. Sorry for missing that. Perhaps the text should even mention
the old break-off version, that is curl > v7.9.8 as far as I can
tell...?
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]