Re: An interaction with ce_match_stat_basic() and autocrlf
- From
Linus Torvalds <torvalds@linux-foundation.org>
- Date
- Jan 8, 2008, 16:10 UTC
- Message-ID
- <alpine.LFD.1.00.0801080748080.3148@woody.linux-foundation.org>
- In-Reply-To
- <7vfxx8tt1z.fsf@gitster.siamese.dyndns.org>
On Tue, 8 Jan 2008, Junio C Hamano wrote:
> > This is caused partly by the breakage in size_only codepath of > diff.c::diff_populate_filespec().
Only partially.
The more fundamental behaviour (that of git update-index) is caused by ie_modified() thinking that when DATA_CHANGED is true, it cannot possibly need to call "ce_modified_check_fs()":
>From ie_modified():
/* Immediately after read-tree or update-index --cacheinfo,
* the length field is zero. For other cases the ce_size
* should match the SHA1 recorded in the index entry.
*/
if ((changed & DATA_CHANGED) && ce->ce_size != htonl(0))
return changed;and that DATA_CHANGED comes from ce_match_stat_basic() which notices that the size has changed.
Similarly, I think that the problem with "diff" not realizing they might be the same comes from ie_match_stat(), which has a similar problem in not realizing that DATA_CHANGED could possibly still mean that it's the same.
This patch should fix it, but I suspect we should think hard about that change to ie_modified(), and see what the performance issues are (ie that code has tried to avoid doing the more expensive ce_modified_check_fs() for a reason).
The change to diff.c is similarly interesting. It is logically wrong to use the worktree_file there (since we have to read the object anyway), but since "reuse_worktree_file" is also tied into the whole refresh logic, I think the diff.c change is correct.
I dunno. This is not meant to be applied, it is meant to be thought about.
Linus
--- diff.c | 2 +- read-cache.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/diff.c b/diff.c index b18c140..9f699b7 100644 --- a/diff.c +++ b/diff.c @@ -1512,7 +1512,7 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int ce = active_cache[pos]; if ((lstat(name, &st) < 0) || !S_ISREG(st.st_mode) || /* careful! */ - ce_match_stat(ce, &st, 0) || + ce_modified(ce, &st, 0) || hashcmp(sha1, ce->sha1)) return 0; /* we return 1 only when we can stat, it is a regular file, diff --git a/read-cache.c b/read-cache.c index 7db5588..e1fc880 100644 --- a/read-cache.c +++ b/read-cache.c @@ -253,12 +253,14 @@ int ie_modified(struct index_state *istate, if (changed & (MODE_CHANGED | TYPE_CHANGED)) return changed; +#if 0 /* Immediately after read-tree or update-index --cacheinfo, * the length field is zero. For other cases the ce_size * should match the SHA1 recorded in the index entry. */ if ((changed & DATA_CHANGED) && ce->ce_size != htonl(0)) return changed; +#endif changed_fs = ce_modified_check_fs(ce, st); if (changed_fs)