Re: [PATCHv2] git-mv: Keep moved index entries inact

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Mon, 28 Jul 2008, SZEDER Gábor wrote:
>
>> there is a race somewhere in these 'git-mv: Keep moved index entries
>> inact' changes.
>> 
>> The test cases 'git mv should overwrite symlink to a file' or 'git mv
>> should overwrite file with a symlink' fail occasionaly.  It's quite
>> non-deterministic:  I have run t7001-mv.sh in a loop (see below) and
>> one or the other usually fails around 50 runs (but sometimes only
>> after 150).  Adding some tracing echos to the tests shows that both
>> tests fail when running 'git diff-files' at the end.
>
> To make it more convenient to test: with this patch it fails all the time:

It's because we rename(2) but do not read back ctime, and reuse the cached
data from the old path that was renamed.  After the failed test that moves
a regular file "move" to "symlink":

$ stat symlink
  File: `symlink'
  Size: 2               Blocks: 8          IO Block: 4096   regular file
Device: 30ah/778d       Inode: 18104337    Links: 1
Access: (0664/-rw-rw-r--)  Uid: ( 1012/   junio)   Gid: (   40/     src)
Access: 2008-07-28 11:49:55.000000000 -0700
Modify: 2008-07-28 11:48:41.000000000 -0700
Change: 2008-07-28 11:48:42.000000000 -0700

But the cached stat information looks like this:

$ ../../git-ls-files --stat
ctime=1217270921, mtime=1217270921, ino=18104337, mode=100644, uid=1012, gid=40symlink

We need to refresh the entry to pick up potential ctime changes.


 read-cache.c       |    7 ++++++-
 builtin-ls-files.c |   21 +++++++++++++++------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 1cae361..834096f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -40,7 +40,7 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
 
 void rename_index_entry_at(struct index_state *istate, int nr, const char *new_name)
 {
-	struct cache_entry *old = istate->cache[nr], *new;
+	struct cache_entry *old = istate->cache[nr], *new, *refreshed;
 	int namelen = strlen(new_name);
 
 	new = xmalloc(cache_entry_size(namelen));
@@ -51,6 +51,11 @@ void rename_index_entry_at(struct index_state *istate, int nr, const char *new_n
 
 	cache_tree_invalidate_path(istate->cache_tree, old->name);
 	remove_index_entry_at(istate, nr);
+
+	/* the renaming could have smudged the ctime */
+	refreshed = refresh_cache_entry(new, 0);
+	if (refreshed && refreshed != new)
+		new = refreshed;
 	add_index_entry(istate, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
 }
 
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index e8d568e..a6b30c8 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -16,6 +16,7 @@ static int show_deleted;
 static int show_cached;
 static int show_others;
 static int show_stage;
+static int show_stat;
 static int show_unmerged;
 static int show_modified;
 static int show_killed;
@@ -205,16 +206,20 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 		tag = alttag;
 	}
 
-	if (!show_stage) {
-		fputs(tag, stdout);
-	} else {
+	if (show_stage)
 		printf("%s%06o %s %d\t",
 		       tag,
 		       ce->ce_mode,
 		       abbrev ? find_unique_abbrev(ce->sha1,abbrev)
 				: sha1_to_hex(ce->sha1),
 		       ce_stage(ce));
-	}
+	else if (show_stat)
+		printf("ctime=%u, mtime=%u, ino=%u, mode=%o, uid=%u, gid=%u\t",
+		       ce->ce_ctime, ce->ce_mtime, ce->ce_ino,
+		       ce->ce_mode, ce->ce_uid, ce->ce_gid);
+
+	else
+		fputs(tag, stdout);
 	write_name_quoted(ce->name + offset, stdout, line_terminator);
 }
 
@@ -235,7 +240,7 @@ static void show_files(struct dir_struct *dir, const char *prefix)
 		if (show_killed)
 			show_killed_files(dir);
 	}
-	if (show_cached | show_stage) {
+	if (show_cached | show_stage | show_stat) {
 		for (i = 0; i < active_nr; i++) {
 			struct cache_entry *ce = active_cache[i];
 			int dtype = ce_to_dtype(ce);
@@ -488,6 +493,10 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			show_stage = 1;
 			continue;
 		}
+		if (!strcmp(arg, "-S") || !strcmp(arg, "--stat")) {
+			show_stat = 1;
+			continue;
+		}
 		if (!strcmp(arg, "-k") || !strcmp(arg, "--killed")) {
 			show_killed = 1;
 			require_work_tree = 1;
@@ -593,7 +602,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 
 	/* With no flags, we default to showing the cached files */
 	if (!(show_stage | show_deleted | show_others | show_unmerged |
-	      show_killed | show_modified))
+	      show_killed | show_modified | show_stat))
 		show_cached = 1;
 
 	read_cache();