Git doesn't always add files to a commit (amend)

All of lore.kernel.org
 help / color / mirror / Atom feed

* Git doesn't always add files to a commit (amend)
@ 2016-07-04 13:50 Yuri Kanivetsky
  2016-07-04 15:48 ` Duy Nguyen
  2016-07-04 17:48 ` [PATCH] cache-tree.c: fix i-t-a check skipping directory updates sometimes Nguyễn Thái Ngọc Duy
  0 siblings, 2 replies; 30+ messages in thread
From: Yuri Kanivetsky @ 2016-07-04 13:50 UTC (permalink / raw)
  To: git

Hi,

When intent to add a directory is made (`git add -N`), and then
contents of any but the first file is staged, `git commit -v --amend`
doesn't add it to the commit, see for yourself:

    #!/usr/bin/env bash
    set -eu
    rm -rf 1
    mkdir 1
    cd 1
    git init
    echo 1 > 1 && git add 1 && git commit -m 1
    mkdir 2
    echo 2/1 > 2/1
    echo 2/2 > 2/2
    git add -N 2
    # git add 2/1   # this file is added
    git add 2/2   # as opposed to this one
    git commit --amend -m 1
    git --no-pager log -p
    git reset
    git --no-pager status

I'm running git-2.9.0.

Regards,
Yuri

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Git doesn't always add files to a commit (amend)
  2016-07-04 13:50 Git doesn't always add files to a commit (amend) Yuri Kanivetsky
@ 2016-07-04 15:48 ` Duy Nguyen
  2016-07-04 17:48 ` [PATCH] cache-tree.c: fix i-t-a check skipping directory updates sometimes Nguyễn Thái Ngọc Duy
  1 sibling, 0 replies; 30+ messages in thread
From: Duy Nguyen @ 2016-07-04 15:48 UTC (permalink / raw)
  To: Yuri Kanivetsky; +Cc: git

On Mon, Jul 04, 2016 at 04:50:38PM +0300, Yuri Kanivetsky wrote:
> Hi,
> 
> When intent to add a directory is made (`git add -N`), and then
> contents of any but the first file is staged, `git commit -v --amend`
> doesn't add it to the commit

Oops, a bug since 2012. Thanks for the report. I know what's wrong and
will prepare a proper patch in a couple of hours.

> , see for yourself:
> 
>     #!/usr/bin/env bash
>     set -eu
>     rm -rf 1
>     mkdir 1
>     cd 1
>     git init
>     echo 1 > 1 && git add 1 && git commit -m 1
>     mkdir 2
>     echo 2/1 > 2/1
>     echo 2/2 > 2/2
>     git add -N 2
>     # git add 2/1   # this file is added
>     git add 2/2   # as opposed to this one
>     git commit --amend -m 1
>     git --no-pager log -p
>     git reset
>     git --no-pager status
> 
> I'm running git-2.9.0.
> 
> Regards,
> Yuri
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH] cache-tree.c: fix i-t-a check skipping directory updates sometimes
  2016-07-04 13:50 Git doesn't always add files to a commit (amend) Yuri Kanivetsky
  2016-07-04 15:48 ` Duy Nguyen
@ 2016-07-04 17:48 ` Nguyễn Thái Ngọc Duy
  2016-07-06 17:43   ` Junio C Hamano
  2016-07-06 18:48   ` [PATCH v2 0/2] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  1 sibling, 2 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-04 17:48 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

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.

Reported-by: Yuri Kanivetsky <yuri.kanivetsky@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@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

-- 
2.8.2.537.g0965dd9

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] cache-tree.c: fix i-t-a check skipping directory updates sometimes
  2016-07-04 17:48 ` [PATCH] cache-tree.c: fix i-t-a check skipping directory updates sometimes Nguyễn Thái Ngọc Duy
@ 2016-07-06 17:43   ` Junio C Hamano
  2016-07-06 17:55     ` Junio C Hamano
  2016-07-06 18:48   ` [PATCH v2 0/2] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  1 sibling, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2016-07-06 17:43 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, yuri.kanivetsky

Nguyễn Thái Ngọc Duy  <pclouds@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.kanivetsky@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] cache-tree.c: fix i-t-a check skipping directory updates sometimes
  2016-07-06 17:43   ` Junio C Hamano
@ 2016-07-06 17:55     ` Junio C Hamano
  0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2016-07-06 17:55 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, yuri.kanivetsky

Junio C Hamano <gitster@pobox.com> writes:

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> 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.

Here is a fix-up to the test part to avoid "touch" if we are not
interested in timestamp, and to avoid "git" command in the upstream
of a pipe, plus an additional test to help further bugfix to make
sure a directory that becomes empty with this culling is not
included in the end result.

Thanks for looking into this topic.

 t/t2203-add-intent.sh | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 12d701c..1fc8d3f 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -87,11 +87,33 @@ test_expect_success 'cache-tree does not ignore dir that has i-t-a entries' '
 	(
 		cd ita-in-dir &&
 		mkdir 2 &&
-		touch 1 2/1 2/2 3 &&
+		for f in 1 2/1 2/2 3
+		do
+			echo "$f" >"$f"
+		done &&
 		git add 1 2/2 3 &&
 		git add -N 2/1 &&
-		git commit -m comitted &&
-		git ls-tree -r HEAD | grep 2/2
+		git commit -m committed &&
+		git ls-tree -r HEAD >actual &&
+		grep 2/2 actual
+	)
+'
+
+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 2 &&
+		for f in 1 2/1 2/2 3
+		do
+			echo "$f" >"$f"
+		done &&
+		git add 1 3 &&
+		git add -N 2/1 &&
+		git commit -m committed &&
+		git ls-tree HEAD >actual &&
+		! grep 2 actual
 	)
 '
 

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 0/2] cache-tree building fix in the presence of ita entries
  2016-07-04 17:48 ` [PATCH] cache-tree.c: fix i-t-a check skipping directory updates sometimes Nguyễn Thái Ngọc Duy
  2016-07-06 17:43   ` Junio C Hamano
@ 2016-07-06 18:48   ` Nguyễn Thái Ngọc Duy
  2016-07-06 18:48     ` [PATCH v2 1/2] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
                       ` (2 more replies)
  1 sibling, 3 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-06 18:48 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

1/2 is the same as before with some test fixups from Junio.

2/2 fixes the empty tree generation found by Junio. I split it out
because the commit message in 1/2 is already long. And because it can
happen even without 1/2 (i.e. yet another bug :( though really hard to
happen).

Suppose a/b/c contains all i-t-a entries. Without 1/2, we skip adding
'c' to the tree object 'a/b' (by luck). But if 'a/b' has _nothing_
else but 'a/b/c' then again we will have an empty tree object, which
should not be added in 'a'. It's a cascading effect all the way up.

Nguyễn Thái Ngọc Duy (2):
  cache-tree.c: fix i-t-a entry skipping directory updates sometimes
  cache-tree: do not generate empty trees as a result of all i-t-a subentries

 cache-tree.c          | 20 ++++++++++++++++++--
 t/t2203-add-intent.sh | 29 +++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

-- 
2.8.2.537.g0965dd9

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 1/2] cache-tree.c: fix i-t-a entry skipping directory updates sometimes
  2016-07-06 18:48   ` [PATCH v2 0/2] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
@ 2016-07-06 18:48     ` Nguyễn Thái Ngọc Duy
  2016-07-06 18:48     ` [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
  2016-07-09  5:23     ` [PATCH v3 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  2 siblings, 0 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-06 18:48 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

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.

Reported-by: Yuri Kanivetsky <yuri.kanivetsky@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache-tree.c          |  4 ++--
 t/t2203-add-intent.sh | 17 +++++++++++++++++
 2 files changed, 19 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..24aed2e 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -82,5 +82,22 @@ 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 &&
+		for f in 1 2/1 2/2 3
+		do
+			echo "$f" >"$f"
+		done &&
+		git add 1 2/2 3 &&
+		git add -N 2/1 &&
+		git commit -m committed &&
+		git ls-tree -r HEAD >actual &&
+		grep 2/2 actual
+	)
+'
+
 test_done

-- 
2.8.2.537.g0965dd9

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-06 18:48   ` [PATCH v2 0/2] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  2016-07-06 18:48     ` [PATCH v2 1/2] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
@ 2016-07-06 18:48     ` Nguyễn Thái Ngọc Duy
  2016-07-06 19:26       ` Junio C Hamano
  2016-07-09  5:23     ` [PATCH v3 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  2 siblings, 1 reply; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-06 18:48 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

If a subdirectory contains nothing but i-t-a entries, we generate an
empty tree object and add it to its parent tree. Which is wrong. Such
a subdirectory should not be added. We ignore i-t-a files _and_
directories.

Note that this has a cascading effect. If subdir 'a/b/c' contains
nothing but i-t-a entries, we ignore it. But then if 'a/b' contains
only (the non-existing) 'a/b/c', then we should ignore 'a/b' while
building 'a' too. And it goes all the way up to top directory.

Noticed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache-tree.c          | 16 ++++++++++++++++
 t/t2203-add-intent.sh | 12 ++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/cache-tree.c b/cache-tree.c
index c2676e8..75e73d7 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -380,6 +380,13 @@ static int update_one(struct cache_tree *it,
 			continue;
 		}
 
+		/*
+		 * "sub" can be an empty tree if all subentries are i-t-a.
+		 */
+		if (sub && sub->cache_tree->entry_count < 0 &&
+		    !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
+			continue;
+
 		strbuf_grow(&buffer, entlen + 100);
 		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
 		strbuf_add(&buffer, sha1, 20);
@@ -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"));
 	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
 
-- 
2.8.2.537.g0965dd9


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-06 18:48     ` [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
@ 2016-07-06 19:26       ` Junio C Hamano
  2016-07-07 17:12         ` Duy Nguyen
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2016-07-06 19:26 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, yuri.kanivetsky

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-06 19:26       ` Junio C Hamano
@ 2016-07-07 17:12         ` Duy Nguyen
  2016-07-07 18:52           ` Junio C Hamano
  0 siblings, 1 reply; 30+ messages in thread
From: Duy Nguyen @ 2016-07-07 17:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, yuri.kanivetsky

On Wed, Jul 06, 2016 at 12:26:19PM -0700, Junio C Hamano wrote:
> 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?

You're right. If an empty index can produce an empty tree, then an
index full of i-t-a entries should also be able to produce an empty
tree.

git-commit not failing when --allow-empty is not given is another
(known) problem, where it miscounts the number of real index entries.
It's not right to "fix" it in here.

I'll deal with that separately. Let's focus on cache-tree only this
time. So how about this on top?

-- 8< --
diff --git a/cache-tree.c b/cache-tree.c
index 75e73d7..2d50640 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -433,15 +433,6 @@ 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"));
 	istate->cache_changed |= CACHE_TREE_CHANGED;
 	return 0;
 }
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index a19f06b..1a01279 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -104,10 +104,17 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '
 	git init ita-in-dir &&
 	(
 		cd ita-in-dir &&
+		echo ground >ground &&
+		git add ground &&
 		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
+		git commit -m committed &&
+		git ls-tree -r HEAD >actual &&
+		cat >expected <<\EOF &&
+100644 blob b649b43b0708f5604ac912f5a15f7da2bad51a1b	ground
+EOF
+		test_cmp expected actual
 	)
 '
 
-- 8< --

> > +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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-07 17:12         ` Duy Nguyen
@ 2016-07-07 18:52           ` Junio C Hamano
  2016-07-08 15:39             ` Duy Nguyen
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2016-07-07 18:52 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: git, yuri.kanivetsky

Duy Nguyen <pclouds@gmail.com> writes:

> I'll deal with that separately. Let's focus on cache-tree only this
> time. So how about this on top?

I was hoping that you would limit the scope of the test to check if
write-tree does the right thing.  i.e. not test "git commit", but
test "git write-tree".


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-07 18:52           ` Junio C Hamano
@ 2016-07-08 15:39             ` Duy Nguyen
  2016-07-08 15:53               ` Junio C Hamano
  0 siblings, 1 reply; 30+ messages in thread
From: Duy Nguyen @ 2016-07-08 15:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, yuri.kanivetsky

On Thu, Jul 07, 2016 at 11:52:58AM -0700, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
> 
> > I'll deal with that separately. Let's focus on cache-tree only this
> > time. So how about this on top?
> 
> I was hoping that you would limit the scope of the test to check if
> write-tree does the right thing.  i.e. not test "git commit", but
> test "git write-tree".
> 

Yeah that's better. So the squash patch is something like this

-- 8< --
diff --git a/cache-tree.c b/cache-tree.c
index 75e73d7..2d50640 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -433,15 +433,6 @@ 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"));
 	istate->cache_changed |= CACHE_TREE_CHANGED;
 	return 0;
 }
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index a19f06b..80880b7 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -107,7 +107,9 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '
 		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
+		git write-tree >actual &&
+		echo 4b825dc642cb6eb9a060e54bf8d69288fbee4904 >empty-tree &&
+		test_cmp empty-tree actual
 	)
 '
 
-- 8< --
--
Duy

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-08 15:39             ` Duy Nguyen
@ 2016-07-08 15:53               ` Junio C Hamano
  2016-07-08 16:36                 ` Duy Nguyen
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2016-07-08 15:53 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: git, yuri.kanivetsky

Duy Nguyen <pclouds@gmail.com> writes:

> Yeah that's better. So the squash patch is something like this

Rather...

> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index a19f06b..80880b7 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -107,7 +107,9 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '
>  		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
> +		git write-tree >actual &&
> +		echo 4b825dc642cb6eb9a060e54bf8d69288fbee4904 >empty-tree &&
> +		test_cmp empty-tree actual

	written=$(git write-tree) &&
        git ls-tree "$written" >actual &&
        ! grep 1 actual

That way, we have one less thing to worry about when the hash
function changes in the future.  You may want to rename 1/2/3
to something more readable (e.g. dir/2/3) and grep for "dir"
instead, though.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-08 15:53               ` Junio C Hamano
@ 2016-07-08 16:36                 ` Duy Nguyen
  2016-07-08 17:23                   ` Junio C Hamano
  0 siblings, 1 reply; 30+ messages in thread
From: Duy Nguyen @ 2016-07-08 16:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Yuri Kanivetsky

On Fri, Jul 8, 2016 at 5:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> Yeah that's better. So the squash patch is something like this
>
> Rather...
>
>> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
>> index a19f06b..80880b7 100755
>> --- a/t/t2203-add-intent.sh
>> +++ b/t/t2203-add-intent.sh
>> @@ -107,7 +107,9 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '
>>               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
>> +             git write-tree >actual &&
>> +             echo 4b825dc642cb6eb9a060e54bf8d69288fbee4904 >empty-tree &&
>> +             test_cmp empty-tree actual
>
>         written=$(git write-tree) &&
>         git ls-tree "$written" >actual &&
>         ! grep 1 actual
>
> That way, we have one less thing to worry about when the hash
> function changes in the future.  You may want to rename 1/2/3
> to something more readable (e.g. dir/2/3) and grep for "dir"
> instead, though.

I thought about that too, then did a grep which showed empty sha1 tree
was used elsewhere. And thought of sending a patch to define
$EMPTY_SHA1 in test-lib-functions.sh or somewhere so we don't hard
code it everywhere, but I didn't. But yeah ls-tree works. The last
line could be test_must_be_empty actual (a bit stricter than grep 1)
-- 
Duy

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-08 16:36                 ` Duy Nguyen
@ 2016-07-08 17:23                   ` Junio C Hamano
  0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2016-07-08 17:23 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List, Yuri Kanivetsky

Duy Nguyen <pclouds@gmail.com> writes:

> I thought about that too, then did a grep which showed empty sha1 tree
> was used elsewhere. And thought of sending a patch to define
> $EMPTY_SHA1 in test-lib-functions.sh or somewhere so we don't hard
> code it everywhere, but I didn't. But yeah ls-tree works. The last
> line could be test_must_be_empty actual (a bit stricter than grep 1)

I am OK if you moved $EMPTY_TREE from t4010 to test-lib.sh, added
$EMPTY_BLOB next to it while at it, and remove their definition from
t1501.

t0000, t1100, t3404, t4054 and t5504 have references to 4b825dc64
that may want to be updated to use $EMPTY_TREE.

t5308, t7011, and t7012 defines HI_SHA1 and NULL_SHA1, which may
want to be updated to use $EMPTY_BLOB.

t/lib-pack.sh has one instance of 69de29bb2d1d that may want to be
updated to use $EMPTY_BLOB.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v3 0/4] cache-tree building fix in the presence of ita entries
  2016-07-06 18:48   ` [PATCH v2 0/2] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  2016-07-06 18:48     ` [PATCH v2 1/2] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
  2016-07-06 18:48     ` [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
@ 2016-07-09  5:23     ` Nguyễn Thái Ngọc Duy
  2016-07-09  5:23       ` [PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE Nguyễn Thái Ngọc Duy
                         ` (4 more replies)
  2 siblings, 5 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-09  5:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

v3 fixes 2/2 (which is 4/4 now), allowing cache-tree to generate an
empty tree if the index contains nothing but i-t-a entries.

Since empty tree SHA-1 is involved and we don't want to make it harder
to move away from SHA-1 in future, 1/2 and 2/2 are added to keep SHA-1
for empty tree (and blob while we're at it) in one place.

Note that I didn't make lib-pack.sh and t5308 use $_EMPTY_BLOB because
the actual SHA-1 characters matter (t5308) and I'm not so sure about
variable expansion in the case/esac block and not wanting to check all
the shells out there again (lib-pack.sh).


Nguyễn Thái Ngọc Duy (4):
  test-lib.sh: introduce and use $_EMPTY_TREE
  test-lib.sh: introduce and use $_EMPTY_BLOB
  cache-tree.c: fix i-t-a entry skipping directory updates sometimes
  cache-tree: do not generate empty trees as a result of all i-t-a subentries

 cache-tree.c                         | 11 +++++++++--
 t/t0000-basic.sh                     |  2 +-
 t/t1011-read-tree-sparse-checkout.sh |  8 ++++----
 t/t1100-commit-tree-options.sh       |  2 +-
 t/t1700-split-index.sh               | 24 ++++++++++++------------
 t/t2203-add-intent.sh                | 31 +++++++++++++++++++++++++++++++
 t/t3102-ls-tree-wildcards.sh         |  8 ++++----
 t/t4010-diff-pathspec.sh             |  6 ++----
 t/t4054-diff-bogus-tree.sh           | 10 ++++------
 t/t5504-fetch-receive-strict.sh      |  4 ++--
 t/t7011-skip-worktree-reading.sh     | 12 +++++-------
 t/t7012-skip-worktree-writing.sh     | 10 ++++------
 t/t7063-status-untracked-cache.sh    |  6 +++---
 t/t7508-status.sh                    |  2 +-
 t/test-lib.sh                        |  5 ++++-
 15 files changed, 87 insertions(+), 54 deletions(-)

-- 
2.8.2.537.g0965dd9


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE
  2016-07-09  5:23     ` [PATCH v3 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
@ 2016-07-09  5:23       ` Nguyễn Thái Ngọc Duy
  2016-07-12 20:40         ` Junio C Hamano
  2016-07-09  5:23       ` [PATCH v3 2/4] test-lib.sh: introduce and use $_EMPTY_BLOB Nguyễn Thái Ngọc Duy
                         ` (3 subsequent siblings)
  4 siblings, 1 reply; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-09  5:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

This is a special SHA1. Let's keep it at one place, easier to replace
later when the hash change comes, easier to recognize. Start with an
underscore to reduce the chances somebody may override it without
realizing it's predefined.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t0000-basic.sh                |  2 +-
 t/t1100-commit-tree-options.sh  |  2 +-
 t/t4010-diff-pathspec.sh        |  6 ++----
 t/t4054-diff-bogus-tree.sh      | 10 ++++------
 t/t5504-fetch-receive-strict.sh |  4 ++--
 t/test-lib.sh                   |  4 +++-
 6 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 60811a3..48214e9 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -834,7 +834,7 @@ test_expect_success 'git write-tree should be able to write an empty tree' '
 '
 
 test_expect_success 'validate object ID of a known tree' '
-	test "$tree" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904
+	test "$tree" = $_EMPTY_TREE
 '
 
 # Various types of objects
diff --git a/t/t1100-commit-tree-options.sh b/t/t1100-commit-tree-options.sh
index b7e9b4fc..2adb123 100755
--- a/t/t1100-commit-tree-options.sh
+++ b/t/t1100-commit-tree-options.sh
@@ -15,7 +15,7 @@ Also make sure that command line parser understands the normal
 . ./test-lib.sh
 
 cat >expected <<EOF
-tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904
+tree $_EMPTY_TREE
 author Author Name <author@email> 1117148400 +0000
 committer Committer Name <committer@email> 1117150200 +0000
 
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index 43c488b..7c98912 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -78,16 +78,14 @@ test_expect_success 'diff-tree pathspec' '
 	test_cmp expected current
 '
 
-EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-
 test_expect_success 'diff-tree with wildcard shows dir also matches' '
-	git diff-tree --name-only $EMPTY_TREE $tree -- "f*" >result &&
+	git diff-tree --name-only $_EMPTY_TREE $tree -- "f*" >result &&
 	echo file0 >expected &&
 	test_cmp expected result
 '
 
 test_expect_success 'diff-tree -r with wildcard' '
-	git diff-tree -r --name-only $EMPTY_TREE $tree -- "*file1" >result &&
+	git diff-tree -r --name-only $_EMPTY_TREE $tree -- "*file1" >result &&
 	echo path1/file1 >expected &&
 	test_cmp expected result
 '
diff --git a/t/t4054-diff-bogus-tree.sh b/t/t4054-diff-bogus-tree.sh
index 1d6efab..6b48512 100755
--- a/t/t4054-diff-bogus-tree.sh
+++ b/t/t4054-diff-bogus-tree.sh
@@ -3,8 +3,6 @@
 test_description='test diff with a bogus tree containing the null sha1'
 . ./test-lib.sh
 
-empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-
 test_expect_success 'create bogus tree' '
 	bogus_tree=$(
 		printf "100644 fooQQQQQQQQQQQQQQQQQQQQQ" |
@@ -22,13 +20,13 @@ test_expect_success 'create tree with matching file' '
 
 test_expect_success 'raw diff shows null sha1 (addition)' '
 	echo ":000000 100644 $_z40 $_z40 A	foo" >expect &&
-	git diff-tree $empty_tree $bogus_tree >actual &&
+	git diff-tree $_EMPTY_TREE $bogus_tree >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'raw diff shows null sha1 (removal)' '
 	echo ":100644 000000 $_z40 $_z40 D	foo" >expect &&
-	git diff-tree $bogus_tree $empty_tree >actual &&
+	git diff-tree $bogus_tree $_EMPTY_TREE >actual &&
 	test_cmp expect actual
 '
 
@@ -57,11 +55,11 @@ test_expect_success 'raw diff shows null sha1 (index)' '
 '
 
 test_expect_success 'patch fails due to bogus sha1 (addition)' '
-	test_must_fail git diff-tree -p $empty_tree $bogus_tree
+	test_must_fail git diff-tree -p $_EMPTY_TREE $bogus_tree
 '
 
 test_expect_success 'patch fails due to bogus sha1 (removal)' '
-	test_must_fail git diff-tree -p $bogus_tree $empty_tree
+	test_must_fail git diff-tree -p $bogus_tree $_EMPTY_TREE
 '
 
 test_expect_success 'patch fails due to bogus sha1 (modification)' '
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 44f3d5f..39cc971 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -115,8 +115,8 @@ test_expect_success 'push with transfer.fsckobjects' '
 	test_cmp exp act
 '
 
-cat >bogus-commit <<\EOF
-tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904
+cat >bogus-commit <<EOF
+tree $_EMPTY_TREE
 author Bugs Bunny 1234567890 +0000
 committer Bugs Bunny <bugs@bun.ni> 1234567890 +0000
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0055ebb..327caa5 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -162,6 +162,8 @@ _x40="$_x05$_x05$_x05$_x05$_x05$_x05$_x05$_x05"
 # Zero SHA-1
 _z40=0000000000000000000000000000000000000000
 
+_EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+
 # Line feed
 LF='
 '
@@ -170,7 +172,7 @@ LF='
 # when case-folding filenames
 u200c=$(printf '\342\200\214')
 
-export _x05 _x40 _z40 LF u200c
+export _x05 _x40 _z40 LF u200c _EMPTY_TREE
 
 # Each test should start with something like this, after copyright notices:
 #
-- 
2.8.2.537.g0965dd9


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 2/4] test-lib.sh: introduce and use $_EMPTY_BLOB
  2016-07-09  5:23     ` [PATCH v3 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  2016-07-09  5:23       ` [PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE Nguyễn Thái Ngọc Duy
@ 2016-07-09  5:23       ` Nguyễn Thái Ngọc Duy
  2016-07-09  5:23       ` [PATCH v3 3/4] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
                         ` (2 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-09  5:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

Similar to $_EMPTY_TREE this makes it easier to recognize this special
SHA-1 and change hash later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t1011-read-tree-sparse-checkout.sh |  8 ++++----
 t/t1700-split-index.sh               | 24 ++++++++++++------------
 t/t3102-ls-tree-wildcards.sh         |  8 ++++----
 t/t7011-skip-worktree-reading.sh     | 12 +++++-------
 t/t7012-skip-worktree-writing.sh     | 10 ++++------
 t/t7063-status-untracked-cache.sh    |  6 +++---
 t/t7508-status.sh                    |  2 +-
 t/test-lib.sh                        |  3 ++-
 8 files changed, 35 insertions(+), 38 deletions(-)

diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index 0c74bee..21cfa2e 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -15,11 +15,11 @@ test_description='sparse checkout tests
 . "$TEST_DIRECTORY"/lib-read-tree.sh
 
 test_expect_success 'setup' '
-	cat >expected <<-\EOF &&
+	cat >expected <<-EOF &&
 	100644 77f0ba1734ed79d12881f81b36ee134de6a3327b 0	init.t
-	100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	sub/added
-	100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	sub/addedtoo
-	100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	subsub/added
+	100644 $_EMPTY_BLOB 0	sub/added
+	100644 $_EMPTY_BLOB 0	sub/addedtoo
+	100644 $_EMPTY_BLOB 0	subsub/added
 	EOF
 	cat >expected.swt <<-\EOF &&
 	H init.t
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 8aef49f..305f210 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -33,14 +33,14 @@ test_expect_success 'add one file' '
 	git update-index --add one &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $_EMPTY_BLOB 0	one
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
 	test-dump-split-index .git/index | sed "/^own/d" >actual &&
 	cat >expect <<EOF &&
 base $base
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $_EMPTY_BLOB 0	one
 replacements:
 deletions:
 EOF
@@ -51,7 +51,7 @@ test_expect_success 'disable split index' '
 	git update-index --no-split-index &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $_EMPTY_BLOB 0	one
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
@@ -67,7 +67,7 @@ test_expect_success 'enable split index again, "one" now belongs to base index"'
 	git update-index --split-index &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $_EMPTY_BLOB 0	one
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
@@ -105,7 +105,7 @@ test_expect_success 'add another file, which stays index' '
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
 100644 2e0996000b7e9019eabcad29391bf0f5c7702f0b 0	one
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	two
+100644 $_EMPTY_BLOB 0	two
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
@@ -113,7 +113,7 @@ EOF
 	q_to_tab >expect <<EOF &&
 $BASE
 100644 2e0996000b7e9019eabcad29391bf0f5c7702f0b 0Q
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	two
+100644 $_EMPTY_BLOB 0	two
 replacements: 0
 deletions:
 EOF
@@ -159,14 +159,14 @@ test_expect_success 'add original file back' '
 	git update-index --add one &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $_EMPTY_BLOB 0	one
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
 	test-dump-split-index .git/index | sed "/^own/d" >actual &&
 	cat >expect <<EOF &&
 $BASE
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $_EMPTY_BLOB 0	one
 replacements:
 deletions: 0
 EOF
@@ -178,8 +178,8 @@ test_expect_success 'add new file' '
 	git update-index --add two &&
 	git ls-files --stage >actual &&
 	cat >expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	two
+100644 $_EMPTY_BLOB 0	one
+100644 $_EMPTY_BLOB 0	two
 EOF
 	test_cmp expect actual
 '
@@ -188,8 +188,8 @@ test_expect_success 'unify index, two files remain' '
 	git update-index --no-split-index &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	two
+100644 $_EMPTY_BLOB 0	one
+100644 $_EMPTY_BLOB 0	two
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh
index 4d4b02e..5cf84fa 100755
--- a/t/t3102-ls-tree-wildcards.sh
+++ b/t/t3102-ls-tree-wildcards.sh
@@ -12,16 +12,16 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'ls-tree a[a] matches literally' '
-	cat >expect <<-\EOF &&
-	100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391	a[a]/three
+	cat >expect <<-EOF &&
+	100644 blob $_EMPTY_BLOB	a[a]/three
 	EOF
 	git ls-tree -r HEAD "a[a]" >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'ls-tree outside prefix' '
-	cat >expect <<-\EOF &&
-	100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391	../a[a]/three
+	cat >expect <<-EOF &&
+	100644 blob $_EMPTY_BLOB	../a[a]/three
 	EOF
 	( cd aa && git ls-tree -r HEAD "../a[a]"; ) >actual &&
 	test_cmp expect actual
diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
index 88d60c1..7fe0c9f 100755
--- a/t/t7011-skip-worktree-reading.sh
+++ b/t/t7011-skip-worktree-reading.sh
@@ -23,17 +23,15 @@ S sub/1
 H sub/2
 EOF
 
-NULL_SHA1=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
-
 setup_absent() {
 	test -f 1 && rm 1
 	git update-index --remove 1 &&
-	git update-index --add --cacheinfo 100644 $NULL_SHA1 1 &&
+	git update-index --add --cacheinfo 100644 $_EMPTY_BLOB 1 &&
 	git update-index --skip-worktree 1
 }
 
 test_absent() {
-	echo "100644 $NULL_SHA1 0	1" > expected &&
+	echo "100644 $_EMPTY_BLOB 0	1" > expected &&
 	git ls-files --stage 1 > result &&
 	test_cmp expected result &&
 	test ! -f 1
@@ -42,12 +40,12 @@ test_absent() {
 setup_dirty() {
 	git update-index --force-remove 1 &&
 	echo dirty > 1 &&
-	git update-index --add --cacheinfo 100644 $NULL_SHA1 1 &&
+	git update-index --add --cacheinfo 100644 $_EMPTY_BLOB 1 &&
 	git update-index --skip-worktree 1
 }
 
 test_dirty() {
-	echo "100644 $NULL_SHA1 0	1" > expected &&
+	echo "100644 $_EMPTY_BLOB 0	1" > expected &&
 	git ls-files --stage 1 > result &&
 	test_cmp expected result &&
 	echo dirty > expected
@@ -120,7 +118,7 @@ test_expect_success 'grep with skip-worktree file' '
 	test "$(git grep --no-ext-grep test)" = "1:test"
 '
 
-echo ":000000 100644 $_z40 $NULL_SHA1 A	1" > expected
+echo ":000000 100644 $_z40 $_EMPTY_BLOB A	1" > expected
 test_expect_success 'diff-index does not examine skip-worktree absent entries' '
 	setup_absent &&
 	git diff-index HEAD -- 1 > result &&
diff --git a/t/t7012-skip-worktree-writing.sh b/t/t7012-skip-worktree-writing.sh
index 9ceaa40..003e36f 100755
--- a/t/t7012-skip-worktree-writing.sh
+++ b/t/t7012-skip-worktree-writing.sh
@@ -53,17 +53,15 @@ test_expect_success 'read-tree removes worktree, dirty case' '
 	git update-index --no-skip-worktree added
 '
 
-NULL_SHA1=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
-
 setup_absent() {
 	test -f 1 && rm 1
 	git update-index --remove 1 &&
-	git update-index --add --cacheinfo 100644 $NULL_SHA1 1 &&
+	git update-index --add --cacheinfo 100644 $_EMPTY_BLOB 1 &&
 	git update-index --skip-worktree 1
 }
 
 test_absent() {
-	echo "100644 $NULL_SHA1 0	1" > expected &&
+	echo "100644 $_EMPTY_BLOB 0	1" > expected &&
 	git ls-files --stage 1 > result &&
 	test_cmp expected result &&
 	test ! -f 1
@@ -72,12 +70,12 @@ test_absent() {
 setup_dirty() {
 	git update-index --force-remove 1 &&
 	echo dirty > 1 &&
-	git update-index --add --cacheinfo 100644 $NULL_SHA1 1 &&
+	git update-index --add --cacheinfo 100644 $_EMPTY_BLOB 1 &&
 	git update-index --skip-worktree 1
 }
 
 test_dirty() {
-	echo "100644 $NULL_SHA1 0	1" > expected &&
+	echo "100644 $_EMPTY_BLOB 0	1" > expected &&
 	git ls-files --stage 1 > result &&
 	test_cmp expected result &&
 	echo dirty > expected
diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index a971884..8d23259 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -53,7 +53,7 @@ A  two
 EOF
 
 cat >../dump.expect <<EOF &&
-info/exclude e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+info/exclude $_EMPTY_BLOB
 core.excludesfile 0000000000000000000000000000000000000000
 exclude_per_dir .gitignore
 flags 00000006
@@ -137,7 +137,7 @@ EOF
 test_expect_success 'verify untracked cache dump' '
 	test-dump-untracked-cache >../actual &&
 	cat >../expect <<EOF &&
-info/exclude e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+info/exclude $_EMPTY_BLOB
 core.excludesfile 0000000000000000000000000000000000000000
 exclude_per_dir .gitignore
 flags 00000006
@@ -184,7 +184,7 @@ EOF
 test_expect_success 'verify untracked cache dump' '
 	test-dump-untracked-cache >../actual &&
 	cat >../expect <<EOF &&
-info/exclude e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+info/exclude $_EMPTY_BLOB
 core.excludesfile 0000000000000000000000000000000000000000
 exclude_per_dir .gitignore
 flags 00000006
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index c3ed7cb..2b534ec 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -803,7 +803,7 @@ EOF
 '
 
 cat >expect <<EOF
-:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0000000000000000000000000000000000000000 M	dir1/modified
+:100644 100644 $_EMPTY_BLOB 0000000000000000000000000000000000000000 M	dir1/modified
 EOF
 test_expect_success 'status refreshes the index' '
 	touch dir2/added &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 327caa5..e1785a2 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -163,6 +163,7 @@ _x40="$_x05$_x05$_x05$_x05$_x05$_x05$_x05$_x05"
 _z40=0000000000000000000000000000000000000000
 
 _EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+_EMPTY_BLOB=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
 
 # Line feed
 LF='
@@ -172,7 +173,7 @@ LF='
 # when case-folding filenames
 u200c=$(printf '\342\200\214')
 
-export _x05 _x40 _z40 LF u200c _EMPTY_TREE
+export _x05 _x40 _z40 LF u200c _EMPTY_TREE _EMPTY_BLOB
 
 # Each test should start with something like this, after copyright notices:
 #
-- 
2.8.2.537.g0965dd9


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 3/4] cache-tree.c: fix i-t-a entry skipping directory updates sometimes
  2016-07-09  5:23     ` [PATCH v3 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  2016-07-09  5:23       ` [PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE Nguyễn Thái Ngọc Duy
  2016-07-09  5:23       ` [PATCH v3 2/4] test-lib.sh: introduce and use $_EMPTY_BLOB Nguyễn Thái Ngọc Duy
@ 2016-07-09  5:23       ` Nguyễn Thái Ngọc Duy
  2016-07-09  5:23       ` [PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
  2016-07-16  5:06       ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  4 siblings, 0 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-09  5:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

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.

Reported-by: Yuri Kanivetsky <yuri.kanivetsky@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache-tree.c          |  4 ++--
 t/t2203-add-intent.sh | 17 +++++++++++++++++
 2 files changed, 19 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..24aed2e 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -82,5 +82,22 @@ 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 &&
+		for f in 1 2/1 2/2 3
+		do
+			echo "$f" >"$f"
+		done &&
+		git add 1 2/2 3 &&
+		git add -N 2/1 &&
+		git commit -m committed &&
+		git ls-tree -r HEAD >actual &&
+		grep 2/2 actual
+	)
+'
+
 test_done

-- 
2.8.2.537.g0965dd9

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-09  5:23     ` [PATCH v3 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
                         ` (2 preceding siblings ...)
  2016-07-09  5:23       ` [PATCH v3 3/4] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
@ 2016-07-09  5:23       ` Nguyễn Thái Ngọc Duy
  2016-07-12 20:49         ` Junio C Hamano
  2016-07-16  5:06       ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  4 siblings, 1 reply; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-09  5:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

If a subdirectory contains nothing but i-t-a entries, we generate an
empty tree object and add it to its parent tree. Which is wrong. Such
a subdirectory should not be added.

Note that this has a cascading effect. If subdir 'a/b/c' contains
nothing but i-t-a entries, we ignore it. But then if 'a/b' contains
only (the non-existing) 'a/b/c', then we should ignore 'a/b' while
building 'a' too. And it goes all the way up to top directory.

Noticed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache-tree.c          |  7 +++++++
 t/t2203-add-intent.sh | 14 ++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/cache-tree.c b/cache-tree.c
index c2676e8..2d50640 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -380,6 +380,13 @@ static int update_one(struct cache_tree *it,
 			continue;
 		}
 
+		/*
+		 * "sub" can be an empty tree if subentries are i-t-a.
+		 */
+		if (sub && sub->cache_tree->entry_count < 0 &&
+		    !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
+			continue;
+
 		strbuf_grow(&buffer, entlen + 100);
 		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
 		strbuf_add(&buffer, sha1, 20);
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 24aed2e..f4b2fac 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -99,5 +99,19 @@ 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 &&
+		git write-tree >actual &&
+		echo $_EMPTY_TREE >expected &&
+		test_cmp expected actual
+	)
+'
+
 test_done
 
-- 
2.8.2.537.g0965dd9


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE
  2016-07-09  5:23       ` [PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE Nguyễn Thái Ngọc Duy
@ 2016-07-12 20:40         ` Junio C Hamano
  2016-07-13 15:04           ` Duy Nguyen
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2016-07-12 20:40 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, yuri.kanivetsky

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

> This is a special SHA1. Let's keep it at one place, easier to replace
> later when the hash change comes, easier to recognize. Start with an
> underscore to reduce the chances somebody may override it without
> realizing it's predefined.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  t/t0000-basic.sh                |  2 +-
>  t/t1100-commit-tree-options.sh  |  2 +-
>  t/t4010-diff-pathspec.sh        |  6 ++----
>  t/t4054-diff-bogus-tree.sh      | 10 ++++------
>  t/t5504-fetch-receive-strict.sh |  4 ++--
>  t/test-lib.sh                   |  4 +++-
>  6 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index 60811a3..48214e9 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -834,7 +834,7 @@ test_expect_success 'git write-tree should be able to write an empty tree' '
>  '
>  
>  test_expect_success 'validate object ID of a known tree' '
> -	test "$tree" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904
> +	test "$tree" = $_EMPTY_TREE
>  '

I doubt the point of, and I'd rather not to see, the leading
underscore.  Are there existing uses of the name that want it to
mean something different?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-09  5:23       ` [PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
@ 2016-07-12 20:49         ` Junio C Hamano
  2016-07-13 14:59           ` Duy Nguyen
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2016-07-12 20:49 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, yuri.kanivetsky

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

> diff --git a/cache-tree.c b/cache-tree.c
> index c2676e8..2d50640 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -380,6 +380,13 @@ static int update_one(struct cache_tree *it,
>  			continue;
>  		}
>  
> +		/*
> +		 * "sub" can be an empty tree if subentries are i-t-a.
> +		 */
> +		if (sub && sub->cache_tree->entry_count < 0 &&
> +		    !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
> +			continue;
> +

Looks sensible, except that it is unclear if it is correct to assume
that "subentries are ita" always equals to "entry_count < 0" here.
I _think_ it is OK, as the function itself does use the latter as
the sign that it hit to_invalidate=1 case when it returns.

Thanks.

>  		strbuf_grow(&buffer, entlen + 100);
>  		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
>  		strbuf_add(&buffer, sha1, 20);
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index 24aed2e..f4b2fac 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -99,5 +99,19 @@ 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 &&
> +		git write-tree >actual &&
> +		echo $_EMPTY_TREE >expected &&
> +		test_cmp expected actual
> +	)
> +'
> +
>  test_done

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-12 20:49         ` Junio C Hamano
@ 2016-07-13 14:59           ` Duy Nguyen
  0 siblings, 0 replies; 30+ messages in thread
From: Duy Nguyen @ 2016-07-13 14:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Yuri Kanivetsky

On Tue, Jul 12, 2016 at 10:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> diff --git a/cache-tree.c b/cache-tree.c
>> index c2676e8..2d50640 100644
>> --- a/cache-tree.c
>> +++ b/cache-tree.c
>> @@ -380,6 +380,13 @@ static int update_one(struct cache_tree *it,
>>                       continue;
>>               }
>>
>> +             /*
>> +              * "sub" can be an empty tree if subentries are i-t-a.
>> +              */
>> +             if (sub && sub->cache_tree->entry_count < 0 &&
>> +                 !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
>> +                     continue;
>> +
>
> Looks sensible, except that it is unclear if it is correct to assume
> that "subentries are ita" always equals to "entry_count < 0" here.
> I _think_ it is OK, as the function itself does use the latter as
> the sign that it hit to_invalidate=1 case when it returns.

I had the same concern and double checked it. If one day we have a new
entry_count < 0 case that's not i-t-a, we'll need to refactor this
code a bit.
-- 
Duy

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE
  2016-07-12 20:40         ` Junio C Hamano
@ 2016-07-13 15:04           ` Duy Nguyen
  0 siblings, 0 replies; 30+ messages in thread
From: Duy Nguyen @ 2016-07-13 15:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Yuri Kanivetsky

On Tue, Jul 12, 2016 at 10:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> This is a special SHA1. Let's keep it at one place, easier to replace
>> later when the hash change comes, easier to recognize. Start with an
>> underscore to reduce the chances somebody may override it without
>> realizing it's predefined.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  t/t0000-basic.sh                |  2 +-
>>  t/t1100-commit-tree-options.sh  |  2 +-
>>  t/t4010-diff-pathspec.sh        |  6 ++----
>>  t/t4054-diff-bogus-tree.sh      | 10 ++++------
>>  t/t5504-fetch-receive-strict.sh |  4 ++--
>>  t/test-lib.sh                   |  4 +++-
>>  6 files changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
>> index 60811a3..48214e9 100755
>> --- a/t/t0000-basic.sh
>> +++ b/t/t0000-basic.sh
>> @@ -834,7 +834,7 @@ test_expect_success 'git write-tree should be able to write an empty tree' '
>>  '
>>
>>  test_expect_success 'validate object ID of a known tree' '
>> -     test "$tree" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904
>> +     test "$tree" = $_EMPTY_TREE
>>  '
>
> I doubt the point of, and I'd rather not to see, the leading
> underscore.  Are there existing uses of the name that want it to
> mean something different?

No. There is EMPTY_TREE in use, but it's exactly what we expect. It's
probably still a good idea to separate "global" variables from
per-file ones. But I don't feel strongly about this, so if a re-roll
is required (or somebody votes for underscore removal, including you),
I'll remove the underscore.
-- 
Duy

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v4 0/4] cache-tree building fix in the presence of ita entries
  2016-07-09  5:23     ` [PATCH v3 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
                         ` (3 preceding siblings ...)
  2016-07-09  5:23       ` [PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
@ 2016-07-16  5:06       ` Nguyễn Thái Ngọc Duy
  2016-07-16  5:06         ` [PATCH v4 1/4] test-lib.sh: introduce and use $EMPTY_TREE Nguyễn Thái Ngọc Duy
                           ` (4 more replies)
  4 siblings, 5 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-16  5:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

v4 removes the leading underscore from _EMPTY_BLOB and _EMPTY_TREE and
updates 4/4 slightly like this.

diff --git a/cache-tree.c b/cache-tree.c
index 2d50640..f28b1f4 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -325,6 +325,7 @@ static int update_one(struct cache_tree *it,
 		const unsigned char *sha1;
 		unsigned mode;
 		int expected_missing = 0;
+		int contains_ita = 0;
 
 		path = ce->name;
 		pathlen = ce_namelen(ce);
@@ -341,7 +342,8 @@ static int update_one(struct cache_tree *it,
 			i += sub->count;
 			sha1 = sub->cache_tree->sha1;
 			mode = S_IFDIR;
-			if (sub->cache_tree->entry_count < 0) {
+			contains_ita = sub->cache_tree->entry_count < 0;
+			if (contains_ita) {
 				to_invalidate = 1;
 				expected_missing = 1;
 			}
@@ -381,10 +383,9 @@ static int update_one(struct cache_tree *it,
 		}
 
 		/*
-		 * "sub" can be an empty tree if subentries are i-t-a.
+		 * "sub" can be an empty tree if all subentries are i-t-a.
 		 */
-		if (sub && sub->cache_tree->entry_count < 0 &&
-		    !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
+		if (contains_ita && !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
 			continue;
 
 		strbuf_grow(&buffer, entlen + 100);

Nguyễn Thái Ngọc Duy (4):
  test-lib.sh: introduce and use $EMPTY_TREE
  test-lib.sh: introduce and use $EMPTY_BLOB
  cache-tree.c: fix i-t-a entry skipping directory updates sometimes
  cache-tree: do not generate empty trees as a result of all i-t-a
    subentries

 cache-tree.c                         | 14 +++++++++++---
 t/t0000-basic.sh                     |  2 +-
 t/t1011-read-tree-sparse-checkout.sh |  8 ++++----
 t/t1100-commit-tree-options.sh       |  2 +-
 t/t1700-split-index.sh               | 24 ++++++++++++------------
 t/t2203-add-intent.sh                | 31 +++++++++++++++++++++++++++++++
 t/t3102-ls-tree-wildcards.sh         |  8 ++++----
 t/t4010-diff-pathspec.sh             |  2 --
 t/t4054-diff-bogus-tree.sh           | 10 ++++------
 t/t5504-fetch-receive-strict.sh      |  4 ++--
 t/t7011-skip-worktree-reading.sh     | 12 +++++-------
 t/t7012-skip-worktree-writing.sh     | 10 ++++------
 t/t7063-status-untracked-cache.sh    |  6 +++---
 t/t7508-status.sh                    |  2 +-
 t/test-lib.sh                        |  5 ++++-
 15 files changed, 87 insertions(+), 53 deletions(-)

-- 
2.9.1.566.gbd532d4


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v4 1/4] test-lib.sh: introduce and use $EMPTY_TREE
  2016-07-16  5:06       ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
@ 2016-07-16  5:06         ` Nguyễn Thái Ngọc Duy
  2016-07-16  5:06         ` [PATCH v4 2/4] test-lib.sh: introduce and use $EMPTY_BLOB Nguyễn Thái Ngọc Duy
                           ` (3 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-16  5:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

This is a special SHA1. Let's keep it at one place, easier to replace
later when the hash change comes, easier to recognize.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t0000-basic.sh                |  2 +-
 t/t1100-commit-tree-options.sh  |  2 +-
 t/t4010-diff-pathspec.sh        |  2 --
 t/t4054-diff-bogus-tree.sh      | 10 ++++------
 t/t5504-fetch-receive-strict.sh |  4 ++--
 t/test-lib.sh                   |  4 +++-
 6 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 60811a3..1aa5093 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -834,7 +834,7 @@ test_expect_success 'git write-tree should be able to write an empty tree' '
 '
 
 test_expect_success 'validate object ID of a known tree' '
-	test "$tree" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904
+	test "$tree" = $EMPTY_TREE
 '
 
 # Various types of objects
diff --git a/t/t1100-commit-tree-options.sh b/t/t1100-commit-tree-options.sh
index b7e9b4fc..ae66ba5 100755
--- a/t/t1100-commit-tree-options.sh
+++ b/t/t1100-commit-tree-options.sh
@@ -15,7 +15,7 @@ Also make sure that command line parser understands the normal
 . ./test-lib.sh
 
 cat >expected <<EOF
-tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904
+tree $EMPTY_TREE
 author Author Name <author@email> 1117148400 +0000
 committer Committer Name <committer@email> 1117150200 +0000
 
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index 43c488b..35b35a8 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -78,8 +78,6 @@ test_expect_success 'diff-tree pathspec' '
 	test_cmp expected current
 '
 
-EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-
 test_expect_success 'diff-tree with wildcard shows dir also matches' '
 	git diff-tree --name-only $EMPTY_TREE $tree -- "f*" >result &&
 	echo file0 >expected &&
diff --git a/t/t4054-diff-bogus-tree.sh b/t/t4054-diff-bogus-tree.sh
index 1d6efab..18f42c5 100755
--- a/t/t4054-diff-bogus-tree.sh
+++ b/t/t4054-diff-bogus-tree.sh
@@ -3,8 +3,6 @@
 test_description='test diff with a bogus tree containing the null sha1'
 . ./test-lib.sh
 
-empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-
 test_expect_success 'create bogus tree' '
 	bogus_tree=$(
 		printf "100644 fooQQQQQQQQQQQQQQQQQQQQQ" |
@@ -22,13 +20,13 @@ test_expect_success 'create tree with matching file' '
 
 test_expect_success 'raw diff shows null sha1 (addition)' '
 	echo ":000000 100644 $_z40 $_z40 A	foo" >expect &&
-	git diff-tree $empty_tree $bogus_tree >actual &&
+	git diff-tree $EMPTY_TREE $bogus_tree >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'raw diff shows null sha1 (removal)' '
 	echo ":100644 000000 $_z40 $_z40 D	foo" >expect &&
-	git diff-tree $bogus_tree $empty_tree >actual &&
+	git diff-tree $bogus_tree $EMPTY_TREE >actual &&
 	test_cmp expect actual
 '
 
@@ -57,11 +55,11 @@ test_expect_success 'raw diff shows null sha1 (index)' '
 '
 
 test_expect_success 'patch fails due to bogus sha1 (addition)' '
-	test_must_fail git diff-tree -p $empty_tree $bogus_tree
+	test_must_fail git diff-tree -p $EMPTY_TREE $bogus_tree
 '
 
 test_expect_success 'patch fails due to bogus sha1 (removal)' '
-	test_must_fail git diff-tree -p $bogus_tree $empty_tree
+	test_must_fail git diff-tree -p $bogus_tree $EMPTY_TREE
 '
 
 test_expect_success 'patch fails due to bogus sha1 (modification)' '
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 44f3d5f..9b19cff 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -115,8 +115,8 @@ test_expect_success 'push with transfer.fsckobjects' '
 	test_cmp exp act
 '
 
-cat >bogus-commit <<\EOF
-tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904
+cat >bogus-commit <<EOF
+tree $EMPTY_TREE
 author Bugs Bunny 1234567890 +0000
 committer Bugs Bunny <bugs@bun.ni> 1234567890 +0000
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0055ebb..85f4c6d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -162,6 +162,8 @@ _x40="$_x05$_x05$_x05$_x05$_x05$_x05$_x05$_x05"
 # Zero SHA-1
 _z40=0000000000000000000000000000000000000000
 
+EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+
 # Line feed
 LF='
 '
@@ -170,7 +172,7 @@ LF='
 # when case-folding filenames
 u200c=$(printf '\342\200\214')
 
-export _x05 _x40 _z40 LF u200c
+export _x05 _x40 _z40 LF u200c EMPTY_TREE
 
 # Each test should start with something like this, after copyright notices:
 #
-- 
2.9.1.566.gbd532d4


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v4 2/4] test-lib.sh: introduce and use $EMPTY_BLOB
  2016-07-16  5:06       ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  2016-07-16  5:06         ` [PATCH v4 1/4] test-lib.sh: introduce and use $EMPTY_TREE Nguyễn Thái Ngọc Duy
@ 2016-07-16  5:06         ` Nguyễn Thái Ngọc Duy
  2016-07-16  5:06         ` [PATCH v4 3/4] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
                           ` (2 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-16  5:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

Similar to $EMPTY_TREE this makes it easier to recognize this special
SHA-1 and change hash later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t1011-read-tree-sparse-checkout.sh |  8 ++++----
 t/t1700-split-index.sh               | 24 ++++++++++++------------
 t/t3102-ls-tree-wildcards.sh         |  8 ++++----
 t/t7011-skip-worktree-reading.sh     | 12 +++++-------
 t/t7012-skip-worktree-writing.sh     | 10 ++++------
 t/t7063-status-untracked-cache.sh    |  6 +++---
 t/t7508-status.sh                    |  2 +-
 t/test-lib.sh                        |  3 ++-
 8 files changed, 35 insertions(+), 38 deletions(-)

diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index 0c74bee..2563d18 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -15,11 +15,11 @@ test_description='sparse checkout tests
 . "$TEST_DIRECTORY"/lib-read-tree.sh
 
 test_expect_success 'setup' '
-	cat >expected <<-\EOF &&
+	cat >expected <<-EOF &&
 	100644 77f0ba1734ed79d12881f81b36ee134de6a3327b 0	init.t
-	100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	sub/added
-	100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	sub/addedtoo
-	100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	subsub/added
+	100644 $EMPTY_BLOB 0	sub/added
+	100644 $EMPTY_BLOB 0	sub/addedtoo
+	100644 $EMPTY_BLOB 0	subsub/added
 	EOF
 	cat >expected.swt <<-\EOF &&
 	H init.t
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 8aef49f..292a072 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -33,14 +33,14 @@ test_expect_success 'add one file' '
 	git update-index --add one &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $EMPTY_BLOB 0	one
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
 	test-dump-split-index .git/index | sed "/^own/d" >actual &&
 	cat >expect <<EOF &&
 base $base
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $EMPTY_BLOB 0	one
 replacements:
 deletions:
 EOF
@@ -51,7 +51,7 @@ test_expect_success 'disable split index' '
 	git update-index --no-split-index &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $EMPTY_BLOB 0	one
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
@@ -67,7 +67,7 @@ test_expect_success 'enable split index again, "one" now belongs to base index"'
 	git update-index --split-index &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $EMPTY_BLOB 0	one
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
@@ -105,7 +105,7 @@ test_expect_success 'add another file, which stays index' '
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
 100644 2e0996000b7e9019eabcad29391bf0f5c7702f0b 0	one
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	two
+100644 $EMPTY_BLOB 0	two
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
@@ -113,7 +113,7 @@ EOF
 	q_to_tab >expect <<EOF &&
 $BASE
 100644 2e0996000b7e9019eabcad29391bf0f5c7702f0b 0Q
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	two
+100644 $EMPTY_BLOB 0	two
 replacements: 0
 deletions:
 EOF
@@ -159,14 +159,14 @@ test_expect_success 'add original file back' '
 	git update-index --add one &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $EMPTY_BLOB 0	one
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
 	test-dump-split-index .git/index | sed "/^own/d" >actual &&
 	cat >expect <<EOF &&
 $BASE
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
+100644 $EMPTY_BLOB 0	one
 replacements:
 deletions: 0
 EOF
@@ -178,8 +178,8 @@ test_expect_success 'add new file' '
 	git update-index --add two &&
 	git ls-files --stage >actual &&
 	cat >expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	two
+100644 $EMPTY_BLOB 0	one
+100644 $EMPTY_BLOB 0	two
 EOF
 	test_cmp expect actual
 '
@@ -188,8 +188,8 @@ test_expect_success 'unify index, two files remain' '
 	git update-index --no-split-index &&
 	git ls-files --stage >ls-files.actual &&
 	cat >ls-files.expect <<EOF &&
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	one
-100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	two
+100644 $EMPTY_BLOB 0	one
+100644 $EMPTY_BLOB 0	two
 EOF
 	test_cmp ls-files.expect ls-files.actual &&
 
diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh
index 4d4b02e..e804377 100755
--- a/t/t3102-ls-tree-wildcards.sh
+++ b/t/t3102-ls-tree-wildcards.sh
@@ -12,16 +12,16 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'ls-tree a[a] matches literally' '
-	cat >expect <<-\EOF &&
-	100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391	a[a]/three
+	cat >expect <<-EOF &&
+	100644 blob $EMPTY_BLOB	a[a]/three
 	EOF
 	git ls-tree -r HEAD "a[a]" >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'ls-tree outside prefix' '
-	cat >expect <<-\EOF &&
-	100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391	../a[a]/three
+	cat >expect <<-EOF &&
+	100644 blob $EMPTY_BLOB	../a[a]/three
 	EOF
 	( cd aa && git ls-tree -r HEAD "../a[a]"; ) >actual &&
 	test_cmp expect actual
diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
index 88d60c1..84f4145 100755
--- a/t/t7011-skip-worktree-reading.sh
+++ b/t/t7011-skip-worktree-reading.sh
@@ -23,17 +23,15 @@ S sub/1
 H sub/2
 EOF
 
-NULL_SHA1=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
-
 setup_absent() {
 	test -f 1 && rm 1
 	git update-index --remove 1 &&
-	git update-index --add --cacheinfo 100644 $NULL_SHA1 1 &&
+	git update-index --add --cacheinfo 100644 $EMPTY_BLOB 1 &&
 	git update-index --skip-worktree 1
 }
 
 test_absent() {
-	echo "100644 $NULL_SHA1 0	1" > expected &&
+	echo "100644 $EMPTY_BLOB 0	1" > expected &&
 	git ls-files --stage 1 > result &&
 	test_cmp expected result &&
 	test ! -f 1
@@ -42,12 +40,12 @@ test_absent() {
 setup_dirty() {
 	git update-index --force-remove 1 &&
 	echo dirty > 1 &&
-	git update-index --add --cacheinfo 100644 $NULL_SHA1 1 &&
+	git update-index --add --cacheinfo 100644 $EMPTY_BLOB 1 &&
 	git update-index --skip-worktree 1
 }
 
 test_dirty() {
-	echo "100644 $NULL_SHA1 0	1" > expected &&
+	echo "100644 $EMPTY_BLOB 0	1" > expected &&
 	git ls-files --stage 1 > result &&
 	test_cmp expected result &&
 	echo dirty > expected
@@ -120,7 +118,7 @@ test_expect_success 'grep with skip-worktree file' '
 	test "$(git grep --no-ext-grep test)" = "1:test"
 '
 
-echo ":000000 100644 $_z40 $NULL_SHA1 A	1" > expected
+echo ":000000 100644 $_z40 $EMPTY_BLOB A	1" > expected
 test_expect_success 'diff-index does not examine skip-worktree absent entries' '
 	setup_absent &&
 	git diff-index HEAD -- 1 > result &&
diff --git a/t/t7012-skip-worktree-writing.sh b/t/t7012-skip-worktree-writing.sh
index 9ceaa40..9d1abe5 100755
--- a/t/t7012-skip-worktree-writing.sh
+++ b/t/t7012-skip-worktree-writing.sh
@@ -53,17 +53,15 @@ test_expect_success 'read-tree removes worktree, dirty case' '
 	git update-index --no-skip-worktree added
 '
 
-NULL_SHA1=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
-
 setup_absent() {
 	test -f 1 && rm 1
 	git update-index --remove 1 &&
-	git update-index --add --cacheinfo 100644 $NULL_SHA1 1 &&
+	git update-index --add --cacheinfo 100644 $EMPTY_BLOB 1 &&
 	git update-index --skip-worktree 1
 }
 
 test_absent() {
-	echo "100644 $NULL_SHA1 0	1" > expected &&
+	echo "100644 $EMPTY_BLOB 0	1" > expected &&
 	git ls-files --stage 1 > result &&
 	test_cmp expected result &&
 	test ! -f 1
@@ -72,12 +70,12 @@ test_absent() {
 setup_dirty() {
 	git update-index --force-remove 1 &&
 	echo dirty > 1 &&
-	git update-index --add --cacheinfo 100644 $NULL_SHA1 1 &&
+	git update-index --add --cacheinfo 100644 $EMPTY_BLOB 1 &&
 	git update-index --skip-worktree 1
 }
 
 test_dirty() {
-	echo "100644 $NULL_SHA1 0	1" > expected &&
+	echo "100644 $EMPTY_BLOB 0	1" > expected &&
 	git ls-files --stage 1 > result &&
 	test_cmp expected result &&
 	echo dirty > expected
diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index a971884..a828a5f 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -53,7 +53,7 @@ A  two
 EOF
 
 cat >../dump.expect <<EOF &&
-info/exclude e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+info/exclude $EMPTY_BLOB
 core.excludesfile 0000000000000000000000000000000000000000
 exclude_per_dir .gitignore
 flags 00000006
@@ -137,7 +137,7 @@ EOF
 test_expect_success 'verify untracked cache dump' '
 	test-dump-untracked-cache >../actual &&
 	cat >../expect <<EOF &&
-info/exclude e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+info/exclude $EMPTY_BLOB
 core.excludesfile 0000000000000000000000000000000000000000
 exclude_per_dir .gitignore
 flags 00000006
@@ -184,7 +184,7 @@ EOF
 test_expect_success 'verify untracked cache dump' '
 	test-dump-untracked-cache >../actual &&
 	cat >../expect <<EOF &&
-info/exclude e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+info/exclude $EMPTY_BLOB
 core.excludesfile 0000000000000000000000000000000000000000
 exclude_per_dir .gitignore
 flags 00000006
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index c3ed7cb..a42aef8 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -803,7 +803,7 @@ EOF
 '
 
 cat >expect <<EOF
-:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0000000000000000000000000000000000000000 M	dir1/modified
+:100644 100644 $EMPTY_BLOB 0000000000000000000000000000000000000000 M	dir1/modified
 EOF
 test_expect_success 'status refreshes the index' '
 	touch dir2/added &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 85f4c6d..9f36091 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -163,6 +163,7 @@ _x40="$_x05$_x05$_x05$_x05$_x05$_x05$_x05$_x05"
 _z40=0000000000000000000000000000000000000000
 
 EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+EMPTY_BLOB=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
 
 # Line feed
 LF='
@@ -172,7 +173,7 @@ LF='
 # when case-folding filenames
 u200c=$(printf '\342\200\214')
 
-export _x05 _x40 _z40 LF u200c EMPTY_TREE
+export _x05 _x40 _z40 LF u200c EMPTY_TREE EMPTY_BLOB
 
 # Each test should start with something like this, after copyright notices:
 #
-- 
2.9.1.566.gbd532d4


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v4 3/4] cache-tree.c: fix i-t-a entry skipping directory updates sometimes
  2016-07-16  5:06       ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
  2016-07-16  5:06         ` [PATCH v4 1/4] test-lib.sh: introduce and use $EMPTY_TREE Nguyễn Thái Ngọc Duy
  2016-07-16  5:06         ` [PATCH v4 2/4] test-lib.sh: introduce and use $EMPTY_BLOB Nguyễn Thái Ngọc Duy
@ 2016-07-16  5:06         ` Nguyễn Thái Ngọc Duy
  2016-07-16  5:06         ` [PATCH v4 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
  2016-07-18 20:48         ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Junio C Hamano
  4 siblings, 0 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-16  5:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

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.

Reported-by: Yuri Kanivetsky <yuri.kanivetsky@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache-tree.c          |  4 ++--
 t/t2203-add-intent.sh | 17 +++++++++++++++++
 2 files changed, 19 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..24aed2e 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -82,5 +82,22 @@ 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 &&
+		for f in 1 2/1 2/2 3
+		do
+			echo "$f" >"$f"
+		done &&
+		git add 1 2/2 3 &&
+		git add -N 2/1 &&
+		git commit -m committed &&
+		git ls-tree -r HEAD >actual &&
+		grep 2/2 actual
+	)
+'
+
 test_done

-- 
2.9.1.566.gbd532d4

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v4 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries
  2016-07-16  5:06       ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
                           ` (2 preceding siblings ...)
  2016-07-16  5:06         ` [PATCH v4 3/4] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
@ 2016-07-16  5:06         ` Nguyễn Thái Ngọc Duy
  2016-07-18 20:48         ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Junio C Hamano
  4 siblings, 0 replies; 30+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-16  5:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, yuri.kanivetsky, Nguyễn Thái Ngọc Duy

If a subdirectory contains nothing but i-t-a entries, we generate an
empty tree object and add it to its parent tree. Which is wrong. Such
a subdirectory should not be added.

Note that this has a cascading effect. If subdir 'a/b/c' contains
nothing but i-t-a entries, we ignore it. But then if 'a/b' contains
only (the non-existing) 'a/b/c', then we should ignore 'a/b' while
building 'a' too. And it goes all the way up to top directory.

Noticed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache-tree.c          | 10 +++++++++-
 t/t2203-add-intent.sh | 14 ++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/cache-tree.c b/cache-tree.c
index c2676e8..f28b1f4 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -325,6 +325,7 @@ static int update_one(struct cache_tree *it,
 		const unsigned char *sha1;
 		unsigned mode;
 		int expected_missing = 0;
+		int contains_ita = 0;
 
 		path = ce->name;
 		pathlen = ce_namelen(ce);
@@ -341,7 +342,8 @@ static int update_one(struct cache_tree *it,
 			i += sub->count;
 			sha1 = sub->cache_tree->sha1;
 			mode = S_IFDIR;
-			if (sub->cache_tree->entry_count < 0) {
+			contains_ita = sub->cache_tree->entry_count < 0;
+			if (contains_ita) {
 				to_invalidate = 1;
 				expected_missing = 1;
 			}
@@ -380,6 +382,12 @@ static int update_one(struct cache_tree *it,
 			continue;
 		}
 
+		/*
+		 * "sub" can be an empty tree if all subentries are i-t-a.
+		 */
+		if (contains_ita && !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
+			continue;
+
 		strbuf_grow(&buffer, entlen + 100);
 		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
 		strbuf_add(&buffer, sha1, 20);
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 24aed2e..8f22c43 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -99,5 +99,19 @@ 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 &&
+		git write-tree >actual &&
+		echo $EMPTY_TREE >expected &&
+		test_cmp expected actual
+	)
+'
+
 test_done
 
-- 
2.9.1.566.gbd532d4


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/4] cache-tree building fix in the presence of ita entries
  2016-07-16  5:06       ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
                           ` (3 preceding siblings ...)
  2016-07-16  5:06         ` [PATCH v4 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
@ 2016-07-18 20:48         ` Junio C Hamano
  4 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2016-07-18 20:48 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, yuri.kanivetsky

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

> v4 removes the leading underscore from _EMPTY_BLOB and _EMPTY_TREE and
> updates 4/4 slightly like this.
>
> diff --git a/cache-tree.c b/cache-tree.c
> index 2d50640..f28b1f4 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -325,6 +325,7 @@ static int update_one(struct cache_tree *it,
>  		const unsigned char *sha1;
>  		unsigned mode;
>  		int expected_missing = 0;
> +		int contains_ita = 0;
>  
>  		path = ce->name;
>  		pathlen = ce_namelen(ce);
> @@ -341,7 +342,8 @@ static int update_one(struct cache_tree *it,
>  			i += sub->count;
>  			sha1 = sub->cache_tree->sha1;
>  			mode = S_IFDIR;
> -			if (sub->cache_tree->entry_count < 0) {
> +			contains_ita = sub->cache_tree->entry_count < 0;
> +			if (contains_ita) {
>  				to_invalidate = 1;
>  				expected_missing = 1;
>  			}
> @@ -381,10 +383,9 @@ static int update_one(struct cache_tree *it,
>  		}
>  
>  		/*
> -		 * "sub" can be an empty tree if subentries are i-t-a.
> +		 * "sub" can be an empty tree if all subentries are i-t-a.
>  		 */
> -		if (sub && sub->cache_tree->entry_count < 0 &&
> -		    !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
> +		if (contains_ita && !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
>  			continue;
>  
>  		strbuf_grow(&buffer, entlen + 100);

This makes quite a lot of sense; even though I do not think it would
change the behaviour within the context of current code, it
definitely is easier to understand and prevents future mistakes.

Will queue.

Thanks.

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2016-07-18 20:48 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-04 13:50 Git doesn't always add files to a commit (amend) Yuri Kanivetsky
2016-07-04 15:48 ` Duy Nguyen
2016-07-04 17:48 ` [PATCH] cache-tree.c: fix i-t-a check skipping directory updates sometimes Nguyễn Thái Ngọc Duy
2016-07-06 17:43   ` Junio C Hamano
2016-07-06 17:55     ` Junio C Hamano
2016-07-06 18:48   ` [PATCH v2 0/2] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
2016-07-06 18:48     ` [PATCH v2 1/2] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
2016-07-06 18:48     ` [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
2016-07-06 19:26       ` Junio C Hamano
2016-07-07 17:12         ` Duy Nguyen
2016-07-07 18:52           ` Junio C Hamano
2016-07-08 15:39             ` Duy Nguyen
2016-07-08 15:53               ` Junio C Hamano
2016-07-08 16:36                 ` Duy Nguyen
2016-07-08 17:23                   ` Junio C Hamano
2016-07-09  5:23     ` [PATCH v3 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
2016-07-09  5:23       ` [PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE Nguyễn Thái Ngọc Duy
2016-07-12 20:40         ` Junio C Hamano
2016-07-13 15:04           ` Duy Nguyen
2016-07-09  5:23       ` [PATCH v3 2/4] test-lib.sh: introduce and use $_EMPTY_BLOB Nguyễn Thái Ngọc Duy
2016-07-09  5:23       ` [PATCH v3 3/4] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
2016-07-09  5:23       ` [PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
2016-07-12 20:49         ` Junio C Hamano
2016-07-13 14:59           ` Duy Nguyen
2016-07-16  5:06       ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Nguyễn Thái Ngọc Duy
2016-07-16  5:06         ` [PATCH v4 1/4] test-lib.sh: introduce and use $EMPTY_TREE Nguyễn Thái Ngọc Duy
2016-07-16  5:06         ` [PATCH v4 2/4] test-lib.sh: introduce and use $EMPTY_BLOB Nguyễn Thái Ngọc Duy
2016-07-16  5:06         ` [PATCH v4 3/4] cache-tree.c: fix i-t-a entry skipping directory updates sometimes Nguyễn Thái Ngọc Duy
2016-07-16  5:06         ` [PATCH v4 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries Nguyễn Thái Ngọc Duy
2016-07-18 20:48         ` [PATCH v4 0/4] cache-tree building fix in the presence of ita entries Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.