Re: WARNING: THIS PATCH CAN BREAK YOUR REPO, was Re: [PATCH 2/3] Only repack active packs by skipping over kept packs.

From: Jan Harkes <jaharkes@cs.cmu.edu>
Date: Oct 30, 2006, 20:52 UTC
Message-ID: <20061030205200.GA20236@delft.aura.cs.cmu.edu>
In-Reply-To: <20061030202611.GA5775@spearce.org>

On Mon, Oct 30, 2006 at 03:26:11PM -0500, Shawn Pearce wrote:

Show 12 quoted lines

> Actually the breakage is easier to reproduce without trashing
> a repository.
> 
> Do the above so you have everything in one pack.  Now use rev-list
> to simulate the object list construction in pack-objects as though
> we were doing a 'git repack -a -d':
> 
>   git-rev-list --objects --all \
>     --unpacked=.git/objects/pack/pack-*.pack \
> 	| wc -l
> 
> gives me 102 (WRONG WRONG WRONG WRONG!!!!!!)

The problem seems to be that as soon as we hit something that is found in a pack that is not on the ignore list, that object and all it's parents are marked as uninteresting. So if the kept pack contains a slice of commits (v1.4.3..v1.4.3.3) the revision walker will only return the recent stuff (v1.4.3.3..) and drop the older data (..v1.4.3).

The following patch does fix the problem Nicolas reported, but for some reason I'm still getting only 102 objects (only tags and the commits they refer to?) with your test.

Jan

----

diff --git a/revision.c b/revision.c
index 280e92b..a69c873 100644
--- a/revision.c
+++ b/revision.c
@@ -418,9 +418,6 @@ static void limit_list(struct rev_info *
 
 		if (revs->max_age != -1 && (commit->date < revs->max_age))
 			obj->flags |= UNINTERESTING;
-		if (revs->unpacked &&
-		    has_sha1_pack(obj->sha1, revs->ignore_packed))
-			obj->flags |= UNINTERESTING;
 		add_parents_to_list(revs, commit, &list);
 		if (obj->flags & UNINTERESTING) {
 			mark_parents_uninteresting(commit);
@@ -1149,17 +1146,18 @@ struct commit *get_revision(struct rev_i
 		 * that we'd otherwise have done in limit_list().
 		 */
 		if (!revs->limited) {
-			if ((revs->unpacked &&
-			     has_sha1_pack(commit->object.sha1,
-					   revs->ignore_packed)) ||
-			    (revs->max_age != -1 &&
-			     (commit->date < revs->max_age)))
+			if (revs->max_age != -1 &&
+			    (commit->date < revs->max_age))
 				continue;
 			add_parents_to_list(revs, commit, &revs->commits);
 		}
 		if (commit->object.flags & SHOWN)
 			continue;
 
+		if (revs->unpacked && has_sha1_pack(commit->object.sha1,
+						    revs->ignore_packed))
+		    continue;
+
 		/* We want to show boundary commits only when their
 		 * children are shown.  When path-limiter is in effect,