[PATCH] test-url-normalize.c: Fix gcc errors and sparse warnings

[PATCH] test-url-normalize.c: Fix gcc errors and sparse warnings

[Ramsay Jones]

Sparse issues an "non-ANSI function declaration of function 'main'"
warning when NO_CURL is set. In order to suppress the warning, we
simply add the function prototype.

When NO_CURL and USE_CURL_MULTI are not defined, then gcc issues the
following error:

      CC test-url-normalize.o
  test-url-normalize.c: In function `run_http_options':
  test-url-normalize.c:49: error: `max_requests' undeclared (first use in this function)
  test-url-normalize.c:49: error: (Each undeclared identifier is reported only once
  test-url-normalize.c:49: error: for each function it appears in.)
  make: *** [test-url-normalize.o] Error 1

In order to fix the error, we simply protect the use of the 'max_requests'
variable with an preprocessor conditional.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Kyle,

When you next update the patches in your 'km/http-curl-config-per-url'
branch, could you please squash this (or something like it) into the
relevant patches.

Thanks!

ATB,
Ramsay Jones

 test-url-normalize.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test-url-normalize.c b/test-url-normalize.c
index abfa4eb..86ce553 100644
--- a/test-url-normalize.c
+++ b/test-url-normalize.c
@@ -1,6 +1,6 @@
 #ifdef NO_CURL
 
-int main()
+int main(void)
 {
 	return 125;
 }
@@ -45,8 +45,10 @@ static int run_http_options(const char *file,
 		printf("%s\n", curl_ssl_try ? "true" : "false");
 	else if (!strcmp("minsessions", opt_lc.buf))
 		printf("%d\n", min_curl_sessions);
+#ifdef USE_CURL_MULTI
 	else if (!strcmp("maxrequests", opt_lc.buf))
 		printf("%d\n", max_requests);
+#endif
 	else if (!strcmp("lowspeedlimit", opt_lc.buf))
 		printf("%ld\n", curl_low_speed_limit);
 	else if (!strcmp("lowspeedtime", opt_lc.buf))
-- 
1.8.3

Re: [PATCH] test-url-normalize.c: Fix gcc errors and sparse warnings

[Kyle J. McKay]

On Jul 24, 2013, at 12:07, Ramsay Jones wrote:

>
> Sparse issues an "non-ANSI function declaration of function 'main'"
> warning when NO_CURL is set. In order to suppress the warning, we
> simply add the function prototype.
>
> When NO_CURL and USE_CURL_MULTI are not defined, then gcc issues the
> following error:
>
>      CC test-url-normalize.o
>  test-url-normalize.c: In function `run_http_options':
>  test-url-normalize.c:49: error: `max_requests' undeclared (first  
> use in this function)
>  test-url-normalize.c:49: error: (Each undeclared identifier is  
> reported only once
>  test-url-normalize.c:49: error: for each function it appears in.)
>  make: *** [test-url-normalize.o] Error 1
>
> In order to fix the error, we simply protect the use of the  
> 'max_requests'
> variable with an preprocessor conditional.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>
> Hi Kyle,
>
> When you next update the patches in your 'km/http-curl-config-per-url'
> branch, could you please squash this (or something like it) into the
> relevant patches.
>
> Thanks!
>
> ATB,
> Ramsay Jones

Thanks.

And hopefully Junio can add this as an incremental patch against next  
as it looks good to me.

Re: [PATCH] test-url-normalize.c: Fix gcc errors and sparse warnings

[Junio C Hamano]

"Kyle J. McKay" <mackyle@gmail.com> writes:

> And hopefully Junio can add this as an incremental patch against next
> as it looks good to me.

Not likely to happen until the weekend.

If I had time for such a clean-up, I would rather be doing the
restructuring of the whole thing to make Peff's "git config --url
<class> <variable> $URL" easier to code myself ;-)

[PATCH 0/3] "git config --get-urlmatch $section.$key $url"

[Junio C Hamano]

So here is a bit of refactoring to extract the logic to find the
entry $section.<urlpattern>.$key from the configuration that best
matches the given $url to answer "what value $section.$key should be
for $url?" out of http.c (primarily because we never want to link
"git cofnig" with the cURL library), and use it from "git config" to
implement Peff's idea to extend "git config".

The first step is a pure code movement, plus some renaming of the
functions.

The second step is to factor out the code to handle --bool, --int, etc.
as a helper so that the new codepath can use it.

The last step currently duplicates the logic in http_options(), but
we might want to refactor it further so that the two functions can
share more code.  We hopefully can get rid of test-url-normalize and
instead use "git config --get-urlmatch" in the tests that protect
the http.<url>.config topic.

Junio C Hamano (3):
  url-match: split out URL matching logic out of http.c
  builtin/config: refactor collect_config()
  config: --get-urlmatch

 Makefile             |   2 +
 builtin/config.c     | 183 ++++++++++++++++---
 http.c               | 500 +--------------------------------------------------
 test-url-normalize.c |   2 -
 url-match.c          | 468 +++++++++++++++++++++++++++++++++++++++++++++++
 url-match.h          |  35 ++++
 6 files changed, 671 insertions(+), 519 deletions(-)
 create mode 100644 url-match.c
 create mode 100644 url-match.h

-- 
1.8.4-rc0-137-g17832d4

[PATCH 1/3] url-match: split out URL matching logic out of http.c

[Junio C Hamano]

Move reusable URL matching code out of http.c, because it is not
limited to http, and we do not necessarily want to link its users
with the cURL library.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile             |   2 +
 http.c               | 500 +--------------------------------------------------
 test-url-normalize.c |   2 -
 url-match.c          | 468 +++++++++++++++++++++++++++++++++++++++++++++++
 url-match.h          |  35 ++++
 5 files changed, 509 insertions(+), 498 deletions(-)
 create mode 100644 url-match.c
 create mode 100644 url-match.h

diff --git a/Makefile b/Makefile
index f83879c..92ccfb3 100644
--- a/Makefile
+++ b/Makefile
@@ -722,6 +722,7 @@ LIB_H += tree-walk.h
 LIB_H += tree.h
 LIB_H += unpack-trees.h
 LIB_H += url.h
+LIB_H += url-match.h
 LIB_H += userdiff.h
 LIB_H += utf8.h
 LIB_H += varint.h
@@ -869,6 +870,7 @@ LIB_OBJS += tree.o
 LIB_OBJS += tree-walk.o
 LIB_OBJS += unpack-trees.o
 LIB_OBJS += url.o
+LIB_OBJS += url-match.o
 LIB_OBJS += usage.o
 LIB_OBJS += userdiff.o
 LIB_OBJS += utf8.o
diff --git a/http.c b/http.c
index c636d3c..a91a00b 100644
--- a/http.c
+++ b/http.c
@@ -3,6 +3,7 @@
 #include "sideband.h"
 #include "run-command.h"
 #include "url.h"
+#include "url-match.h"
 #include "credential.h"
 #include "version.h"
 #include "pkt-line.h"
@@ -56,33 +57,6 @@ enum http_option_type {
 	OPT_MAX
 };
 
-struct url_info {
-	char *url;		/* normalized url on success, must be freed, otherwise NULL */
-	const char *err;	/* if !url, a brief reason for the failure, otherwise NULL */
-
-	/* the rest of the fields are only set if url != NULL */
-
-	size_t url_len;		/* total length of url (which is now normalized) */
-	size_t scheme_len;	/* length of scheme name (excluding final :) */
-	size_t user_off;	/* offset into url to start of user name (0 => none) */
-	size_t user_len;	/* length of user name; if user_off != 0 but
-				   user_len == 0, an empty user name was given */
-	size_t passwd_off;	/* offset into url to start of passwd (0 => none) */
-	size_t passwd_len;	/* length of passwd; if passwd_off != 0 but
-				   passwd_len == 0, an empty passwd was given */
-	size_t host_off;	/* offset into url to start of host name (0 => none) */
-	size_t host_len;	/* length of host name; this INCLUDES any ':portnum';
-				 * file urls may have host_len == 0 */
-	size_t port_len;	/* if a portnum is present (port_len != 0), it has
-				 * this length (excluding the leading ':') at the
-				 * end of the host name (always 0 for file urls) */
-	size_t path_off;	/* offset into url to the start of the url path;
-				 * this will always point to a '/' character
-				 * after the url has been normalized */
-	size_t path_len;	/* length of path portion excluding any trailing
-				 * '?...' and '#...' portion; will always be >= 1 */
-};
-
 static size_t http_option_max_matched_len[OPT_MAX];
 static int http_option_user_matched[OPT_MAX];
 
@@ -197,472 +171,6 @@ static void process_curl_messages(void)
 }
 #endif
 
-#define URL_ALPHA "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
-#define URL_DIGIT "0123456789"
-#define URL_ALPHADIGIT URL_ALPHA URL_DIGIT
-#define URL_SCHEME_CHARS URL_ALPHADIGIT "+.-"
-#define URL_HOST_CHARS URL_ALPHADIGIT ".-[:]" /* IPv6 literals need [:] */
-#define URL_UNSAFE_CHARS " <>\"%{}|\\^`" /* plus 0x00-0x1F,0x7F-0xFF */
-#define URL_GEN_RESERVED ":/?#[]@"
-#define URL_SUB_RESERVED "!$&'()*+,;="
-#define URL_RESERVED URL_GEN_RESERVED URL_SUB_RESERVED /* only allowed delims */
-
-static int append_normalized_escapes(struct strbuf *buf,
-				     const char *from,
-				     size_t from_len,
-				     const char *esc_extra,
-				     const char *esc_ok)
-{
-	/*
-	 * Append to strbuf 'buf' characters from string 'from' with length
-	 * 'from_len' while unescaping characters that do not need to be escaped
-	 * and escaping characters that do.  The set of characters to escape
-	 * (the complement of which is unescaped) starts out as the RFC 3986
-	 * unsafe characters (0x00-0x1F,0x7F-0xFF," <>\"#%{}|\\^`").  If
-	 * 'esc_extra' is not NULL, those additional characters will also always
-	 * be escaped.  If 'esc_ok' is not NULL, those characters will be left
-	 * escaped if found that way, but will not be unescaped otherwise (used
-	 * for delimiters).  If a %-escape sequence is encountered that is not
-	 * followed by 2 hexadecimal digits, the sequence is invalid and
-	 * false (0) will be returned.  Otherwise true (1) will be returned for
-	 * success.
-	 *
-	 * Note that all %-escape sequences will be normalized to UPPERCASE
-	 * as indicated in RFC 3986.  Unless included in esc_extra or esc_ok
-	 * alphanumerics and "-._~" will always be unescaped as per RFC 3986.
-	 */
-
-	while (from_len) {
-		int ch = *from++;
-		int was_esc = 0;
-
-		from_len--;
-		if (ch == '%') {
-			if (from_len < 2 ||
-			    !isxdigit((unsigned char)from[0]) ||
-			    !isxdigit((unsigned char)from[1]))
-				return 0;
-			ch = hexval_table[(unsigned char)*from++] << 4;
-			ch |= hexval_table[(unsigned char)*from++];
-			from_len -= 2;
-			was_esc = 1;
-		}
-		if ((unsigned char)ch <= 0x1F || (unsigned char)ch >= 0x7F ||
-		    strchr(URL_UNSAFE_CHARS, ch) ||
-		    (esc_extra && strchr(esc_extra, ch)) ||
-		    (was_esc && strchr(esc_ok, ch)))
-			strbuf_addf(buf, "%%%02X", (unsigned char)ch);
-		else
-			strbuf_addch(buf, ch);
-	}
-
-	return 1;
-}
-
-static char *http_options_url_normalize(const char *url, struct url_info *out_info)
-{
-	/*
-	 * Normalize NUL-terminated url using the following rules:
-	 *
-	 * 1. Case-insensitive parts of url will be converted to lower case
-	 * 2. %-encoded characters that do not need to be will be unencoded
-	 * 3. Characters that are not %-encoded and must be will be encoded
-	 * 4. All %-encodings will be converted to upper case hexadecimal
-	 * 5. Leading 0s are removed from port numbers
-	 * 6. If the default port for the scheme is given it will be removed
-	 * 7. A path part (including empty) not starting with '/' has one added
-	 * 8. Any dot segments (. or ..) in the path are resolved and removed
-	 * 9. IPv6 host literals are allowed (but not normalized or validated)
-	 *
-	 * The rules are based on information in RFC 3986.
-	 *
-	 * Please note this function requires a full URL including a scheme
-	 * and host part (except for file: URLs which may have an empty host).
-	 *
-	 * The return value is a newly allocated string that must be freed
-	 * or NULL if the url is not valid.
-	 *
-	 * If out_info is non-NULL, the url and err fields therein will always
-	 * be set.  If a non-NULL value is returned, it will be stored in
-	 * out_info->url as well, out_info->err will be set to NULL and the
-	 * other fields of *out_info will also be filled in.  If a NULL value
-	 * is returned, NULL will be stored in out_info->url and out_info->err
-	 * will be set to a brief, translated, error message, but no other
-	 * fields will be filled in.
-	 *
-	 * This is NOT a URL validation function.  Full URL validation is NOT
-	 * performed.  Some invalid host names are passed through this function
-	 * undetected.  However, most all other problems that make a URL invalid
-	 * will be detected (including a missing host for non file: URLs).
-	 */
-
-	size_t url_len = strlen(url);
-	struct strbuf norm;
-	size_t spanned;
-	size_t scheme_len, user_off=0, user_len=0, passwd_off=0, passwd_len=0;
-	size_t host_off=0, host_len=0, port_len=0, path_off, path_len, result_len;
-	const char *slash_ptr, *at_ptr, *colon_ptr, *path_start;
-	char *result;
-
-	/*
-	 * Copy lowercased scheme and :// suffix, %-escapes are not allowed
-	 * First character of scheme must be URL_ALPHA
-	 */
-	spanned = strspn(url, URL_SCHEME_CHARS);
-	if (!spanned || !isalpha(url[0]) || spanned + 3 > url_len ||
-	    url[spanned] != ':' || url[spanned+1] != '/' || url[spanned+2] != '/') {
-		if (out_info) {
-			out_info->url = NULL;
-			out_info->err = _("invalid URL scheme name or missing '://' suffix");
-		}
-		return NULL; /* Bad scheme and/or missing "://" part */
-	}
-	strbuf_init(&norm, url_len);
-	scheme_len = spanned;
-	spanned += 3;
-	url_len -= spanned;
-	while (spanned--)
-		strbuf_addch(&norm, tolower(*url++));
-
-
-	/*
-	 * Copy any username:password if present normalizing %-escapes
-	 */
-	at_ptr = strchr(url, '@');
-	slash_ptr = url + strcspn(url, "/?#");
-	if (at_ptr && at_ptr < slash_ptr) {
-		user_off = norm.len;
-		if (at_ptr > url) {
-			if (!append_normalized_escapes(&norm, url, at_ptr - url,
-						       "", URL_RESERVED)) {
-				if (out_info) {
-					out_info->url = NULL;
-					out_info->err = _("invalid %XX escape sequence");
-				}
-				strbuf_release(&norm);
-				return NULL;
-			}
-			colon_ptr = strchr(norm.buf + scheme_len + 3, ':');
-			if (colon_ptr) {
-				passwd_off = (colon_ptr + 1) - norm.buf;
-				passwd_len = norm.len - passwd_off;
-				user_len = (passwd_off - 1) - (scheme_len + 3);
-			} else {
-				user_len = norm.len - (scheme_len + 3);
-			}
-		}
-		strbuf_addch(&norm, '@');
-		url_len -= (++at_ptr - url);
-		url = at_ptr;
-	}
-
-
-	/*
-	 * Copy the host part excluding any port part, no %-escapes allowed
-	 */
-	if (!url_len || strchr(":/?#", *url)) {
-		/* Missing host invalid for all URL schemes except file */
-		if (strncmp(norm.buf, "file:", 5)) {
-			if (out_info) {
-				out_info->url = NULL;
-				out_info->err = _("missing host and scheme is not 'file:'");
-			}
-			strbuf_release(&norm);
-			return NULL;
-		}
-	} else {
-		host_off = norm.len;
-	}
-	colon_ptr = slash_ptr - 1;
-	while (colon_ptr > url && *colon_ptr != ':' && *colon_ptr != ']')
-		colon_ptr--;
-	if (*colon_ptr != ':') {
-		colon_ptr = slash_ptr;
-	} else if (!host_off && colon_ptr < slash_ptr && colon_ptr + 1 != slash_ptr) {
-		/* file: URLs may not have a port number */
-		if (out_info) {
-			out_info->url = NULL;
-			out_info->err = _("a 'file:' URL may not have a port number");
-		}
-		strbuf_release(&norm);
-		return NULL;
-	}
-	spanned = strspn(url, URL_HOST_CHARS);
-	if (spanned < colon_ptr - url) {
-		/* Host name has invalid characters */
-		if (out_info) {
-			out_info->url = NULL;
-			out_info->err = _("invalid characters in host name");
-		}
-		strbuf_release(&norm);
-		return NULL;
-	}
-	while (url < colon_ptr) {
-		strbuf_addch(&norm, tolower(*url++));
-		url_len--;
-	}
-
-
-	/*
-	 * Check the port part and copy if not the default (after removing any
-	 * leading 0s); no %-escapes allowed
-	 */
-	if (colon_ptr < slash_ptr) {
-		/* skip the ':' and leading 0s but not the last one if all 0s */
-		url++;
-		url += strspn(url, "0");
-		if (url == slash_ptr && url[-1] == '0')
-			url--;
-		if (url == slash_ptr) {
-			/* Skip ":" port with no number, it's same as default */
-		} else if (slash_ptr - url == 2 &&
-			   !strncmp(norm.buf, "http:", 5) &&
-			   !strncmp(url, "80", 2)) {
-			/* Skip http :80 as it's the default */
-		} else if (slash_ptr - url == 3 &&
-			   !strncmp(norm.buf, "https:", 6) &&
-			   !strncmp(url, "443", 3)) {
-			/* Skip https :443 as it's the default */
-		} else {
-			/*
-			 * Port number must be all digits with leading 0s removed
-			 * and since all the protocols we deal with have a 16-bit
-			 * port number it must also be in the range 1..65535
-			 * 0 is not allowed because that means "next available"
-			 * on just about every system and therefore cannot be used
-			 */
-			unsigned long pnum = 0;
-			spanned = strspn(url, URL_DIGIT);
-			if (spanned < slash_ptr - url) {
-				/* port number has invalid characters */
-				if (out_info) {
-					out_info->url = NULL;
-					out_info->err = _("invalid port number");
-				}
-				strbuf_release(&norm);
-				return NULL;
-			}
-			if (slash_ptr - url <= 5)
-				pnum = strtoul(url, NULL, 10);
-			if (pnum == 0 || pnum > 65535) {
-				/* port number not in range 1..65535 */
-				if (out_info) {
-					out_info->url = NULL;
-					out_info->err = _("invalid port number");
-				}
-				strbuf_release(&norm);
-				return NULL;
-			}
-			strbuf_addch(&norm, ':');
-			strbuf_add(&norm, url, slash_ptr - url);
-			port_len = slash_ptr - url;
-		}
-		url_len -= slash_ptr - colon_ptr;
-		url = slash_ptr;
-	}
-	if (host_off)
-		host_len = norm.len - host_off;
-
-
-	/*
-	 * Now copy the path resolving any . and .. segments being careful not
-	 * to corrupt the URL by unescaping any delimiters, but do add an
-	 * initial '/' if it's missing and do normalize any %-escape sequences.
-	 */
-	path_off = norm.len;
-	path_start = norm.buf + path_off;
-	strbuf_addch(&norm, '/');
-	if (*url == '/') {
-		url++;
-		url_len--;
-	}
-	for (;;) {
-		const char *seg_start = norm.buf + norm.len;
-		const char *next_slash = url + strcspn(url, "/?#");
-		int skip_add_slash = 0;
-		/*
-		 * RFC 3689 indicates that any . or .. segments should be
-		 * unescaped before being checked for.
-		 */
-		if (!append_normalized_escapes(&norm, url, next_slash - url, "",
-					       URL_RESERVED)) {
-			if (out_info) {
-				out_info->url = NULL;
-				out_info->err = _("invalid %XX escape sequence");
-			}
-			strbuf_release(&norm);
-			return NULL;
-		}
-		if (!strcmp(seg_start, ".")) {
-			/* ignore a . segment; be careful not to remove initial '/' */
-			if (seg_start == path_start + 1) {
-				strbuf_setlen(&norm, norm.len - 1);
-				skip_add_slash = 1;
-			} else {
-				strbuf_setlen(&norm, norm.len - 2);
-			}
-		} else if (!strcmp(seg_start, "..")) {
-			/*
-			 * ignore a .. segment and remove the previous segment;
-			 * be careful not to remove initial '/' from path
-			 */
-			const char *prev_slash = norm.buf + norm.len - 3;
-			if (prev_slash == path_start) {
-				/* invalid .. because no previous segment to remove */
-				if (out_info) {
-					out_info->url = NULL;
-					out_info->err = _("invalid '..' path segment");
-				}
-				strbuf_release(&norm);
-				return NULL;
-			}
-			while (*--prev_slash != '/') {}
-			if (prev_slash == path_start) {
-				strbuf_setlen(&norm, prev_slash - norm.buf + 1);
-				skip_add_slash = 1;
-			} else {
-				strbuf_setlen(&norm, prev_slash - norm.buf);
-			}
-		}
-		url_len -= next_slash - url;
-		url = next_slash;
-		/* if the next char is not '/' done with the path */
-		if (*url != '/')
-			break;
-		url++;
-		url_len--;
-		if (!skip_add_slash)
-			strbuf_addch(&norm, '/');
-	}
-	path_len = norm.len - path_off;
-
-
-	/*
-	 * Now simply copy the rest, if any, only normalizing %-escapes and
-	 * being careful not to corrupt the URL by unescaping any delimiters.
-	 */
-	if (*url) {
-		if (!append_normalized_escapes(&norm, url, url_len, "", URL_RESERVED)) {
-			if (out_info) {
-				out_info->url = NULL;
-				out_info->err = _("invalid %XX escape sequence");
-			}
-			strbuf_release(&norm);
-			return NULL;
-		}
-	}
-
-
-	result = strbuf_detach(&norm, &result_len);
-	if (out_info) {
-		out_info->url = result;
-		out_info->err = NULL;
-		out_info->url_len = result_len;
-		out_info->scheme_len = scheme_len;
-		out_info->user_off = user_off;
-		out_info->user_len = user_len;
-		out_info->passwd_off = passwd_off;
-		out_info->passwd_len = passwd_len;
-		out_info->host_off = host_off;
-		out_info->host_len = host_len;
-		out_info->port_len = port_len;
-		out_info->path_off = path_off;
-		out_info->path_len = path_len;
-	}
-	return result;
-}
-
-static size_t http_options_url_match_prefix(const char *url,
-					    const char *url_prefix,
-					    size_t url_prefix_len)
-{
-	/*
-	 * url_prefix matches url if url_prefix is an exact match for url or it
-	 * is a prefix of url and the match ends on a path component boundary.
-	 * Both url and url_prefix are considered to have an implicit '/' on the
-	 * end for matching purposes if they do not already.
-	 *
-	 * url must be NUL terminated.  url_prefix_len is the length of
-	 * url_prefix which need not be NUL terminated.
-	 *
-	 * The return value is the length of the match in characters (including
-	 * the final '/' even if it's implicit) or 0 for no match.
-	 *
-	 * Passing NULL as url and/or url_prefix will always cause 0 to be
-	 * returned without causing any faults.
-	 */
-	if (!url || !url_prefix)
-		return 0;
-	if (!url_prefix_len || (url_prefix_len == 1 && *url_prefix == '/'))
-		return (!*url || *url == '/') ? 1 : 0;
-	if (url_prefix[url_prefix_len - 1] == '/')
-		url_prefix_len--;
-	if (strncmp(url, url_prefix, url_prefix_len))
-		return 0;
-	if ((strlen(url) == url_prefix_len) || (url[url_prefix_len] == '/'))
-		return url_prefix_len + 1;
-	return 0;
-}
-
-static int http_options_match_urls(const struct url_info *url,
-				   const struct url_info *url_prefix,
-				   int *exactusermatch)
-{
-	/*
-	 * url_prefix matches url if the scheme, host and port of url_prefix
-	 * are the same as those of url and the path portion of url_prefix
-	 * is the same as the path portion of url or it is a prefix that
-	 * matches at a '/' boundary.  If url_prefix contains a user name,
-	 * that must also exactly match the user name in url.
-	 *
-	 * If the user, host, port and path match in this fashion, the returned
-	 * value is the length of the path match including any implicit
-	 * final '/'.  For example, "http://me@example.com/path" is matched by
-	 * "http://example.com" with a path length of 1.
-	 *
-	 * If there is a match and exactusermatch is not NULL, then
-	 * *exactusermatch will be set to true if both url and url_prefix
-	 * contained a user name or false if url_prefix did not have a
-	 * user name.  If there is no match *exactusermatch is left untouched.
-	 */
-	int usermatched = 0;
-	int pathmatchlen;
-
-	if (!url || !url_prefix || !url->url || !url_prefix->url)
-		return 0;
-
-	/* check the scheme */
-	if (url_prefix->scheme_len != url->scheme_len ||
-	    strncmp(url->url, url_prefix->url, url->scheme_len))
-		return 0; /* schemes do not match */
-
-	/* check the user name if url_prefix has one */
-	if (url_prefix->user_off) {
-		if (!url->user_off || url->user_len != url_prefix->user_len ||
-		    strncmp(url->url + url->user_off,
-			    url_prefix->url + url_prefix->user_off,
-			    url->user_len))
-			return 0; /* url_prefix has a user but it's not a match */
-		usermatched = 1;
-	}
-
-	/* check the host and port */
-	if (url_prefix->host_len != url->host_len ||
-	    strncmp(url->url + url->host_off,
-		    url_prefix->url + url_prefix->host_off, url->host_len))
-		return 0; /* host names and/or ports do not match */
-
-	/* check the path */
-	pathmatchlen = http_options_url_match_prefix(
-		url->url + url->path_off,
-		url_prefix->url + url_prefix->path_off,
-		url_prefix->url_len - url_prefix->path_off);
-
-	if (pathmatchlen && exactusermatch)
-		*exactusermatch = usermatched;
-	return pathmatchlen;
-}
-
 static int match_is_ignored(size_t matchlen, int usermatch, enum http_option_type opt)
 {
 	/*
@@ -720,11 +228,11 @@ static int http_options(const char *var, const char *value, void *cb)
 		if (!info || !info->url)
 			return 0;
 		config_url = xmemdupz(key, dot - key);
-		norm_url = http_options_url_normalize(config_url, &norm_info);
+		norm_url = url_normalize(config_url, &norm_info);
 		free(config_url);
 		if (!norm_url)
 			return 0;
-		matchlen = http_options_match_urls(info, &norm_info, &usermatch);
+		matchlen = match_urls(info, &norm_info, &usermatch);
 		free(norm_url);
 		if (!matchlen)
 			return 0;
@@ -967,7 +475,7 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
 
 	memset(http_option_max_matched_len, 0, sizeof(http_option_max_matched_len));
 	memset(http_option_user_matched, 0, sizeof(http_option_user_matched));
-	http_options_url_normalize(url, &info);
+	url_normalize(url, &info);
 	git_config(http_options, &info);
 	free(info.url);
 
diff --git a/test-url-normalize.c b/test-url-normalize.c
index 4f870dc..0b809eb 100644
--- a/test-url-normalize.c
+++ b/test-url-normalize.c
@@ -65,8 +65,6 @@ static int run_http_options(const char *file,
 	return 0;
 }
 
-#define url_normalize(u,i) http_options_url_normalize(u,i)
-
 int main(int argc, char **argv)
 {
 	const char *usage = "test-url-normalize [-p | -l] <url1> | <url1> <url2>"
diff --git a/url-match.c b/url-match.c
new file mode 100644
index 0000000..424a0a0
--- /dev/null
+++ b/url-match.c
@@ -0,0 +1,468 @@
+#include "cache.h"
+#include "url-match.h"
+
+#define URL_ALPHA "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+#define URL_DIGIT "0123456789"
+#define URL_ALPHADIGIT URL_ALPHA URL_DIGIT
+#define URL_SCHEME_CHARS URL_ALPHADIGIT "+.-"
+#define URL_HOST_CHARS URL_ALPHADIGIT ".-[:]" /* IPv6 literals need [:] */
+#define URL_UNSAFE_CHARS " <>\"%{}|\\^`" /* plus 0x00-0x1F,0x7F-0xFF */
+#define URL_GEN_RESERVED ":/?#[]@"
+#define URL_SUB_RESERVED "!$&'()*+,;="
+#define URL_RESERVED URL_GEN_RESERVED URL_SUB_RESERVED /* only allowed delims */
+
+static int append_normalized_escapes(struct strbuf *buf,
+				     const char *from,
+				     size_t from_len,
+				     const char *esc_extra,
+				     const char *esc_ok)
+{
+	/*
+	 * Append to strbuf 'buf' characters from string 'from' with length
+	 * 'from_len' while unescaping characters that do not need to be escaped
+	 * and escaping characters that do.  The set of characters to escape
+	 * (the complement of which is unescaped) starts out as the RFC 3986
+	 * unsafe characters (0x00-0x1F,0x7F-0xFF," <>\"#%{}|\\^`").  If
+	 * 'esc_extra' is not NULL, those additional characters will also always
+	 * be escaped.  If 'esc_ok' is not NULL, those characters will be left
+	 * escaped if found that way, but will not be unescaped otherwise (used
+	 * for delimiters).  If a %-escape sequence is encountered that is not
+	 * followed by 2 hexadecimal digits, the sequence is invalid and
+	 * false (0) will be returned.  Otherwise true (1) will be returned for
+	 * success.
+	 *
+	 * Note that all %-escape sequences will be normalized to UPPERCASE
+	 * as indicated in RFC 3986.  Unless included in esc_extra or esc_ok
+	 * alphanumerics and "-._~" will always be unescaped as per RFC 3986.
+	 */
+
+	while (from_len) {
+		int ch = *from++;
+		int was_esc = 0;
+
+		from_len--;
+		if (ch == '%') {
+			if (from_len < 2 ||
+			    !isxdigit((unsigned char)from[0]) ||
+			    !isxdigit((unsigned char)from[1]))
+				return 0;
+			ch = hexval_table[(unsigned char)*from++] << 4;
+			ch |= hexval_table[(unsigned char)*from++];
+			from_len -= 2;
+			was_esc = 1;
+		}
+		if ((unsigned char)ch <= 0x1F || (unsigned char)ch >= 0x7F ||
+		    strchr(URL_UNSAFE_CHARS, ch) ||
+		    (esc_extra && strchr(esc_extra, ch)) ||
+		    (was_esc && strchr(esc_ok, ch)))
+			strbuf_addf(buf, "%%%02X", (unsigned char)ch);
+		else
+			strbuf_addch(buf, ch);
+	}
+
+	return 1;
+}
+
+char *url_normalize(const char *url, struct url_info *out_info)
+{
+	/*
+	 * Normalize NUL-terminated url using the following rules:
+	 *
+	 * 1. Case-insensitive parts of url will be converted to lower case
+	 * 2. %-encoded characters that do not need to be will be unencoded
+	 * 3. Characters that are not %-encoded and must be will be encoded
+	 * 4. All %-encodings will be converted to upper case hexadecimal
+	 * 5. Leading 0s are removed from port numbers
+	 * 6. If the default port for the scheme is given it will be removed
+	 * 7. A path part (including empty) not starting with '/' has one added
+	 * 8. Any dot segments (. or ..) in the path are resolved and removed
+	 * 9. IPv6 host literals are allowed (but not normalized or validated)
+	 *
+	 * The rules are based on information in RFC 3986.
+	 *
+	 * Please note this function requires a full URL including a scheme
+	 * and host part (except for file: URLs which may have an empty host).
+	 *
+	 * The return value is a newly allocated string that must be freed
+	 * or NULL if the url is not valid.
+	 *
+	 * If out_info is non-NULL, the url and err fields therein will always
+	 * be set.  If a non-NULL value is returned, it will be stored in
+	 * out_info->url as well, out_info->err will be set to NULL and the
+	 * other fields of *out_info will also be filled in.  If a NULL value
+	 * is returned, NULL will be stored in out_info->url and out_info->err
+	 * will be set to a brief, translated, error message, but no other
+	 * fields will be filled in.
+	 *
+	 * This is NOT a URL validation function.  Full URL validation is NOT
+	 * performed.  Some invalid host names are passed through this function
+	 * undetected.  However, most all other problems that make a URL invalid
+	 * will be detected (including a missing host for non file: URLs).
+	 */
+
+	size_t url_len = strlen(url);
+	struct strbuf norm;
+	size_t spanned;
+	size_t scheme_len, user_off=0, user_len=0, passwd_off=0, passwd_len=0;
+	size_t host_off=0, host_len=0, port_len=0, path_off, path_len, result_len;
+	const char *slash_ptr, *at_ptr, *colon_ptr, *path_start;
+	char *result;
+
+	/*
+	 * Copy lowercased scheme and :// suffix, %-escapes are not allowed
+	 * First character of scheme must be URL_ALPHA
+	 */
+	spanned = strspn(url, URL_SCHEME_CHARS);
+	if (!spanned || !isalpha(url[0]) || spanned + 3 > url_len ||
+	    url[spanned] != ':' || url[spanned+1] != '/' || url[spanned+2] != '/') {
+		if (out_info) {
+			out_info->url = NULL;
+			out_info->err = _("invalid URL scheme name or missing '://' suffix");
+		}
+		return NULL; /* Bad scheme and/or missing "://" part */
+	}
+	strbuf_init(&norm, url_len);
+	scheme_len = spanned;
+	spanned += 3;
+	url_len -= spanned;
+	while (spanned--)
+		strbuf_addch(&norm, tolower(*url++));
+
+
+	/*
+	 * Copy any username:password if present normalizing %-escapes
+	 */
+	at_ptr = strchr(url, '@');
+	slash_ptr = url + strcspn(url, "/?#");
+	if (at_ptr && at_ptr < slash_ptr) {
+		user_off = norm.len;
+		if (at_ptr > url) {
+			if (!append_normalized_escapes(&norm, url, at_ptr - url,
+						       "", URL_RESERVED)) {
+				if (out_info) {
+					out_info->url = NULL;
+					out_info->err = _("invalid %XX escape sequence");
+				}
+				strbuf_release(&norm);
+				return NULL;
+			}
+			colon_ptr = strchr(norm.buf + scheme_len + 3, ':');
+			if (colon_ptr) {
+				passwd_off = (colon_ptr + 1) - norm.buf;
+				passwd_len = norm.len - passwd_off;
+				user_len = (passwd_off - 1) - (scheme_len + 3);
+			} else {
+				user_len = norm.len - (scheme_len + 3);
+			}
+		}
+		strbuf_addch(&norm, '@');
+		url_len -= (++at_ptr - url);
+		url = at_ptr;
+	}
+
+
+	/*
+	 * Copy the host part excluding any port part, no %-escapes allowed
+	 */
+	if (!url_len || strchr(":/?#", *url)) {
+		/* Missing host invalid for all URL schemes except file */
+		if (strncmp(norm.buf, "file:", 5)) {
+			if (out_info) {
+				out_info->url = NULL;
+				out_info->err = _("missing host and scheme is not 'file:'");
+			}
+			strbuf_release(&norm);
+			return NULL;
+		}
+	} else {
+		host_off = norm.len;
+	}
+	colon_ptr = slash_ptr - 1;
+	while (colon_ptr > url && *colon_ptr != ':' && *colon_ptr != ']')
+		colon_ptr--;
+	if (*colon_ptr != ':') {
+		colon_ptr = slash_ptr;
+	} else if (!host_off && colon_ptr < slash_ptr && colon_ptr + 1 != slash_ptr) {
+		/* file: URLs may not have a port number */
+		if (out_info) {
+			out_info->url = NULL;
+			out_info->err = _("a 'file:' URL may not have a port number");
+		}
+		strbuf_release(&norm);
+		return NULL;
+	}
+	spanned = strspn(url, URL_HOST_CHARS);
+	if (spanned < colon_ptr - url) {
+		/* Host name has invalid characters */
+		if (out_info) {
+			out_info->url = NULL;
+			out_info->err = _("invalid characters in host name");
+		}
+		strbuf_release(&norm);
+		return NULL;
+	}
+	while (url < colon_ptr) {
+		strbuf_addch(&norm, tolower(*url++));
+		url_len--;
+	}
+
+
+	/*
+	 * Check the port part and copy if not the default (after removing any
+	 * leading 0s); no %-escapes allowed
+	 */
+	if (colon_ptr < slash_ptr) {
+		/* skip the ':' and leading 0s but not the last one if all 0s */
+		url++;
+		url += strspn(url, "0");
+		if (url == slash_ptr && url[-1] == '0')
+			url--;
+		if (url == slash_ptr) {
+			/* Skip ":" port with no number, it's same as default */
+		} else if (slash_ptr - url == 2 &&
+			   !strncmp(norm.buf, "http:", 5) &&
+			   !strncmp(url, "80", 2)) {
+			/* Skip http :80 as it's the default */
+		} else if (slash_ptr - url == 3 &&
+			   !strncmp(norm.buf, "https:", 6) &&
+			   !strncmp(url, "443", 3)) {
+			/* Skip https :443 as it's the default */
+		} else {
+			/*
+			 * Port number must be all digits with leading 0s removed
+			 * and since all the protocols we deal with have a 16-bit
+			 * port number it must also be in the range 1..65535
+			 * 0 is not allowed because that means "next available"
+			 * on just about every system and therefore cannot be used
+			 */
+			unsigned long pnum = 0;
+			spanned = strspn(url, URL_DIGIT);
+			if (spanned < slash_ptr - url) {
+				/* port number has invalid characters */
+				if (out_info) {
+					out_info->url = NULL;
+					out_info->err = _("invalid port number");
+				}
+				strbuf_release(&norm);
+				return NULL;
+			}
+			if (slash_ptr - url <= 5)
+				pnum = strtoul(url, NULL, 10);
+			if (pnum == 0 || pnum > 65535) {
+				/* port number not in range 1..65535 */
+				if (out_info) {
+					out_info->url = NULL;
+					out_info->err = _("invalid port number");
+				}
+				strbuf_release(&norm);
+				return NULL;
+			}
+			strbuf_addch(&norm, ':');
+			strbuf_add(&norm, url, slash_ptr - url);
+			port_len = slash_ptr - url;
+		}
+		url_len -= slash_ptr - colon_ptr;
+		url = slash_ptr;
+	}
+	if (host_off)
+		host_len = norm.len - host_off;
+
+
+	/*
+	 * Now copy the path resolving any . and .. segments being careful not
+	 * to corrupt the URL by unescaping any delimiters, but do add an
+	 * initial '/' if it's missing and do normalize any %-escape sequences.
+	 */
+	path_off = norm.len;
+	path_start = norm.buf + path_off;
+	strbuf_addch(&norm, '/');
+	if (*url == '/') {
+		url++;
+		url_len--;
+	}
+	for (;;) {
+		const char *seg_start = norm.buf + norm.len;
+		const char *next_slash = url + strcspn(url, "/?#");
+		int skip_add_slash = 0;
+		/*
+		 * RFC 3689 indicates that any . or .. segments should be
+		 * unescaped before being checked for.
+		 */
+		if (!append_normalized_escapes(&norm, url, next_slash - url, "",
+					       URL_RESERVED)) {
+			if (out_info) {
+				out_info->url = NULL;
+				out_info->err = _("invalid %XX escape sequence");
+			}
+			strbuf_release(&norm);
+			return NULL;
+		}
+		if (!strcmp(seg_start, ".")) {
+			/* ignore a . segment; be careful not to remove initial '/' */
+			if (seg_start == path_start + 1) {
+				strbuf_setlen(&norm, norm.len - 1);
+				skip_add_slash = 1;
+			} else {
+				strbuf_setlen(&norm, norm.len - 2);
+			}
+		} else if (!strcmp(seg_start, "..")) {
+			/*
+			 * ignore a .. segment and remove the previous segment;
+			 * be careful not to remove initial '/' from path
+			 */
+			const char *prev_slash = norm.buf + norm.len - 3;
+			if (prev_slash == path_start) {
+				/* invalid .. because no previous segment to remove */
+				if (out_info) {
+					out_info->url = NULL;
+					out_info->err = _("invalid '..' path segment");
+				}
+				strbuf_release(&norm);
+				return NULL;
+			}
+			while (*--prev_slash != '/') {}
+			if (prev_slash == path_start) {
+				strbuf_setlen(&norm, prev_slash - norm.buf + 1);
+				skip_add_slash = 1;
+			} else {
+				strbuf_setlen(&norm, prev_slash - norm.buf);
+			}
+		}
+		url_len -= next_slash - url;
+		url = next_slash;
+		/* if the next char is not '/' done with the path */
+		if (*url != '/')
+			break;
+		url++;
+		url_len--;
+		if (!skip_add_slash)
+			strbuf_addch(&norm, '/');
+	}
+	path_len = norm.len - path_off;
+
+
+	/*
+	 * Now simply copy the rest, if any, only normalizing %-escapes and
+	 * being careful not to corrupt the URL by unescaping any delimiters.
+	 */
+	if (*url) {
+		if (!append_normalized_escapes(&norm, url, url_len, "", URL_RESERVED)) {
+			if (out_info) {
+				out_info->url = NULL;
+				out_info->err = _("invalid %XX escape sequence");
+			}
+			strbuf_release(&norm);
+			return NULL;
+		}
+	}
+
+
+	result = strbuf_detach(&norm, &result_len);
+	if (out_info) {
+		out_info->url = result;
+		out_info->err = NULL;
+		out_info->url_len = result_len;
+		out_info->scheme_len = scheme_len;
+		out_info->user_off = user_off;
+		out_info->user_len = user_len;
+		out_info->passwd_off = passwd_off;
+		out_info->passwd_len = passwd_len;
+		out_info->host_off = host_off;
+		out_info->host_len = host_len;
+		out_info->port_len = port_len;
+		out_info->path_off = path_off;
+		out_info->path_len = path_len;
+	}
+	return result;
+}
+
+static size_t http_options_url_match_prefix(const char *url,
+					    const char *url_prefix,
+					    size_t url_prefix_len)
+{
+	/*
+	 * url_prefix matches url if url_prefix is an exact match for url or it
+	 * is a prefix of url and the match ends on a path component boundary.
+	 * Both url and url_prefix are considered to have an implicit '/' on the
+	 * end for matching purposes if they do not already.
+	 *
+	 * url must be NUL terminated.  url_prefix_len is the length of
+	 * url_prefix which need not be NUL terminated.
+	 *
+	 * The return value is the length of the match in characters (including
+	 * the final '/' even if it's implicit) or 0 for no match.
+	 *
+	 * Passing NULL as url and/or url_prefix will always cause 0 to be
+	 * returned without causing any faults.
+	 */
+	if (!url || !url_prefix)
+		return 0;
+	if (!url_prefix_len || (url_prefix_len == 1 && *url_prefix == '/'))
+		return (!*url || *url == '/') ? 1 : 0;
+	if (url_prefix[url_prefix_len - 1] == '/')
+		url_prefix_len--;
+	if (strncmp(url, url_prefix, url_prefix_len))
+		return 0;
+	if ((strlen(url) == url_prefix_len) || (url[url_prefix_len] == '/'))
+		return url_prefix_len + 1;
+	return 0;
+}
+
+int match_urls(const struct url_info *url,
+	       const struct url_info *url_prefix,
+	       int *exactusermatch)
+{
+	/*
+	 * url_prefix matches url if the scheme, host and port of url_prefix
+	 * are the same as those of url and the path portion of url_prefix
+	 * is the same as the path portion of url or it is a prefix that
+	 * matches at a '/' boundary.  If url_prefix contains a user name,
+	 * that must also exactly match the user name in url.
+	 *
+	 * If the user, host, port and path match in this fashion, the returned
+	 * value is the length of the path match including any implicit
+	 * final '/'.  For example, "http://me@example.com/path" is matched by
+	 * "http://example.com" with a path length of 1.
+	 *
+	 * If there is a match and exactusermatch is not NULL, then
+	 * *exactusermatch will be set to true if both url and url_prefix
+	 * contained a user name or false if url_prefix did not have a
+	 * user name.  If there is no match *exactusermatch is left untouched.
+	 */
+	int usermatched = 0;
+	int pathmatchlen;
+
+	if (!url || !url_prefix || !url->url || !url_prefix->url)
+		return 0;
+
+	/* check the scheme */
+	if (url_prefix->scheme_len != url->scheme_len ||
+	    strncmp(url->url, url_prefix->url, url->scheme_len))
+		return 0; /* schemes do not match */
+
+	/* check the user name if url_prefix has one */
+	if (url_prefix->user_off) {
+		if (!url->user_off || url->user_len != url_prefix->user_len ||
+		    strncmp(url->url + url->user_off,
+			    url_prefix->url + url_prefix->user_off,
+			    url->user_len))
+			return 0; /* url_prefix has a user but it's not a match */
+		usermatched = 1;
+	}
+
+	/* check the host and port */
+	if (url_prefix->host_len != url->host_len ||
+	    strncmp(url->url + url->host_off,
+		    url_prefix->url + url_prefix->host_off, url->host_len))
+		return 0; /* host names and/or ports do not match */
+
+	/* check the path */
+	pathmatchlen = http_options_url_match_prefix(
+		url->url + url->path_off,
+		url_prefix->url + url_prefix->path_off,
+		url_prefix->url_len - url_prefix->path_off);
+
+	if (pathmatchlen && exactusermatch)
+		*exactusermatch = usermatched;
+	return pathmatchlen;
+}
diff --git a/url-match.h b/url-match.h
new file mode 100644
index 0000000..e9cecd3
--- /dev/null
+++ b/url-match.h
@@ -0,0 +1,35 @@
+#ifndef URL_MATCH_H
+
+struct url_info {
+	/* normalized url on success, must be freed, otherwise NULL */
+	char *url;
+	/* if !url, a brief reason for the failure, otherwise NULL */
+	const char *err;
+
+	/* the rest of the fields are only set if url != NULL */
+
+	size_t url_len;		/* total length of url (which is now normalized) */
+	size_t scheme_len;	/* length of scheme name (excluding final :) */
+	size_t user_off;	/* offset into url to start of user name (0 => none) */
+	size_t user_len;	/* length of user name; if user_off != 0 but
+				   user_len == 0, an empty user name was given */
+	size_t passwd_off;	/* offset into url to start of passwd (0 => none) */
+	size_t passwd_len;	/* length of passwd; if passwd_off != 0 but
+				   passwd_len == 0, an empty passwd was given */
+	size_t host_off;	/* offset into url to start of host name (0 => none) */
+	size_t host_len;	/* length of host name; this INCLUDES any ':portnum';
+				 * file urls may have host_len == 0 */
+	size_t port_len;	/* if a portnum is present (port_len != 0), it has
+				 * this length (excluding the leading ':') at the
+				 * end of the host name (always 0 for file urls) */
+	size_t path_off;	/* offset into url to the start of the url path;
+				 * this will always point to a '/' character
+				 * after the url has been normalized */
+	size_t path_len;	/* length of path portion excluding any trailing
+				 * '?...' and '#...' portion; will always be >= 1 */
+};
+
+extern char *url_normalize(const char *, struct url_info *);
+extern int match_urls(const struct url_info *url, const struct url_info *url_prefix, int *exactusermatch);
+
+#endif /* URL_MATCH_H */
-- 
1.8.4-rc0-137-g17832d4

[PATCH 2/3] builtin/config: refactor collect_config()

[Junio C Hamano]

In order to reuse the logic to format the configuration value while
honouring the requested type, split this function into two.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/config.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 33c9bf9..12c5073 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -100,25 +100,13 @@ struct strbuf_list {
 	int alloc;
 };
 
-static int collect_config(const char *key_, const char *value_, void *cb)
+static int format_config(struct strbuf *buf, const char *key_, const char *value_)
 {
-	struct strbuf_list *values = cb;
-	struct strbuf *buf;
-	char value[256];
-	const char *vptr = value;
 	int must_free_vptr = 0;
 	int must_print_delim = 0;
+	char value[256];
+	const char *vptr = value;
 
-	if (!use_key_regexp && strcmp(key_, key))
-		return 0;
-	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
-		return 0;
-	if (regexp != NULL &&
-	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
-		return 0;
-
-	ALLOC_GROW(values->items, values->nr + 1, values->alloc);
-	buf = &values->items[values->nr++];
 	strbuf_init(buf, 0);
 
 	if (show_keys) {
@@ -126,7 +114,7 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 		must_print_delim = 1;
 	}
 	if (types == TYPE_INT)
-		sprintf(value, "%d", git_config_int(key_, value_?value_:""));
+		sprintf(value, "%d", git_config_int(key_, value_ ? value_ : ""));
 	else if (types == TYPE_BOOL)
 		vptr = git_config_bool(key_, value_) ? "true" : "false";
 	else if (types == TYPE_BOOL_OR_INT) {
@@ -154,15 +142,27 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 	strbuf_addch(buf, term);
 
 	if (must_free_vptr)
-		/* If vptr must be freed, it's a pointer to a
-		 * dynamically allocated buffer, it's safe to cast to
-		 * const.
-		*/
 		free((char *)vptr);
-
 	return 0;
 }
 
+static int collect_config(const char *key_, const char *value_, void *cb)
+{
+	struct strbuf_list *values = cb;
+
+	if (!use_key_regexp && strcmp(key_, key))
+		return 0;
+	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
+		return 0;
+	if (regexp != NULL &&
+	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
+		return 0;
+
+	ALLOC_GROW(values->items, values->nr + 1, values->alloc);
+
+	return format_config(&values->items[values->nr++], key_, value_);
+}
+
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = CONFIG_GENERIC_ERROR;
-- 
1.8.4-rc0-137-g17832d4

[PATCH 3/3] config: --get-urlmatch

[Junio C Hamano]

"git config --get-urlmatch $section[.$variable] $url" is a way to
learn what the configured value for $section.$variable is for the
given URL, using the logic introduced by the http.<url>.config
topic.  In addition to $section.$variable, entries in the
configuration file(s) that match $section.<urlpattern>.$variable are
looked up and the one with <urlpattern> that matches the given $url
the best is used to answer the query.

This can still be further refactored to remove code from http_options()
in http.c, I think.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/config.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/builtin/config.c b/builtin/config.c
index 12c5073..c1d32ae 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -2,6 +2,7 @@
 #include "cache.h"
 #include "color.h"
 #include "parse-options.h"
+#include "url-match.h"
 
 static const char *const builtin_config_usage[] = {
 	N_("git config [options]"),
@@ -41,6 +42,7 @@ static int respect_includes = -1;
 #define ACTION_SET_ALL (1<<12)
 #define ACTION_GET_COLOR (1<<13)
 #define ACTION_GET_COLORBOOL (1<<14)
+#define ACTION_GET_URLMATCH (1<<15)
 
 #define TYPE_BOOL (1<<0)
 #define TYPE_INT (1<<1)
@@ -57,6 +59,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"), ACTION_GET),
 	OPT_BIT(0, "get-all", &actions, N_("get all values: key [value-regex]"), ACTION_GET_ALL),
 	OPT_BIT(0, "get-regexp", &actions, N_("get values for regexp: name-regex [value-regex]"), ACTION_GET_REGEXP),
+	OPT_BIT(0, "get-urlmatch", &actions, N_("get value specific for the URL: section[.var] URL"), ACTION_GET_URLMATCH),
 	OPT_BIT(0, "replace-all", &actions, N_("replace all matching variables: name value [value_regex]"), ACTION_REPLACE_ALL),
 	OPT_BIT(0, "add", &actions, N_("add a new variable: name value"), ACTION_ADD),
 	OPT_BIT(0, "unset", &actions, N_("remove a variable: name [value-regex]"), ACTION_UNSET),
@@ -348,6 +351,140 @@ static int get_colorbool(int print)
 		return get_colorbool_found ? 0 : 1;
 }
 
+struct urlmatch_collect {
+	struct string_list vars;
+	struct url_info url;
+	const char *section;
+	const char *key;
+};
+
+struct urlmatch_item {
+	size_t max_matched_len;
+	char user_matched;
+	char value_is_null;
+	struct strbuf value;
+};
+
+static int urlmatch_collect(const char *var, const char *value, void *cb)
+{
+	struct string_list_item *item;
+	struct urlmatch_collect *collect = cb;
+	struct urlmatch_item *matched;
+	struct url_info *url = &collect->url;
+	const char *key, *dot;
+	size_t matchlen = 0;
+	int usermatch = 0;
+
+	key = skip_prefix(var, collect->section);
+	if (!key || *(key++) != '.')
+		return 0; /* not interested */
+	dot = strrchr(key, '.');
+	if (dot) {
+		char *config_url, *norm_url;
+		struct url_info norm_info;
+		int matchlen;
+
+		config_url = xmemdupz(key, dot - key);
+		norm_url = url_normalize(config_url, &norm_info);
+		free(config_url);
+		if (!norm_url)
+			return 0;
+		matchlen = match_urls(url, &norm_info, &usermatch);
+		free(norm_url);
+		if (!matchlen)
+			return 0;
+		key = dot + 1;
+	}
+
+	if (collect->key && strcmp(key, collect->key))
+		return 0;
+
+	item = string_list_insert(&collect->vars, key);
+	if (!item->util) {
+		matched = xcalloc(1, sizeof(*matched));
+		item->util = matched;
+		strbuf_init(&matched->value, 0);
+	} else {
+		matched = item->util;
+		/*
+		 * Is our match shorter?  Is our match the same
+		 * length, and without user while the current
+		 * candidate is with user?  Then we cannot use it.
+		 */
+		if (matchlen < matched->max_matched_len ||
+		    ((matchlen == matched->max_matched_len) &&
+		     (!usermatch && matched->user_matched)))
+			return 0;
+		/*
+		 * Otherwise, release the old one and replace
+		 * with ours.
+		 */
+		strbuf_release(&matched->value);
+	}
+
+	matched->max_matched_len = matchlen;
+	matched->user_matched = usermatch;
+	if (value) {
+		strbuf_addstr(&matched->value, value);
+		matched->value_is_null = 0;
+	} else {
+		matched->value_is_null = 1;
+	}
+	return 0;
+}
+
+static int get_urlmatch(const char *var, const char *url)
+{
+	const char *section_tail;
+	struct string_list_item *item;
+	struct urlmatch_collect collect = { STRING_LIST_INIT_DUP };
+
+	if (!url_normalize(url, &collect.url))
+		die(collect.url.err);
+
+	section_tail = strchr(var, '.');
+	if (section_tail) {
+		collect.section = xmemdupz(var, section_tail - var);
+		collect.key = strrchr(var, '.') + 1;
+		show_keys = 0;
+	} else {
+		collect.section = var;
+		collect.key = NULL;
+		show_keys = 1;
+	}
+
+	git_config_with_options(urlmatch_collect, &collect,
+				given_config_file, respect_includes);
+
+	for_each_string_list_item(item, &collect.vars) {
+		struct urlmatch_item *matched = item->util;
+		struct strbuf key = STRBUF_INIT;
+		struct strbuf buf = STRBUF_INIT;
+
+		strbuf_addstr(&key, collect.section);
+		strbuf_addch(&key, '.');
+		strbuf_addstr(&key, item->string);
+		format_config(&buf, key.buf,
+			      matched->value_is_null ? NULL : matched->value.buf);
+		fwrite(buf.buf, 1, buf.len, stdout);
+		strbuf_release(&key);
+		strbuf_release(&buf);
+
+		strbuf_release(&matched->value);
+	}
+	string_list_clear(&collect.vars, 1);
+
+	/*
+	 * section name may have been copied to replace the dot, in which
+	 * case it needs to be freed.  key name is either NULL (e.g. 'http'
+	 * alone) or points into var (e.g. 'http.savecookies'), and we do
+	 * not own the storage.
+	 */
+	if (collect.section != var)
+		free((void *)collect.section);
+	return 0;
+}
+
 int cmd_config(int argc, const char **argv, const char *prefix)
 {
 	int nongit = !startup_info->have_repository;
@@ -499,6 +636,10 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 1, 2);
 		return get_value(argv[0], argv[1]);
 	}
+	else if (actions == ACTION_GET_URLMATCH) {
+		check_argc(argc, 2, 2);
+		return get_urlmatch(argv[0], argv[1]);
+	}
 	else if (actions == ACTION_UNSET) {
 		check_argc(argc, 1, 2);
 		if (argc == 2)
-- 
1.8.4-rc0-137-g17832d4

Re: [PATCH 3/3] config: --get-urlmatch

[Jeff King]

On Mon, Jul 29, 2013 at 03:49:10PM -0700, Junio C Hamano wrote:

> "git config --get-urlmatch $section[.$variable] $url" is a way to
> learn what the configured value for $section.$variable is for the
> given URL, using the logic introduced by the http.<url>.config
> topic.

That interface looks good to me. It matches the --get-all/--get-regexp
options of git-config. It does not quite match --get-color, which is
really a type (like --bool or --int), but I think --get-color is the odd
one out there.

> This can still be further refactored to remove code from http_options()
> in http.c, I think.

Yeah, that would be the ultimate goal. I think you would just need to
keep the same string_list there, with one urlmatch_item per key that we
care about.

> ---
>  builtin/config.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++++++

It seems like some of the logic here should be reusable for http.c.
Should it be in config.c instead?

> +struct urlmatch_collect {
> +	struct string_list vars;
> +	struct url_info url;
> +	const char *section;
> +	const char *key;
> +};
> +
> +struct urlmatch_item {
> +	size_t max_matched_len;
> +	char user_matched;
> +	char value_is_null;
> +	struct strbuf value;
> +};

I think you ultimately want such a string_list for matching arbitrary
numbers of keys, but do you need it for the git-config case?

You will always be matching collect->key, so you will only ever insert a
single item into the collect->vars list. IOW, this could be:

  struct urlmatch_collect {
    struct url_info url;
    const char *section;
    const char *key;
    struct urlmatch_item match;
  };

I don't mind if it is more complicated than this single-case needs to be
if the code is also being used to http.c, but I haven't seen that yet
(and this code would not used as-is, as you would want to call the
function with many potential collect->key values, but without
re-normalizing the URL over and over).

And of course, it needs docs and tests. :) The latter can probably come
by converting some of the test-url-normalize tests to just use
git-config instead.

-Peff

Re: [PATCH 3/3] config: --get-urlmatch

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> +struct urlmatch_item {
>> +	size_t max_matched_len;
>> +	char user_matched;
>> +	char value_is_null;
>> +	struct strbuf value;
>> +};
>
> I think you ultimately want such a string_list for matching arbitrary
> numbers of keys, but do you need it for the git-config case?

"git config" does not know the semantics of each key, nor available
set of keys, no?  The string-list is only to support

    git config --get-urlmatch http http://www.google.com/

i.e. "list everything under http.* hierarchy".

And unlike http_option() which can incrementally always call back
the setter (and let it override older value), the command has to
read everything through and then give us the final value, so I do
not think we can get away without one.

> You will always be matching collect->key, so you will only ever insert a
> single item into the collect->vars list. IOW, this could be:
>
>   struct urlmatch_collect {
>     struct url_info url;
>     const char *section;
>     const char *key;
>     struct urlmatch_item match;
>   };
>
> I don't mind if it is more complicated than this single-case needs to be
> if the code is also being used to http.c, but I haven't seen that yet

That is in the works, of course ;-)

Re: [PATCH 3/3] config: --get-urlmatch

[Jeff King]

On Mon, Jul 29, 2013 at 06:33:43PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> >> +struct urlmatch_item {
> >> +	size_t max_matched_len;
> >> +	char user_matched;
> >> +	char value_is_null;
> >> +	struct strbuf value;
> >> +};
> >
> > I think you ultimately want such a string_list for matching arbitrary
> > numbers of keys, but do you need it for the git-config case?
> 
> "git config" does not know the semantics of each key, nor available
> set of keys, no?  The string-list is only to support
> 
>     git config --get-urlmatch http http://www.google.com/
> 
> i.e. "list everything under http.* hierarchy".

Ah, I missed that you could leave "key" empty. I had expected
collect->key to be filled in, at which point you only ever have one such
key (and you do not need to know the semantics, only which one is the
"winner").

-Peff

Re: [PATCH 3/3] config: --get-urlmatch

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Ah, I missed that you could leave "key" empty.

Yes, the general syntax is

	git config [--<type>] --get-urlmatch <section>[.<key>] <url>

and giving <section> without a specific <key> would list all the
variables in the section that apply to <url>.

This is why we should do documentation at some point before
publishing a patch series; sorry about that.

Re: [PATCH 3/3] config: --get-urlmatch

[Kyle J. McKay]

On Jul 29, 2013, at 15:49, Junio C Hamano wrote:

> "git config --get-urlmatch $section[.$variable] $url" is a way to
> learn what the configured value for $section.$variable is for the
> given URL, using the logic introduced by the http.<url>.config
> topic.  In addition to $section.$variable, entries in the
> configuration file(s) that match $section.<urlpattern>.$variable are
> looked up and the one with <urlpattern> that matches the given $url
> the best is used to answer the query.
>
> This can still be further refactored to remove code from  
> http_options()
> in http.c, I think.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin/config.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++ 
> +++++++++
> 1 file changed, 141 insertions(+)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index 12c5073..c1d32ae 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
[...]
> +static int get_urlmatch(const char *var, const char *url)
> +{
> +	const char *section_tail;
> +	struct string_list_item *item;
> +	struct urlmatch_collect collect = { STRING_LIST_INIT_DUP };
> +
> +	if (!url_normalize(url, &collect.url))
> +		die(collect.url.err);

The value now stored in collect.url.url is never freed.

> +
> +	section_tail = strchr(var, '.');
> +	if (section_tail) {
> +		collect.section = xmemdupz(var, section_tail - var);
> +		collect.key = strrchr(var, '.') + 1;
> +		show_keys = 0;
> +	} else {
> +		collect.section = var;
> +		collect.key = NULL;
> +		show_keys = 1;
> +	}
> +
> +	git_config_with_options(urlmatch_collect, &collect,
> +				given_config_file, respect_includes);
> +
> +	for_each_string_list_item(item, &collect.vars) {
> +		struct urlmatch_item *matched = item->util;
> +		struct strbuf key = STRBUF_INIT;
> +		struct strbuf buf = STRBUF_INIT;
> +
> +		strbuf_addstr(&key, collect.section);
> +		strbuf_addch(&key, '.');
> +		strbuf_addstr(&key, item->string);
> +		format_config(&buf, key.buf,
> +			      matched->value_is_null ? NULL : matched->value.buf);
> +		fwrite(buf.buf, 1, buf.len, stdout);
> +		strbuf_release(&key);
> +		strbuf_release(&buf);
> +
> +		strbuf_release(&matched->value);
> +	}
> +	string_list_clear(&collect.vars, 1);

Needs something like this here:

+	free(collect.url.url);

> +
> +	/*
> +	 * section name may have been copied to replace the dot, in which
> +	 * case it needs to be freed.  key name is either NULL (e.g. 'http'
> +	 * alone) or points into var (e.g. 'http.savecookies'), and we do
> +	 * not own the storage.
> +	 */
> +	if (collect.section != var)
> +		free((void *)collect.section);
> +	return 0;
> +}

Still needed after 4/3 except it's now config.url.url instead.

[PATCH 4/3] url-match: generalize configuration collection logic

[Junio C Hamano]

Instead of always collecting the found value in the strbuf in the
string list item for each found key, allow a set of callback
functions to be specified by the user to be called back at strategic
places.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 builtin/config.c | 117 +++++++++++++++++--------------------------------------
 url-match.c      |  75 +++++++++++++++++++++++++++++++++++
 url-match.h      |  22 +++++++++++
 3 files changed, 133 insertions(+), 81 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index c1d32ae..173c18f 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -351,79 +351,17 @@ static int get_colorbool(int print)
 		return get_colorbool_found ? 0 : 1;
 }
 
-struct urlmatch_collect {
-	struct string_list vars;
-	struct url_info url;
-	const char *section;
-	const char *key;
-};
-
-struct urlmatch_item {
-	size_t max_matched_len;
-	char user_matched;
+struct urlmatch_config_item {
+	struct urlmatch_item match;
 	char value_is_null;
 	struct strbuf value;
 };
 
-static int urlmatch_collect(const char *var, const char *value, void *cb)
+static int urlmatch_collect_fn(const char *var, const char *value,
+			       void *cb, void *matched_cb)
 {
-	struct string_list_item *item;
-	struct urlmatch_collect *collect = cb;
-	struct urlmatch_item *matched;
-	struct url_info *url = &collect->url;
-	const char *key, *dot;
-	size_t matchlen = 0;
-	int usermatch = 0;
-
-	key = skip_prefix(var, collect->section);
-	if (!key || *(key++) != '.')
-		return 0; /* not interested */
-	dot = strrchr(key, '.');
-	if (dot) {
-		char *config_url, *norm_url;
-		struct url_info norm_info;
-		int matchlen;
-
-		config_url = xmemdupz(key, dot - key);
-		norm_url = url_normalize(config_url, &norm_info);
-		free(config_url);
-		if (!norm_url)
-			return 0;
-		matchlen = match_urls(url, &norm_info, &usermatch);
-		free(norm_url);
-		if (!matchlen)
-			return 0;
-		key = dot + 1;
-	}
-
-	if (collect->key && strcmp(key, collect->key))
-		return 0;
+	struct urlmatch_config_item *matched = matched_cb;
 
-	item = string_list_insert(&collect->vars, key);
-	if (!item->util) {
-		matched = xcalloc(1, sizeof(*matched));
-		item->util = matched;
-		strbuf_init(&matched->value, 0);
-	} else {
-		matched = item->util;
-		/*
-		 * Is our match shorter?  Is our match the same
-		 * length, and without user while the current
-		 * candidate is with user?  Then we cannot use it.
-		 */
-		if (matchlen < matched->max_matched_len ||
-		    ((matchlen == matched->max_matched_len) &&
-		     (!usermatch && matched->user_matched)))
-			return 0;
-		/*
-		 * Otherwise, release the old one and replace
-		 * with ours.
-		 */
-		strbuf_release(&matched->value);
-	}
-
-	matched->max_matched_len = matchlen;
-	matched->user_matched = usermatch;
 	if (value) {
 		strbuf_addstr(&matched->value, value);
 		matched->value_is_null = 0;
@@ -433,35 +371,52 @@ static int urlmatch_collect(const char *var, const char *value, void *cb)
 	return 0;
 }
 
+static void *urlmatch_alloc_fn(const char *var, const char *value, void *cb)
+{
+	return xcalloc(1, sizeof(struct urlmatch_config_item));
+}
+
+static void urlmatch_clear_fn(void *matched_cb)
+{
+	struct urlmatch_config_item *matched = matched_cb;
+	strbuf_init(&matched->value, 0);
+}
+
 static int get_urlmatch(const char *var, const char *url)
 {
 	const char *section_tail;
 	struct string_list_item *item;
-	struct urlmatch_collect collect = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+
+	config.fn = urlmatch_collect_fn;
+	config.cascade_fn = NULL;
+	config.item_alloc = urlmatch_alloc_fn;
+	config.item_clear = urlmatch_clear_fn;
+	config.cb = NULL;
 
-	if (!url_normalize(url, &collect.url))
-		die(collect.url.err);
+	if (!url_normalize(url, &config.url))
+		die(config.url.err);
 
 	section_tail = strchr(var, '.');
 	if (section_tail) {
-		collect.section = xmemdupz(var, section_tail - var);
-		collect.key = strrchr(var, '.') + 1;
+		config.section = xmemdupz(var, section_tail - var);
+		config.key = strrchr(var, '.') + 1;
 		show_keys = 0;
 	} else {
-		collect.section = var;
-		collect.key = NULL;
+		config.section = var;
+		config.key = NULL;
 		show_keys = 1;
 	}
 
-	git_config_with_options(urlmatch_collect, &collect,
+	git_config_with_options(urlmatch_config_entry, &config,
 				given_config_file, respect_includes);
 
-	for_each_string_list_item(item, &collect.vars) {
-		struct urlmatch_item *matched = item->util;
+	for_each_string_list_item(item, &config.vars) {
+		struct urlmatch_config_item *matched = item->util;
 		struct strbuf key = STRBUF_INIT;
 		struct strbuf buf = STRBUF_INIT;
 
-		strbuf_addstr(&key, collect.section);
+		strbuf_addstr(&key, config.section);
 		strbuf_addch(&key, '.');
 		strbuf_addstr(&key, item->string);
 		format_config(&buf, key.buf,
@@ -472,7 +427,7 @@ static int get_urlmatch(const char *var, const char *url)
 
 		strbuf_release(&matched->value);
 	}
-	string_list_clear(&collect.vars, 1);
+	string_list_clear(&config.vars, 1);
 
 	/*
 	 * section name may have been copied to replace the dot, in which
@@ -480,8 +435,8 @@ static int get_urlmatch(const char *var, const char *url)
 	 * alone) or points into var (e.g. 'http.savecookies'), and we do
 	 * not own the storage.
 	 */
-	if (collect.section != var)
-		free((void *)collect.section);
+	if (config.section != var)
+		free((void *)config.section);
 	return 0;
 }
 
diff --git a/url-match.c b/url-match.c
index 424a0a0..290ba40 100644
--- a/url-match.c
+++ b/url-match.c
@@ -466,3 +466,78 @@ int match_urls(const struct url_info *url,
 		*exactusermatch = usermatched;
 	return pathmatchlen;
 }
+
+int urlmatch_config_entry(const char *var, const char *value, void *cb)
+{
+	struct string_list_item *item;
+	struct urlmatch_config *collect = cb;
+	struct urlmatch_item *matched;
+	struct url_info *url = &collect->url;
+	const char *key, *dot;
+	struct strbuf synthkey = STRBUF_INIT;
+	size_t matched_len = 0;
+	int user_matched = 0;
+	int retval;
+
+	key = skip_prefix(var, collect->section);
+	if (!key || *(key++) != '.') {
+		if (collect->cascade_fn)
+			return collect->cascade_fn(var, value, cb);
+		return 0; /* not interested */
+	}
+	dot = strrchr(key, '.');
+	if (dot) {
+		char *config_url, *norm_url;
+		struct url_info norm_info;
+
+		config_url = xmemdupz(key, dot - key);
+		norm_url = url_normalize(config_url, &norm_info);
+		free(config_url);
+		if (!norm_url)
+			return 0;
+		matched_len = match_urls(url, &norm_info, &user_matched);
+		free(norm_url);
+		if (!matched_len)
+			return 0;
+		key = dot + 1;
+	}
+
+	if (collect->key && strcmp(key, collect->key))
+		return 0;
+
+	item = string_list_insert(&collect->vars, key);
+	if (!item->util) {
+		if (collect->item_alloc)
+			matched = collect->item_alloc(var, value, cb);
+		else
+			matched = xcalloc(1, sizeof(*matched));
+		item->util = matched;
+	} else {
+		matched = item->util;
+		/*
+		 * Is our match shorter?  Is our match the same
+		 * length, and without user while the current
+		 * candidate is with user?  Then we cannot use it.
+		 */
+		if (matched_len < matched->matched_len ||
+		    ((matched_len == matched->matched_len) &&
+		     (!user_matched && matched->user_matched)))
+			return 0;
+		/*
+		 * Otherwise, clear the old one and replace it
+		 * with this one.
+		 */
+		if (collect->item_clear)
+			collect->item_clear(matched);
+	}
+
+	matched->matched_len = matched_len;
+	matched->user_matched = user_matched;
+	strbuf_addstr(&synthkey, collect->section);
+	strbuf_addch(&synthkey, '.');
+	strbuf_addstr(&synthkey, key);
+	retval = collect->fn(synthkey.buf, value, collect->cb, matched);
+
+	strbuf_release(&synthkey);
+	return retval;
+}
diff --git a/url-match.h b/url-match.h
index e9cecd3..c3a5783 100644
--- a/url-match.h
+++ b/url-match.h
@@ -1,4 +1,5 @@
 #ifndef URL_MATCH_H
+#include "string-list.h"
 
 struct url_info {
 	/* normalized url on success, must be freed, otherwise NULL */
@@ -32,4 +33,25 @@ struct url_info {
 extern char *url_normalize(const char *, struct url_info *);
 extern int match_urls(const struct url_info *url, const struct url_info *url_prefix, int *exactusermatch);
 
+struct urlmatch_item {
+	size_t matched_len;
+	char user_matched;
+	/* possibly more */
+};
+
+struct urlmatch_config {
+	struct string_list vars;
+	struct url_info url;
+	const char *section;
+	const char *key;
+
+	void *cb;
+	int (*fn)(const char *var, const char *value, void *cb, void *matched);
+	int (*cascade_fn)(const char *var, const char *value, void *cb);
+	void *(*item_alloc)(const char *var, const char *value, void *cb);
+	void (*item_clear)(void *);
+};
+
+extern int urlmatch_config_entry(const char *var, const char *value, void *cb);
+
 #endif /* URL_MATCH_H */
-- 
1.8.4-rc0-137-g17832d4

[PATCH 5/3] revert most of the http_options() change

[Junio C Hamano]

With the previous preparation step, the earlier 1bb00006 (config:
add support for http.<url>.* settings, 2013-07-21) that introduced
many repeated changes:

        -if (!strcmp("http.key", var)) {
        +if (!strcmp("key", key)) {
        +       if (match_is_shorter(..., OPT_KEY_NAME))
        +               return 0;
                ... original processing for KEY_NAME ...
         }

can be reverted.

This allows us to also get rid of the "repeating yourself to the
death" enum http_option_type, and new code (like db/http-savecookies
patch) that wants to add a new configuration to the http.* subsystem
does not have to conflict with http.<urlpattern>.variable series at
all.

This is more-or-less how I want the endgame to look like. Not even
compile tested, but it is getting late so I'll show it to the list
in case people may want to play with it and polish it while I am
away for the night ;-).

If people can agree that this is going in the right direction,
perhaps we should rebuild Kyle's series without detouring of adding
and then yanking what 1bb00006 (config: add support for http.<url>.*
settings, 2013-07-21) did in the next cycle.

 http.c               | 181 +++++++++------------------------------------------
 test-url-normalize.c |   9 ++-
 2 files changed, 39 insertions(+), 151 deletions(-)

diff --git a/http.c b/http.c
index a91a00b..c7f513b 100644
--- a/http.c
+++ b/http.c
@@ -31,35 +31,6 @@ static CURL *curl_default;
 
 char curl_errorstr[CURL_ERROR_SIZE];
 
-enum http_option_type {
-	OPT_POST_BUFFER,
-	OPT_MIN_SESSIONS,
-	OPT_SSL_VERIFY,
-	OPT_SSL_TRY,
-	OPT_SSL_CERT,
-	OPT_SSL_CAINFO,
-	OPT_LOW_SPEED,
-	OPT_LOW_TIME,
-	OPT_NO_EPSV,
-	OPT_HTTP_PROXY,
-	OPT_COOKIE_FILE,
-	OPT_USER_AGENT,
-	OPT_PASSWD_REQ,
-#ifdef USE_CURL_MULTI
-	OPT_MAX_REQUESTS,
-#endif
-#if LIBCURL_VERSION_NUM >= 0x070903
-	OPT_SSL_KEY,
-#endif
-#if LIBCURL_VERSION_NUM >= 0x070908
-	OPT_SSL_CAPATH,
-#endif
-	OPT_MAX
-};
-
-static size_t http_option_max_matched_len[OPT_MAX];
-static int http_option_user_matched[OPT_MAX];
-
 static int curl_ssl_verify = -1;
 static int curl_ssl_try;
 static const char *ssl_cert;
@@ -171,119 +142,37 @@ static void process_curl_messages(void)
 }
 #endif
 
-static int match_is_ignored(size_t matchlen, int usermatch, enum http_option_type opt)
+static int http_options(const char *var, const char *value, void *cb, void *matched)
 {
-	/*
-	 * Compare matchlen to the last matched path length of option opt and
-	 * return true if matchlen is shorter than the last matched length
-	 * (meaning the config setting should be ignored).  Upon seeing the
-	 * _same_ key (i.e. new key has the same match length which is therefore
-	 * not shorter) the new setting will override the previous setting
-	 * unless the new setting did not match the user and the previous match
-	 * did.  Otherwise return false and record matchlen as the current last
-	 * matched path length of option opt and usermatch as the last user
-	 * matching state for option opt.
-	 */
-	if (matchlen < http_option_max_matched_len[opt])
-		return 1;
-	if (matchlen > http_option_max_matched_len[opt]) {
-		http_option_max_matched_len[opt] = matchlen;
-		http_option_user_matched[opt] = usermatch;
-		return 0;
-	}
-	/*
-	 * If a previous match of the same length explicitly matched the user
-	 * name, but the current match matched on any user, ignore it.
-	 */
-	if (!usermatch && http_option_user_matched[opt])
-		return 1;
-	http_option_user_matched[opt] = usermatch;
-	return 0;
-}
-
-static int http_options(const char *var, const char *value, void *cb)
-{
-	const struct url_info *info = cb;
-	const char *key, *dot;
-	size_t matchlen = 0;
-	int usermatch = 0;
-
-	key = skip_prefix(var, "http.");
-	if (!key)
-		return git_default_config(var, value, cb);
-
-	/*
-	 * If the part following the leading "http." contains a '.' then check
-	 * to see if the part between "http." and the last '.' is a match to
-	 * url.  If it's not then ignore the setting.  Otherwise set key to
-	 * point to the option which is the part after the final '.' and
-	 * use key in subsequent comparisons to determine the option type.
-	 */
-	dot = strrchr(key, '.');
-	if (dot) {
-		char *config_url;
-		struct url_info norm_info;
-		char *norm_url;
-
-		if (!info || !info->url)
-			return 0;
-		config_url = xmemdupz(key, dot - key);
-		norm_url = url_normalize(config_url, &norm_info);
-		free(config_url);
-		if (!norm_url)
-			return 0;
-		matchlen = match_urls(info, &norm_info, &usermatch);
-		free(norm_url);
-		if (!matchlen)
-			return 0;
-		key = dot + 1;
-	}
-
-	if (!strcmp("sslverify", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_SSL_VERIFY))
-			return 0;
+	if (!strcmp("http.sslverify", var)) {
 		curl_ssl_verify = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp("sslcert", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_SSL_CERT))
-			return 0;
+	if (!strcmp("http.sslcert", var)) {
 		return git_config_string(&ssl_cert, var, value);
 	}
 #if LIBCURL_VERSION_NUM >= 0x070903
-	if (!strcmp("sslkey", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_SSL_KEY))
-			return 0;
+	if (!strcmp("http.sslkey", var)) {
 		return git_config_string(&ssl_key, var, value);
 	}
 #endif
 #if LIBCURL_VERSION_NUM >= 0x070908
-	if (!strcmp("sslcapath", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_SSL_CAPATH))
-			return 0;
+	if (!strcmp("http.sslcapath", var)) {
 		return git_config_string(&ssl_capath, var, value);
 	}
 #endif
-	if (!strcmp("sslcainfo", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_SSL_CAINFO))
-			return 0;
+	if (!strcmp("http.sslcainfo", var)) {
 		return git_config_string(&ssl_cainfo, var, value);
 	}
-	if (!strcmp("sslcertpasswordprotected", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_PASSWD_REQ))
-			return 0;
+	if (!strcmp("http.sslcertpasswordprotected", var)) {
 		ssl_cert_password_required = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp("ssltry", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_SSL_TRY))
-			return 0;
+	if (!strcmp("http.ssltry", var)) {
 		curl_ssl_try = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp("minsessions", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_MIN_SESSIONS))
-			return 0;
+	if (!strcmp("http.minsessions", var)) {
 		min_curl_sessions = git_config_int(var, value);
 #ifndef USE_CURL_MULTI
 		if (min_curl_sessions > 1)
@@ -292,56 +181,40 @@ static int http_options(const char *var, const char *value, void *cb)
 		return 0;
 	}
 #ifdef USE_CURL_MULTI
-	if (!strcmp("maxrequests", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_MAX_REQUESTS))
-			return 0;
+	if (!strcmp("http.maxrequests", var)) {
 		max_requests = git_config_int(var, value);
 		return 0;
 	}
 #endif
-	if (!strcmp("lowspeedlimit", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_LOW_SPEED))
-			return 0;
+	if (!strcmp("http.lowspeedlimit", var)) {
 		curl_low_speed_limit = (long)git_config_int(var, value);
 		return 0;
 	}
-	if (!strcmp("lowspeedtime", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_LOW_TIME))
-			return 0;
+	if (!strcmp("http.lowspeedtime", var)) {
 		curl_low_speed_time = (long)git_config_int(var, value);
 		return 0;
 	}
 
-	if (!strcmp("noepsv", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_NO_EPSV))
-			return 0;
+	if (!strcmp("http.noepsv", var)) {
 		curl_ftp_no_epsv = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp("proxy", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_HTTP_PROXY))
-			return 0;
+	if (!strcmp("http.proxy", var)) {
 		return git_config_string(&curl_http_proxy, var, value);
 	}
 
-	if (!strcmp("cookiefile", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_COOKIE_FILE))
-			return 0;
+	if (!strcmp("http.cookiefile", var)) {
 		return git_config_string(&curl_cookie_file, var, value);
 	}
 
-	if (!strcmp("postbuffer", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_POST_BUFFER))
-			return 0;
+	if (!strcmp("http.postbuffer", var)) {
 		http_post_buffer = git_config_int(var, value);
 		if (http_post_buffer < LARGE_PACKET_MAX)
 			http_post_buffer = LARGE_PACKET_MAX;
 		return 0;
 	}
 
-	if (!strcmp("useragent", key)) {
-		if (match_is_ignored(matchlen, usermatch, OPT_USER_AGENT))
-			return 0;
+	if (!strcmp("http.useragent", var)) {
 		return git_config_string(&user_agent, var, value);
 	}
 
@@ -469,15 +342,23 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
 {
 	char *low_speed_limit;
 	char *low_speed_time;
-	struct url_info info;
+	char *normalized_url;
+	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+
+	config.fn = http_options;
+	config.cascade_fn = git_default_config;
+	config.item_alloc = NULL;
+	config.item_clear = NULL;
+	config.cb = NULL;
 
 	http_is_verbose = 0;
+	normalized_url = url_normalize(url, &config.url);
+
+	config.section = "http";
+	config.key = NULL;
 
-	memset(http_option_max_matched_len, 0, sizeof(http_option_max_matched_len));
-	memset(http_option_user_matched, 0, sizeof(http_option_user_matched));
-	url_normalize(url, &info);
-	git_config(http_options, &info);
-	free(info.url);
+	git_config(urlmatch_config_entry, &config);
+	free(normalized_url);
 
 	curl_global_init(CURL_GLOBAL_ALL);
 
diff --git a/test-url-normalize.c b/test-url-normalize.c
index 0b809eb..fab94e5 100644
--- a/test-url-normalize.c
+++ b/test-url-normalize.c
@@ -15,8 +15,15 @@ static int run_http_options(const char *file,
 {
 	struct strbuf opt_lc;
 	size_t i, len;
+	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
 
-	if (git_config_with_options(http_options, (void *)info, file, 0))
+	config.fn = http_options;
+	config.cascade_fn = git_default_config;
+	config.item_alloc = NULL;
+	config.item_clear = NULL;
+	config.cb = NULL;
+
+	if (git_config_with_options(urlmatch_config_entry, &config, file, 0))
 		return 1;
 
 	len = strlen(opt);

Re: [PATCH 5/3] revert most of the http_options() change

[Kyle J. McKay]

On Jul 29, 2013, at 19:13, Junio C Hamano wrote:

> With the previous preparation step, the earlier 1bb00006 (config:
> add support for http.<url>.* settings, 2013-07-21) that introduced
> many repeated changes:
>
>        -if (!strcmp("http.key", var)) {
>        +if (!strcmp("key", key)) {
>        +       if (match_is_shorter(..., OPT_KEY_NAME))
>        +               return 0;
>                ... original processing for KEY_NAME ...
>         }
>
> can be reverted.
>
> This allows us to also get rid of the "repeating yourself to the
> death" enum http_option_type, and new code (like db/http-savecookies
> patch) that wants to add a new configuration to the http.* subsystem
> does not have to conflict with http.<urlpattern>.variable series at
> all.
>
> This is more-or-less how I want the endgame to look like. Not even
> compile tested, but it is getting late so I'll show it to the list
> in case people may want to play with it and polish it while I am
> away for the night ;-).
>
> If people can agree that this is going in the right direction,
> perhaps we should rebuild Kyle's series without detouring of adding
> and then yanking what 1bb00006 (config: add support for http.<url>.*
> settings, 2013-07-21) did in the next cycle.
>
> http.c               | 181 ++++++++ 
> +------------------------------------------
> test-url-normalize.c |   9 ++-
> 2 files changed, 39 insertions(+), 151 deletions(-)
>
> diff --git a/http.c b/http.c
> index a91a00b..c7f513b 100644
> --- a/http.c
> +++ b/http.c
[...]
> @@ -469,15 +342,23 @@ void http_init(struct remote *remote, const  
> char *url, int proactive_auth)
> {
> 	char *low_speed_limit;
> 	char *low_speed_time;
> -	struct url_info info;
> +	char *normalized_url;
> +	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
> +
> +	config.fn = http_options;
> +	config.cascade_fn = git_default_config;
> +	config.item_alloc = NULL;
> +	config.item_clear = NULL;
> +	config.cb = NULL;

Missing this:
+	config.section = "http";


[...]
> diff --git a/test-url-normalize.c b/test-url-normalize.c
> index 0b809eb..fab94e5 100644
> --- a/test-url-normalize.c
> +++ b/test-url-normalize.c
> @@ -15,8 +15,15 @@ static int run_http_options(const char *file,
> {
> 	struct strbuf opt_lc;
> 	size_t i, len;
> +	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
>
> -	if (git_config_with_options(http_options, (void *)info, file, 0))
> +	config.fn = http_options;
> +	config.cascade_fn = git_default_config;
> +	config.item_alloc = NULL;
> +	config.item_clear = NULL;
> +	config.cb = NULL;


Missing this:
+	config.section = "http";

Without these it segfaults running the tests.

However, now the tests that actually check the results using config  
files are failing:

> # failed 1 among 11 test(s)

I'm looking at it now to see if I can fix the problem.

Re: [PATCH 5/3] revert most of the http_options() change

[Kyle J. McKay]

On Jul 30, 2013, at 12:14, Kyle J. McKay wrote:

> On Jul 29, 2013, at 19:13, Junio C Hamano wrote:
>
>> With the previous preparation step, the earlier 1bb00006 (config:
>> add support for http.<url>.* settings, 2013-07-21) that introduced
>> many repeated changes:
>>
>>       -if (!strcmp("http.key", var)) {
>>       +if (!strcmp("key", key)) {
>>       +       if (match_is_shorter(..., OPT_KEY_NAME))
>>       +               return 0;
>>               ... original processing for KEY_NAME ...
>>        }
>>
>> can be reverted.
>>
>> This allows us to also get rid of the "repeating yourself to the
>> death" enum http_option_type, and new code (like db/http-savecookies
>> patch) that wants to add a new configuration to the http.* subsystem
>> does not have to conflict with http.<urlpattern>.variable series at
>> all.
>>
>> This is more-or-less how I want the endgame to look like. Not even
>> compile tested, but it is getting late so I'll show it to the list
>> in case people may want to play with it and polish it while I am
>> away for the night ;-).
>>
>> If people can agree that this is going in the right direction,
>> perhaps we should rebuild Kyle's series without detouring of adding
>> and then yanking what 1bb00006 (config: add support for http.<url>.*
>> settings, 2013-07-21) did in the next cycle.
>>
>> http.c               | 181 ++++++++ 
>> +------------------------------------------
>> test-url-normalize.c |   9 ++-
>> 2 files changed, 39 insertions(+), 151 deletions(-)
>>
>> diff --git a/http.c b/http.c
>> index a91a00b..c7f513b 100644
>> --- a/http.c
>> +++ b/http.c
> [...]
>> @@ -469,15 +342,23 @@ void http_init(struct remote *remote, const  
>> char *url, int proactive_auth)
>> {
>> 	char *low_speed_limit;
>> 	char *low_speed_time;
>> -	struct url_info info;
>> +	char *normalized_url;
>> +	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
>> +
>> +	config.fn = http_options;
>> +	config.cascade_fn = git_default_config;
>> +	config.item_alloc = NULL;
>> +	config.item_clear = NULL;
>> +	config.cb = NULL;
>
> Missing this:
> +	config.section = "http";
>
>
> [...]
>> diff --git a/test-url-normalize.c b/test-url-normalize.c
>> index 0b809eb..fab94e5 100644
>> --- a/test-url-normalize.c
>> +++ b/test-url-normalize.c
>> @@ -15,8 +15,15 @@ static int run_http_options(const char *file,
>> {
>> 	struct strbuf opt_lc;
>> 	size_t i, len;
>> +	struct urlmatch_config config = { STRING_LIST_INIT_DUP };

Also needs this:

+	memcpy(&config.url, info, sizeof(struct url_info));

>> -	if (git_config_with_options(http_options, (void *)info, file, 0))
>> +	config.fn = http_options;
>> +	config.cascade_fn = git_default_config;
>> +	config.item_alloc = NULL;
>> +	config.item_clear = NULL;
>> +	config.cb = NULL;
>
>
> Missing this:
> +	config.section = "http";
>
> Without these it segfaults running the tests.
>
> However, now the tests that actually check the results using config  
> files are failing:
>
>> # failed 1 among 11 test(s)
>
> I'm looking at it now to see if I can fix the problem.

And now all the t5200-url-normalize tests pass again.

FYI, I couldn't get the patches to apply against next or pu without  
some minor tweaks that were just conflict resolutions having to do  
with git_config_with_options changing its signature.

Re: [PATCH 5/3] revert most of the http_options() change

[Junio C Hamano]

"Kyle J. McKay" <mackyle@gmail.com> writes:

> And now all the t5200-url-normalize tests pass again.
>
> FYI, I couldn't get the patches to apply against next or pu without
> some minor tweaks that were just conflict resolutions having to do
> with git_config_with_options changing its signature.

Thanks.

I built these five patches directly on top of your series, i.e. on
top of cea9928d (docs: update http.<url>.* options documentation,
2013-07-25).

I'll rebuild the series with your fixes and I may even queue it to
'pu', but with some random thoughts:

 * "url-match.[ch]" are ugly names.  I'd be happier with
   "urlmatch.[ch]".

 * In the end result, http_options() looks mostly identical to that
   of the 'master', but it got an extra "void *matched", only to
   support "git config --get-urlmatch".  I however do not have to,
   if I do a few things:

   - Instead of implementing urlmatch_config_item that extends
     urlmatch_item, have a separate string_list, keyed by the
     variable names, and point the string_list with the generic cb
     pointer I already have in urlmatch_config.  The fn() on the
     "git config" side has to index this second string_list with the
     variable and keep track of the value from the best candidate we
     have seen so far.

   - The above allows us to lose item_alloc and item_clear callback
     functions from urlmatch_config, as we will not be doing the
     structure inheritance to extend urlmatch_item.

   - We still need cascade_fn callback, though.

 * With the above, http_options() does not have to change in the
   series.  We could restructure the series perhaps this way:

   - http.sslCertPasswordProtected parsing fix.

   - Add urlmatch.[ch], which would be your "config: improve support
     for http.<url>.* settings" and yesterday's "url-match: split
     out URL matching logic out of http.c", and a half of
     "url-match: generalize configuration collection logic".

   - Update http.c to use urlmatch.[ch] to make http.c match the
     result of the endgame patch [5/3], and add necessary end user
     documentation to Documentation/config.txt.

   - Add test-url-normalize and t/t5200

   - Update builtin/config.c for "--get-urlmatch", to make
     builtin/config.c match the result of the endgame patch [5/3],
     add end user documentation to Documentation/git-config.txt.

   - Add some tests for "git config --get-urlmatch".

Re: [PATCH 0/3] "git config --get-urlmatch $section.$key $url"

[Ramsay Jones]

Junio C Hamano wrote:
> So here is a bit of refactoring to extract the logic to find the
> entry $section.<urlpattern>.$key from the configuration that best
> matches the given $url to answer "what value $section.$key should be
> for $url?" out of http.c (primarily because we never want to link
> "git cofnig" with the cURL library), and use it from "git config" to
> implement Peff's idea to extend "git config".
> 
> The first step is a pure code movement, plus some renaming of the
> functions.
> 
> The second step is to factor out the code to handle --bool, --int, etc.
> as a helper so that the new codepath can use it.
> 
> The last step currently duplicates the logic in http_options(), but
> we might want to refactor it further so that the two functions can
> share more code.  We hopefully can get rid of test-url-normalize and
> instead use "git config --get-urlmatch" in the tests that protect
> the http.<url>.config topic.

I haven't been following this topic too closely and I don't have any
feel for how long it will take to get to the end-game. However, unless
the removal of test-url-normalize is coming soon, could I request that
you apply my patch (or squash it into this series)? At present, I have
to apply the patch before building the next and pu branches; OK it's not
too onerous, but still ... :-P


ATB,
Ramsay Jones

Re: [PATCH 0/3] "git config --get-urlmatch $section.$key $url"

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Junio C Hamano wrote:
>> So here is a bit of refactoring to extract the logic to find the
>> entry $section.<urlpattern>.$key from the configuration that best
>> matches the given $url to answer "what value $section.$key should be
>> for $url?" out of http.c (primarily because we never want to link
>> "git cofnig" with the cURL library), and use it from "git config" to
>> implement Peff's idea to extend "git config".
>> 
>> The first step is a pure code movement, plus some renaming of the
>> functions.
>> 
>> The second step is to factor out the code to handle --bool, --int, etc.
>> as a helper so that the new codepath can use it.
>> 
>> The last step currently duplicates the logic in http_options(), but
>> we might want to refactor it further so that the two functions can
>> share more code.  We hopefully can get rid of test-url-normalize and
>> instead use "git config --get-urlmatch" in the tests that protect
>> the http.<url>.config topic.
>
> I haven't been following this topic too closely and I don't have any
> feel for how long it will take to get to the end-game. However, unless
> the removal of test-url-normalize is coming soon, could I request that
> you apply my patch (or squash it into this series)? At present, I have
> to apply the patch before building the next and pu branches; OK it's not
> too onerous, but still ... :-P

Will squash in.  Thanks for a reminder.