git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 28, 2008, 19:19 UTC
Message-ID
<7vwsj5rf48.fsf@gitster.siamese.dyndns.org>
In-Reply-To
<alpine.DEB.1.00.0807281605330.8986@racer>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 13 quoted lines
> 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();
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 24 of 56 in “Submodule support in git mv, git rm”
  1. Petr BaudisJul 16, 2008
  2. 1/7 git-mv: Remove dead code branchPetr Baudis, Jul 16, 2008
  3. 2/7 t7400: Add short "git submodule add" testsuitePetr Baudis, Jul 16, 2008
  4. 3/7 git submodule add: Fix naming clash handlingPetr Baudis, Jul 16, 2008
  5. 4/7 submodule.*: Introduce simple C interface for submodule lookup by pathPetr Baudis, Jul 16, 2008
  6. 5/7 git mv: Support moving submodulesPetr Baudis, Jul 16, 2008
  7. Junio C HamanoJul 17, 2008
  8. Petr BaudisJul 17, 2008
  9. git-mv: Keep moved index entries inactPetr Baudis, Jul 17, 2008
  10. git mv: Support moving submodulesPetr Baudis, Jul 17, 2008
  11. Junio C HamanoJul 19, 2008
  12. Petr BaudisJul 21, 2008
  13. [PATCHv2] git-mv: Keep moved index entries inactPetr Baudis, Jul 21, 2008
  14. Junio C HamanoJul 21, 2008
  15. Junio C HamanoJul 26, 2008
  16. Petr BaudisJul 27, 2008
  17. t/t7001-mv.sh: Propose ability to use git-mv on conflicting entriesPetr Baudis, Jul 27, 2008
  18. Junio C HamanoJul 28, 2008
  19. Junio C HamanoJul 28, 2008
  20. SZEDER GáborJul 28, 2008
  21. Johannes SchindelinJul 28, 2008
  22. Johannes SchindelinJul 28, 2008
  23. Johannes SchindelinJul 28, 2008
  24. Junio C HamanoJul 28, 2008
  25. Johannes SchindelinJul 28, 2008
  26. Johannes SchindelinJul 28, 2008
  27. Petr BaudisJul 29, 2008
  28. Junio C HamanoJul 29, 2008
  29. Junio C HamanoJul 29, 2008
  30. Not going beyond symbolic linksJunio C Hamano, Aug 4, 2008
  31. 1/2 update-index: refuse to add working tree items beyond symlinksJunio C Hamano, Aug 4, 2008
  32. 2/2 add: refuse to add working tree items beyond symlinksJunio C Hamano, Aug 4, 2008
  33. Linus TorvaldsAug 5, 2008
  34. Junio C HamanoAug 5, 2008
  35. Linus TorvaldsAug 5, 2008
  36. Johannes SchindelinAug 5, 2008
  37. Linus TorvaldsAug 5, 2008
  38. Junio C HamanoAug 5, 2008
  39. Dmitry PotapovAug 5, 2008
  40. Junio C HamanoAug 5, 2008
  41. Linus TorvaldsAug 5, 2008
  42. Junio C HamanoAug 5, 2008
  43. Johannes SchindelinAug 5, 2008
  44. Junio C HamanoAug 5, 2008
  45. david@lang.hmAug 5, 2008
  46. Junio C HamanoAug 7, 2008
  47. Junio C HamanoAug 8, 2008
  48. Linus TorvaldsAug 8, 2008
  49. Johannes SchindelinJul 21, 2008
  50. Petr BaudisJul 21, 2008
  51. Junio C HamanoJul 21, 2008
  52. 6/7 git rm: Support for removing submodulesPetr Baudis, Jul 16, 2008
  53. Johannes SchindelinJul 16, 2008
  54. Petr BaudisJul 17, 2008
  55. Johannes SchindelinJul 17, 2008
  56. 7/7 t7403: Submodule git mv, git rm testsuitePetr Baudis, Jul 16, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.