Re: Bug: write-tree corrupts intent-to-add index state

On Tue, Nov 06, 2012 at 12:53:27AM -0800, Jonathon Mah wrote:

It appears the index forgot that those files were new. So not only
has the intent-to-add status been lost, but git status shows a file
 existing in neither HEAD nor the index as not-new-but-modified.

Tracing through it, I narrowed it down to git write-tree affecting
the index state. As far as I can tell, it's incorrect for that
command to change the index. I'm now out of my (shallow) depth in
the git codebase, so I'm throwing it out there for someone to tackle
(hopefully). I've attached a failing test case.

I played with your test a bit and found that if we skip the commit
step, the bug disappears. I checked and believe that i-t-a flag in
index is preserved, which leaves cache-tree as the culprit. If
cache-tree is (incorrectly) valid, diff operations won't bother
checking index.

We used to not allow writing tree with i-t-a entries present. Then we
allowed it, but forgot that we need to invalidate any paths that
contain i-t-a entries, otherwise cache-tree won't truly reflect
index.

The below patch seems to do the trick, but I'd rather do the
invalidation inside update_one() so we don't need to traverse over the
index again. I haven't succesfully put cache-tree.c back in my head
again to make it happen. Anybody is welcome to step up, or I'll finish
it this weekend.

-- 8< --

diff --git a/cache-tree.c b/cache-tree.c
index 28ed657..30a8018 100644
--- a/cache-tree.c
+++ b/cache-tree.c

@@ -381,6 +381,9 @@ int cache_tree_update(struct cache_tree *it,
 	i = update_one(it, cache, entries, "", 0, flags);
 	if (i < 0)
 		return i;
+	for (i = 0; i < entries; i++)
+		if (cache[i]->ce_flags & CE_INTENT_TO_ADD)
+			cache_tree_invalidate_path(it, cache[i]->name);
 	return 0;
 }
 

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index ec35409..3ddbd89 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh

@@ -62,5 +62,25 @@ test_expect_success 'can "commit -a" with an i-t-a entry' '
 	git commit -a -m all
 '
 
+test_expect_success 'cache-tree invalidates i-t-a paths' '
+	git reset --hard &&
+	mkdir dir &&
+	: >dir/foo &&
+	git add dir/foo &&
+	git commit -m foo &&
+
+	: >dir/bar &&
+	git add -N dir/bar &&
+	git diff --cached --name-only >actual &&
+	echo dir/bar >expect &&
+	test_cmp expect actual &&
+
+	git write-tree >/dev/null &&
+
+	git diff --cached --name-only >actual &&
+	echo dir/bar >expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 8< --