Re: 'git commit --short' without touching index?

[Jeff King <peff@peff.net>]

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