Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries

[Junio C Hamano <gitster@pobox.com>]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> @@ -426,6 +433,15 @@ int cache_tree_update(struct index_state *istate, int flags)
>  	i = update_one(it, cache, entries, "", 0, &skip, flags);
>  	if (i < 0)
>  		return i;
> +	/*
> +	 * Top dir can become empty if all entries are i-t-a (even
> +	 * from subdirs). Note that we do allow to create an empty
> +	 * tree from an empty index. Only error when an empty tree is
> +	 * a result of the i-t-a thing.
> +	 */
> +	if (it->entry_count < 0 &&
> +	    !hashcmp(it->sha1, EMPTY_TREE_SHA1_BIN))
> +		return error(_("cannot build a tree from just intent-to-add entries"));

The test would not let you tell between the two possible ways the
last step "git commit" fails.

Did it fail due to the protection this change adds (i.e. you should
be checking if "git write-tree" fails if that is the case we want to
cover), or did it fail because you recorded an empty tree as the
root commit without giving the --allow-empty option?

In any case, I am not sure about the logic in the comment, either.
"git commit --allow-empty" should be able to create a new commit
without any files in it, no?

>  	istate->cache_changed |= CACHE_TREE_CHANGED;
>  	return 0;
>  }
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index 24aed2e..a19f06b 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -99,5 +99,17 @@ test_expect_success 'cache-tree does not ignore dir that has i-t-a entries' '
>  	)
>  '
>  
> +test_expect_success 'cache-tree does skip dir that becomes empty' '
> +	rm -fr ita-in-dir &&
> +	git init ita-in-dir &&
> +	(
> +		cd ita-in-dir &&
> +		mkdir -p 1/2/3 &&
> +		echo 4 >1/2/3/4 &&
> +		git add -N 1/2/3/4 &&
> +		test_must_fail git commit -m committed
> +	)
> +'
> +
>  test_done