[PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Daniel Barkalow]

The new parser is different from the one in builtin-push in two ways:
the default is to use the current branch's remote, if there is one,
before "origin"; and config is used in preference to remotes.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 Makefile       |    5 +-
 builtin-push.c |  190 ++++++-----------------------------------------------
 remote.c       |  201 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 remote.h       |   18 +++++
 4 files changed, 242 insertions(+), 172 deletions(-)
 create mode 100644 remote.c
 create mode 100644 remote.h

diff --git a/Makefile b/Makefile
index e0a1308..dd64b7d 100644
--- a/Makefile
+++ b/Makefile
@@ -288,7 +288,8 @@ LIB_H = \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
-	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h mailmap.h
+	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
+	mailmap.h remote.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -310,7 +311,7 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o
+	convert.o attr.o decorate.o progress.o mailmap.o remote.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/builtin-push.c b/builtin-push.c
index cb78401..0e602f3 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -5,17 +5,13 @@
 #include "refs.h"
 #include "run-command.h"
 #include "builtin.h"
-
-#define MAX_URI (16)
+#include "remote.h"
 
 static const char push_usage[] = "git-push [--all] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]";
 
 static int all, tags, force, thin = 1, verbose;
 static const char *receivepack;
 
-#define BUF_SIZE (2084)
-static char buffer[BUF_SIZE];
-
 static const char **refspec;
 static int refspec_nr;
 
@@ -137,175 +133,29 @@ static void set_refspecs(const char **refs, int nr)
 	expand_refspecs();
 }
 
-static int get_remotes_uri(const char *repo, const char *uri[MAX_URI])
-{
-	int n = 0;
-	FILE *f = fopen(git_path("remotes/%s", repo), "r");
-	int has_explicit_refspec = refspec_nr || all || tags;
-
-	if (!f)
-		return -1;
-	while (fgets(buffer, BUF_SIZE, f)) {
-		int is_refspec;
-		char *s, *p;
-
-		if (!prefixcmp(buffer, "URL:")) {
-			is_refspec = 0;
-			s = buffer + 4;
-		} else if (!prefixcmp(buffer, "Push:")) {
-			is_refspec = 1;
-			s = buffer + 5;
-		} else
-			continue;
-
-		/* Remove whitespace at the head.. */
-		while (isspace(*s))
-			s++;
-		if (!*s)
-			continue;
-
-		/* ..and at the end */
-		p = s + strlen(s);
-		while (isspace(p[-1]))
-			*--p = 0;
-
-		if (!is_refspec) {
-			if (n < MAX_URI)
-				uri[n++] = xstrdup(s);
-			else
-				error("more than %d URL's specified, ignoring the rest", MAX_URI);
-		}
-		else if (is_refspec && !has_explicit_refspec) {
-			if (!wildcard_ref(s))
-				add_refspec(xstrdup(s));
-		}
-	}
-	fclose(f);
-	if (!n)
-		die("remote '%s' has no URL", repo);
-	return n;
-}
-
-static const char **config_uri;
-static const char *config_repo;
-static int config_repo_len;
-static int config_current_uri;
-static int config_get_refspecs;
-static int config_get_receivepack;
-
-static int get_remote_config(const char* key, const char* value)
-{
-	if (!prefixcmp(key, "remote.") &&
-	    !strncmp(key + 7, config_repo, config_repo_len)) {
-		if (!strcmp(key + 7 + config_repo_len, ".url")) {
-			if (config_current_uri < MAX_URI)
-				config_uri[config_current_uri++] = xstrdup(value);
-			else
-				error("more than %d URL's specified, ignoring the rest", MAX_URI);
-		}
-		else if (config_get_refspecs &&
-			 !strcmp(key + 7 + config_repo_len, ".push")) {
-			if (!wildcard_ref(value))
-				add_refspec(xstrdup(value));
-		}
-		else if (config_get_receivepack &&
-			 !strcmp(key + 7 + config_repo_len, ".receivepack")) {
-			if (!receivepack) {
-				char *rp = xmalloc(strlen(value) + 16);
-				sprintf(rp, "--receive-pack=%s", value);
-				receivepack = rp;
-			} else
-				error("more than one receivepack given, using the first");
-		}
-	}
-	return 0;
-}
-
-static int get_config_remotes_uri(const char *repo, const char *uri[MAX_URI])
-{
-	config_repo_len = strlen(repo);
-	config_repo = repo;
-	config_current_uri = 0;
-	config_uri = uri;
-	config_get_refspecs = !(refspec_nr || all || tags);
-	config_get_receivepack = (receivepack == NULL);
-
-	git_config(get_remote_config);
-	return config_current_uri;
-}
-
-static int get_branches_uri(const char *repo, const char *uri[MAX_URI])
-{
-	const char *slash = strchr(repo, '/');
-	int n = slash ? slash - repo : 1000;
-	FILE *f = fopen(git_path("branches/%.*s", n, repo), "r");
-	char *s, *p;
-	int len;
-
-	if (!f)
-		return 0;
-	s = fgets(buffer, BUF_SIZE, f);
-	fclose(f);
-	if (!s)
-		return 0;
-	while (isspace(*s))
-		s++;
-	if (!*s)
-		return 0;
-	p = s + strlen(s);
-	while (isspace(p[-1]))
-		*--p = 0;
-	len = p - s;
-	if (slash)
-		len += strlen(slash);
-	p = xmalloc(len + 1);
-	strcpy(p, s);
-	if (slash)
-		strcat(p, slash);
-	uri[0] = p;
-	return 1;
-}
-
-/*
- * Read remotes and branches file, fill the push target URI
- * list.  If there is no command line refspecs, read Push: lines
- * to set up the *refspec list as well.
- * return the number of push target URIs
- */
-static int read_config(const char *repo, const char *uri[MAX_URI])
-{
-	int n;
-
-	if (*repo != '/') {
-		n = get_remotes_uri(repo, uri);
-		if (n > 0)
-			return n;
-
-		n = get_config_remotes_uri(repo, uri);
-		if (n > 0)
-			return n;
-
-		n = get_branches_uri(repo, uri);
-		if (n > 0)
-			return n;
-	}
-
-	uri[0] = repo;
-	return 1;
-}
-
 static int do_push(const char *repo)
 {
-	const char *uri[MAX_URI];
-	int i, n, errs;
+	int i, errs;
 	int common_argc;
 	const char **argv;
 	int argc;
+	struct remote *remote = remote_get(repo);
 
-	n = read_config(repo, uri);
-	if (n <= 0)
+	if (!remote)
 		die("bad repository '%s'", repo);
 
+	if (remote->receivepack) {
+		char *rp = xmalloc(strlen(remote->receivepack) + 16);
+		sprintf(rp, "--receive-pack=%s", remote->receivepack);
+		receivepack = rp;
+	}
+	if (!refspec && !all && !tags && remote->push_refspec_nr) {
+		for (i = 0; i < remote->push_refspec_nr; i++) {
+			if (!wildcard_ref(remote->push_refspec[i]))
+				add_refspec(remote->push_refspec[i]);
+		}
+	}
+
 	argv = xmalloc((refspec_nr + 10) * sizeof(char *));
 	argv[0] = "dummy-send-pack";
 	argc = 1;
@@ -318,12 +168,12 @@ static int do_push(const char *repo)
 	common_argc = argc;
 
 	errs = 0;
-	for (i = 0; i < n; i++) {
+	for (i = 0; i < remote->uri_nr; i++) {
 		int err;
 		int dest_argc = common_argc;
 		int dest_refspec_nr = refspec_nr;
 		const char **dest_refspec = refspec;
-		const char *dest = uri[i];
+		const char *dest = remote->uri[i];
 		const char *sender = "send-pack";
 		if (!prefixcmp(dest, "http://") ||
 		    !prefixcmp(dest, "https://"))
@@ -341,7 +191,7 @@ static int do_push(const char *repo)
 		if (!err)
 			continue;
 
-		error("failed to push to '%s'", uri[i]);
+		error("failed to push to '%s'", remote->uri[i]);
 		switch (err) {
 		case -ERR_RUN_COMMAND_FORK:
 			error("unable to fork for %s", sender);
@@ -362,7 +212,7 @@ static int do_push(const char *repo)
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	const char *repo = "origin";	/* default repository */
+	const char *repo = NULL;	/* default repository */
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/remote.c b/remote.c
new file mode 100644
index 0000000..32a0acf
--- /dev/null
+++ b/remote.c
@@ -0,0 +1,201 @@
+#include "cache.h"
+#include "remote.h"
+#include "refs.h"
+
+static struct remote **remotes;
+static int allocated_remotes;
+
+#define BUF_SIZE (2084)
+static char buffer[BUF_SIZE];
+
+static void add_push_refspec(struct remote *remote, const char *ref)
+{
+	int nr = remote->push_refspec_nr + 1;
+	remote->push_refspec =
+		xrealloc(remote->push_refspec, nr * sizeof(char *));
+	remote->push_refspec[nr-1] = ref;
+	remote->push_refspec_nr = nr;
+}
+
+static void add_uri(struct remote *remote, const char *uri)
+{
+	int nr = remote->uri_nr + 1;
+	remote->uri =
+		xrealloc(remote->uri, nr * sizeof(char *));
+	remote->uri[nr-1] = uri;
+	remote->uri_nr = nr;
+}
+
+static struct remote *make_remote(const char *name, int len)
+{
+	int i, empty = -1;
+
+	for (i = 0; i < allocated_remotes; i++) {
+		if (!remotes[i]) {
+			if (empty < 0)
+				empty = i;
+		} else {
+			if (len ? (!strncmp(name, remotes[i]->name, len) &&
+				   !remotes[i]->name[len]) :
+			    !strcmp(name, remotes[i]->name))
+				return remotes[i];
+		}
+	}
+
+	if (empty < 0) {
+		empty = allocated_remotes;
+		allocated_remotes += allocated_remotes ? allocated_remotes : 1;
+		remotes = xrealloc(remotes,
+				   sizeof(*remotes) * allocated_remotes);
+		memset(remotes + empty, 0,
+		       (allocated_remotes - empty) * sizeof(*remotes));
+	}
+	remotes[empty] = xcalloc(1, sizeof(struct remote));
+	if (len)
+		remotes[empty]->name = xstrndup(name, len);
+	else
+		remotes[empty]->name = xstrdup(name);
+	return remotes[empty];
+}
+
+static void read_remotes_file(struct remote *remote)
+{
+	FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
+
+	if (!f)
+		return;
+	while (fgets(buffer, BUF_SIZE, f)) {
+		int value_list;
+		char *s, *p;
+
+		if (!prefixcmp(buffer, "URL:")) {
+			value_list = 0;
+			s = buffer + 4;
+		} else if (!prefixcmp(buffer, "Push:")) {
+			value_list = 1;
+			s = buffer + 5;
+		} else
+			continue;
+
+		while (isspace(*s))
+			s++;
+		if (!*s)
+			continue;
+
+		p = s + strlen(s);
+		while (isspace(p[-1]))
+			*--p = 0;
+
+		switch (value_list) {
+		case 0:
+			add_uri(remote, xstrdup(s));
+			break;
+		case 1:
+			add_push_refspec(remote, xstrdup(s));
+			break;
+		}
+	}
+}
+
+static void read_branches_file(struct remote *remote)
+{
+	const char *slash = strchr(remote->name, '/');
+	int n = slash ? slash - remote->name : 1000;
+	FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
+	char *s, *p;
+	int len;
+
+	if (!f)
+		return;
+	s = fgets(buffer, BUF_SIZE, f);
+	fclose(f);
+	if (!s)
+		return;
+	while (isspace(*s))
+		s++;
+	if (!*s)
+		return;
+	p = s + strlen(s);
+	while (isspace(p[-1]))
+		*--p = 0;
+	len = p - s;
+	if (slash)
+		len += strlen(slash);
+	p = xmalloc(len + 1);
+	strcpy(p, s);
+	if (slash)
+		strcat(p, slash);
+	add_uri(remote, p);
+}
+
+static char *default_remote_name = NULL;
+static const char *current_branch = NULL;
+static int current_branch_len = 0;
+
+static int handle_config(const char *key, const char *value)
+{
+	const char *name;
+	const char *subkey;
+	struct remote *remote;
+	if (!prefixcmp(key, "branch.") && current_branch &&
+	    !strncmp(key + 7, current_branch, current_branch_len) &&
+	    !strcmp(key + 7 + current_branch_len, ".remote")) {
+		free(default_remote_name);
+		default_remote_name = xstrdup(value);
+	}
+	if (prefixcmp(key,  "remote."))
+		return 0;
+	name = key + 7;
+	subkey = strrchr(name, '.');
+	if (!subkey)
+		return error("Config with no key for remote %s", name);
+	remote = make_remote(name, subkey - name);
+	if (!strcmp(subkey, ".url")) {
+		add_uri(remote, xstrdup(value));
+	} else if (!strcmp(subkey, ".push")) {
+		add_push_refspec(remote, xstrdup(value));
+	} else if (!strcmp(subkey, ".receivepack")) {
+		if (!remote->receivepack)
+			remote->receivepack = xstrdup(value);
+		else
+			error("more than one receivepack given, using the first");
+	}
+	return 0;
+}
+
+static void read_config(void)
+{
+	unsigned char sha1[20];
+	const char *head_ref;
+	int flag;
+	if (default_remote_name) // did this already
+		return;
+	default_remote_name = xstrdup("origin");
+	current_branch = NULL;
+	head_ref = resolve_ref("HEAD", sha1, 0, &flag);
+	if (head_ref && (flag & REF_ISSYMREF) &&
+	    !prefixcmp(head_ref, "refs/heads/")) {
+		current_branch = head_ref + strlen("refs/heads/");
+		current_branch_len = strlen(current_branch);
+	}
+	git_config(handle_config);
+}
+
+struct remote *remote_get(const char *name)
+{
+	struct remote *ret;
+
+	read_config();
+	if (!name)
+		name = default_remote_name;
+	ret = make_remote(name, 0);
+	if (*name == '/')
+		add_uri(ret, name);
+	if (!ret->uri)
+		read_remotes_file(ret);
+	if (!ret->uri)
+		read_branches_file(ret);
+	if (!ret->uri)
+		return NULL;
+	return ret;
+}
diff --git a/remote.h b/remote.h
new file mode 100644
index 0000000..73747a8
--- /dev/null
+++ b/remote.h
@@ -0,0 +1,18 @@
+#ifndef REMOTE_H
+#define REMOTE_H
+
+struct remote {
+	const char *name;
+
+	const char **uri;
+	int uri_nr;
+
+	const char **push_refspec;
+	int push_refspec_nr;
+
+	const char *receivepack;
+};
+
+struct remote *remote_get(const char *name);
+
+#endif
-- 
1.5.2.rc2.22.g9bca

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Junio C Hamano]

This seems to break t9400, with "fatal: bad repository 'gitcvs.git",
upon "git push".

: gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v
* expecting success: cvs -Q co -d cvswork master &&
   test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))" = "empty/1.1/"
cvs checkout: Updating cvswork
U cvswork/empty
*   ok 1: basic checkout

* expecting success: echo testfile1 >testfile1 &&
   git add testfile1 &&
   git commit -q -m "Add testfile1" &&
   git push gitcvs.git >/dev/null &&
   cd cvswork &&
   cvs -Q update &&
   test "$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))" = "testfile1/1.1/" &&
   diff -q testfile1 ../testfile1
fatal: bad repository 'gitcvs.git'
* FAIL 2: cvs update (create new file)
	echo testfile1 >testfile1 &&
	   git add testfile1 &&
	   git commit -q -m "Add testfile1" &&
	   git push gitcvs.git >/dev/null &&
	   cd cvswork &&
	   cvs -Q update &&
	   test "$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))" = "testfile1/1.1/" &&
	   diff -q testfile1 ../testfile1
: gitster t/db/remote; 

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Daniel Barkalow]

On Thu, 10 May 2007, Junio C Hamano wrote:

> This seems to break t9400, with "fatal: bad repository 'gitcvs.git",
> upon "git push".
> 
> : gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v
> * expecting success: cvs -Q co -d cvswork master &&
>    test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))" = "empty/1.1/"
> cvs checkout: Updating cvswork
> U cvswork/empty
> *   ok 1: basic checkout
> 
> * expecting success: echo testfile1 >testfile1 &&
>    git add testfile1 &&
>    git commit -q -m "Add testfile1" &&
>    git push gitcvs.git >/dev/null &&

The man page doesn't think this is valid, since it only claims absolute 
paths to work for local repositories. (Current builtin-push seems to be 
sloppy; it has the die() that triggers in my version, but its 
read_config() always returns a value that doesn't trigger it, suggesting 
that there's some sort of bug.)

Following patch fixes t9400 to use absolute paths (assuming that's what it 
expects "git push gitcvs.git" to mean).

=== cut here ===
Local repository names for git-push are only documented with absolute 
paths, even though current git-push is lenient. Use an absolute path in 
t9400-git-cvsserver-server.sh.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index f17be6b..178b9b4 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -45,7 +45,7 @@ test_expect_success 'cvs update (create new file)' \
   'echo testfile1 >testfile1 &&
    git add testfile1 &&
    git commit -q -m "Add testfile1" &&
-   git push gitcvs.git >/dev/null &&
+   git push $(pwd)/gitcvs.git >/dev/null &&
    cd cvswork &&
    cvs -Q update &&
    test "$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))" = "testfile1/1.1/" &&
@@ -56,7 +56,7 @@ test_expect_success 'cvs update (update existing file)' \
   'echo line 2 >>testfile1 &&
    git add testfile1 &&
    git commit -q -m "Append to testfile1" &&
-   git push gitcvs.git >/dev/null &&
+   git push $(pwd)/gitcvs.git >/dev/null &&
    cd cvswork &&
    cvs -Q update &&
    test "$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))" = "testfile1/1.2/" &&
@@ -69,7 +69,7 @@ test_expect_failure "cvs update w/o -d doesn't create subdir (TODO)" \
    echo >test/empty &&
    git add test &&
    git commit -q -m "Single Subdirectory" &&
-   git push gitcvs.git >/dev/null &&
+   git push $(pwd)/gitcvs.git >/dev/null &&
    cd cvswork &&
    cvs -Q update &&
    test ! -d test'
@@ -82,7 +82,7 @@ test_expect_success 'cvs update (subdirectories)' \
       git add $dir;
    done) &&
    git commit -q -m "deep sub directory structure" &&
-   git push gitcvs.git >/dev/null &&
+   git push $(pwd)/gitcvs.git >/dev/null &&
    cd cvswork &&
    cvs -Q update -d &&
    (for dir in A A/B A/B/C A/D E; do
@@ -100,7 +100,7 @@ cd "$WORKDIR"
 test_expect_success 'cvs update (delete file)' \
   'git rm testfile1 &&
    git commit -q -m "Remove testfile1" &&
-   git push gitcvs.git >/dev/null &&
+   git push $(pwd)/gitcvs.git >/dev/null &&
    cd cvswork &&
    cvs -Q update &&
    test -z "$(grep testfile1 CVS/Entries)" &&
@@ -111,7 +111,7 @@ test_expect_success 'cvs update (re-add deleted file)' \
   'echo readded testfile >testfile1 &&
    git add testfile1 &&
    git commit -q -m "Re-Add testfile1" &&
-   git push gitcvs.git >/dev/null &&
+   git push $(pwd)/gitcvs.git >/dev/null &&
    cd cvswork &&
    cvs -Q update &&
    test "$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))" = "testfile1/1.4/" &&

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Thu, 10 May 2007, Junio C Hamano wrote:
>
>> This seems to break t9400, with "fatal: bad repository 'gitcvs.git",
>> upon "git push".
>> 
>> : gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v
>> * expecting success: cvs -Q co -d cvswork master &&
>>    test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))" = "empty/1.1/"
>> cvs checkout: Updating cvswork
>> U cvswork/empty
>> *   ok 1: basic checkout
>> 
>> * expecting success: echo testfile1 >testfile1 &&
>>    git add testfile1 &&
>>    git commit -q -m "Add testfile1" &&
>>    git push gitcvs.git >/dev/null &&
>
> The man page doesn't think this is valid, since it only claims absolute 
> paths to work for local repositories.

Does it?  I suspect we need to fix the manpage then, as it is
fairly common to do 

	$ git fetch ../next-door-neighbour

and expect the opposite to work as well.

And I think it does today.

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Daniel Barkalow]

On Thu, 10 May 2007, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Thu, 10 May 2007, Junio C Hamano wrote:
> >
> >> This seems to break t9400, with "fatal: bad repository 'gitcvs.git",
> >> upon "git push".
> >> 
> >> : gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v
> >> * expecting success: cvs -Q co -d cvswork master &&
> >>    test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))" = "empty/1.1/"
> >> cvs checkout: Updating cvswork
> >> U cvswork/empty
> >> *   ok 1: basic checkout
> >> 
> >> * expecting success: echo testfile1 >testfile1 &&
> >>    git add testfile1 &&
> >>    git commit -q -m "Add testfile1" &&
> >>    git push gitcvs.git >/dev/null &&
> >
> > The man page doesn't think this is valid, since it only claims absolute 
> > paths to work for local repositories.
> 
> Does it?  I suspect we need to fix the manpage then, as it is
> fairly common to do 
> 
> 	$ git fetch ../next-door-neighbour
> 
> and expect the opposite to work as well.
> 
> And I think it does today.

Hmm, and I guess URIs on the command line work the same way. How about 
requiring a '/' somewhere in a repository argument in order to treat it as 
a repository instead of a remote name? Then "../next-door-neighbour" would 
work, "./gitcvs.git" would work (in the odd case where you actually have a 
bare repository sitting in your working directory), but we'd avoid the 
current default of pushing to a bare repository in "./origin/" if nothing 
at all is configured.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

>> And I think it does today.
>
> Hmm, and I guess URIs on the command line work the same way. How about 
> requiring a '/' somewhere in a repository argument in order to treat it as 
> a repository instead of a remote name? Then "../next-door-neighbour" would 
> work, "./gitcvs.git" would work (in the odd case where you actually have a 
> bare repository sitting in your working directory), but we'd avoid the 
> current default of pushing to a bare repository in "./origin/" if nothing 
> at all is configured.

When I wrote the message you are responding to, I thought this
was a regression from the current behaviour, which (IIRC--it's
getting late and I am tired to double check) essentially says if
the token is a name of the directory, the target repository is a
local one, but "we'd avoid..." part seems to suggest that you
actually did this deliberately as a fix to some problem in the
current behaviour.  I am not however sure what it exactly is.
Could you care to elaborate the part after "we'd avoid..." to
clarify what the problem is, please?

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Daniel Barkalow]

On Thu, 10 May 2007, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> >> And I think it does today.
> >
> > Hmm, and I guess URIs on the command line work the same way. How about 
> > requiring a '/' somewhere in a repository argument in order to treat it as 
> > a repository instead of a remote name? Then "../next-door-neighbour" would 
> > work, "./gitcvs.git" would work (in the odd case where you actually have a 
> > bare repository sitting in your working directory), but we'd avoid the 
> > current default of pushing to a bare repository in "./origin/" if nothing 
> > at all is configured.
> 
> When I wrote the message you are responding to, I thought this
> was a regression from the current behaviour, which (IIRC--it's
> getting late and I am tired to double check) essentially says if
> the token is a name of the directory, the target repository is a
> local one, but "we'd avoid..." part seems to suggest that you
> actually did this deliberately as a fix to some problem in the
> current behaviour.  I am not however sure what it exactly is.
> Could you care to elaborate the part after "we'd avoid..." to
> clarify what the problem is, please?

The problem, in general, is that, if the remote name you specify (or 
"origin" if you don't specify any) is not configured as a remote, it is 
treated as a filename in the current directory for a local push. E.g.:

$ git init
$ git push
fatal: 'origin': unable to chdir or not a git archive
fatal: The remote end hung up unexpectedly

It's actually trying to push to ./origin/, which is totally nuts as a 
default repository to push to. Similarly, if you typo an actual remote 
name. Furthermore, builtin-push.c has an error message for the situation 
where the repository specification is wrong, suggesting that there is 
some invalid repository specification, but it isn't reachable. And it 
carefully prevents remote names from starting with a '/', suggestion that 
that is the distinguishing characteristic between directly-specified 
repository URIs and configured remotes (which can't really be right, of 
course).

I think the right answer is to say that configured remotes cannot contain 
slashes, and directly-specified URIs must contain slashes, and it'll all 
be clear.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> ..., "./gitcvs.git" would work (in the odd case where you actually have a 
> bare repository sitting in your working directory), ...

This is a bit of tangent, because it does not change the issue
we are discussing, and I suspect you already know this.

But to avoid future confusion by people on the list who read
this in the archive...

	"foo.git" does _NOT_ mean the directory is a bare
	repository.  It is perfectly normal to have "foo.git/"
	that has a working tree whose repository data lives in
	"foo.git/.git".

I _think_ (meaning, I haven't polled the userbase) requiring '/'
may not break people's existing setup too badly, while it _is_ a
regression in the sense that we suddenly start disallowing
something we allowed for a long time, perhaps without no
apparent good reason.

I hope you'll correct me about "without no apparent good reason"
part, which was what I was asking in my previous message.  With
a good rationale, it is much easier to sell a change that is a
regression in the strictest sense but is unlikely to hurt people
in practice.

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Thu, 10 May 2007, Junio C Hamano wrote:
>
>> Daniel Barkalow <barkalow@iabervon.org> writes:
>> 
>> >> And I think it does today.
>> >
>> > Hmm, and I guess URIs on the command line work the same way. How about 
>> > requiring a '/' somewhere in a repository argument in order to treat it as 
>> > a repository instead of a remote name? Then "../next-door-neighbour" would 
>> > work, "./gitcvs.git" would work (in the odd case where you actually have a 
>> > bare repository sitting in your working directory), but we'd avoid the 
>> > current default of pushing to a bare repository in "./origin/" if nothing 
>> > at all is configured.
>> 
>> When I wrote the message you are responding to, I thought this
>> was a regression from the current behaviour, which (IIRC--it's
>> getting late and I am tired to double check) essentially says if
>> the token is a name of the directory, the target repository is a
>> local one, but "we'd avoid..." part seems to suggest that you
>> actually did this deliberately as a fix to some problem in the
>> current behaviour.  I am not however sure what it exactly is.
>> Could you care to elaborate the part after "we'd avoid..." to
>> clarify what the problem is, please?
>
> The problem, in general, is that, if the remote name you specify (or 
> "origin" if you don't specify any) is not configured as a remote, it is 
> treated as a filename in the current directory for a local push. E.g.:
>
> $ git init
> $ git push
> fatal: 'origin': unable to chdir or not a git archive
> fatal: The remote end hung up unexpectedly

Ahh.  You were trying to give it a better error message.

I think I lied in the previous message.  I said we try to see if
it is a local directory name before using that name, but we do
not do it, and leave the error detection to the lower level on
the other side (push spawns send-pack which in turn spawns
receive-pack) instead.

Perhaps an alternative is to see if the name is configured as a
remote (if so, we obviously use it), and if not do stat() to see
if it is a directory (if so, use it as a local repository).
Then we do not have to impose new restriction of slash at all,
although it might complicate the code a bit more.

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Daniel Barkalow]

On Thu, 10 May 2007, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Thu, 10 May 2007, Junio C Hamano wrote:
> >
> >> Daniel Barkalow <barkalow@iabervon.org> writes:
> >> 
> >> >> And I think it does today.
> >> >
> >> > Hmm, and I guess URIs on the command line work the same way. How about 
> >> > requiring a '/' somewhere in a repository argument in order to treat it as 
> >> > a repository instead of a remote name? Then "../next-door-neighbour" would 
> >> > work, "./gitcvs.git" would work (in the odd case where you actually have a 
> >> > bare repository sitting in your working directory), but we'd avoid the 
> >> > current default of pushing to a bare repository in "./origin/" if nothing 
> >> > at all is configured.
> >> 
> >> When I wrote the message you are responding to, I thought this
> >> was a regression from the current behaviour, which (IIRC--it's
> >> getting late and I am tired to double check) essentially says if
> >> the token is a name of the directory, the target repository is a
> >> local one, but "we'd avoid..." part seems to suggest that you
> >> actually did this deliberately as a fix to some problem in the
> >> current behaviour.  I am not however sure what it exactly is.
> >> Could you care to elaborate the part after "we'd avoid..." to
> >> clarify what the problem is, please?
> >
> > The problem, in general, is that, if the remote name you specify (or 
> > "origin" if you don't specify any) is not configured as a remote, it is 
> > treated as a filename in the current directory for a local push. E.g.:
> >
> > $ git init
> > $ git push
> > fatal: 'origin': unable to chdir or not a git archive
> > fatal: The remote end hung up unexpectedly
> 
> Ahh.  You were trying to give it a better error message.
> 
> I think I lied in the previous message.  I said we try to see if
> it is a local directory name before using that name, but we do
> not do it, and leave the error detection to the lower level on
> the other side (push spawns send-pack which in turn spawns
> receive-pack) instead.
> 
> Perhaps an alternative is to see if the name is configured as a
> remote (if so, we obviously use it), and if not do stat() to see
> if it is a directory (if so, use it as a local repository).
> Then we do not have to impose new restriction of slash at all,
> although it might complicate the code a bit more.

The problem I see with allowing paths without slashes is if you've got a 
subproject with a name similar to a remote name, and type the wrong one 
(particularly due to tab-completion), or if you've got a remote name you 
use in other projects that matches a subproject in a project where you 
aren't using that remote name. I think that a repository in your working 
directory is unlikely to be something you actually want to push to or 
fetch from (and if this is actually what you want, ./directory is the 
usual unixy thing for saying, no really, I want a relative path in the 
current directory, and would work here; and it would be good practice 
anyway, so that you don't get tripped up if you create a remote 
configuration with that name later). Obviously, ../something has a slash; 
it should also take ':' to mean a URI (which I didn't realize last night), 
so that "person@machine:directory.git" is a URI.

I think, as a general rule, that it would be cleanest to distinguish 
lexically between repository names that indicate configured remotes and 
repository names that are URIs, particularly if we only break usage that 
people only do for writing git regression tests.

	-Daniel
*This .sig left intentionally blank*

[PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Daniel Barkalow]

The new parser is different from the one in builtin-push in two ways:
the default is to use the current branch's remote, if there is one,
before "origin"; and config is used in preference to remotes.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 Makefile       |    5 +-
 builtin-push.c |  190 ++++++-----------------------------------------------
 remote.c       |  203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 remote.h       |   18 +++++
 4 files changed, 244 insertions(+), 172 deletions(-)
 create mode 100644 remote.c
 create mode 100644 remote.h

diff --git a/Makefile b/Makefile
index 7cf146b..134fbd2 100644
--- a/Makefile
+++ b/Makefile
@@ -296,7 +296,8 @@ LIB_H = \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
-	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h mailmap.h
+	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
+	mailmap.h remote.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -318,7 +319,7 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o
+	convert.o attr.o decorate.o progress.o mailmap.o remote.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/builtin-push.c b/builtin-push.c
index cb78401..0e602f3 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -5,17 +5,13 @@
 #include "refs.h"
 #include "run-command.h"
 #include "builtin.h"
-
-#define MAX_URI (16)
+#include "remote.h"
 
 static const char push_usage[] = "git-push [--all] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]";
 
 static int all, tags, force, thin = 1, verbose;
 static const char *receivepack;
 
-#define BUF_SIZE (2084)
-static char buffer[BUF_SIZE];
-
 static const char **refspec;
 static int refspec_nr;
 
@@ -137,175 +133,29 @@ static void set_refspecs(const char **refs, int nr)
 	expand_refspecs();
 }
 
-static int get_remotes_uri(const char *repo, const char *uri[MAX_URI])
-{
-	int n = 0;
-	FILE *f = fopen(git_path("remotes/%s", repo), "r");
-	int has_explicit_refspec = refspec_nr || all || tags;
-
-	if (!f)
-		return -1;
-	while (fgets(buffer, BUF_SIZE, f)) {
-		int is_refspec;
-		char *s, *p;
-
-		if (!prefixcmp(buffer, "URL:")) {
-			is_refspec = 0;
-			s = buffer + 4;
-		} else if (!prefixcmp(buffer, "Push:")) {
-			is_refspec = 1;
-			s = buffer + 5;
-		} else
-			continue;
-
-		/* Remove whitespace at the head.. */
-		while (isspace(*s))
-			s++;
-		if (!*s)
-			continue;
-
-		/* ..and at the end */
-		p = s + strlen(s);
-		while (isspace(p[-1]))
-			*--p = 0;
-
-		if (!is_refspec) {
-			if (n < MAX_URI)
-				uri[n++] = xstrdup(s);
-			else
-				error("more than %d URL's specified, ignoring the rest", MAX_URI);
-		}
-		else if (is_refspec && !has_explicit_refspec) {
-			if (!wildcard_ref(s))
-				add_refspec(xstrdup(s));
-		}
-	}
-	fclose(f);
-	if (!n)
-		die("remote '%s' has no URL", repo);
-	return n;
-}
-
-static const char **config_uri;
-static const char *config_repo;
-static int config_repo_len;
-static int config_current_uri;
-static int config_get_refspecs;
-static int config_get_receivepack;
-
-static int get_remote_config(const char* key, const char* value)
-{
-	if (!prefixcmp(key, "remote.") &&
-	    !strncmp(key + 7, config_repo, config_repo_len)) {
-		if (!strcmp(key + 7 + config_repo_len, ".url")) {
-			if (config_current_uri < MAX_URI)
-				config_uri[config_current_uri++] = xstrdup(value);
-			else
-				error("more than %d URL's specified, ignoring the rest", MAX_URI);
-		}
-		else if (config_get_refspecs &&
-			 !strcmp(key + 7 + config_repo_len, ".push")) {
-			if (!wildcard_ref(value))
-				add_refspec(xstrdup(value));
-		}
-		else if (config_get_receivepack &&
-			 !strcmp(key + 7 + config_repo_len, ".receivepack")) {
-			if (!receivepack) {
-				char *rp = xmalloc(strlen(value) + 16);
-				sprintf(rp, "--receive-pack=%s", value);
-				receivepack = rp;
-			} else
-				error("more than one receivepack given, using the first");
-		}
-	}
-	return 0;
-}
-
-static int get_config_remotes_uri(const char *repo, const char *uri[MAX_URI])
-{
-	config_repo_len = strlen(repo);
-	config_repo = repo;
-	config_current_uri = 0;
-	config_uri = uri;
-	config_get_refspecs = !(refspec_nr || all || tags);
-	config_get_receivepack = (receivepack == NULL);
-
-	git_config(get_remote_config);
-	return config_current_uri;
-}
-
-static int get_branches_uri(const char *repo, const char *uri[MAX_URI])
-{
-	const char *slash = strchr(repo, '/');
-	int n = slash ? slash - repo : 1000;
-	FILE *f = fopen(git_path("branches/%.*s", n, repo), "r");
-	char *s, *p;
-	int len;
-
-	if (!f)
-		return 0;
-	s = fgets(buffer, BUF_SIZE, f);
-	fclose(f);
-	if (!s)
-		return 0;
-	while (isspace(*s))
-		s++;
-	if (!*s)
-		return 0;
-	p = s + strlen(s);
-	while (isspace(p[-1]))
-		*--p = 0;
-	len = p - s;
-	if (slash)
-		len += strlen(slash);
-	p = xmalloc(len + 1);
-	strcpy(p, s);
-	if (slash)
-		strcat(p, slash);
-	uri[0] = p;
-	return 1;
-}
-
-/*
- * Read remotes and branches file, fill the push target URI
- * list.  If there is no command line refspecs, read Push: lines
- * to set up the *refspec list as well.
- * return the number of push target URIs
- */
-static int read_config(const char *repo, const char *uri[MAX_URI])
-{
-	int n;
-
-	if (*repo != '/') {
-		n = get_remotes_uri(repo, uri);
-		if (n > 0)
-			return n;
-
-		n = get_config_remotes_uri(repo, uri);
-		if (n > 0)
-			return n;
-
-		n = get_branches_uri(repo, uri);
-		if (n > 0)
-			return n;
-	}
-
-	uri[0] = repo;
-	return 1;
-}
-
 static int do_push(const char *repo)
 {
-	const char *uri[MAX_URI];
-	int i, n, errs;
+	int i, errs;
 	int common_argc;
 	const char **argv;
 	int argc;
+	struct remote *remote = remote_get(repo);
 
-	n = read_config(repo, uri);
-	if (n <= 0)
+	if (!remote)
 		die("bad repository '%s'", repo);
 
+	if (remote->receivepack) {
+		char *rp = xmalloc(strlen(remote->receivepack) + 16);
+		sprintf(rp, "--receive-pack=%s", remote->receivepack);
+		receivepack = rp;
+	}
+	if (!refspec && !all && !tags && remote->push_refspec_nr) {
+		for (i = 0; i < remote->push_refspec_nr; i++) {
+			if (!wildcard_ref(remote->push_refspec[i]))
+				add_refspec(remote->push_refspec[i]);
+		}
+	}
+
 	argv = xmalloc((refspec_nr + 10) * sizeof(char *));
 	argv[0] = "dummy-send-pack";
 	argc = 1;
@@ -318,12 +168,12 @@ static int do_push(const char *repo)
 	common_argc = argc;
 
 	errs = 0;
-	for (i = 0; i < n; i++) {
+	for (i = 0; i < remote->uri_nr; i++) {
 		int err;
 		int dest_argc = common_argc;
 		int dest_refspec_nr = refspec_nr;
 		const char **dest_refspec = refspec;
-		const char *dest = uri[i];
+		const char *dest = remote->uri[i];
 		const char *sender = "send-pack";
 		if (!prefixcmp(dest, "http://") ||
 		    !prefixcmp(dest, "https://"))
@@ -341,7 +191,7 @@ static int do_push(const char *repo)
 		if (!err)
 			continue;
 
-		error("failed to push to '%s'", uri[i]);
+		error("failed to push to '%s'", remote->uri[i]);
 		switch (err) {
 		case -ERR_RUN_COMMAND_FORK:
 			error("unable to fork for %s", sender);
@@ -362,7 +212,7 @@ static int do_push(const char *repo)
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	const char *repo = "origin";	/* default repository */
+	const char *repo = NULL;	/* default repository */
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/remote.c b/remote.c
new file mode 100644
index 0000000..1dd2e77
--- /dev/null
+++ b/remote.c
@@ -0,0 +1,203 @@
+#include "cache.h"
+#include "remote.h"
+#include "refs.h"
+
+static struct remote **remotes;
+static int allocated_remotes;
+
+#define BUF_SIZE (2084)
+static char buffer[BUF_SIZE];
+
+static void add_push_refspec(struct remote *remote, const char *ref)
+{
+	int nr = remote->push_refspec_nr + 1;
+	remote->push_refspec =
+		xrealloc(remote->push_refspec, nr * sizeof(char *));
+	remote->push_refspec[nr-1] = ref;
+	remote->push_refspec_nr = nr;
+}
+
+static void add_uri(struct remote *remote, const char *uri)
+{
+	int nr = remote->uri_nr + 1;
+	remote->uri =
+		xrealloc(remote->uri, nr * sizeof(char *));
+	remote->uri[nr-1] = uri;
+	remote->uri_nr = nr;
+}
+
+static struct remote *make_remote(const char *name, int len)
+{
+	int i, empty = -1;
+
+	for (i = 0; i < allocated_remotes; i++) {
+		if (!remotes[i]) {
+			if (empty < 0)
+				empty = i;
+		} else {
+			if (len ? (!strncmp(name, remotes[i]->name, len) &&
+				   !remotes[i]->name[len]) :
+			    !strcmp(name, remotes[i]->name))
+				return remotes[i];
+		}
+	}
+
+	if (empty < 0) {
+		empty = allocated_remotes;
+		allocated_remotes += allocated_remotes ? allocated_remotes : 1;
+		remotes = xrealloc(remotes,
+				   sizeof(*remotes) * allocated_remotes);
+		memset(remotes + empty, 0,
+		       (allocated_remotes - empty) * sizeof(*remotes));
+	}
+	remotes[empty] = xcalloc(1, sizeof(struct remote));
+	if (len)
+		remotes[empty]->name = xstrndup(name, len);
+	else
+		remotes[empty]->name = xstrdup(name);
+	return remotes[empty];
+}
+
+static void read_remotes_file(struct remote *remote)
+{
+	FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
+
+	if (!f)
+		return;
+	while (fgets(buffer, BUF_SIZE, f)) {
+		int value_list;
+		char *s, *p;
+
+		if (!prefixcmp(buffer, "URL:")) {
+			value_list = 0;
+			s = buffer + 4;
+		} else if (!prefixcmp(buffer, "Push:")) {
+			value_list = 1;
+			s = buffer + 5;
+		} else
+			continue;
+
+		while (isspace(*s))
+			s++;
+		if (!*s)
+			continue;
+
+		p = s + strlen(s);
+		while (isspace(p[-1]))
+			*--p = 0;
+
+		switch (value_list) {
+		case 0:
+			add_uri(remote, xstrdup(s));
+			break;
+		case 1:
+			add_push_refspec(remote, xstrdup(s));
+			break;
+		}
+	}
+}
+
+static void read_branches_file(struct remote *remote)
+{
+	const char *slash = strchr(remote->name, '/');
+	int n = slash ? slash - remote->name : 1000;
+	FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
+	char *s, *p;
+	int len;
+
+	if (!f)
+		return;
+	s = fgets(buffer, BUF_SIZE, f);
+	fclose(f);
+	if (!s)
+		return;
+	while (isspace(*s))
+		s++;
+	if (!*s)
+		return;
+	p = s + strlen(s);
+	while (isspace(p[-1]))
+		*--p = 0;
+	len = p - s;
+	if (slash)
+		len += strlen(slash);
+	p = xmalloc(len + 1);
+	strcpy(p, s);
+	if (slash)
+		strcat(p, slash);
+	add_uri(remote, p);
+}
+
+static char *default_remote_name = NULL;
+static const char *current_branch = NULL;
+static int current_branch_len = 0;
+
+static int handle_config(const char *key, const char *value)
+{
+	const char *name;
+	const char *subkey;
+	struct remote *remote;
+	if (!prefixcmp(key, "branch.") && current_branch &&
+	    !strncmp(key + 7, current_branch, current_branch_len) &&
+	    !strcmp(key + 7 + current_branch_len, ".remote")) {
+		free(default_remote_name);
+		default_remote_name = xstrdup(value);
+	}
+	if (prefixcmp(key,  "remote."))
+		return 0;
+	name = key + 7;
+	subkey = strrchr(name, '.');
+	if (!subkey)
+		return error("Config with no key for remote %s", name);
+	remote = make_remote(name, subkey - name);
+	if (!strcmp(subkey, ".url")) {
+		add_uri(remote, xstrdup(value));
+	} else if (!strcmp(subkey, ".push")) {
+		add_push_refspec(remote, xstrdup(value));
+	} else if (!strcmp(subkey, ".receivepack")) {
+		if (!remote->receivepack)
+			remote->receivepack = xstrdup(value);
+		else
+			error("more than one receivepack given, using the first");
+	}
+	return 0;
+}
+
+static void read_config(void)
+{
+	unsigned char sha1[20];
+	const char *head_ref;
+	int flag;
+	if (default_remote_name) // did this already
+		return;
+	default_remote_name = xstrdup("origin");
+	current_branch = NULL;
+	head_ref = resolve_ref("HEAD", sha1, 0, &flag);
+	if (head_ref && (flag & REF_ISSYMREF) &&
+	    !prefixcmp(head_ref, "refs/heads/")) {
+		current_branch = head_ref + strlen("refs/heads/");
+		current_branch_len = strlen(current_branch);
+	}
+	git_config(handle_config);
+}
+
+struct remote *remote_get(const char *name)
+{
+	struct remote *ret;
+
+	read_config();
+	if (!name)
+		name = default_remote_name;
+	ret = make_remote(name, 0);
+	if (name[0] != '/') {
+		if (!ret->uri)
+			read_remotes_file(ret);
+		if (!ret->uri)
+			read_branches_file(ret);
+	}
+	if (!ret->uri)
+		add_uri(ret, name);
+	if (!ret->uri)
+		return NULL;
+	return ret;
+}
diff --git a/remote.h b/remote.h
new file mode 100644
index 0000000..73747a8
--- /dev/null
+++ b/remote.h
@@ -0,0 +1,18 @@
+#ifndef REMOTE_H
+#define REMOTE_H
+
+struct remote {
+	const char *name;
+
+	const char **uri;
+	int uri_nr;
+
+	const char **push_refspec;
+	int push_refspec_nr;
+
+	const char *receivepack;
+};
+
+struct remote *remote_get(const char *name);
+
+#endif
-- 
1.5.2.rc2.45.g3d9b43-dirty

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> The new parser is different from the one in builtin-push in two ways:
> the default is to use the current branch's remote, if there is one,
> before "origin"; and config is used in preference to remotes.

Please chuck my comment about describing differences in the
previous message---you did it already.  Sorry.

> diff --git a/remote.c b/remote.c
> new file mode 100644
> index 0000000..1dd2e77
> --- /dev/null
> +++ b/remote.c
> @@ -0,0 +1,203 @@
> +#include "cache.h"
> +#include "remote.h"
> +#include "refs.h"
> +
> +static struct remote **remotes;
> +static int allocated_remotes;
> +
> +#define BUF_SIZE (2084)
> +static char buffer[BUF_SIZE];

Heh, inherited a funny constant from the original...  I guess
most likely he meant 2048 ;-).

> +static void read_remotes_file(struct remote *remote)
> +{
> +	FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
> +
> +	if (!f)
> +		return;
> +	while (fgets(buffer, BUF_SIZE, f)) {
> +...
> +	}
> +}

I sense a slight "FILE *" leak here...

[PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Daniel Barkalow]

The new parser is different from the one in builtin-push in two ways:
the default is to use the current branch's remote, if there is one,
before "origin"; and config is used in preference to remotes.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 Makefile       |    5 +-
 builtin-push.c |  190 ++++++----------------------------------------------
 remote.c       |  204 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 remote.h       |   18 +++++
 4 files changed, 245 insertions(+), 172 deletions(-)
 create mode 100644 remote.c
 create mode 100644 remote.h

diff --git a/Makefile b/Makefile
index 7cf146b..134fbd2 100644
--- a/Makefile
+++ b/Makefile
@@ -296,7 +296,8 @@ LIB_H = \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
-	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h mailmap.h
+	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
+	mailmap.h remote.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -318,7 +319,7 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o
+	convert.o attr.o decorate.o progress.o mailmap.o remote.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/builtin-push.c b/builtin-push.c
index cb78401..0e602f3 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -5,17 +5,13 @@
 #include "refs.h"
 #include "run-command.h"
 #include "builtin.h"
-
-#define MAX_URI (16)
+#include "remote.h"
 
 static const char push_usage[] = "git-push [--all] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]";
 
 static int all, tags, force, thin = 1, verbose;
 static const char *receivepack;
 
-#define BUF_SIZE (2084)
-static char buffer[BUF_SIZE];
-
 static const char **refspec;
 static int refspec_nr;
 
@@ -137,175 +133,29 @@ static void set_refspecs(const char **refs, int nr)
 	expand_refspecs();
 }
 
-static int get_remotes_uri(const char *repo, const char *uri[MAX_URI])
-{
-	int n = 0;
-	FILE *f = fopen(git_path("remotes/%s", repo), "r");
-	int has_explicit_refspec = refspec_nr || all || tags;
-
-	if (!f)
-		return -1;
-	while (fgets(buffer, BUF_SIZE, f)) {
-		int is_refspec;
-		char *s, *p;
-
-		if (!prefixcmp(buffer, "URL:")) {
-			is_refspec = 0;
-			s = buffer + 4;
-		} else if (!prefixcmp(buffer, "Push:")) {
-			is_refspec = 1;
-			s = buffer + 5;
-		} else
-			continue;
-
-		/* Remove whitespace at the head.. */
-		while (isspace(*s))
-			s++;
-		if (!*s)
-			continue;
-
-		/* ..and at the end */
-		p = s + strlen(s);
-		while (isspace(p[-1]))
-			*--p = 0;
-
-		if (!is_refspec) {
-			if (n < MAX_URI)
-				uri[n++] = xstrdup(s);
-			else
-				error("more than %d URL's specified, ignoring the rest", MAX_URI);
-		}
-		else if (is_refspec && !has_explicit_refspec) {
-			if (!wildcard_ref(s))
-				add_refspec(xstrdup(s));
-		}
-	}
-	fclose(f);
-	if (!n)
-		die("remote '%s' has no URL", repo);
-	return n;
-}
-
-static const char **config_uri;
-static const char *config_repo;
-static int config_repo_len;
-static int config_current_uri;
-static int config_get_refspecs;
-static int config_get_receivepack;
-
-static int get_remote_config(const char* key, const char* value)
-{
-	if (!prefixcmp(key, "remote.") &&
-	    !strncmp(key + 7, config_repo, config_repo_len)) {
-		if (!strcmp(key + 7 + config_repo_len, ".url")) {
-			if (config_current_uri < MAX_URI)
-				config_uri[config_current_uri++] = xstrdup(value);
-			else
-				error("more than %d URL's specified, ignoring the rest", MAX_URI);
-		}
-		else if (config_get_refspecs &&
-			 !strcmp(key + 7 + config_repo_len, ".push")) {
-			if (!wildcard_ref(value))
-				add_refspec(xstrdup(value));
-		}
-		else if (config_get_receivepack &&
-			 !strcmp(key + 7 + config_repo_len, ".receivepack")) {
-			if (!receivepack) {
-				char *rp = xmalloc(strlen(value) + 16);
-				sprintf(rp, "--receive-pack=%s", value);
-				receivepack = rp;
-			} else
-				error("more than one receivepack given, using the first");
-		}
-	}
-	return 0;
-}
-
-static int get_config_remotes_uri(const char *repo, const char *uri[MAX_URI])
-{
-	config_repo_len = strlen(repo);
-	config_repo = repo;
-	config_current_uri = 0;
-	config_uri = uri;
-	config_get_refspecs = !(refspec_nr || all || tags);
-	config_get_receivepack = (receivepack == NULL);
-
-	git_config(get_remote_config);
-	return config_current_uri;
-}
-
-static int get_branches_uri(const char *repo, const char *uri[MAX_URI])
-{
-	const char *slash = strchr(repo, '/');
-	int n = slash ? slash - repo : 1000;
-	FILE *f = fopen(git_path("branches/%.*s", n, repo), "r");
-	char *s, *p;
-	int len;
-
-	if (!f)
-		return 0;
-	s = fgets(buffer, BUF_SIZE, f);
-	fclose(f);
-	if (!s)
-		return 0;
-	while (isspace(*s))
-		s++;
-	if (!*s)
-		return 0;
-	p = s + strlen(s);
-	while (isspace(p[-1]))
-		*--p = 0;
-	len = p - s;
-	if (slash)
-		len += strlen(slash);
-	p = xmalloc(len + 1);
-	strcpy(p, s);
-	if (slash)
-		strcat(p, slash);
-	uri[0] = p;
-	return 1;
-}
-
-/*
- * Read remotes and branches file, fill the push target URI
- * list.  If there is no command line refspecs, read Push: lines
- * to set up the *refspec list as well.
- * return the number of push target URIs
- */
-static int read_config(const char *repo, const char *uri[MAX_URI])
-{
-	int n;
-
-	if (*repo != '/') {
-		n = get_remotes_uri(repo, uri);
-		if (n > 0)
-			return n;
-
-		n = get_config_remotes_uri(repo, uri);
-		if (n > 0)
-			return n;
-
-		n = get_branches_uri(repo, uri);
-		if (n > 0)
-			return n;
-	}
-
-	uri[0] = repo;
-	return 1;
-}
-
 static int do_push(const char *repo)
 {
-	const char *uri[MAX_URI];
-	int i, n, errs;
+	int i, errs;
 	int common_argc;
 	const char **argv;
 	int argc;
+	struct remote *remote = remote_get(repo);
 
-	n = read_config(repo, uri);
-	if (n <= 0)
+	if (!remote)
 		die("bad repository '%s'", repo);
 
+	if (remote->receivepack) {
+		char *rp = xmalloc(strlen(remote->receivepack) + 16);
+		sprintf(rp, "--receive-pack=%s", remote->receivepack);
+		receivepack = rp;
+	}
+	if (!refspec && !all && !tags && remote->push_refspec_nr) {
+		for (i = 0; i < remote->push_refspec_nr; i++) {
+			if (!wildcard_ref(remote->push_refspec[i]))
+				add_refspec(remote->push_refspec[i]);
+		}
+	}
+
 	argv = xmalloc((refspec_nr + 10) * sizeof(char *));
 	argv[0] = "dummy-send-pack";
 	argc = 1;
@@ -318,12 +168,12 @@ static int do_push(const char *repo)
 	common_argc = argc;
 
 	errs = 0;
-	for (i = 0; i < n; i++) {
+	for (i = 0; i < remote->uri_nr; i++) {
 		int err;
 		int dest_argc = common_argc;
 		int dest_refspec_nr = refspec_nr;
 		const char **dest_refspec = refspec;
-		const char *dest = uri[i];
+		const char *dest = remote->uri[i];
 		const char *sender = "send-pack";
 		if (!prefixcmp(dest, "http://") ||
 		    !prefixcmp(dest, "https://"))
@@ -341,7 +191,7 @@ static int do_push(const char *repo)
 		if (!err)
 			continue;
 
-		error("failed to push to '%s'", uri[i]);
+		error("failed to push to '%s'", remote->uri[i]);
 		switch (err) {
 		case -ERR_RUN_COMMAND_FORK:
 			error("unable to fork for %s", sender);
@@ -362,7 +212,7 @@ static int do_push(const char *repo)
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	const char *repo = "origin";	/* default repository */
+	const char *repo = NULL;	/* default repository */
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/remote.c b/remote.c
new file mode 100644
index 0000000..dbcc74e
--- /dev/null
+++ b/remote.c
@@ -0,0 +1,204 @@
+#include "cache.h"
+#include "remote.h"
+#include "refs.h"
+
+static struct remote **remotes;
+static int allocated_remotes;
+
+#define BUF_SIZE (2048)
+static char buffer[BUF_SIZE];
+
+static void add_push_refspec(struct remote *remote, const char *ref)
+{
+	int nr = remote->push_refspec_nr + 1;
+	remote->push_refspec =
+		xrealloc(remote->push_refspec, nr * sizeof(char *));
+	remote->push_refspec[nr-1] = ref;
+	remote->push_refspec_nr = nr;
+}
+
+static void add_uri(struct remote *remote, const char *uri)
+{
+	int nr = remote->uri_nr + 1;
+	remote->uri =
+		xrealloc(remote->uri, nr * sizeof(char *));
+	remote->uri[nr-1] = uri;
+	remote->uri_nr = nr;
+}
+
+static struct remote *make_remote(const char *name, int len)
+{
+	int i, empty = -1;
+
+	for (i = 0; i < allocated_remotes; i++) {
+		if (!remotes[i]) {
+			if (empty < 0)
+				empty = i;
+		} else {
+			if (len ? (!strncmp(name, remotes[i]->name, len) &&
+				   !remotes[i]->name[len]) :
+			    !strcmp(name, remotes[i]->name))
+				return remotes[i];
+		}
+	}
+
+	if (empty < 0) {
+		empty = allocated_remotes;
+		allocated_remotes += allocated_remotes ? allocated_remotes : 1;
+		remotes = xrealloc(remotes,
+				   sizeof(*remotes) * allocated_remotes);
+		memset(remotes + empty, 0,
+		       (allocated_remotes - empty) * sizeof(*remotes));
+	}
+	remotes[empty] = xcalloc(1, sizeof(struct remote));
+	if (len)
+		remotes[empty]->name = xstrndup(name, len);
+	else
+		remotes[empty]->name = xstrdup(name);
+	return remotes[empty];
+}
+
+static void read_remotes_file(struct remote *remote)
+{
+	FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
+
+	if (!f)
+		return;
+	while (fgets(buffer, BUF_SIZE, f)) {
+		int value_list;
+		char *s, *p;
+
+		if (!prefixcmp(buffer, "URL:")) {
+			value_list = 0;
+			s = buffer + 4;
+		} else if (!prefixcmp(buffer, "Push:")) {
+			value_list = 1;
+			s = buffer + 5;
+		} else
+			continue;
+
+		while (isspace(*s))
+			s++;
+		if (!*s)
+			continue;
+
+		p = s + strlen(s);
+		while (isspace(p[-1]))
+			*--p = 0;
+
+		switch (value_list) {
+		case 0:
+			add_uri(remote, xstrdup(s));
+			break;
+		case 1:
+			add_push_refspec(remote, xstrdup(s));
+			break;
+		}
+	}
+	fclose(f);
+}
+
+static void read_branches_file(struct remote *remote)
+{
+	const char *slash = strchr(remote->name, '/');
+	int n = slash ? slash - remote->name : 1000;
+	FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
+	char *s, *p;
+	int len;
+
+	if (!f)
+		return;
+	s = fgets(buffer, BUF_SIZE, f);
+	fclose(f);
+	if (!s)
+		return;
+	while (isspace(*s))
+		s++;
+	if (!*s)
+		return;
+	p = s + strlen(s);
+	while (isspace(p[-1]))
+		*--p = 0;
+	len = p - s;
+	if (slash)
+		len += strlen(slash);
+	p = xmalloc(len + 1);
+	strcpy(p, s);
+	if (slash)
+		strcat(p, slash);
+	add_uri(remote, p);
+}
+
+static char *default_remote_name = NULL;
+static const char *current_branch = NULL;
+static int current_branch_len = 0;
+
+static int handle_config(const char *key, const char *value)
+{
+	const char *name;
+	const char *subkey;
+	struct remote *remote;
+	if (!prefixcmp(key, "branch.") && current_branch &&
+	    !strncmp(key + 7, current_branch, current_branch_len) &&
+	    !strcmp(key + 7 + current_branch_len, ".remote")) {
+		free(default_remote_name);
+		default_remote_name = xstrdup(value);
+	}
+	if (prefixcmp(key,  "remote."))
+		return 0;
+	name = key + 7;
+	subkey = strrchr(name, '.');
+	if (!subkey)
+		return error("Config with no key for remote %s", name);
+	remote = make_remote(name, subkey - name);
+	if (!strcmp(subkey, ".url")) {
+		add_uri(remote, xstrdup(value));
+	} else if (!strcmp(subkey, ".push")) {
+		add_push_refspec(remote, xstrdup(value));
+	} else if (!strcmp(subkey, ".receivepack")) {
+		if (!remote->receivepack)
+			remote->receivepack = xstrdup(value);
+		else
+			error("more than one receivepack given, using the first");
+	}
+	return 0;
+}
+
+static void read_config(void)
+{
+	unsigned char sha1[20];
+	const char *head_ref;
+	int flag;
+	if (default_remote_name) // did this already
+		return;
+	default_remote_name = xstrdup("origin");
+	current_branch = NULL;
+	head_ref = resolve_ref("HEAD", sha1, 0, &flag);
+	if (head_ref && (flag & REF_ISSYMREF) &&
+	    !prefixcmp(head_ref, "refs/heads/")) {
+		current_branch = head_ref + strlen("refs/heads/");
+		current_branch_len = strlen(current_branch);
+	}
+	git_config(handle_config);
+}
+
+struct remote *remote_get(const char *name)
+{
+	struct remote *ret;
+
+	read_config();
+	if (!name)
+		name = default_remote_name;
+	ret = make_remote(name, 0);
+	if (name[0] != '/') {
+		if (!ret->uri)
+			read_remotes_file(ret);
+		if (!ret->uri)
+			read_branches_file(ret);
+	}
+	if (!ret->uri)
+		add_uri(ret, name);
+	if (!ret->uri)
+		return NULL;
+	return ret;
+}
diff --git a/remote.h b/remote.h
new file mode 100644
index 0000000..73747a8
--- /dev/null
+++ b/remote.h
@@ -0,0 +1,18 @@
+#ifndef REMOTE_H
+#define REMOTE_H
+
+struct remote {
+	const char *name;
+
+	const char **uri;
+	int uri_nr;
+
+	const char **push_refspec;
+	int push_refspec_nr;
+
+	const char *receivepack;
+};
+
+struct remote *remote_get(const char *name);
+
+#endif
-- 
1.5.2.rc2.45.g3d9b43-dirty

Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> +static int handle_config(const char *key, const char *value)
> +{
> +	const char *name;
> +	const char *subkey;
> +	struct remote *remote;
> +	if (!prefixcmp(key, "branch.") && current_branch &&
> +	    !strncmp(key + 7, current_branch, current_branch_len) &&
> +	    !strcmp(key + 7 + current_branch_len, ".remote")) {
> +		free(default_remote_name);
> +		default_remote_name = xstrdup(value);
> +	}
> +	if (prefixcmp(key,  "remote."))
> +		return 0;
> +	name = key + 7;
> +	subkey = strrchr(name, '.');
> +	if (!subkey)
> +		return error("Config with no key for remote %s", name);
> +	remote = make_remote(name, subkey - name);
> +	if (!strcmp(subkey, ".url")) {
> +		add_uri(remote, xstrdup(value));
> +	} else if (!strcmp(subkey, ".push")) {
> +		add_push_refspec(remote, xstrdup(value));
> +	} else if (!strcmp(subkey, ".receivepack")) {
> +		if (!remote->receivepack)
> +			remote->receivepack = xstrdup(value);
> +		else
> +			error("more than one receivepack given, using the first");
> +	}
> +	return 0;
> +}

You forgot to update this part?  With your comments on not
erroring out, which made sense to me, how about this?

diff --git a/remote.c b/remote.c
index dbcc74e..b032e81 100644
--- a/remote.c
+++ b/remote.c
@@ -150,7 +150,26 @@ static int handle_config(const char *key, const char *value)
 	subkey = strrchr(name, '.');
 	if (!subkey)
 		return error("Config with no key for remote %s", name);
+	if (*subkey == '/') {
+		warning("Config remote shorthand cannot begin with '/': %s", name);
+		return 0;
+	}
 	remote = make_remote(name, subkey - name);
+	if (!value) {
+		/* if we ever have a boolean variable, e.g. "remote.*.disabled"
+		 * [remote "frotz"]
+		 *      disabled
+		 * is a valid way to set it to true; we get NULL in value so
+		 * we need to handle it here.
+		 *
+		 * if (!strcmp(subkey, ".disabled")) {
+		 *      val = git_config_bool(key, value);
+		 *      return 0;
+		 * } else
+		 *
+		 */
+		return 0; /* ignore unknown booleans */
+	}
 	if (!strcmp(subkey, ".url")) {
 		add_uri(remote, xstrdup(value));
 	} else if (!strcmp(subkey, ".push")) {