On Mon, Mar 16, 2015 at 09:05:45AM -0700, Junio C Hamano wrote:
> The offending one came from eec3e7e4 (cache-tree: invalidate i-t-a
> paths after generating trees, 2012-12-16), which was a fix to an
> earlier bug where a cache-tree written out of an index with i-t-a
> entries had incorrect information and still claimed it is fully
> valid after write-tree rebuilt it.  The test probably should add
> another path without i-t-a bit, run the same "diff --cached" with
> updated expectation before write-tre, and run the "diff --cached"
> again to make sure it produces a result that match the updated
> expectation.

Would adding another non-i-t-a entry help? Before this patch
"diff --cached" after write-tree shows the i-t-a entry only when
eec3e7e4 is applied. But with this patch we don't show i-t-a entry any
more, before or after write-tree, eec3e7e4 makes no visible difference.

We could even revert eec3e7e4 and the outcome of "diff --cached" would
be the same because we just sort of move the "invalidation" part from
cache-tree to do_oneway_diff(). Not invalidating would speed up "diff
--cached" when i-t-a entries are present. Still it may be a good idea
to invalidate i-t-a paths to be on the safe side. Perhaps a patch like
this to resurrect the test?

-- 8< --
Subject: t2203: enable 'cache-tree invalidates i-t-a paths' test

This test is disabled in the previous patch because the "diff
--cached" expectation is the same, with or without eec3e7e4 [1] where
this test is added.

But it still is a good idea to invalidate i-t-a paths because the
index does _not_ match the cached-tree exactly.  "diff --cached" may
not care, but other operations might. Update the test to catch this
invalidation instead.

[1] cache-tree: invalidate i-t-a paths after generating trees, 2012-12-16

Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 42827b8..1a6c814 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -77,7 +77,7 @@ test_expect_success 'can "commit -a" with an i-t-a entry' '
        git commit -a -m all
 '
 
-test_expect_failure 'cache-tree invalidates i-t-a paths' '
+test_expect_success 'cache-tree invalidates i-t-a paths' '
        git reset --hard &&
        mkdir dir &&
        : >dir/foo &&
@@ -86,14 +86,14 @@ test_expect_failure 'cache-tree invalidates i-t-a paths' '
 
        : >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-dump-cache-tree >actual &&
+       cat >expect <<-\EOF &&
+       invalid                                   (1 subtrees)
+       invalid                                  #(ref)  (1 subtrees)
+       invalid                                  dir/ (0 subtrees)
+       invalid                                  #(ref) dir/ (0 subtrees)
+       EOF
        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