From 86ab4906a7c86719cae33b28dac1a4157d665867 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Mon, 5 Mar 2007 13:10:06 -0800
Subject: [PATCH 01/13] revision walker: Fix --boundary when limited

This cleans up the boundary processing in the commit walker.  It

 - rips out the boundary logic from the commit walker. Placing
   "negative" commits in the revs->commits list was Ok if all we
   cared about "boundary" was the UNINTERESTING limiting case,
   but conceptually it was wrong.

 - makes get_revision_1() function to walk the commits and return
   the results as if there is no funny postprocessing flags such
   as --reverse, --skip nor --max-count.

 - makes get_revision() function the postprocessing phase:

   If reverse is given, wait for get_revision_1() to give
   everything that it would normally give, and then reverse it
   before consuming.

   If skip is given, skip that many before going further.

   If max is given, stop when we gave out that many.

   Now that we are about to return one positive commit, mark
   the parents of that commit to be potential boundaries
   before returning, iff we are doing the boundary processing.

   Return the commit.

 - After get_revision() finishes giving out all the positive
   commits, if we are doing the boundary processing, we look at
   the parents that we marked as potential boundaries earlier,
   see if they are really boundaries, and give them out.

It loses more code than it adds, even when the new gc_boundary()
function, which is purely for early optimization, is counted.

Note that this patch is purely for eyeballing and discussion
only.  It breaks git-bundle's verify logic because the logic
does not use BOUNDARY_SHOW flag for its internal computation
anymore.  After we correct it not to attempt to affect the
boundary processing by setting the BOUNDARY_SHOW flag, we can
remove BOUNDARY_SHOW from revision.h and use that bit assignment
for the new CHILD_SHOWN flag.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 revision.c | 206 ++++++++++++++++++++++++-----------------------------
 revision.h |   8 ++-
 2 files changed, 99 insertions(+), 115 deletions(-)

diff --git a/revision.c b/revision.c
index f5b8ae4f03..5d137ea14a 100644
--- a/revision.c
+++ b/revision.c
@@ -437,36 +437,6 @@ static void limit_list(struct rev_info *revs)
 			continue;
 		p = &commit_list_insert(commit, p)->next;
 	}
-	if (revs->boundary) {
-		/* mark the ones that are on the result list first */
-		for (list = newlist; list; list = list->next) {
-			struct commit *commit = list->item;
-			commit->object.flags |= TMP_MARK;
-		}
-		for (list = newlist; list; list = list->next) {
-			struct commit *commit = list->item;
-			struct object *obj = &commit->object;
-			struct commit_list *parent;
-			if (obj->flags & UNINTERESTING)
-				continue;
-			for (parent = commit->parents;
-			     parent;
-			     parent = parent->next) {
-				struct commit *pcommit = parent->item;
-				if (!(pcommit->object.flags & UNINTERESTING))
-					continue;
-				pcommit->object.flags |= BOUNDARY;
-				if (pcommit->object.flags & TMP_MARK)
-					continue;
-				pcommit->object.flags |= TMP_MARK;
-				p = &commit_list_insert(pcommit, p)->next;
-			}
-		}
-		for (list = newlist; list; list = list->next) {
-			struct commit *commit = list->item;
-			commit->object.flags &= ~TMP_MARK;
-		}
-	}
 	revs->commits = newlist;
 }
 
@@ -1193,17 +1163,6 @@ static void rewrite_parents(struct rev_info *revs, struct commit *commit)
 	}
 }
 
-static void mark_boundary_to_show(struct commit *commit)
-{
-	struct commit_list *p = commit->parents;
-	while (p) {
-		commit = p->item;
-		p = p->next;
-		if (commit->object.flags & BOUNDARY)
-			commit->object.flags |= BOUNDARY_SHOW;
-	}
-}
-
 static int commit_match(struct commit *commit, struct rev_info *opt)
 {
 	if (!opt->grep_filter)
@@ -1235,15 +1194,9 @@ static struct commit *get_revision_1(struct rev_info *revs)
 		 */
 		if (!revs->limited) {
 			if (revs->max_age != -1 &&
-			    (commit->date < revs->max_age)) {
-				if (revs->boundary)
-					commit->object.flags |=
-						BOUNDARY_SHOW | BOUNDARY;
-				else
-					continue;
-			} else
-				add_parents_to_list(revs, commit,
-						&revs->commits);
+			    (commit->date < revs->max_age))
+				continue;
+			add_parents_to_list(revs, commit, &revs->commits);
 		}
 		if (commit->object.flags & SHOWN)
 			continue;
@@ -1252,18 +1205,6 @@ static struct commit *get_revision_1(struct rev_info *revs)
 						    revs->ignore_packed))
 		    continue;
 
-		/* We want to show boundary commits only when their
-		 * children are shown.  When path-limiter is in effect,
-		 * rewrite_parents() drops some commits from getting shown,
-		 * and there is no point showing boundary parents that
-		 * are not shown.  After rewrite_parents() rewrites the
-		 * parents of a commit that is shown, we mark the boundary
-		 * parents with BOUNDARY_SHOW.
-		 */
-		if (commit->object.flags & BOUNDARY_SHOW) {
-			commit->object.flags |= SHOWN;
-			return commit;
-		}
 		if (commit->object.flags & UNINTERESTING)
 			continue;
 		if (revs->min_age != -1 && (commit->date > revs->min_age))
@@ -1286,80 +1227,119 @@ static struct commit *get_revision_1(struct rev_info *revs)
 			if (revs->parents)
 				rewrite_parents(revs, commit);
 		}
-		if (revs->boundary)
-			mark_boundary_to_show(commit);
 		commit->object.flags |= SHOWN;
 		return commit;
 	} while (revs->commits);
 	return NULL;
 }
 
+static void gc_boundary(struct object_array *array)
+{
+	unsigned nr = array->nr;
+	unsigned alloc = array->alloc;
+	struct object_array_entry *objects = array->objects;
+
+	if (alloc <= nr) {
+		unsigned i, j;
+		for (i = j = 0; i < nr; i++) {
+			if (objects[i].item->flags & SHOWN)
+				continue;
+			if (i != j)
+				objects[j] = objects[i];
+			j++;
+		}
+		for (i = j; j < nr; j++)
+			objects[i].item = NULL;
+		array->nr = j;
+	}
+}
+
 struct commit *get_revision(struct rev_info *revs)
 {
 	struct commit *c = NULL;
+	struct commit_list *l;
 
-	if (revs->reverse) {
-		struct commit_list *list;
-
-		/*
-		 * rev_info.reverse is used to note the fact that we
-		 * want to output the list of revisions in reverse
-		 * order.  To accomplish this goal, reverse can have
-		 * different values:
-		 *
-		 *  0  do nothing
-		 *  1  reverse the list
-		 *  2  internal use:  we have already obtained and
-		 *     reversed the list, now we only need to yield
-		 *     its items.
-		 */
-
-		if (revs->reverse == 1) {
-			revs->reverse = 0;
-			list = NULL;
-			while ((c = get_revision(revs)))
-				commit_list_insert(c, &list);
-			revs->commits = list;
-			revs->reverse = 2;
+	if (revs->boundary == 2) {
+		unsigned i;
+		struct object_array *array = &revs->boundary_commits;
+		struct object_array_entry *objects = array->objects;
+		for (i = 0; i < array->nr; i++) {
+			c = (struct commit *)(objects[i].item);
+			if (!c)
+				continue;
+			if (!(c->object.flags & CHILD_SHOWN))
+				continue;
+			if (!(c->object.flags & SHOWN))
+				break;
 		}
-
-		if (!revs->commits)
+		if (array->nr <= i)
 			return NULL;
-		c = revs->commits->item;
-		list = revs->commits->next;
-		free(revs->commits);
-		revs->commits = list;
+
+		c->object.flags |= SHOWN | BOUNDARY;
 		return c;
 	}
 
-	if (0 < revs->skip_count) {
-		while ((c = get_revision_1(revs)) != NULL) {
-			if (revs->skip_count-- <= 0)
-				break;
-		}
+	if (revs->reverse) {
+		l = NULL;
+		while ((c = get_revision_1(revs)))
+			commit_list_insert(c, &l);
+		revs->commits = l;
+		revs->reverse = 0;
 	}
 
-	/* Check the max_count ... */
+	/*
+	 * Now pick up what they want to give us
+	 */
+	c = get_revision_1(revs);
+	while (0 < revs->skip_count) {
+		revs->skip_count--;
+		c = get_revision_1(revs);
+		if (!c)
+			break;
+	}
+
+	/*
+	 * Check the max_count.
+	 */
 	switch (revs->max_count) {
 	case -1:
 		break;
 	case 0:
-		if (revs->boundary) {
-			struct commit_list *list = revs->commits;
-			while (list) {
-				list->item->object.flags |=
-					BOUNDARY_SHOW | BOUNDARY;
-				list = list->next;
-			}
-			/* all remaining commits are boundary commits */
-			revs->max_count = -1;
-			revs->limited = 1;
-		} else
-			return NULL;
+		c = NULL;
+		break;
 	default:
 		revs->max_count--;
 	}
-	if (c)
+
+	if (!revs->boundary)
 		return c;
-	return get_revision_1(revs);
+
+	if (!c) {
+		/*
+		 * get_revision_1() runs out the commits, and
+		 * we are done computing the boundaries.
+		 * switch to boundary commits output mode.
+		 */
+		revs->boundary = 2;
+		return get_revision(revs);
+	}
+
+	/*
+	 * boundary commits are the commits that are parents of the
+	 * ones we got from get_revision_1() but they themselves are
+	 * not returned from get_revision_1().  Before returning
+	 * 'c', we need to mark its parents that they could be boundaries.
+	 */
+
+	for (l = c->parents; l; l = l->next) {
+		struct object *p;
+		p = &(l->item->object);
+		if (p->flags & CHILD_SHOWN)
+			continue;
+		p->flags |= CHILD_SHOWN;
+		gc_boundary(&revs->boundary_commits);
+		add_object_array(p, NULL, &revs->boundary_commits);
+	}
+
+	return c;
 }
diff --git a/revision.h b/revision.h
index 5fec1846f3..6579a446ea 100644
--- a/revision.h
+++ b/revision.h
@@ -10,6 +10,7 @@
 #define BOUNDARY_SHOW	(1u<<6)
 #define ADDED		(1u<<7)	/* Parents already parsed and added? */
 #define SYMMETRIC_LEFT	(1u<<8)
+#define CHILD_SHOWN	(1u<<9)
 
 struct rev_info;
 struct log_info;
@@ -21,6 +22,9 @@ struct rev_info {
 	struct commit_list *commits;
 	struct object_array pending;
 
+	/* Parents of shown commits */
+	struct object_array boundary_commits;
+
 	/* Basic information */
 	const char *prefix;
 	void *prune_data;
@@ -40,10 +44,10 @@ struct rev_info {
 			edge_hint:1,
 			limited:1,
 			unpacked:1, /* see also ignore_packed below */
-			boundary:1,
+			boundary:2,
 			left_right:1,
 			parents:1,
-			reverse:2;
+			reverse:1;
 
 	/* Diff flags */
 	unsigned int	diff:1,

From 2b064697a5b0610254f52523ce5b70c81118da80 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Mon, 5 Mar 2007 16:10:28 -0800
Subject: [PATCH 02/13] revision traversal: retire BOUNDARY_SHOW

This removes the flag internally used by revision traversal to
decide which commits are indeed boundaries and renames it to
CHILD_SHOWN.  builtin-bundle uses the symbol for its
verification, but I think the logic it uses it is wrong.  The
flag is still useful but it is local to the git-bundle, so it is
renamed to PREREQ_MARK.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-bundle.c | 6 ++++--
 revision.c       | 9 ++++++++-
 revision.h       | 3 +--
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 279b8f8e58..76bd204de8 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -160,6 +160,8 @@ static int fork_with_pipe(const char **argv, int *in, int *out)
 	return pid;
 }
 
+#define PREREQ_MARK (1u<<16)
+
 static int verify_bundle(struct bundle_header *header)
 {
 	/*
@@ -179,7 +181,7 @@ static int verify_bundle(struct bundle_header *header)
 		struct ref_list_entry *e = p->list + i;
 		struct object *o = parse_object(e->sha1);
 		if (o) {
-			o->flags |= BOUNDARY_SHOW;
+			o->flags |= PREREQ_MARK;
 			add_pending_object(&revs, o, e->name);
 			continue;
 		}
@@ -202,7 +204,7 @@ static int verify_bundle(struct bundle_header *header)
 
 	i = req_nr;
 	while (i && (commit = get_revision(&revs)))
-		if (commit->object.flags & BOUNDARY_SHOW)
+		if (commit->object.flags & PREREQ_MARK)
 			i--;
 
 	for (i = 0; i < req_nr; i++)
diff --git a/revision.c b/revision.c
index 5d137ea14a..2d27ccf70b 100644
--- a/revision.c
+++ b/revision.c
@@ -1285,17 +1285,21 @@ struct commit *get_revision(struct rev_info *revs)
 			commit_list_insert(c, &l);
 		revs->commits = l;
 		revs->reverse = 0;
+		c = NULL;
 	}
 
 	/*
 	 * Now pick up what they want to give us
 	 */
-	c = get_revision_1(revs);
+	if (!(c = get_revision_1(revs)))
+		return NULL;
 	while (0 < revs->skip_count) {
 		revs->skip_count--;
 		c = get_revision_1(revs);
 		if (!c)
 			break;
+		/* Although we grabbed it, it is not shown. */
+		c->object.flags &= ~SHOWN;
 	}
 
 	/*
@@ -1305,6 +1309,9 @@ struct commit *get_revision(struct rev_info *revs)
 	case -1:
 		break;
 	case 0:
+		/* Although we grabbed it, it is not shown. */
+		if (c)
+			c->object.flags &= ~SHOWN;
 		c = NULL;
 		break;
 	default:
diff --git a/revision.h b/revision.h
index 6579a446ea..1885f8d233 100644
--- a/revision.h
+++ b/revision.h
@@ -7,10 +7,9 @@
 #define SHOWN		(1u<<3)
 #define TMP_MARK	(1u<<4) /* for isolated cases; clean after use */
 #define BOUNDARY	(1u<<5)
-#define BOUNDARY_SHOW	(1u<<6)
+#define CHILD_SHOWN	(1u<<6)
 #define ADDED		(1u<<7)	/* Parents already parsed and added? */
 #define SYMMETRIC_LEFT	(1u<<8)
-#define CHILD_SHOWN	(1u<<9)
 
 struct rev_info;
 struct log_info;

From c2dea5a11cc7106d0beec9931e527467b7189ab3 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Mon, 5 Mar 2007 16:16:32 -0800
Subject: [PATCH 03/13] git-bundle: various fixups

verify_bundle() returned with an error early only when all
prerequisite commits were missing.  It should error out much
earlier when some are missing.

When the rev-list is limited in ways other than revision range
(e.g. --max-count or --max-age), create_bundle() listed all
positive refs given from the command line as if they are
available, but resulting pack may not have some of them.  Add a
logic to make sure all of them are included, and error out
otherwise.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-bundle.c | 46 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 76bd204de8..d4dd57d159 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -189,7 +189,7 @@ static int verify_bundle(struct bundle_header *header)
 			error(message);
 		error("%s %s", sha1_to_hex(e->sha1), e->name);
 	}
-	if (revs.pending.nr == 0)
+	if (revs.pending.nr != p->nr)
 		return ret;
 	req_nr = revs.pending.nr;
 	setup_revisions(2, argv, &revs, NULL);
@@ -279,6 +279,7 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	int pid, in, out, i, status;
 	char buffer[1024];
 	struct rev_info revs;
+	struct object_array tips;
 
 	bundle_fd = (!strcmp(path, "-") ? 1 :
 			open(path, O_CREAT | O_WRONLY, 0666));
@@ -316,19 +317,23 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	argc = setup_revisions(argc, argv, &revs, NULL);
 	if (argc > 1)
 		return error("unrecognized argument: %s'", argv[1]);
+
+	memset(&tips, 0, sizeof(tips));
 	for (i = 0; i < revs.pending.nr; i++) {
 		struct object_array_entry *e = revs.pending.objects + i;
-		if (!(e->item->flags & UNINTERESTING)) {
-			unsigned char sha1[20];
-			char *ref;
-			if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)
-				continue;
-			write_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);
-			write_or_die(bundle_fd, " ", 1);
-			write_or_die(bundle_fd, ref, strlen(ref));
-			write_or_die(bundle_fd, "\n", 1);
-			free(ref);
-		}
+		unsigned char sha1[20];
+		char *ref;
+
+		if (e->item->flags & UNINTERESTING)
+			continue;
+		if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)
+			continue;
+		write_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);
+		write_or_die(bundle_fd, " ", 1);
+		write_or_die(bundle_fd, ref, strlen(ref));
+		write_or_die(bundle_fd, "\n", 1);
+		add_object_array(e->item, e->name, &tips);
+		free(ref);
 	}
 
 	/* end header */
@@ -356,7 +361,22 @@ static int create_bundle(struct bundle_header *header, const char *path,
 			return -1;
 	if (!WIFEXITED(status) || WEXITSTATUS(status))
 		return error ("pack-objects died");
-	return 0;
+
+	/*
+	 * Make sure the refs we wrote out is correct; --max-count and
+	 * other limiting options could have prevented all the tips
+	 * from getting output.
+	 */
+	status = 0;
+	for (i = 0; i < tips.nr; i++) {
+		if (!(tips.objects[i].item->flags & SHOWN)) {
+			status = 1;
+			error("%s: not included in the resulting pack",
+			      tips.objects[i].name);
+		}
+	}
+
+	return status;
 }
 
 static int unbundle(struct bundle_header *header, int bundle_fd,

From c33d8593851ef1b3b59fc9b8022019cb955e8d59 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Mon, 5 Mar 2007 18:23:57 -0800
Subject: [PATCH 04/13] revision traversal: SHOWN means shown

This moves the code to set SHOWN on the commit from get_revision_1()
back to get_revision(), so that the bit means what it originally
meant: this commit has been given back to the caller.

Also it fixes the --reverse breakage Dscho pointed out.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 revision.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/revision.c b/revision.c
index 2d27ccf70b..35a171105a 100644
--- a/revision.c
+++ b/revision.c
@@ -1227,7 +1227,6 @@ static struct commit *get_revision_1(struct rev_info *revs)
 			if (revs->parents)
 				rewrite_parents(revs, commit);
 		}
-		commit->object.flags |= SHOWN;
 		return commit;
 	} while (revs->commits);
 	return NULL;
@@ -1280,11 +1279,22 @@ struct commit *get_revision(struct rev_info *revs)
 	}
 
 	if (revs->reverse) {
+		int limit = -1;
+
+		if (0 <= revs->max_count) {
+			limit = revs->max_count;
+			if (0 < revs->skip_count)
+				limit += revs->skip_count;
+		}
 		l = NULL;
-		while ((c = get_revision_1(revs)))
+		while ((c = get_revision_1(revs))) {
 			commit_list_insert(c, &l);
+			if ((0 < limit) && !--limit)
+				break;
+		}
 		revs->commits = l;
 		revs->reverse = 0;
+		revs->max_count = -1;
 		c = NULL;
 	}
 
@@ -1298,8 +1308,6 @@ struct commit *get_revision(struct rev_info *revs)
 		c = get_revision_1(revs);
 		if (!c)
 			break;
-		/* Although we grabbed it, it is not shown. */
-		c->object.flags &= ~SHOWN;
 	}
 
 	/*
@@ -1310,16 +1318,18 @@ struct commit *get_revision(struct rev_info *revs)
 		break;
 	case 0:
 		/* Although we grabbed it, it is not shown. */
-		if (c)
-			c->object.flags &= ~SHOWN;
 		c = NULL;
 		break;
 	default:
 		revs->max_count--;
 	}
 
-	if (!revs->boundary)
+	if (c)
+		c->object.flags |= SHOWN;
+
+	if (!revs->boundary) {
 		return c;
+	}
 
 	if (!c) {
 		/*
@@ -1341,7 +1351,7 @@ struct commit *get_revision(struct rev_info *revs)
 	for (l = c->parents; l; l = l->next) {
 		struct object *p;
 		p = &(l->item->object);
-		if (p->flags & CHILD_SHOWN)
+		if (p->flags & (CHILD_SHOWN | SHOWN))
 			continue;
 		p->flags |= CHILD_SHOWN;
 		gc_boundary(&revs->boundary_commits);

From 80e25ceece17fd575735c40dc923086154402cbe Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Mon, 5 Mar 2007 16:17:27 -0800
Subject: [PATCH 05/13] git-bundle: make verify a bit more chatty.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-bundle.c | 55 ++++++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index d4dd57d159..d0c361763c 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -160,9 +160,28 @@ static int fork_with_pipe(const char **argv, int *in, int *out)
 	return pid;
 }
 
+static int list_refs(struct ref_list *r, int argc, const char **argv)
+{
+	int i;
+
+	for (i = 0; i < r->nr; i++) {
+		if (argc > 1) {
+			int j;
+			for (j = 1; j < argc; j++)
+				if (!strcmp(r->list[i].name, argv[j]))
+					break;
+			if (j == argc)
+				continue;
+		}
+		printf("%s %s\n", sha1_to_hex(r->list[i].sha1),
+				r->list[i].name);
+	}
+	return 0;
+}
+
 #define PREREQ_MARK (1u<<16)
 
-static int verify_bundle(struct bundle_header *header)
+static int verify_bundle(struct bundle_header *header, int verbose)
 {
 	/*
 	 * Do fast check, then if any prereqs are missing then go line by line
@@ -218,27 +237,24 @@ static int verify_bundle(struct bundle_header *header)
 	for (i = 0; i < refs.nr; i++)
 		clear_commit_marks((struct commit *)refs.objects[i].item, -1);
 
+	if (verbose) {
+		struct ref_list *r;
+
+		r = &header->references;
+		printf("The bundle contains %d ref%s\n",
+		       r->nr, (1 < r->nr) ? "s" : "");
+		list_refs(r, 0, NULL);
+		r = &header->prerequisites;
+		printf("The bundle requires these %d ref%s\n",
+		       r->nr, (1 < r->nr) ? "s" : "");
+		list_refs(r, 0, NULL);
+	}
 	return ret;
 }
 
 static int list_heads(struct bundle_header *header, int argc, const char **argv)
 {
-	int i;
-	struct ref_list *r = &header->references;
-
-	for (i = 0; i < r->nr; i++) {
-		if (argc > 1) {
-			int j;
-			for (j = 1; j < argc; j++)
-				if (!strcmp(r->list[i].name, argv[j]))
-					break;
-			if (j == argc)
-				continue;
-		}
-		printf("%s %s\n", sha1_to_hex(r->list[i].sha1),
-				r->list[i].name);
-	}
-	return 0;
+	return list_refs(&header->references, argc, argv);
 }
 
 static void show_commit(struct commit *commit)
@@ -385,7 +401,7 @@ static int unbundle(struct bundle_header *header, int bundle_fd,
 	const char *argv_index_pack[] = {"index-pack", "--stdin", NULL};
 	int pid, status, dev_null;
 
-	if (verify_bundle(header))
+	if (verify_bundle(header, 0))
 		return -1;
 	dev_null = open("/dev/null", O_WRONLY);
 	pid = fork_with_pipe(argv_index_pack, &bundle_fd, &dev_null);
@@ -429,7 +445,7 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)
 
 	if (!strcmp(cmd, "verify")) {
 		close(bundle_fd);
-		if (verify_bundle(&header))
+		if (verify_bundle(&header, 1))
 			return 1;
 		fprintf(stderr, "%s is okay\n", bundle_file);
 		return 0;
@@ -449,4 +465,3 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)
 	} else
 		usage(bundle_usage);
 }
-

From 892ae6bf13d3811f36e6fe65c4ff85841e1c4f14 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Tue, 6 Mar 2007 03:00:18 -0800
Subject: [PATCH 06/13] revision --boundary: fix stupid typo

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 revision.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index 35a171105a..f48d7f788a 100644
--- a/revision.c
+++ b/revision.c
@@ -1247,7 +1247,7 @@ static void gc_boundary(struct object_array *array)
 				objects[j] = objects[i];
 			j++;
 		}
-		for (i = j; j < nr; j++)
+		for (i = j; i < nr; i++)
 			objects[i].item = NULL;
 		array->nr = j;
 	}

From 8839ac9442c9ded41bfa369a142fd2e659a44377 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Tue, 6 Mar 2007 03:20:55 -0800
Subject: [PATCH 07/13] revision --boundary: fix uncounted case.

When the list is truly limited and get_revision_1() returned NULL,
the code incorrectly returned it without switching to boundary emiting
mode.  Silly.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 revision.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/revision.c b/revision.c
index f48d7f788a..3c2eb125e6 100644
--- a/revision.c
+++ b/revision.c
@@ -1301,13 +1301,14 @@ struct commit *get_revision(struct rev_info *revs)
 	/*
 	 * Now pick up what they want to give us
 	 */
-	if (!(c = get_revision_1(revs)))
-		return NULL;
-	while (0 < revs->skip_count) {
-		revs->skip_count--;
-		c = get_revision_1(revs);
-		if (!c)
-			break;
+	c = get_revision_1(revs);
+	if (c) {
+		while (0 < revs->skip_count) {
+			revs->skip_count--;
+			c = get_revision_1(revs);
+			if (!c)
+				break;
+		}
 	}
 
 	/*
@@ -1317,7 +1318,6 @@ struct commit *get_revision(struct rev_info *revs)
 	case -1:
 		break;
 	case 0:
-		/* Although we grabbed it, it is not shown. */
 		c = NULL;
 		break;
 	default:

From 8315588b59bea45c4b82451cc3ac337fa5c68526 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Tue, 6 Mar 2007 22:57:07 +0100
Subject: [PATCH 08/13] bundle: fix wrong check of read_header()'s return value
 & add tests

If read_header() fails, it returns <0, not 0. Further, an open(/dev/null)
was not checked for errors.

Also, this adds two tests to make sure that the bundle file looks
correct, by checking if it has the header has the expected form, and that
the pack contains the right amount of objects.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-bundle.c |  4 +++-
 t/t5510-fetch.sh | 21 +++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index d0c361763c..3b3bc2582d 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -404,6 +404,8 @@ static int unbundle(struct bundle_header *header, int bundle_fd,
 	if (verify_bundle(header, 0))
 		return -1;
 	dev_null = open("/dev/null", O_WRONLY);
+	if (dev_null < 0)
+		return error("Could not open /dev/null");
 	pid = fork_with_pipe(argv_index_pack, &bundle_fd, &dev_null);
 	if (pid < 0)
 		return error("Could not spawn index-pack");
@@ -440,7 +442,7 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)
 
 	memset(&header, 0, sizeof(header));
 	if (strcmp(cmd, "create") &&
-			!(bundle_fd = read_header(bundle_file, &header)))
+			(bundle_fd = read_header(bundle_file, &header)) < 0)
 		return 1;
 
 	if (!strcmp(cmd, "verify")) {
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index fa76662dce..ad589dd0df 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -90,6 +90,13 @@ test_expect_success 'create bundle 1' '
 	git bundle create bundle1 master^..master
 '
 
+test_expect_success 'header of bundle looks right' '
+	head -n 1 "$D"/bundle1 | grep "^#" &&
+	head -n 2 "$D"/bundle1 | grep "^-[0-9a-f]\{40\} " &&
+	head -n 3 "$D"/bundle1 | grep "^[0-9a-f]\{40\} " &&
+	head -n 4 "$D"/bundle1 | grep "^$"
+'
+
 test_expect_success 'create bundle 2' '
 	cd "$D" &&
 	git bundle create bundle2 master~2..master
@@ -101,6 +108,20 @@ test_expect_failure 'unbundle 1' '
 	git fetch "$D/bundle1" master:master
 '
 
+test_expect_success 'bundle 1 has only 3 files ' '
+	cd "$D" &&
+	(
+		while read x && test -n "$x"
+		do
+			:;
+		done
+		cat
+	) <bundle1 >bundle.pack &&
+	git index-pack bundle.pack &&
+	verify=$(git verify-pack -v bundle.pack) &&
+	test 4 = $(echo "$verify" | wc -l)
+'
+
 test_expect_success 'unbundle 2' '
 	cd "$D/bundle" &&
 	git fetch ../bundle2 master:master &&

From 18449ab0e9754ae04ae2f232449241ac361f3db4 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Thu, 8 Mar 2007 00:43:05 +0100
Subject: [PATCH 09/13] git-bundle: avoid packing objects which are in the
 prerequisites

When saying something like "--since=1.day.ago" or "--max-count=5",
git-bundle finds the boundary commits which are recorded as
prerequisites. However, it failed to tell pack-objects _not_ to
pack the objects which are in these.

Fix that. And add a test for that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-bundle.c | 15 ++++++++++++---
 t/t5510-fetch.sh | 11 +++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 3b3bc2582d..70d4479193 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -305,6 +305,10 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	/* write signature */
 	write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
 
+	/* init revs to list objects for pack-objects later */
+	save_commit_buffer = 0;
+	init_revisions(&revs, NULL);
+
 	/* write prerequisites */
 	memcpy(argv_boundary + 3, argv + 1, argc * sizeof(const char *));
 	argv_boundary[0] = "rev-list";
@@ -316,8 +320,15 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	if (pid < 0)
 		return -1;
 	while ((i = read_string(out, buffer, sizeof(buffer))) > 0)
-		if (buffer[0] == '-')
+		if (buffer[0] == '-') {
+			unsigned char sha1[20];
 			write_or_die(bundle_fd, buffer, i);
+			if (!get_sha1_hex(buffer + 1, sha1)) {
+				struct object *object = parse_object(sha1);
+				object->flags |= UNINTERESTING;
+				add_pending_object(&revs, object, buffer);
+			}
+		}
 	while ((i = waitpid(pid, &status, 0)) < 0)
 		if (errno != EINTR)
 			return error("rev-list died");
@@ -325,8 +336,6 @@ static int create_bundle(struct bundle_header *header, const char *path,
 		return error("rev-list died %d", WEXITSTATUS(status));
 
 	/* write references */
-	save_commit_buffer = 0;
-	init_revisions(&revs, NULL);
 	revs.tag_objects = 1;
 	revs.tree_objects = 1;
 	revs.blob_objects = 1;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index ad589dd0df..ee3f397a9b 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -128,4 +128,15 @@ test_expect_success 'unbundle 2' '
 	test "tip" = "$(git log -1 --pretty=oneline master | cut -b42-)"
 '
 
+test_expect_success 'bundle does not prerequisite objects' '
+	cd "$D" &&
+	touch file2 &&
+	git add file2 &&
+	git commit -m add.file2 file2 &&
+	git bundle create bundle3 -1 HEAD &&
+	sed "1,4d" < bundle3 > bundle.pack &&
+	git index-pack bundle.pack &&
+	test 4 = $(git verify-pack -v bundle.pack | wc -l)
+'
+
 test_done

From 9e64d109f905afb225f48409c4e0e068b2203332 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Thu, 8 Mar 2007 01:27:44 +0100
Subject: [PATCH 10/13] git-bundle: Make thin packs

Thin packs are way smaller, but they rely on the receiving end to have the
base objects. However, Git's pack protocol also uses thin packs by
default. So make the packs contained in bundles thin, since bundles are
just another transport.

The patch looks a bit bigger than intended, mainly because --thin
_implies_ that pack-objects should run its own rev-list. Therefore, this
patch removes all the stuff we used to roll rev-list ourselves.

This commit also changes behaviour slightly: since we now know early
enough if a specified ref is _not_ contained in the pack, we can avoid
putting that ref into the pack. So, we don't die() here, but warn()
instead, and skip that ref.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-bundle.c | 85 +++++++++++++++---------------------------------
 1 file changed, 26 insertions(+), 59 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 70d4479193..6163358191 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -257,45 +257,15 @@ static int list_heads(struct bundle_header *header, int argc, const char **argv)
 	return list_refs(&header->references, argc, argv);
 }
 
-static void show_commit(struct commit *commit)
-{
-	write_or_die(1, sha1_to_hex(commit->object.sha1), 40);
-	write_or_die(1, "\n", 1);
-	if (commit->parents) {
-		free_commit_list(commit->parents);
-		commit->parents = 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');
-	int len = ep ? ep - p->name : strlen(p->name);
-	write_or_die(1, sha1_to_hex(p->item->sha1), 40);
-	write_or_die(1, " ", 1);
-	if (len)
-		write_or_die(1, p->name, len);
-	write_or_die(1, "\n", 1);
-}
-
-static void show_edge(struct commit *commit)
-{
-	; /* nothing to do */
-}
-
 static int create_bundle(struct bundle_header *header, const char *path,
 		int argc, const char **argv)
 {
 	int bundle_fd = -1;
 	const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
-	const char **argv_pack = xmalloc(4 * sizeof(const char *));
+	const char **argv_pack = xmalloc(5 * sizeof(const char *));
 	int pid, in, out, i, status;
 	char buffer[1024];
 	struct rev_info revs;
-	struct object_array tips;
 
 	bundle_fd = (!strcmp(path, "-") ? 1 :
 			open(path, O_CREAT | O_WRONLY, 0666));
@@ -319,16 +289,20 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	pid = fork_with_pipe(argv_boundary, NULL, &out);
 	if (pid < 0)
 		return -1;
-	while ((i = read_string(out, buffer, sizeof(buffer))) > 0)
+	while ((i = read_string(out, buffer, sizeof(buffer))) > 0) {
+		unsigned char sha1[20];
 		if (buffer[0] == '-') {
-			unsigned char sha1[20];
 			write_or_die(bundle_fd, buffer, i);
 			if (!get_sha1_hex(buffer + 1, sha1)) {
 				struct object *object = parse_object(sha1);
 				object->flags |= UNINTERESTING;
 				add_pending_object(&revs, object, buffer);
 			}
+		} else if (!get_sha1_hex(buffer, sha1)) {
+			struct object *object = parse_object(sha1);
+			object->flags |= SHOWN;
 		}
+	}
 	while ((i = waitpid(pid, &status, 0)) < 0)
 		if (errno != EINTR)
 			return error("rev-list died");
@@ -336,14 +310,10 @@ static int create_bundle(struct bundle_header *header, const char *path,
 		return error("rev-list died %d", WEXITSTATUS(status));
 
 	/* write references */
-	revs.tag_objects = 1;
-	revs.tree_objects = 1;
-	revs.blob_objects = 1;
 	argc = setup_revisions(argc, argv, &revs, NULL);
 	if (argc > 1)
 		return error("unrecognized argument: %s'", argv[1]);
 
-	memset(&tips, 0, sizeof(tips));
 	for (i = 0; i < revs.pending.nr; i++) {
 		struct object_array_entry *e = revs.pending.objects + i;
 		unsigned char sha1[20];
@@ -353,11 +323,20 @@ static int create_bundle(struct bundle_header *header, const char *path,
 			continue;
 		if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)
 			continue;
+		/*
+		 * Make sure the refs we wrote out is correct; --max-count and
+		 * other limiting options could have prevented all the tips
+		 * from getting output.
+		 */
+		if (!(e->item->flags & SHOWN)) {
+			warn("ref '%s' is excluded by the rev-list options",
+				e->name);
+			continue;
+		}
 		write_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);
 		write_or_die(bundle_fd, " ", 1);
 		write_or_die(bundle_fd, ref, strlen(ref));
 		write_or_die(bundle_fd, "\n", 1);
-		add_object_array(e->item, e->name, &tips);
 		free(ref);
 	}
 
@@ -368,39 +347,27 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	argv_pack[0] = "pack-objects";
 	argv_pack[1] = "--all-progress";
 	argv_pack[2] = "--stdout";
-	argv_pack[3] = NULL;
+	argv_pack[3] = "--thin";
+	argv_pack[4] = NULL;
 	in = -1;
 	out = bundle_fd;
 	pid = fork_with_pipe(argv_pack, &in, &out);
 	if (pid < 0)
 		return error("Could not spawn pack-objects");
-	close(1);
-	dup2(in, 1);
+	for (i = 0; i < revs.pending.nr; i++) {
+		struct object *object = revs.pending.objects[i].item;
+		if (object->flags & UNINTERESTING)
+			write(in, "^", 1);
+		write(in, sha1_to_hex(object->sha1), 40);
+		write(in, "\n", 1);
+	}
 	close(in);
-	prepare_revision_walk(&revs);
-	mark_edges_uninteresting(revs.commits, &revs, show_edge);
-	traverse_commit_list(&revs, show_commit, show_object);
-	close(1);
 	while (waitpid(pid, &status, 0) < 0)
 		if (errno != EINTR)
 			return -1;
 	if (!WIFEXITED(status) || WEXITSTATUS(status))
 		return error ("pack-objects died");
 
-	/*
-	 * Make sure the refs we wrote out is correct; --max-count and
-	 * other limiting options could have prevented all the tips
-	 * from getting output.
-	 */
-	status = 0;
-	for (i = 0; i < tips.nr; i++) {
-		if (!(tips.objects[i].item->flags & SHOWN)) {
-			status = 1;
-			error("%s: not included in the resulting pack",
-			      tips.objects[i].name);
-		}
-	}
-
 	return status;
 }
 

From 263703fff38d252907d1c7ae9977038715e2e22f Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Fri, 9 Mar 2007 03:48:27 +0100
Subject: [PATCH 11/13] git-bundle: handle thin packs in subcommand "unbundle"

The patch to make the packs in a bundle thin forgot the receiving side.
D'oh.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-bundle.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 6163358191..33b533f821 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -374,7 +374,8 @@ static int create_bundle(struct bundle_header *header, const char *path,
 static int unbundle(struct bundle_header *header, int bundle_fd,
 		int argc, const char **argv)
 {
-	const char *argv_index_pack[] = {"index-pack", "--stdin", NULL};
+	const char *argv_index_pack[] = {"index-pack",
+		"--fix-thin", "--stdin", NULL};
 	int pid, status, dev_null;
 
 	if (verify_bundle(header, 0))

From d58c6184e345b9c8c8bfe8cc3eb1bbfe2f5ee4f9 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Fri, 9 Mar 2007 03:48:46 +0100
Subject: [PATCH 12/13] git-bundle: die if a given ref is not included in
 bundle

The earlier patch tried to be nice by just warning, but it seems
more likely that the user wants to adjust the parameters.

Also, it prevents a bundle containing _all_ revisions in the case
when the user only gave one ref, but also rev-list options which
excluded the ref.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-bundle.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 33b533f821..ca3de60e44 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -328,11 +328,9 @@ static int create_bundle(struct bundle_header *header, const char *path,
 		 * other limiting options could have prevented all the tips
 		 * from getting output.
 		 */
-		if (!(e->item->flags & SHOWN)) {
-			warn("ref '%s' is excluded by the rev-list options",
+		if (!(e->item->flags & SHOWN))
+			die("ref '%s' is excluded by the rev-list options",
 				e->name);
-			continue;
-		}
 		write_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);
 		write_or_die(bundle_fd, " ", 1);
 		write_or_die(bundle_fd, ref, strlen(ref));

From 2e578f9a4f08ade4e8da52614c566e5dc1c8ca00 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Fri, 9 Mar 2007 03:50:06 +0100
Subject: [PATCH 13/13] git-bundle: prevent overwriting existing bundles

Not only does it prevent accidentally losing older bundles, but it
also fixes a subtle bug: when writing into an existing bundle,
git-pack-objects would not truncate the bundle. Therefore,
fetching from the bundle would trigger an error in unpack-objects:
"fatal: pack has junk at the end".

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-bundle.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index ca3de60e44..55f6d0abcf 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -268,9 +268,9 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	struct rev_info revs;
 
 	bundle_fd = (!strcmp(path, "-") ? 1 :
-			open(path, O_CREAT | O_WRONLY, 0666));
+			open(path, O_CREAT | O_EXCL | O_WRONLY, 0666));
 	if (bundle_fd < 0)
-		return error("Could not write to '%s'", path);
+		return error("Could not create '%s': %s", path, strerror(errno));
 
 	/* write signature */
 	write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));