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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 6, 2016, 19:26 UTC
Message-ID: <xmqqeg76sh44.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160706184829.31825-3-pclouds@gmail.com>

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

Show 13 quoted lines

> @@ -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?

Show 24 quoted lines

>  	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