Fix extraneous lstat's in 'git checkout -f'

[Linus Torvalds <torvalds@linux-foundation.org>]

In our 'oneway_merge()' we always do an 'lstat()' to see if we might need 
to mark the entry for updating. 

We really shouldn't need to do that when the cache entry is already marked 
as being ce_uptodate(). However, since this function will actually match 
the lstat() information with the CE_MATCH_IGNORE_VALID, we need to be a 
bit more careful: it's not sufficient to check ce_uptodate(), we need to 
also double-check that the ce_uptodate() isn't because of CE_VALID (which 
will be "uptodate" regardless of whether the lstat() matches or not).

This explains why it's not sufficient to check _just_ the CE_UPTODATE bit.

But if both CE_UPTODATE is set, and CE_VALID is clear, then we know that 
the lstat() will always match the cache entry information, and there's no 
point in doing another one.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

Quite frankly, I'd like for us to at least think about removing CE_VALID. 

Does anybody use it (and "core.ignorestat" that turns it on, along with 
the "--assume-unchanged" flag to update-index)

I wonder if we have other places where we have optimized away the lstat() 
just because we decided that it was already up-to-date - without 
realizing that something could have been marked up-to-date just because 
it was marked CE_VALID.

The original reasoning behind CE_VALID was to avoid the cost of lstat's on 
operating systems where it is slow, but we are now so good at optimizing 
them away (thanks to CE_UPTODATE) that I wonder whether it's not better to 
just always rely on CE_UPTODATE.

In fact, a quick grep made me just look at "verify_uptodate()", and how it 
does ie_match_stat(CE_MATCH_IGNORE_VALID). But look at how we've already 
short-circuited this if ce_uptodate() is true, and we never get there if 
we've preloaded the index or done any other operation that might have 
already validated the entry?

I dunno. This at least doesn't make things worse, since I started thinking 
about this _after_ having first done a patch that just did the 
"ce_uptodate()" test.

 unpack-trees.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index f9d12aa..9920a6f 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -985,6 +985,18 @@ int bind_merge(struct cache_entry **src,
 }
 
 /*
+ * Do we _know_ the stat information will match?
+ *
+ * Note that if CE_VALID is set, then we might have a
+ * uptodate cache entry even if the stat info might not
+ * match.
+ */
+static inline int known_uptodate(struct cache_entry *ce)
+{
+	return ce_uptodate(ce) && !(ce->ce_flags & CE_VALID);
+}
+
+/*
  * One-way merge.
  *
  * The rule is:
@@ -1004,7 +1016,7 @@ int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)
 
 	if (old && same(old, a)) {
 		int update = 0;
-		if (o->reset) {
+		if (o->reset && !known_uptodate(old)) {
 			struct stat st;
 			if (lstat(old->name, &st) ||
 			    ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID))