Re: [PATCH] Allow git to use any HTTP authentication method.

Tay Ray Chuan [off-list ref] writes:

On Mon, Dec 28, 2009 at 11:53 PM, Shawn O. Pearce [off-list ref] wrote:

quoted

Martin Storsj? [off-list ref] wrote:

quoted

Should I send in a new patch that removes the http.authAny option and
always enables this, or send a rewritten version of the patch that already
is in 'next'?

I'm not Junio, but I would suggest sending in a new patch series,
and asking Junio politely to revert the one that is currently in
next before merging in the new series.

If we really are killing http.authAny before it hits master, there
is no reason for it to appear in the final project history.

hmm, a few days back Junio (added to Cc list) sent out an email
regarding branch shuffling and dropping topics from 'next'. Junio,
could we piggyback on this?

If people want to go that way that is fine by me, but unlike the ones that
are _ejected_ from next without trace, if we are going to have the primary
feature the patch introduces and changing only a minor detail of external
interface, I don't think we gain much by hinding that story from the
history, especially for something that has been cooking in 'next'.

A separate follow-up commit would be more honest about the feature's
history.  Also a follow-up patch to remove conditionals is much easier to
review than a resend of a rewritten patch, especially when the original
was reviewed adequately for its primary codepath to implement the feature.

Would it be just a matter of queueing something like this on top of
b8ac923 (Add an option for using any HTTP authentication scheme, not only
basic, 2009-11-27)?

-- >8 -- 
From: "Shawn O. Pearce" <redacted>,
Subject: Remove http.authAny

Back when the feature to use different HTTP authentication methods was
originally written, it needed an extra HTTP request for everything when
the feature was in effect, because we didn't reuse curl sessions.

However, b8ac923 (Add an option for using any HTTP authentication scheme,
not only basic, 2009-11-27) builds on top of an updated codebase that does
reuse curl sessions; there is no need to manually avoid the extra overhead
by making this configurable anymore.

---
 Documentation/config.txt |    7 -------
 http.c                   |   17 +----------------
 2 files changed, 1 insertions(+), 23 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a54ede3..b77d66d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -1158,13 +1158,6 @@ http.noEPSV::
 	support EPSV mode. Can be overridden by the 'GIT_CURL_FTP_NO_EPSV'
 	environment variable. Default is false (curl will use EPSV).
 
-http.authAny::
-	Allow any HTTP authentication method, not only basic. Enabling
-	this lowers the performance slightly, by having to do requests
-	without any authentication to discover the authentication method
-	to use. Can be overridden by the 'GIT_HTTP_AUTH_ANY'
-	environment variable. Default is false.
-
 i18n.commitEncoding::
 	Character encoding the commit messages are stored in; git itself
 	does not care per se, but this information is necessary e.g. when

diff --git a/http.c b/http.c
index aeb69b3..01e0fdc 100644
--- a/http.c
+++ b/http.c

@@ -40,9 +40,6 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static char *user_name, *user_pass;
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-static int curl_http_auth_any = 0;
-#endif
 
 #if LIBCURL_VERSION_NUM >= 0x071700
 /* Use CURLOPT_KEYPASSWD as is */

@@ -197,12 +194,6 @@ static int http_options(const char *var, const char *value, void *cb)
 			http_post_buffer = LARGE_PACKET_MAX;
 		return 0;
 	}
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-	if (!strcmp("http.authany", var)) {
-		curl_http_auth_any = git_config_bool(var, value);
-		return 0;
-	}
-#endif
 
 	/* Fall back on the default ones */
 	return git_default_config(var, value, cb);

@@ -254,8 +245,7 @@ static CURL *get_curl_handle(void)
 	curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
 #endif
 #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-	if (curl_http_auth_any)
-		curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
+	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif
 
 	init_curl_http_auth(result);

@@ -408,11 +398,6 @@ void http_init(struct remote *remote)
 	if (getenv("GIT_CURL_FTP_NO_EPSV"))
 		curl_ftp_no_epsv = 1;
 
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-	if (getenv("GIT_HTTP_AUTH_ANY"))
-		curl_http_auth_any = 1;
-#endif
-
 	if (remote && remote->url && remote->url[0]) {
 		http_auth_init(remote->url[0]);
 		if (!ssl_cert_password_required &&

On Mon, 28 Dec 2009, Junio C Hamano wrote:

Would it be just a matter of queueing something like this on top of
b8ac923 (Add an option for using any HTTP authentication scheme, not only
basic, 2009-11-27)?

Looks good to me:

Acked-by: Martin Storsjo <redacted>

// Martin

Junio C Hamano [off-list ref] wrote:

-- >8 -- 
From: "Shawn O. Pearce" <redacted>,
Subject: Remove http.authAny

Ack.

But I'm not sure I should be the author, you did all of the legwork
and therefore should get credit for it.  :-)

-- 
Shawn.