Re: [PATCHv2] git-mv: Keep moved index entries inact
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:04
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCHv2] git-mv: Keep moved index entries inact · Johannes Schindelin <hidden>
- 2016-06-15 · Re: [PATCHv2] git-mv: Keep moved index entries inact · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCHv2] git-mv: Keep moved index entries inact · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCHv2] git-mv: Keep moved index entries inact · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCHv2] git-mv: Keep moved index entries inact · Petr Baudis <hidden>
Johannes Schindelin [off-list ref] writes:
On Mon, 28 Jul 2008, SZEDER Gábor wrote:quoted
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();