Re: [PATCH/RFC] Makefile: do not depend on curl-config
On Wed, Apr 30, 2014 at 9:46 PM, Sebastian Schuberth sschube...@gmail.com wrote: On Wed, Apr 30, 2014 at 6:52 PM, Johannes Schindelin johannes.schinde...@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? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
On Mon, May 5, 2014 at 12:53 PM, Erik Faye-Lund kusmab...@gmail.com wrote: 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? Fine with me. -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH/RFC] Makefile: do not depend on curl-config
Am 05.05.2014 12:53, schrieb Erik Faye-Lund: On Wed, Apr 30, 2014 at 9:46 PM, Sebastian Schuberth sschube...@gmail.com wrote: On Wed, Apr 30, 2014 at 6:52 PM, Johannes Schindelin johannes.schinde...@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? Yes! -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
On Mon, Apr 28, 2014 at 6:29 PM, Erik Faye-Lund kusmab...@gmail.com wrote: 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 kusmab...@gmail.com Junio, this patch still applies, and is required on Git for Windows for http-push to work, even after f3f11fa (Makefile: default to -lcurl when no CURL_CONFIG or CURLDIR). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
Erik Faye-Lund kusmab...@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 kusmab...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
On Wed, Apr 30, 2014 at 8:27 AM, Junio C Hamano gits...@pobox.com wrote: 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 is fine by me FWIW. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
On Wed, Apr 30, 2014 at 5:27 PM, Junio C Hamano gits...@pobox.com wrote: Erik Faye-Lund kusmab...@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 kusmab...@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. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
Hi kusma, On Wed, 30 Apr 2014, Erik Faye-Lund 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. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
On Wed, Apr 30, 2014 at 6:52 PM, Johannes Schindelin johannes.schinde...@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. -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
