git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Cc: Thomas Rast <trast@inf.ethz.ch>, Jeff King <peff@peff.net>,
	Piotr Krukowiecki <piotr.krukowiecki@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: git status: small difference between stating whole repository and small subdirectory
Date: Tue, 21 Feb 2012 19:32:49 -0800	[thread overview]
Message-ID: <7vvcmzczku.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <CACsJy8DE86qzA1=GiKZFRCt5aH8X4iMyDvfrhnqwmbq52szhHg@mail.gmail.com> (Nguyen Thai Ngoc Duy's message of "Tue, 21 Feb 2012 21:45:13 +0700")

Nguyen Thai Ngoc Duy <pclouds@gmail.com> 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".

  parent reply	other threads:[~2012-02-22  3:33 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-10  9:42 git status: small difference between stating whole repository and small subdirectory Piotr Krukowiecki
2012-02-10 12:33 ` Nguyen Thai Ngoc Duy
2012-02-10 13:46   ` Piotr Krukowiecki
2012-02-10 14:37     ` Nguyen Thai Ngoc Duy
2012-02-13 16:54       ` Piotr Krukowiecki
2012-02-10 16:18 ` Piotr Krukowiecki
2012-02-14 11:34   ` Thomas Rast
2012-02-15  8:57     ` Piotr Krukowiecki
2012-02-15 11:01       ` Nguyen Thai Ngoc Duy
2012-02-15 15:14         ` Piotr Krukowiecki
2012-02-16 13:22           ` Piotr Krukowiecki
2012-02-15 19:03       ` Jeff King
2012-02-16 13:37         ` Piotr Krukowiecki
2012-02-16 14:05           ` Thomas Rast
2012-02-16 20:15             ` Junio C Hamano
2012-02-17 16:55             ` Piotr Krukowiecki
2012-02-16 19:20           ` Jeff King
2012-02-17 17:19             ` Piotr Krukowiecki
2012-02-17 20:37               ` Jeff King
2012-02-17 22:25                 ` Junio C Hamano
2012-02-17 22:29                   ` Jeff King
2012-02-20  8:25                     ` Piotr Krukowiecki
2012-02-20 14:06                       ` Jeff King
2012-02-20 14:09                         ` Thomas Rast
2012-02-20 14:36                           ` Nguyen Thai Ngoc Duy
2012-02-20 14:39                             ` Jeff King
2012-02-20 15:11                               ` Jeff King
2012-02-20 18:45                                 ` Thomas Rast
2012-02-20 20:35                                   ` Jeff King
2012-02-20 22:04                                     ` Junio C Hamano
2012-02-20 22:41                                       ` Jeff King
2012-02-20 23:31                                         ` Junio C Hamano
2012-02-21  7:21                                           ` Piotr Krukowiecki
2012-02-20 20:08                                 ` Junio C Hamano
2012-02-20 20:17                                   ` Jeff King
2012-02-21 14:45                             ` Nguyen Thai Ngoc Duy
2012-02-21 19:16                               ` Junio C Hamano
2012-02-22  2:12                                 ` Nguyen Thai Ngoc Duy
2012-02-22  2:55                                   ` Junio C Hamano
2012-02-22 12:54                                     ` Nguyen Thai Ngoc Duy
2012-02-22 13:17                                       ` Thomas Rast
2012-02-22 10:34                                 ` Nguyen Thai Ngoc Duy
2012-02-22  3:32                               ` Junio C Hamano [this message]
2012-04-10 15:16                                 ` Piotr Krukowiecki
2012-04-10 16:23                                   ` Junio C Hamano
2012-04-10 18:00                                     ` Jeff King
2012-02-20 19:57                           ` Junio C Hamano
2012-02-20 19:59                             ` Thomas Rast
2012-02-20 14:16                         ` Nguyen Thai Ngoc Duy
2012-02-20 14:22                           ` Jeff King
2012-02-20 19:56                         ` Junio C Hamano
2012-02-20 20:09                           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vvcmzczku.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=piotr.krukowiecki@gmail.com \
    --cc=trast@inf.ethz.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).