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

[PATCH v4 0/7] mv: fix out-of-cone file/directory move logic

From
Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
Date
Jun 23, 2022, 11:41 UTC
Message-ID
<20220623114120.12768-1-shaoxuan.yuan02@gmail.com>
In-Reply-To
<20220331091755.385961-1-shaoxuan.yuan02@gmail.com>
## Changes since WIP v3 ##
1. Fix style-nits.
2. Move "mv: update sparsity after moving from out-of-cone to in-cone" to the 
   (2/7) position (was (7/7) position in WIP v3). Make this move so that we
   can drop "git sparse-checkout reapply" from the tests, as suggested by
   Victoria [1]. The reason is we need to check out the moved cache_entry
   for all out-of-cone to in-cone moves, so this commit works better if being the
   first one.
3. Fix the commit message of "mv: use flags mode for update_mode", as suggested
   here [2].
4. Add "Helped-by" and "Suggested-by" trailers.
5. In "mv: add check_dir_in_index() and solve general dir check issue", change
   the 'check_dir_in_index()' to no more accept 'namelen' as argument. The
   original "namelen + 1" logic can be erroneous, for example when 'name' already
   has a trailing slash. So just use strlen() to save the trouble.
## Notes ##

As discussed in WIP v3, we can postpone the "in-cone to out-of-cone" move to a later series. Hence, this series' functionality part has been done, remove the WIP prefix.

[1] https://lore.kernel.org/git/adb795ba-56ce-8441-0c38-a3e6b0a6e861@github.com/ [2] https://lore.kernel.org/git/01b39c63-5652-4293-0424-ff99b6f9f7d2@github.com/

Shaoxuan Yuan (7):
  t7002: add tests for moving out-of-cone file/directory
  mv: update sparsity after moving from out-of-cone to in-cone
  mv: decouple if/else-if checks using goto
  mv: check if out-of-cone file exists in index with SKIP_WORKTREE bit
  mv: check if <destination> exists in index to handle overwriting
  mv: use flags mode for update_mode
  mv: add check_dir_in_index() and solve general dir check issue
 builtin/mv.c                  | 239 +++++++++++++++++++++++++---------
 t/t7002-mv-sparse-checkout.sh |  84 ++++++++++++
 2 files changed, 259 insertions(+), 64 deletions(-)
Range-diff against v3:
1:  574cbdfdb4 ! 1:  90c38f479b t7002: add tests for moving out-of-cone file/directory
    @@ t/t7002-mv-sparse-checkout.sh: test_expect_success 'refuse to move file to non-s
     +	test_when_finished "cleanup_sparse_checkout" &&
     +	setup_sparse_checkout &&
     +
    -+	git mv --sparse folder1 sub 1>actual 2>stderr &&
    ++	git mv --sparse folder1 sub 2>stderr &&
     +	test_must_be_empty stderr &&
     +
    -+	git sparse-checkout reapply &&
     +	test_path_is_dir sub/folder1 &&
     +	test_path_is_file sub/folder1/file1
     +'
    @@ t/t7002-mv-sparse-checkout.sh: test_expect_success 'refuse to move file to non-s
     +	test_when_finished "cleanup_sparse_checkout" &&
     +	setup_sparse_checkout &&
     +
    -+	git mv --sparse folder1/file1 sub 1>actual 2>stderr &&
    ++	git mv --sparse folder1/file1 sub 2>stderr &&
     +	test_must_be_empty stderr &&
     +
    -+	git sparse-checkout reapply &&
    -+	! test_path_is_dir sub/folder1 &&
     +	test_path_is_file sub/file1
     +'
     +
7:  6fa630203b ! 2:  c6fcaf8313 mv: update sparsity after moving from out-of-cone to in-cone
    @@ Commit message
         2. The moved cache entry is checked out in the working tree to reflect
            the updated sparsity.
     
    +    Helped-by: Victoria Dye <vdye@github.com>
         Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
     
      ## builtin/mv.c ##
    @@ builtin/mv.c: int cmd_mv(int argc, const char **argv, const char *prefix)
      		assert(pos >= 0);
      		rename_cache_entry_at(pos, dst);
     +
    -+		if (mode & SPARSE) {
    -+			if (path_in_sparse_checkout(dst, &the_index)) {
    -+				int dst_pos;
    ++		if ((mode & SPARSE) &&
    ++		    (path_in_sparse_checkout(dst, &the_index))) {
    ++			int dst_pos;
     +
    -+				dst_pos = cache_name_pos(dst, strlen(dst));
    -+				active_cache[dst_pos]->ce_flags &= ~CE_SKIP_WORKTREE;
    ++			dst_pos = cache_name_pos(dst, strlen(dst));
    ++			active_cache[dst_pos]->ce_flags &= ~CE_SKIP_WORKTREE;
     +
    -+				if (checkout_entry(active_cache[dst_pos], &state, NULL, NULL))
    -+					die(_("cannot checkout %s"), ce->name);
    -+			}
    ++			if (checkout_entry(active_cache[dst_pos], &state, NULL, NULL))
    ++				die(_("cannot checkout %s"), ce->name);
     +		}
      	}
      
      	if (gitmodules_modified)
    -
    - ## t/t7002-mv-sparse-checkout.sh ##
    -@@ t/t7002-mv-sparse-checkout.sh: test_expect_success 'can move out-of-cone directory with --sparse' '
    - 	git mv --sparse folder1 sub 1>actual 2>stderr &&
    - 	test_must_be_empty stderr &&
    - 
    --	git sparse-checkout reapply &&
    - 	test_path_is_dir sub/folder1 &&
    - 	test_path_is_file sub/folder1/file1
    - '
    -@@ t/t7002-mv-sparse-checkout.sh: test_expect_success 'can move out-of-cone file with --sparse' '
    - 	git mv --sparse folder1/file1 sub 1>actual 2>stderr &&
    - 	test_must_be_empty stderr &&
    - 
    --	git sparse-checkout reapply &&
    - 	! test_path_is_dir sub/folder1 &&
    - 	test_path_is_file sub/file1
    - '
    -@@ t/t7002-mv-sparse-checkout.sh: test_expect_success 'refuse to move sparse file to existing destination' '
    - 	test_cmp expect stderr
    - '
    - 
    --test_expect_failure 'move sparse file to existing destination with --force and --sparse' '
    -+test_expect_success 'move sparse file to existing destination with --force and --sparse' '
    - 	test_when_finished "cleanup_sparse_checkout" &&
    - 	mkdir folder1 &&
    - 	touch folder1/file1 &&
2:  aac267091b ! 3:  aee896a3f2 mv: decouple if/else-if checks using goto
    @@ Commit message
         Refactor to decouple this if/else-if chain by using goto to jump ahead.
     
         Suggested-by: Derrick Stolee <derrickstolee@github.com>
    +    Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
     
      ## builtin/mv.c ##
     @@ builtin/mv.c: int cmd_mv(int argc, const char **argv, const char *prefix)
3:  b0e90cfa01 = 4:  82c10486ec mv: check if out-of-cone file exists in index with SKIP_WORKTREE bit
4:  c0c9dc9575 ! 5:  59dcf1a55f mv: check if <destination> exists in index to handle overwriting
    @@ t/t7002-mv-sparse-checkout.sh: test_expect_success 'can move out-of-cone file wi
      	test_when_finished "cleanup_sparse_checkout" &&
      	mkdir folder1 &&
      	touch folder1/file1 &&
    +@@ t/t7002-mv-sparse-checkout.sh: test_expect_failure 'refuse to move sparse file to existing destination' '
    + 	test_cmp expect stderr
    + '
    + 
    +-test_expect_failure 'move sparse file to existing destination with --force and --sparse' '
    ++test_expect_success 'move sparse file to existing destination with --force and --sparse' '
    + 	test_when_finished "cleanup_sparse_checkout" &&
    + 	mkdir folder1 &&
    + 	touch folder1/file1 &&
5:  3088724e72 ! 6:  b8e3094178 mv: use flags mode for update_mode
    @@ Commit message
         of the file and make it use "flags" mode (each state is a different
         bit in the word).
     
    +    Change the flag assignments from '=' (single assignment) to '|='
    +    (additive). Also change flag evaluation from '==' to '&', etc.
    +
         [1] https://lore.kernel.org/git/22aadea2-9330-aa9e-7b6a-834585189144@github.com/
     
    +    Helped-by: Victoria Dye <vdye@github.com>
    +    Helped-by: Derrick Stolee <derrickstolee@github.com>
         Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
     
      ## builtin/mv.c ##
    @@ builtin/mv.c: static const char * const builtin_mv_usage[] = {
     +	WORKING_DIRECTORY = (1 << 1),
     +	INDEX = (1 << 2),
     +	SPARSE = (1 << 3),
    -+	SKIP_WORKTREE_DIR = (1 << 4),
     +};
     +
      #define DUP_BASENAME 1
    @@ builtin/mv.c: int cmd_mv(int argc, const char **argv, const char *prefix)
      		if (show_only)
      			continue;
     -		if (mode != INDEX && mode != SPARSE && rename(src, dst) < 0) {
    -+		if (!(mode & (INDEX | SPARSE | SKIP_WORKTREE_DIR)) &&
    -+			rename(src, dst) < 0) {
    ++		if (!(mode & (INDEX | SPARSE)) &&
    ++		    rename(src, dst) < 0) {
      			if (ignore_errors)
      				continue;
      			die_errno(_("renaming '%s' failed"), src);
    @@ builtin/mv.c: int cmd_mv(int argc, const char **argv, const char *prefix)
      		}
      
     -		if (mode == WORKING_DIRECTORY)
    -+		if (mode & (WORKING_DIRECTORY | SKIP_WORKTREE_DIR))
    ++		if (mode & (WORKING_DIRECTORY))
      			continue;
      
      		pos = cache_name_pos(src, strlen(src));
6:  0c7ba28ddc ! 7:  080f34ee01 mv: add check_dir_in_index() and solve general dir check issue
    @@ Commit message
         instead of "bad source"; also user now can supply a "--sparse" flag so
         this operation can be carried out successfully.
     
    +    Helped-by: Victoria Dye <vdye@github.com>
    +    Helped-by: Derrick Stolee <derrickstolee@github.com>
         Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
     
      ## builtin/mv.c ##
    +@@ builtin/mv.c: enum update_mode {
    + 	WORKING_DIRECTORY = (1 << 1),
    + 	INDEX = (1 << 2),
    + 	SPARSE = (1 << 3),
    ++	SKIP_WORKTREE_DIR = (1 << 4),
    + };
    + 
    + #define DUP_BASENAME 1
     @@ builtin/mv.c: static int index_range_of_same_dir(const char *src, int length,
      	return last - first;
      }
    @@ builtin/mv.c: static int index_range_of_same_dir(const char *src, int length,
     + * marked with CE_SKIP_WORKTREE, the directory would be present in working tree).
     + * Return 1 otherwise.
     + */
    -+static int check_dir_in_index(const char *name, int namelen)
    ++static int check_dir_in_index(const char *name)
     +{
    -+	int ret = 1;
     +	const char *with_slash = add_slash(name);
    -+	int length = namelen + 1;
    ++	int length = strlen(with_slash);
     +
     +	int pos = cache_name_pos(with_slash, length);
     +	const struct cache_entry *ce;
    @@ builtin/mv.c: static int index_range_of_same_dir(const char *src, int length,
     +	if (pos < 0) {
     +		pos = -pos - 1;
     +		if (pos >= the_index.cache_nr)
    -+			return ret;
    ++			return 1;
     +		ce = active_cache[pos];
     +		if (strncmp(with_slash, ce->name, length))
    -+			return ret;
    ++			return 1;
     +		if (ce_skip_worktree(ce))
    -+			return ret = 0;
    ++			return 0;
     +	}
    -+	return ret;
    ++	return 1;
     +}
     +
      int cmd_mv(int argc, const char **argv, const char *prefix)
    @@ builtin/mv.c: int cmd_mv(int argc, const char **argv, const char *prefix)
      			pos = cache_name_pos(src, length);
      			if (pos < 0) {
     +				const char *src_w_slash = add_slash(src);
    -+				if (!check_dir_in_index(src, length) &&
    -+					!path_in_sparse_checkout(src_w_slash, &the_index)) {
    ++				if (!path_in_sparse_checkout(src_w_slash, &the_index) &&
    ++				    !check_dir_in_index(src)) {
     +					modes[i] |= SKIP_WORKTREE_DIR;
     +					goto dir_check;
     +				}
    @@ builtin/mv.c: int cmd_mv(int argc, const char **argv, const char *prefix)
     +dir_check:
     +		if (S_ISDIR(st.st_mode)) {
      			int j, dst_len, n;
    --			int first = cache_name_pos(src, length), last;
    -+			int first, last;
    -+			first = cache_name_pos(src, length);
    + 			int first = cache_name_pos(src, length), last;
    + 
    +@@ builtin/mv.c: int cmd_mv(int argc, const char **argv, const char *prefix)
    + 			printf(_("Renaming %s to %s\n"), src, dst);
    + 		if (show_only)
    + 			continue;
    +-		if (!(mode & (INDEX | SPARSE)) &&
    ++		if (!(mode & (INDEX | SPARSE | SKIP_WORKTREE_DIR)) &&
    + 		    rename(src, dst) < 0) {
    + 			if (ignore_errors)
    + 				continue;
    +@@ builtin/mv.c: int cmd_mv(int argc, const char **argv, const char *prefix)
    + 							      1);
    + 		}
    + 
    +-		if (mode & (WORKING_DIRECTORY))
    ++		if (mode & (WORKING_DIRECTORY | SKIP_WORKTREE_DIR))
    + 			continue;
      
    - 			if (first >= 0) {
    - 				prepare_move_submodule(src, first,
    + 		pos = cache_name_pos(src, strlen(src));
     
      ## t/t7002-mv-sparse-checkout.sh ##
     @@ t/t7002-mv-sparse-checkout.sh: test_expect_success 'refuse to move file to non-skip-worktree sparse path' '
base-commit: ddbc07872e86265dc30aefa5f4d881f762120044
-- 
2.35.1
Previous: Shaoxuan YuanNext: Shaoxuan Yuan
Message 68 of 95 in “[WIP v1 0/4] mv: fix out-of-cone file/directory move logic”
  1. Shaoxuan YuanMar 31, 2022
  2. 1/4 mv: check if out-of-cone file exists in index with SKIP_WORKTREE bitShaoxuan Yuan, Mar 31, 2022
  3. Victoria DyeMar 31, 2022
  4. Derrick StoleeApr 1, 2022
  5. 2/4 mv: add check_dir_in_index() and solve general dir check issueShaoxuan Yuan, Mar 31, 2022
  6. Ævar Arnfjörð BjarmasonMar 31, 2022
  7. Shaoxuan YuanApr 1, 2022
  8. Victoria DyeMar 31, 2022
  9. Shaoxuan YuanApr 1, 2022
  10. Derrick StoleeApr 1, 2022
  11. Shaoxuan YuanApr 4, 2022
  12. Shaoxuan YuanApr 4, 2022
  13. Derrick StoleeApr 4, 2022
  14. 3/4 mv: add advise_to_reapply hint for moving file into coneShaoxuan Yuan, Mar 31, 2022
  15. Ævar Arnfjörð BjarmasonMar 31, 2022
  16. Shaoxuan YuanApr 1, 2022
  17. Ævar Arnfjörð BjarmasonApr 1, 2022
  18. Eric SunshineApr 3, 2022
  19. Victoria DyeMar 31, 2022
  20. Derrick StoleeApr 1, 2022
  21. 4/4 t7002: add tests for moving out-of-cone file/directoryShaoxuan Yuan, Mar 31, 2022
  22. Ævar Arnfjörð BjarmasonMar 31, 2022
  23. Victoria DyeMar 31, 2022
  24. Shaoxuan YuanMar 31, 2022
  25. Victoria DyeMar 31, 2022
  26. Shaoxuan YuanApr 1, 2022
  27. Shaoxuan YuanApr 8, 2022
  28. 0/5 mv: fix out-of-cone file/directory move logicShaoxuan Yuan, May 27, 2022
  29. 1/5 t7002: add tests for moving out-of-cone file/directoryShaoxuan Yuan, May 27, 2022
  30. Ævar Arnfjörð BjarmasonMay 27, 2022
  31. Derrick StoleeMay 27, 2022
  32. Victoria DyeMay 27, 2022
  33. 2/5 mv: check if out-of-cone file exists in index with SKIP_WORKTREE bitShaoxuan Yuan, May 27, 2022
  34. Derrick StoleeMay 27, 2022
  35. Victoria DyeMay 27, 2022
  36. Shaoxuan YuanMay 31, 2022
  37. 3/5 mv: check if <destination> exists in index to handle overwritingShaoxuan Yuan, May 27, 2022
  38. Victoria DyeMay 27, 2022
  39. 4/5 mv: add check_dir_in_index() and solve general dir check issueShaoxuan Yuan, May 27, 2022
  40. Derrick StoleeMay 27, 2022
  41. Shaoxuan YuanMay 31, 2022
  42. Derrick StoleeMay 31, 2022
  43. 5/5 mv: use update_sparsity() after touching sparse contentsShaoxuan Yuan, May 27, 2022
  44. Ævar Arnfjörð BjarmasonMay 27, 2022
  45. Victoria DyeMay 27, 2022
  46. Junio C HamanoMay 27, 2022
  47. Victoria DyeMay 27, 2022
  48. Shaoxuan YuanJun 16, 2022
  49. Victoria DyeJun 16, 2022
  50. Shaoxuan YuanJun 17, 2022
  51. 0/7 mv: fix out-of-cone file/directory move logicShaoxuan Yuan, Jun 19, 2022
  52. 1/7 t7002: add tests for moving out-of-cone file/directoryShaoxuan Yuan, Jun 19, 2022
  53. Victoria DyeJun 21, 2022
  54. 2/7 mv: decouple if/else-if checks using gotoShaoxuan Yuan, Jun 19, 2022
  55. 3/7 mv: check if out-of-cone file exists in index with SKIP_WORKTREE bitShaoxuan Yuan, Jun 19, 2022
  56. 4/7 mv: check if <destination> exists in index to handle overwritingShaoxuan Yuan, Jun 19, 2022
  57. 5/7 mv: use flags mode for update_modeShaoxuan Yuan, Jun 19, 2022
  58. Victoria DyeJun 21, 2022
  59. Shaoxuan YuanJun 22, 2022
  60. 6/7 mv: add check_dir_in_index() and solve general dir check issueShaoxuan Yuan, Jun 19, 2022
  61. Victoria DyeJun 21, 2022
  62. 7/7 mv: update sparsity after moving from out-of-cone to in-coneShaoxuan Yuan, Jun 19, 2022
  63. Victoria DyeJun 21, 2022
  64. Victoria DyeJun 21, 2022
  65. Derrick StoleeJun 23, 2022
  66. Junio C HamanoJun 23, 2022
  67. Shaoxuan YuanJun 24, 2022
  68. 0/7 mv: fix out-of-cone file/directory move logicShaoxuan Yuan, Jun 23, 2022
  69. 1/7 t7002: add tests for moving out-of-cone file/directoryShaoxuan Yuan, Jun 23, 2022
  70. 2/7 mv: update sparsity after moving from out-of-cone to in-coneShaoxuan Yuan, Jun 23, 2022
  71. Derrick StoleeJun 23, 2022
  72. Shaoxuan YuanJun 24, 2022
  73. Derrick StoleeJun 27, 2022
  74. 3/7 mv: decouple if/else-if checks using gotoShaoxuan Yuan, Jun 23, 2022
  75. 4/7 mv: check if out-of-cone file exists in index with SKIP_WORKTREE bitShaoxuan Yuan, Jun 23, 2022
  76. 5/7 mv: check if <destination> exists in index to handle overwritingShaoxuan Yuan, Jun 23, 2022
  77. 6/7 mv: use flags mode for update_modeShaoxuan Yuan, Jun 23, 2022
  78. Derrick StoleeJun 23, 2022
  79. 7/7 mv: add check_dir_in_index() and solve general dir check issueShaoxuan Yuan, Jun 23, 2022
  80. Derrick StoleeJun 23, 2022
  81. Shaoxuan YuanJun 24, 2022
  82. Derrick StoleeJun 27, 2022
  83. Derrick StoleeJun 23, 2022
  84. Junio C HamanoJun 23, 2022
  85. 0/8 mv: fix out-of-cone file/directory move logicShaoxuan Yuan, Jun 30, 2022
  86. 1/8 t7002: add tests for moving out-of-cone file/directoryShaoxuan Yuan, Jun 30, 2022
  87. 2/8 t1092: mv directory from out-of-cone to in-coneShaoxuan Yuan, Jun 30, 2022
  88. 3/8 mv: update sparsity after moving from out-of-cone to in-coneShaoxuan Yuan, Jun 30, 2022
  89. 4/8 mv: decouple if/else-if checks using gotoShaoxuan Yuan, Jun 30, 2022
  90. 5/8 mv: check if out-of-cone file exists in index with SKIP_WORKTREE bitShaoxuan Yuan, Jun 30, 2022
  91. 6/8 mv: check if <destination> exists in index to handle overwritingShaoxuan Yuan, Jun 30, 2022
  92. 7/8 mv: use flags mode for update_modeShaoxuan Yuan, Jun 30, 2022
  93. 8/8 mv: add check_dir_in_index() and solve general dir check issueShaoxuan Yuan, Jun 30, 2022
  94. Derrick StoleeJul 1, 2022
  95. Junio C HamanoJul 1, 2022

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.