git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Erik Faye-Lund <kusmabite@gmail.com>
Date
Apr 30, 2014, 15:53 UTC
Message-ID
<CABPQNSZUCPd=1Eu8VUCP01tkdYkBC=xspFZuDuywuYZUH8ewvw@mail.gmail.com>
In-Reply-To
<xmqqfvkuhm77.fsf@gitster.dls.corp.google.com>
On Wed, Apr 30, 2014 at 5:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 52 quoted lines
> 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?
Not very. I've successfully tested it on two systems, msysGit and RedHat 5.
Show 8 quoted lines
> 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.

Show 20 quoted lines
> 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...?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Previous: 'Dave Borowitz' via msysGitNext: Johannes Schindelin
Message 5 of 10 in “Makefile: do not depend on curl-config”
  1. Makefile: do not depend on curl-configErik Faye-Lund, Apr 28, 2014
  2. Erik Faye-LundApr 30, 2014
  3. Junio C HamanoApr 30, 2014
  4. 'Dave Borowitz' via msysGitApr 30, 2014
  5. Erik Faye-LundApr 30, 2014
  6. Johannes SchindelinApr 30, 2014
  7. Sebastian SchuberthApr 30, 2014
  8. Erik Faye-LundMay 5, 2014
  9. Sebastian SchuberthMay 5, 2014
  10. Thomas BraunMay 5, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.