[PATCH v2] Fix remote.<remote>.vcs

remote.<remote>.vcs causes remote->foreign_vcs to be set on entry to
transport_get(). Unfortunately, the code assumed that any such entry
is stale from previous round.

Fix this by making VCS set by URL to be volatile w.r.t. transport_get()
instead.

Signed-off-by: Ilari Liusvaara <redacted>
---
 transport.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

Differences from first round:

This makes VCS setting apply to all URLs that don't explicitly override,
instead of it applying to just the first one.

diff --git a/transport.c b/transport.c
index 7714fdb..87581b8 100644
--- a/transport.c
+++ b/transport.c

@@ -912,20 +912,19 @@ static int external_specification_len(const char *url)
 
 struct transport *transport_get(struct remote *remote, const char *url)
 {
+	const char *helper;
 	struct transport *ret = xcalloc(1, sizeof(*ret));
 
 	if (!remote)
 		die("No remote provided to transport_get()");
 
 	ret->remote = remote;
+	helper = remote->foreign_vcs;
 
 	if (!url && remote && remote->url)
 		url = remote->url[0];
 	ret->url = url;
 
-	/* In case previous URL had helper forced, reset it. */
-	remote->foreign_vcs = NULL;
-
 	/* maybe it is a foreign URL? */
 	if (url) {
 		const char *p = url;

@@ -933,11 +932,11 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		while (isalnum(*p))
 			p++;
 		if (!prefixcmp(p, "::"))
-			remote->foreign_vcs = xstrndup(url, p - url);
+			helper = xstrndup(url, p - url);
 	}
 
-	if (remote && remote->foreign_vcs) {
-		transport_helper_init(ret, remote->foreign_vcs);
+	if (helper) {
+		transport_helper_init(ret, helper);
 	} else if (!prefixcmp(url, "rsync:")) {
 		ret->get_refs_list = get_refs_via_rsync;
 		ret->fetch = fetch_objs_via_rsync;

-- 
1.7.0.rc0.19.gb557e6

Heya,

On Wed, Jan 27, 2010 at 18:53, Ilari Liusvaara
[off-list ref] wrote:

Fix this by making VCS set by URL to be volatile w.r.t. transport_get()
instead.

Patch looks good (didn't test it though), but I had to read this line
twice; I'm afraid I don't have any suggestions on how to improve it
though.

-- 
Cheers,

Sverre Rabbelier

On Wed, 27 Jan 2010, Ilari Liusvaara wrote:

remote.<remote>.vcs causes remote->foreign_vcs to be set on entry to
transport_get(). Unfortunately, the code assumed that any such entry
is stale from previous round.

Fix this by making VCS set by URL to be volatile w.r.t. transport_get()
instead.

Signed-off-by: Ilari Liusvaara <redacted>

Except that you missed the "remote == NULL" case (noted below), this is 
what I was thinking of.

Acked-by: Daniel Barkalow <redacted>

Needs to be "helper = remote ? remote->foreign_vcs : NULL", for the same 
reason that the test below had been "remote && remote->foreign_vcs".

On Wed, Jan 27, 2010 at 01:39:00PM -0500, Daniel Barkalow wrote:

On Wed, 27 Jan 2010, Ilari Liusvaara wrote:

quoted

 
 	if (!remote)
 		die("No remote provided to transport_get()");
 
 	ret->remote = remote;
+	helper = remote->foreign_vcs;

Needs to be "helper = remote ? remote->foreign_vcs : NULL", for the same 
reason that the test below had been "remote && remote->foreign_vcs".

Few lines above that:

     if (!remote)
             die("No remote provided to transport_get()");


-Ilari