All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Erik Faye-Lund <kusmabite@gmail.com>,
	Dave Borowitz <dborowitz@google.com>
Cc: git@vger.kernel.org,  patthoyts@gmail.com,
	 marat@slonopotamus.org,  tuomas.silvola@knowit.fi,
	 msysgit@googlegroups.com,  johannes.schindelin@gmx.de
Subject: Re: [PATCH/RFC] Makefile: do not depend on curl-config
Date: Wed, 30 Apr 2014 08:27:24 -0700	[thread overview]
Message-ID: <xmqqfvkuhm77.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1398702545-9860-1-git-send-email-kusmabite@gmail.com> (Erik Faye-Lund's message of "Mon, 28 Apr 2014 18:29:05 +0200")

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.

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

  parent reply	other threads:[~2014-04-30 15:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-28 16:29 [PATCH/RFC] Makefile: do not depend on curl-config Erik Faye-Lund
2014-04-30 13:08 ` Erik Faye-Lund
2014-04-30 15:27 ` Junio C Hamano [this message]
2014-04-30 15:29   ` 'Dave Borowitz' via msysGit
2014-04-30 15:53   ` Erik Faye-Lund
2014-04-30 16:52     ` Johannes Schindelin
2014-04-30 19:46       ` Sebastian Schuberth
2014-05-05 10:53         ` Erik Faye-Lund
2014-05-05 10:56           ` Sebastian Schuberth
2014-05-05 15:03           ` [msysGit] " Thomas Braun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqfvkuhm77.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=dborowitz@google.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=kusmabite@gmail.com \
    --cc=marat@slonopotamus.org \
    --cc=msysgit@googlegroups.com \
    --cc=patthoyts@gmail.com \
    --cc=tuomas.silvola@knowit.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.