RFC patchMakefile: do not depend on curl-config

> 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@gmail.com>

> 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@gmail.com>
> ---
>
> 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

> How old/battle tested is this change?  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.

> Erik Faye-Lund <kusmabite@gmail.com> 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@gmail.com>
>> ---
>>
>> 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?

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

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

>> We can keep this patch in the msysGit repo for the 2.0 release.
>
> FWIW the plan is to switch to mingwGitDevEnv for the 2.0 release. It is
> not quite clear as of yet how patches will be managed with said
> environment.

> On Wed, Apr 30, 2014 at 6:52 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
>>> We can keep this patch in the msysGit repo for the 2.0 release.
>>
>> FWIW the plan is to switch to mingwGitDevEnv for the 2.0 release. It is
>> not quite clear as of yet how patches will be managed with said
>> environment.
>
> The environment is just that: The environment to build Git for
> Windows. This means that patches on top of Git for Windows could still
> be maintained in msysgit/git (or a fork thereof) on GitHub.

>>> FWIW the plan is to switch to mingwGitDevEnv for the 2.0 release. It is
>>> not quite clear as of yet how patches will be managed with said
>>> environment.
>>
>> The environment is just that: The environment to build Git for
>> Windows. This means that patches on top of Git for Windows could still
>> be maintained in msysgit/git (or a fork thereof) on GitHub.
>
> Thanks for the heads up. Even so, are you guys OK with me pushing this
> patch to our downstream repo?

> On Wed, Apr 30, 2014 at 9:46 PM, Sebastian Schuberth
> <sschuberth@gmail.com> wrote:
>> On Wed, Apr 30, 2014 at 6:52 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>>
>>>> We can keep this patch in the msysGit repo for the 2.0 release.
>>>
>>> FWIW the plan is to switch to mingwGitDevEnv for the 2.0 release. It is
>>> not quite clear as of yet how patches will be managed with said
>>> environment.
>>
>> The environment is just that: The environment to build Git for
>> Windows. This means that patches on top of Git for Windows could still
>> be maintained in msysgit/git (or a fork thereof) on GitHub.
> 
> Thanks for the heads up. Even so, are you guys OK with me pushing this
> patch to our downstream repo?