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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 5, 2010, 21:28 UTC
Message-ID: <7vk4p9wrdb.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100705205651.GA32728@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 23 quoted lines

> On Mon, Jul 05, 2010 at 01:10:43PM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > It would be nice if the index-refreshing code only wrote to the index if
>> > there was something worth writing. I'm not sure how hard that would be
>> > to implement, though.
>> 
>> Hmm, don't we already do that with "istate->cache_changed"?
>
> 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?

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.

 builtin/commit.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index c101f00..86d3926 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -343,9 +343,13 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
 	if (!pathspec || !*pathspec) {
 		fd = hold_locked_index(&index_lock, 1);
 		refresh_cache_or_die(refresh_flags);
-		if (write_cache(fd, active_cache, active_nr) ||
-		    commit_locked_index(&index_lock))
-			die("unable to write new_index file");
+		if (active_cache_changed) {
+			if (write_cache(fd, active_cache, active_nr) ||
+			    commit_locked_index(&index_lock))
+				die("unable to write new_index file");
+		} else {
+			rollback_lock_file(&index_lock);
+		}
 		commit_style = COMMIT_AS_IS;
 		return get_index_file();
 	}