[PATCH] builtin-push: resurrect parsing of Push: lines

[PATCH] builtin-push: resurrect parsing of Push: lines

[Johannes Schindelin]

The C'ification of push left these behind.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 builtin-push.c |   26 +++++++++++++++++++++-----
 1 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index a0c1caa..4e659f0 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -68,6 +68,10 @@ static void set_refspecs(const char **re
 	expand_refspecs();
 }
 
+#define MAX_REFSPECS 10
+static int current_refspec = 0;
+static char *refspecs_[MAX_REFSPECS];
+
 static int get_remotes_uri(const char *repo, const char *uri[MAX_URI])
 {
 	int n = 0;
@@ -76,11 +80,17 @@ static int get_remotes_uri(const char *r
 	if (!f)
 		return -1;
 	while (fgets(buffer, BUF_SIZE, f)) {
+		int is_refspec;
 		char *s, *p;
 
-		if (strncmp("URL: ", buffer, 5))
+		if (!strncmp("URL: ", buffer, 5)) {
+			is_refspec = 0;
+			s = buffer + 5;
+		} else if (!strncmp("Push: ", buffer, 6)) {
+			is_refspec = 1;
+			s = buffer + 6;
+		} else
 			continue;
-		s = buffer + 5;
 
 		/* Remove whitespace at the head.. */
 		while (isspace(*s))
@@ -93,9 +103,10 @@ static int get_remotes_uri(const char *r
 		while (isspace(p[-1]))
 			*--p = 0;
 
-		uri[n++] = strdup(s);
-		if (n == MAX_URI)
-			break;
+		if (!is_refspec && n < MAX_URI)
+			uri[n++] = strdup(s);
+		else if (is_refspec && current_refspec < MAX_REFSPECS)
+			refspecs_[current_refspec++] = strdup(s);
 	}
 	fclose(f);
 	if (!n)
@@ -140,6 +151,8 @@ static int get_uri(const char *repo, con
 	int n;
 
 	if (*repo != '/') {
+		current_refspec = 0;
+
 		n = get_remotes_uri(repo, uri);
 		if (n > 0)
 			return n;
@@ -165,6 +178,9 @@ static int do_push(const char *repo)
 	if (n <= 0)
 		die("bad repository '%s'", repo);
 
+	if (refspec_nr == 0)
+		set_refspecs((const char**)refspecs_, current_refspec);
+
 	argv = xmalloc((refspec_nr + 10) * sizeof(char *));
 	argv[0] = "dummy-send-pack";
 	argc = 1;

Re: [PATCH] builtin-push: resurrect parsing of Push: lines

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The C'ification of push left these behind.

> +#define MAX_REFSPECS 10
> +static int current_refspec = 0;
> +static char *refspecs_[MAX_REFSPECS];

While this fix makes perfect sense, I think MAX_URI set to 16 is
reasonable hard limit, but I am not happy about giving hard
limit to MAX_REFSPECS -- that's a regression from the shell
script one.

Re: [PATCH] builtin-push: resurrect parsing of Push: lines

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> The C'ification of push left these behind.
>
>> +#define MAX_REFSPECS 10
>> +static int current_refspec = 0;
>> +static char *refspecs_[MAX_REFSPECS];
>
> While this fix makes perfect sense, I think MAX_URI set to 16 is
> reasonable hard limit, but I am not happy about giving hard
> limit to MAX_REFSPECS -- that's a regression from the shell
> script one.

So how about this as a replacement?

 - You are going to discard refspecs_[] if there were any
   command line stuff anyway, so do not even bother storing them
   in a separate array.  Just do add_refspec() on them if you
   are going to use it after you are done.

 - The function get_uri() is not about URI anymore, so rename it
   appropriately.

 - Fix the double call to get_uri() Sean noticed.

-- >8 --
diff --git a/builtin-push.c b/builtin-push.c
index 4e659f0..9a861b5 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -68,14 +68,11 @@ static void set_refspecs(const char **re
 	expand_refspecs();
 }
 
-#define MAX_REFSPECS 10
-static int current_refspec = 0;
-static char *refspecs_[MAX_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;
 
 	if (!f)
 		return -1;
@@ -103,10 +100,14 @@ static int get_remotes_uri(const char *r
 		while (isspace(p[-1]))
 			*--p = 0;
 
-		if (!is_refspec && n < MAX_URI)
-			uri[n++] = strdup(s);
-		else if (is_refspec && current_refspec < MAX_REFSPECS)
-			refspecs_[current_refspec++] = strdup(s);
+		if (!is_refspec) {
+			if (n < MAX_URI)
+				uri[n++] = strdup(s);
+			else
+				error("more than %d URL's specified, ignoreing the rest", MAX_URI);
+		}
+		else if (is_refspec && !has_explicit_refspec)
+			add_refspec(strdup(s));
 	}
 	fclose(f);
 	if (!n)
@@ -146,13 +147,17 @@ static int get_branches_uri(const char *
 	return 1;
 }
 
-static int get_uri(const char *repo, const char *uri[MAX_URI])
+/*
+ * 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 != '/') {
-		current_refspec = 0;
-
 		n = get_remotes_uri(repo, uri);
 		if (n > 0)
 			return n;
@@ -169,18 +174,15 @@ static int get_uri(const char *repo, con
 static int do_push(const char *repo)
 {
 	const char *uri[MAX_URI];
-	int i, n = get_uri(repo, uri);
+	int i, n;
 	int remote;
 	const char **argv;
 	int argc;
 
-	n = get_uri(repo, uri);
+	n = read_config(repo, uri);
 	if (n <= 0)
 		die("bad repository '%s'", repo);
 
-	if (refspec_nr == 0)
-		set_refspecs((const char**)refspecs_, current_refspec);
-
 	argv = xmalloc((refspec_nr + 10) * sizeof(char *));
 	argv[0] = "dummy-send-pack";
 	argc = 1;

Re: [PATCH] builtin-push: resurrect parsing of Push: lines

[Johannes Schindelin]

Hi,

On Sun, 30 Apr 2006, Junio C Hamano wrote:

> So how about this as a replacement?

Looks good. Will try tomorrow.

Ciao,
Dscho