[PATCH 1/5] upload-pack: no longer call rev-list

[PATCH 1/5] upload-pack: no longer call rev-list

[Johannes Schindelin]

It is trivial to do now, and it is needed for the upcoming shallow
clone stuff.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 upload-pack.c |   93 ++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 60 insertions(+), 33 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index 4572fff..7f7df2a 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -9,6 +9,9 @@
 #include "object.h"
 #include "commit.h"
 #include "exec_cmd.h"
+#include "diff.h"
+#include "revision.h"
+#include "list-objects.h"
 
 static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>";
 
@@ -57,6 +60,40 @@ static ssize_t send_client_data(int fd,
 	return safe_write(fd, data, sz);
 }
 
+FILE *pack_pipe = NULL;
+static void show_commit(struct commit *commit)
+{
+	if (commit->object.flags & BOUNDARY)
+		fputc('-', pack_pipe);
+	if (fputs(sha1_to_hex(commit->object.sha1), pack_pipe) < 0)
+		die("broken output pipe");
+	fputc('\n', pack_pipe);
+	fflush(pack_pipe);
+	free(commit->buffer);
+	commit->buffer = NULL;
+}
+
+static void show_object(struct object_array_entry *p)
+{
+	/* An object with name "foo\n0000000..." can be used to
+	 * confuse downstream git-pack-objects very badly.
+	 */
+	const char *ep = strchr(p->name, '\n');
+	if (ep) {
+		fprintf(pack_pipe, "%s %.*s\n", sha1_to_hex(p->item->sha1),
+		       (int) (ep - p->name),
+		       p->name);
+	}
+	else
+		fprintf(pack_pipe, "%s %s\n",
+				sha1_to_hex(p->item->sha1), p->name);
+}
+
+static void show_edge(struct commit *commit)
+{
+	fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1));
+}
+
 static void create_pack_file(void)
 {
 	/* Pipes between rev-list to pack-objects, pack-objects to us
@@ -78,48 +115,38 @@ static void create_pack_file(void)
 
 	if (!pid_rev_list) {
 		int i;
-		int args;
-		const char **argv;
-		const char **p;
-		char *buf;
+		struct rev_info revs;
 
-		if (create_full_pack) {
-			args = 10;
-			use_thin_pack = 0; /* no point doing it */
-		}
-		else
-			args = have_obj.nr + want_obj.nr + 5;
-		p = xmalloc(args * sizeof(char *));
-		argv = (const char **) p;
-		buf = xmalloc(args * 45);
+		pack_pipe = fdopen(lp_pipe[1], "w");
 
-		dup2(lp_pipe[1], 1);
-		close(0);
-		close(lp_pipe[0]);
-		close(lp_pipe[1]);
-		*p++ = "rev-list";
-		*p++ = use_thin_pack ? "--objects-edge" : "--objects";
 		if (create_full_pack)
-			*p++ = "--all";
-		else {
+			use_thin_pack = 0; /* no point doing it */
+		init_revisions(&revs, NULL);
+		revs.tag_objects = 1;
+		revs.tree_objects = 1;
+		revs.blob_objects = 1;
+		if (use_thin_pack)
+			revs.edge_hint = 1;
+
+		if (create_full_pack) {
+			const char *args[] = {"rev-list", "--all", NULL};
+			setup_revisions(2, args, &revs, NULL);
+		} else {
 			for (i = 0; i < want_obj.nr; i++) {
 				struct object *o = want_obj.objects[i].item;
-				*p++ = buf;
-				memcpy(buf, sha1_to_hex(o->sha1), 41);
-				buf += 41;
+				add_pending_object(&revs, o, NULL);
 			}
-		}
-		if (!create_full_pack)
 			for (i = 0; i < have_obj.nr; i++) {
 				struct object *o = have_obj.objects[i].item;
-				*p++ = buf;
-				*buf++ = '^';
-				memcpy(buf, sha1_to_hex(o->sha1), 41);
-				buf += 41;
+				o->flags |= UNINTERESTING;
+				add_pending_object(&revs, o, NULL);
 			}
-		*p++ = NULL;
-		execv_git_cmd(argv);
-		die("git-upload-pack: unable to exec git-rev-list");
+			setup_revisions(0, NULL, &revs, NULL);
+		}
+		prepare_revision_walk(&revs);
+		mark_edges_uninteresting(revs.commits, &revs, show_edge);
+		traverse_commit_list(&revs, show_commit, show_object);
+		exit(0);
 	}
 
 	if (pipe(pu_pipe) < 0)
-- 
1.4.3.3.gca42

Re: [PATCH 1/5] upload-pack: no longer call rev-list

[Junio C Hamano]

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

> It is trivial to do now,...

May be, but can we do something about these duplicated code?

> @@ -57,6 +60,40 @@ static ssize_t send_client_data(int fd,
>  	return safe_write(fd, data, sz);
>  }
>  
> +FILE *pack_pipe = NULL;
> +static void show_commit(struct commit *commit)
> +{
> +	if (commit->object.flags & BOUNDARY)
> +		fputc('-', pack_pipe);
> +	if (fputs(sha1_to_hex(commit->object.sha1), pack_pipe) < 0)
> +		die("broken output pipe");
> +	fputc('\n', pack_pipe);
> +	fflush(pack_pipe);
> +	free(commit->buffer);
> +	commit->buffer = NULL;
> +}
> +
> +static void show_object(struct object_array_entry *p)
> +{
> +	/* An object with name "foo\n0000000..." can be used to
> +	 * confuse downstream git-pack-objects very badly.
> +	 */
> +	const char *ep = strchr(p->name, '\n');
> +	if (ep) {
> +		fprintf(pack_pipe, "%s %.*s\n", sha1_to_hex(p->item->sha1),
> +		       (int) (ep - p->name),
> +		       p->name);
> +	}
> +	else
> +		fprintf(pack_pipe, "%s %s\n",
> +				sha1_to_hex(p->item->sha1), p->name);
> +}
> +
> +static void show_edge(struct commit *commit)
> +{
> +	fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1));
> +}
> +

Re: [PATCH 1/5] upload-pack: no longer call rev-list

[Johannes Schindelin]

On Mon, 30 Oct 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > It is trivial to do now,...
> 
> May be, but can we do something about these duplicated code?
> 
> > @@ -57,6 +60,40 @@ static ssize_t send_client_data(int fd,
> >  	return safe_write(fd, data, sz);
> >  }
> >  
> > +FILE *pack_pipe = NULL;
> > +static void show_commit(struct commit *commit)
> > [...]

Yes, we could make mark_edges_uninteresting() and traverse_commit_list() 
take a FILE * parameter, which is then passed to the functions. Hmmm?

Except show_commit() should not show "commit " in front of each line. So, 
another parameter?

Ciao,
Dscho

Re: [PATCH 1/5] upload-pack: no longer call rev-list

[Johannes Schindelin]

Hi,

On Tue, 31 Oct 2006, Johannes Schindelin wrote:

> Except show_commit() should not show "commit " in front of each line. 
> So, another parameter?

Sorry, that was not clear. When using traverse_commit_list() from 
upload-pack, the "commit " prefix is _not_ shown. With every other 
user of travsere_commit_list(), it is.

Ciao,
Dscho