Subject: Re: [PATCH 1/3] cache-tree: Create/update cache-tree on checkout

From: Junio C Hamano <[email-masked]>
Date: Sun, 29 Jun 2014 23:31:16 -0700

David Turner <[email-masked]> writes:

> When git checkout checks out a branch, create or update the
> cache-tree so that subsequent operations are faster.
>
> Signed-off-by: David Turner <[email-masked]>
> ---
>  builtin/checkout.c    |  4 ++++
>  cache-tree.c          | 22 ++++++++++++----------
>  cache-tree.h          |  1 +
>  t/t0090-cache-tree.sh | 15 ++++++++++++++-
>  4 files changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 07cf555..df791e8 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -553,6 +553,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
>  		}
>  	}
>  
> +	if (write_cache_as_tree(NULL, WRITE_TREE_DO_NOT_WRITE, "")) {
> +		warn("Unable to write cache_tree");
> +	}
> +

This is not even warn-worthy event, isn't it?  "checkout -m other"
would attempt a three-way merge to replay your local changes
relative to your current HEAD in the context of "other" branch and
will leave conflicts in the index and in the working tree, and it is
perfectly a normal thing that you cannot write the index in such a
state as a tree object.  Perhaps you should check if the index is
unmerged before even attempting to compute the cache tree.

Also it looks very strange that write-cache-as-tree, whose *PRIMARY*
function is to write a tree, receives a "DO NOT WRITE" option here.
I understand that you want a new interface into the cache-tree
subsystem that only updates the cache-tree but it somehow smells
like a sloppy/lazy refactoring that is not done quite right.