Re: git status: small difference between stating whole repository and small subdirectory

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2012, 20:08 UTC
Message-ID: <7vfwe5l13w.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120220151134.GA13135@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>   4. At the end of unpack_trees, we forget about src_index, and copy
>      o->result into *o->dst_index byte for byte. I.e., we overwrite
>      the_index.cache_tree, which has been properly updated the whole
>      time,

I strongly suspect that "properly updated" part needs to be thoroughly audited. I wouldn't be surprised that this behaviour is what we did when we split src_index vs dst_index when he rewrote unpack_trees() in order to emulate the original "unpack-trees is beyond salvation because it does not maintain cache tree correctly, just nuke it" behaviour.

Show 6 quoted lines

> But it does not actually insert the _destination_ tree into the cache
> tree. Which we can do in certain situations, but only if there were no
> paths in the tree that were left unchanged (e.g., you modify "foo", then
> "git checkout HEAD^", which updates "bar". Your tree does not match
> HEAD^, and must be invalidated).  While it would be cool to be able to
> handle those complex cases,...

It may look cool but it may not be a good change. You are spending extra cycles to optimize for the next write-tree that may not happen before the index is further updated.

> I think this implementation matches the intent of the original calls to
> cache_tree_invalidate_path sprinkled throughout unpack-trees.c.

Yes, and as long as we invalidate all the directories that need to be invalidated during the unpack-tree operation, I think it is a correct thing to do.

> But I
> have to say that it seems a little odd for us to be modifying the
> o->src_index throughout the whole thing.

Yes, that part is logically *wrong*. I think it is a remnant from the days when there was no distinction between src_index and dst_index.

> I would think the right thing
> would be to make a deep copy of o->src_index->cache_tree into
> o->result.cache_tree as the very first thing, and then update
> o->result.cache_tree throughout the tree traversal.

Yes.