On Mon, Jul 05, 2010 at 02:28:16PM -0700, Junio C Hamano wrote:
> > Apparently not:
> >
> > $ stat .git/index | grep -i modify
> > Modify: 2010-07-05 16:52:11.000000000 -0400
> > $ git status
> > # On branch master
> > nothing to commit
> > $ stat .git/index | grep -i modify
> > Modify: 2010-07-05 16:53:09.000000000 -0400
> >
> > and it is not just updating some stat-dirtiness. Doing it over and over
> > will keep updating the index. It looks like we unconditionally do the
> > lock and write in cmd_status, but I haven't looked further.
>
> Something like this, plus possibly a similar fix to "git commit $path"
> codepath, perhaps?
Yeah, that looks sane, though the one for "git status" is actually:
diff --git a/builtin/commit.c b/builtin/commit.c
index 8258a16..5e578af 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1090,9 +1090,11 @@ int cmd_status(int argc, const char **argv, const char *prefix)
fd = hold_locked_index(&index_lock, 0);
if (0 <= fd) {
- if (!write_cache(fd, active_cache, active_nr))
+ if (active_cache_changed &&
+ !write_cache(fd, active_cache, active_nr))
commit_locked_index(&index_lock);
- rollback_lock_file(&index_lock);
+ else
+ rollback_lock_file(&index_lock);
}
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
and I did confirm that it works (does not touch the index when nothing
changes, and does write out the index even for a stat-dirty entry).
> We may want to audit all uses of write_cache() and write_index() that are
> not protected with active_cache_changed (or istate->cache_changed); I am
> reluctant to suggest placing that logic into write_index() at this point,
> though, as we may be updating the index in bulk, without marking
> active_cache_changed bit, exactly because we know we will unconditionally
> write the result out.
I don't think changing write_index() is a good idea. There are a lot of
calls, and I would rather have each one opt into this optimization than
break them in a mass change.
I was thinking that we might be able to simplify all of these lines into
a "lock, write, and commit" function that would either rollback or die
on failure, depending on a parameter. But there's often more complex
logic going on between those functions, so I don't think it would really
save us much.
-Peff
--
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