Nguyễn Thái Ngọc Duy <pclo...@gmail.com> writes:
> Commit 3cf773e (cache-tree: fix writing cache-tree when CE_REMOVE is
> present - 2012-12-16) skips i-t-a entries when building trees objects
> from the index. Unfortunately it may skip too much.
>
> The code in question checks if an entry is an i-t-a one, then no tree
> entry will be written. But it does not take into account that
> directories can also be written with the same code. Suppose we have
> this in the index.
>
> a-file
> subdir/file1
> subdir/file2
> subdir/file3
> the-last-file
>
> We write an entry for a-file as normal and move on to subdir/file1,
> where we realize the entry name for this level is simply just
> "subdir", write down an entry for "subdir" then jump three items ahead
> to the-last-file.
>
> That is what happens normally when the first file in subdir is not an
> i-t-a entry. If subdir/file1 is an i-t-a, because of the broken
> condition in this code, we still think "subdir" is an i-t-a file and
> not writing "subdir" down and jump to the-last-file. The result tree
> now only has two items: a-file and the-last-file. subdir should be
> there too (even though it only records two sub-entries, file2 and
> file3).
>
> If the i-t-a entry is subdir/file2 or subdir/file3, this is not a
> problem because we jump over them anyway. Which may explain why the
> bug is hidden for nearly four years.
>
> Fix it by making sure we only skip i-t-a entries when the entry in
> question is actual an index entry, not a directory.
Aha. Good catch.
However, this makes me wonder if subdir has only files all of which
are i-t-a. The resulting top-level tree object should not record
subdir/ as an empty (sub)directory in that case. I do not see where
you are ensuring it in the patch below, though.
>
> Reported-by: Yuri Kanivetsky <yuri.kanivet...@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> ---
> cache-tree.c | 4 ++--
> t/t2203-add-intent.sh | 13 +++++++++++++
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/cache-tree.c b/cache-tree.c
> index ddf0cc9..c2676e8 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -319,7 +319,7 @@ static int update_one(struct cache_tree *it,
> i = 0;
> while (i < entries) {
> const struct cache_entry *ce = cache[i];
> - struct cache_tree_sub *sub;
> + struct cache_tree_sub *sub = NULL;
> const char *path, *slash;
> int pathlen, entlen;
> const unsigned char *sha1;
> @@ -375,7 +375,7 @@ static int update_one(struct cache_tree *it,
> * they are not part of generated trees. Invalidate up
> * to root to force cache-tree users to read elsewhere.
> */
> - if (ce_intent_to_add(ce)) {
> + if (!sub && ce_intent_to_add(ce)) {
> to_invalidate = 1;
> continue;
> }
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index 2a4a749..12d701c 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -82,5 +82,18 @@ test_expect_success 'cache-tree invalidates i-t-a paths' '
> test_cmp expect actual
> '
>
> +test_expect_success 'cache-tree does not ignore dir that has i-t-a entries' '
> + git init ita-in-dir &&
> + (
> + cd ita-in-dir &&
> + mkdir 2 &&
> + touch 1 2/1 2/2 3 &&
> + git add 1 2/2 3 &&
> + git add -N 2/1 &&
> + git commit -m comitted &&
> + git ls-tree -r HEAD | grep 2/2
> + )
> +'
> +
> test_done
--
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
