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

On Mon, Jul 05, 2010 at 02:28:16PM -0700, Junio C Hamano wrote:

quoted

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