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

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
-- 8< -- 
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to