Re: path limiting broken

From: Linus Torvalds <torvalds@osdl.org>
Date: Apr 16, 2006, 16:06 UTC
Message-ID: <Pine.LNX.4.64.0604160850230.3701@g5.osdl.org>
In-Reply-To: <Pine.LNX.4.63.0604161411120.15345@wbgn013.biozentrum.uni-wuerzburg.de>

On Sun, 16 Apr 2006, Johannes Schindelin wrote:

Show 5 quoted lines

> 
> I am not intelligent enough to find out why there are three revisions 
> which get culled.
> 
> Ideas?

Path limiting by default looks at all parents of a merge, and if any of them is identical to the child (within the path limiter), it will pick _that_ parent, and that parent only, and drop all other parents.

(If there are multiple identical parents, it will pick the first one).

For example, if you do

		 HEAD
		   |
		   a
		  / \
		 b   c
		 |  /|
		 | / d
		 |/  |
		 e   f
		 |  /
		 g /
		 |/
		 h

and the file was modified in commit "b" and nowhere else, then think about what the final history should look like.. Since it was modified in "b", it's going to be the same in "a" and "b", so the path limiting will totally drop the whole "c" subtree, and will only ever look at the tree a->b->e->g->h.

After that, it will simplify the history by dropping commits that don't change the path, and the whole history will end up being just "b".

Which is exactly what you want.

Now, realize that if the exact _same_ change is done in "f", the same thing will happen. "f" will never be shown, because "f" simply isn't interesting. We picked up the same change in "b", and while the merge "a" had _both_ parents identical in the file, it would pick only the first one. So we'll never even _look_ at "f".

Now, this is _important_. Pruning merges is really fundamental to giving a reasonable file history, and not just a performance optimization. If the same change came from two branches, we really don't care who did it: we'll hit _one_ of the changes even after pruning. But yes, it could miss the other one.

Here's a patch to show what an unpruned history ends up looking like.

With this patch applied, try the following:

	gitk -- ls-tree.c &
	gitk --no-prune-merges -- ls-tree.c &

and compare the two. You'll see _immediately_ why I think merge pruning is not just a good idea, it's something we _have_ to do.

		Linus
----

diff --git a/revision.c b/revision.c
index 0505f3f..25338b6 100644
--- a/revision.c
+++ b/revision.c
@@ -291,7 +291,7 @@ static void try_to_simplify_commit(struc
 		parse_commit(p);
 		switch (rev_compare_tree(revs, p->tree, commit->tree)) {
 		case REV_TREE_SAME:
-			if (p->object.flags & UNINTERESTING) {
+			if (revs->no_merge_pruning || (p->object.flags & UNINTERESTING)) {
 				/* Even if a merge with an uninteresting
 				 * side branch brought the entire change
 				 * we are interested in, we do not want
@@ -640,6 +640,10 @@ int setup_revisions(int argc, const char
 				revs->unpacked = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--no-prune-merges")) {
+				revs->no_merge_pruning = 1;
+				continue;
+			}
 			*unrecognized++ = arg;
 			left++;
 			continue;
diff --git a/revision.h b/revision.h
index 8970b57..a116305 100644
--- a/revision.h
+++ b/revision.h
@@ -26,6 +26,7 @@ struct rev_info {
 	/* Traversal flags */
 	unsigned int	dense:1,
 			no_merges:1,
+			no_merge_pruning:1,
 			remove_empty_trees:1,
 			lifo:1,
 			topo_order:1,