Nguyen Thai Ngoc Duy <pclouds@xxxxxxxxx> writes:
>> diff --git a/builtin/checkout.c b/builtin/checkout.c
>> index 5bf96ba..c06287a 100644
>> --- a/builtin/checkout.c
>> +++ b/builtin/checkout.c
>> @@ -319,6 +319,10 @@ static void show_local_changes(struct object *head, struct diff_options *opts)
>> die(_("diff_setup_done failed"));
>> add_pending_object(&rev, head, NULL);
>> run_diff_index(&rev, 0);
>> + if (!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES)) {
>> + struct tree *tree = parse_tree_indirect(head->sha1);
>> + prime_cache_tree(&active_cache_tree, tree);
>> + }
>> }
I think this patch is wrong on at least two counts.
* The run_diff_index(&rev, 0) you reused is doing "diff HEAD" and not
"diff --cached HEAD". The added check does not say anything about the
comparison between the index and the tree at the HEAD.
* Even if we added an extra run_diff_index(&rev, 1) there, or added a
call to index_differs_from() to run "diff --cached HEAD" to check what
needs to be checked, it is still not quite right.
On the latter point, imagine what happens in the two invocations of
checkout in the following sequence:
$ git reset --hard master
$ git checkout master
$ git checkout master
The second one should notice that the cache tree is fully valid, so the
internal "diff --cached" it runs should only open the top-level tree
and scan entries in it, without recursing into any of the subtrees, and
realize that the index is in sync with "HEAD", which should be a very
cheap operation (that is the whole point of the current topic of our
discussion looking at the cache-tree). Then the new code calls
prime_cache_tree() to read _everything_?
Probably cache_tree_fully_valid() should be called before deciding that we
need to re-populate the cache tree from "HEAD".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html