[PATCH 2/2] Teach rev-list an option to read revs from the standard input.

[PATCH 2/2] Teach rev-list an option to read revs from the standard input.

[Junio C Hamano]

When --stdin option is given, in addition to the <rev>s listed
on the command line, the command can read one rev parameter per
line from the standard input.  The list of revs ends at the
first empty line or EOF.

Note that you still have to give all the flags from the command
line; only rev arguments (including A..B, A...B, and A^@ notations)
can be give from the standard input.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 Documentation/git-rev-list.txt |    6 ++++++
 builtin-rev-list.c             |   25 +++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 3c4c2fb..28966ad 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -17,6 +17,7 @@ SYNOPSIS
 	     [ \--remove-empty ]
 	     [ \--not ]
 	     [ \--all ]
+	     [ \--stdin ]
 	     [ \--topo-order ]
 	     [ \--parents ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
@@ -171,6 +172,11 @@ limiting may be applied.
 	Pretend as if all the refs in `$GIT_DIR/refs/` are listed on the
 	command line as '<commit>'.
 
+--stdin::
+
+	In addition to the '<commit>' listed on the command
+	line, read them from the standard input.
+
 --merge::
 
 	After a failed merge, show refs that touch files having a
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 8437454..8fe8afb 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -23,6 +23,7 @@ static const char rev_list_usage[] =
 "    --no-merges\n"
 "    --remove-empty\n"
 "    --all\n"
+"    --stdin\n"
 "  ordering output:\n"
 "    --topo-order\n"
 "    --date-order\n"
@@ -304,10 +305,28 @@ static void mark_edges_uninteresting(str
 	}
 }
 
+static void read_revisions_from_stdin(struct rev_info *revs)
+{
+	char line[1000];
+
+	while (fgets(line, sizeof(line), stdin) != NULL) {
+		int len = strlen(line);
+		if (line[len - 1] == '\n')
+			line[--len] = 0;
+		if (!len)
+			break;
+		if (line[0] == '-')
+			die("options not supported in --stdin mode");
+		if (handle_revision_arg(line, revs, 0, 1))
+			die("bad revision '%s'", line);
+	}
+}
+
 int cmd_rev_list(int argc, const char **argv, const char *prefix)
 {
 	struct commit_list *list;
 	int i;
+	int read_from_stdin = 0;
 
 	init_revisions(&revs, prefix);
 	revs.abbrev = 0;
@@ -329,6 +348,12 @@ int cmd_rev_list(int argc, const char **
 			bisect_list = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--stdin")) {
+			if (read_from_stdin++)
+				die("--stdin given twice?");
+			read_revisions_from_stdin(&revs);
+			continue;
+		}
 		usage(rev_list_usage);
 
 	}
-- 
1.4.2.g980c3

Re: [PATCH 2/2] Teach rev-list an option to read revs from the standard input.

[Jakub Narebski]

Junio C Hamano wrote:

> Note that you still have to give all the flags from the command
> line; only rev arguments (including A..B, A...B, and A^@ notations)
> can be give from the standard input.

Does this include ^A notation?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH 2/2] Teach rev-list an option to read revs from the standard input.

[Andy Whitcroft]

Ok, I've tested this with an updated version of my patch to make
send-pack use this (which I'll send out following this message) and it
seems to work pretty well.

-apw

[PATCH] send pack switch to using git rev list stdin v2

[Andy Whitcroft]

send-pack: switch to using git-rev-list --stdin v2

When we are generating packs to update remote repositories we
want to supply as much information as possible about the revisions
that already exist to rev-list in order optimise the pack as much
as possible.  We need to pass two revisions for each branch we are
updating in the remote repository and one for each additional branch.
Where the remote repository has numerous branches we can run out
of command line space to pass them.

Utilise the git-rev-list --stdin mode to allow unlimited numbers
of revision constraints.  This allows us to move back to the much
simpler unordered revision selection code.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/send-pack.c b/send-pack.c
index ac4501d..b403ee9 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -38,9 +38,8 @@ static void exec_pack_objects(void)
 
 static void exec_rev_list(struct ref *refs)
 {
-	struct ref *ref;
-	static const char *args[1000];
-	int i = 0, j;
+	static const char *args[4];
+	int i = 0;
 
 	args[i++] = "rev-list";	/* 0 */
 	if (use_thin_pack)	/* 1 */
@@ -48,43 +47,29 @@ static void exec_rev_list(struct ref *re
 	else
 		args[i++] = "--objects";
 
-	/* First send the ones we care about most */
-	for (ref = refs; ref; ref = ref->next) {
-		if (900 < i)
-			die("git-rev-list environment overflow");
-		if (!is_zero_sha1(ref->new_sha1)) {
-			char *buf = xmalloc(100);
-			args[i++] = buf;
-			snprintf(buf, 50, "%s", sha1_to_hex(ref->new_sha1));
-			buf += 50;
-			if (!is_zero_sha1(ref->old_sha1) &&
-			    has_sha1_file(ref->old_sha1)) {
-				args[i++] = buf;
-				snprintf(buf, 50, "^%s",
-					 sha1_to_hex(ref->old_sha1));
-			}
-		}
-	}
+	args[i++] = "--stdin";
 
-	/* Then a handful of the remainder
-	 * NEEDSWORK: we would be better off if used the newer ones first.
-	 */
-	for (ref = refs, j = i + 16;
-	     i < 900 && i < j && ref;
-	     ref = ref->next) {
-		if (is_zero_sha1(ref->new_sha1) &&
-		    !is_zero_sha1(ref->old_sha1) &&
-		    has_sha1_file(ref->old_sha1)) {
-			char *buf = xmalloc(42);
-			args[i++] = buf;
-			snprintf(buf, 42, "^%s", sha1_to_hex(ref->old_sha1));
-		}
-	}
 	args[i] = NULL;
 	execv_git_cmd(args);
 	die("git-rev-list exec failed (%s)", strerror(errno));
 }
 
+static void builtin_rev_list_generate(struct ref *refs)
+{
+	while (refs) {
+		if (!is_zero_sha1(refs->old_sha1) &&
+		    has_sha1_file(refs->old_sha1)) {
+			printf("^%s\n", sha1_to_hex(refs->old_sha1));
+		}
+		if (!is_zero_sha1(refs->new_sha1)) {
+			printf("%s\n", sha1_to_hex(refs->new_sha1));
+		}
+		refs = refs->next;
+	}
+
+	exit(0);
+}
+
 static void rev_list(int fd, struct ref *refs)
 {
 	int pipe_fd[2];
@@ -111,13 +96,38 @@ static void rev_list(int fd, struct ref 
 	exec_rev_list(refs);
 }
 
+static void rev_list_generate(int fd, struct ref *refs)
+{
+	int pipe_fd[2];
+	pid_t rev_list_generate_pid;
+
+	if (pipe(pipe_fd) < 0)
+		die("rev-list-generate setup: pipe failed");
+	rev_list_generate_pid = fork();
+	if (!rev_list_generate_pid) {
+		dup2(pipe_fd[0], 0);
+		dup2(fd, 1);
+		close(pipe_fd[0]);
+		close(pipe_fd[1]);
+		close(fd);
+		rev_list(fd, refs);
+		die("rev-list setup failed");
+	}
+	if (rev_list_generate_pid < 0)
+		die("rev-list-generate fork failed");
+	dup2(pipe_fd[1], 1);
+	close(pipe_fd[0]);
+	close(pipe_fd[1]);
+	close(fd);
+	builtin_rev_list_generate(refs);
+}
 static void pack_objects(int fd, struct ref *refs)
 {
 	pid_t rev_list_pid;
 
 	rev_list_pid = fork();
 	if (!rev_list_pid) {
-		rev_list(fd, refs);
+		rev_list_generate(fd, refs);
 		die("rev-list setup failed");
 	}
 	if (rev_list_pid < 0)

Re: [PATCH 2/2] Teach rev-list an option to read revs from the standard input.

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

> Ok, I've tested this with an updated version of my patch to make
> send-pack use this (which I'll send out following this message) and it
> seems to work pretty well.

Eh, I have your earlier one placed on "pu" already.  I do not
mind replacing it with the new one but (1) have you checked what
is in "pu"?  (2) how different is this new one compared to it?

Re: [PATCH 2/2] Teach rev-list an option to read revs from the standard input.

[Andy Whitcroft]

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>> Ok, I've tested this with an updated version of my patch to make
>> send-pack use this (which I'll send out following this message) and it
>> seems to work pretty well.
> 
> Eh, I have your earlier one placed on "pu" already.  I do not
> mind replacing it with the new one but (1) have you checked what
> is in "pu"?  (2) how different is this new one compared to it?

There was white space breakage in the patch which I noticed late in the
game, and a problem with the printing lacking newlines.  Your conversion
to direct writes on the fd fixes the bug part.

What you have on 'pu' looks good to me.

/me wasn't aware of the meaning of the pu branch :)

-apw

Re: [PATCH 2/2] Teach rev-list an option to read revs from the standard input.

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

> There was white space breakage in the patch which I noticed late in the
> game, and a problem with the printing lacking newlines.  Your conversion
> to direct writes on the fd fixes the bug part.
>
> What you have on 'pu' looks good to me.
>
> /me wasn't aware of the meaning of the pu branch :)

Thanks.  Will merge into "next" with a plan to have it graduate
to master by next week.