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)
-
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
