Re: [PATCH] cache-tree: populate cache-tree on successful merge

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

David Turner <dturner@twopensource.com> writes:

> When we unpack trees into an existing index, we discard the old index
> and replace it with the new, merged index.  Ensure that this index has
> its cache-tree populated.  This will make subsequent git status and
> commit commands faster.
>
> Signed-off-by: David Turner <dturner@twopensource.com>
> Signed-off-by: Brian Degenhardt <bmd@bmdhacks.com>
> ---
>
> This patch is by my colleague, Brian Degenhardt (as part of his work
> on git at Twitter).  I'm sending it with his and Twitter's approval.

I'd need to tweak the From:/Author: line then, and flip the order of
the sign-off, as Brian wrote and signed off then David relayed (as
attached).

> diff --git a/unpack-trees.c b/unpack-trees.c
> index 2927660..befc247 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1156,6 +1156,13 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>  	o->src_index = NULL;
>  	ret = check_updates(o) ? (-2) : 0;
>  	if (o->dst_index) {
> +		if (!o->result.cache_tree)
> +			o->result.cache_tree = cache_tree();
> +
> +		if (!cache_tree_fully_valid(o->result.cache_tree)) {
> +			cache_tree_update(&o->result, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
> +		}

This does the cache-tree thing unconditionally, not "on successful
merge".  cache_tree_update() would refuse when it sees an unmerged
entry, but somehow the discrepancy between the title and the code
bothers me.

By the way, I wonder if we can lose/revert aecf567c (cache-tree:
create/update cache-tree on checkout, 2014-07-05), now the
underlying unpack_trees() does the necessary cache_tree_update()
when a branch is checked out.

Thanks.

-- >8 --
From: Brian Degenhardt <bmd@bmdhacks.com>
Date: Tue, 28 Jul 2015 15:30:40 -0400
Subject: [PATCH] unpack-trees: populate cache-tree on successful merge

When we unpack trees into an existing index, we discard the old
index and replace it with the new, merged index.  Ensure that this
index has its cache-tree populated.  This will make subsequent git
status and commit commands faster.

Signed-off-by: Brian Degenhardt <bmd@bmdhacks.com>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0090-cache-tree.sh | 24 ++++++++++++++++++++++++
 unpack-trees.c        |  8 ++++++++
 2 files changed, 32 insertions(+)

diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 601d02d..055cc19 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -199,6 +199,30 @@ test_expect_success 'checkout -B gives cache-tree' '
 	test_cache_tree
 '
 
+test_expect_success 'merge --ff-only maintains cache-tree' '
+	git checkout current &&
+	git checkout -b changes &&
+	test_commit llamas &&
+	test_commit pachyderm &&
+	test_cache_tree &&
+	git checkout current &&
+	test_cache_tree &&
+	git merge --ff-only changes &&
+	test_cache_tree
+'
+
+test_expect_success 'merge maintains cache-tree' '
+	git checkout current &&
+	git checkout -b changes2 &&
+	test_commit alpacas &&
+	test_cache_tree &&
+	git checkout current &&
+	test_commit struthio &&
+	test_cache_tree &&
+	git merge changes2 &&
+	test_cache_tree
+'
+
 test_expect_success 'partial commit gives cache-tree' '
 	git checkout -b partial no-children &&
 	test_commit one &&
diff --git a/unpack-trees.c b/unpack-trees.c
index be84ba2..d92f903 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1155,6 +1155,14 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	o->src_index = NULL;
 	ret = check_updates(o) ? (-2) : 0;
 	if (o->dst_index) {
+		if (!ret) {
+			if (!o->result.cache_tree)
+				o->result.cache_tree = cache_tree();
+			if (!cache_tree_fully_valid(o->result.cache_tree))
+				cache_tree_update(&o->result,
+						  WRITE_TREE_SILENT |
+						  WRITE_TREE_REPAIR);
+		}
 		discard_index(o->dst_index);
 		*o->dst_index = o->result;
 	} else {
-- 
2.5.0-370-gf964943