From 367844e5b747883f4456fd17201da6e6f5320f4a Mon Sep 17 00:00:00 2001 From: Shaoxuan Yuan Date: Thu, 30 Jun 2022 10:37:30 +0800 Subject: [PATCH 1/8] t7002: add tests for moving out-of-cone file/directory Add corresponding tests to test following situations: We do not have sufficient coverage of moving files outside of a sparse-checkout cone. Create new tests covering this behavior, keeping in mind that the user can include --sparse (or not), move a file or directory, and the destination can already exist in the index (in this case user can use --force to overwrite existing entry). Helped-by: Victoria Dye Helped-by: Derrick Stolee Signed-off-by: Shaoxuan Yuan Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- t/t7002-mv-sparse-checkout.sh | 84 +++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh index f0f7cbfcdb..023e657c9e 100755 --- a/t/t7002-mv-sparse-checkout.sh +++ b/t/t7002-mv-sparse-checkout.sh @@ -4,6 +4,18 @@ test_description='git mv in sparse working trees' . ./test-lib.sh +setup_sparse_checkout () { + mkdir folder1 && + touch folder1/file1 && + git add folder1 && + git sparse-checkout set --cone sub +} + +cleanup_sparse_checkout () { + git sparse-checkout disable && + git reset --hard +} + test_expect_success 'setup' " mkdir -p sub/dir sub/dir2 && touch a b c sub/d sub/dir/e sub/dir2/e && @@ -196,6 +208,7 @@ test_expect_success 'can move files to non-sparse dir' ' ' test_expect_success 'refuse to move file to non-skip-worktree sparse path' ' + test_when_finished "cleanup_sparse_checkout" && git reset --hard && git sparse-checkout init --no-cone && git sparse-checkout set a !/x y/ !x/y/z && @@ -206,4 +219,75 @@ test_expect_success 'refuse to move file to non-skip-worktree sparse path' ' test_cmp expect stderr ' +test_expect_failure 'refuse to move out-of-cone directory without --sparse' ' + test_when_finished "cleanup_sparse_checkout" && + setup_sparse_checkout && + + test_must_fail git mv folder1 sub 2>stderr && + cat sparse_error_header >expect && + echo folder1/file1 >>expect && + cat sparse_hint >>expect && + test_cmp expect stderr +' + +test_expect_failure 'can move out-of-cone directory with --sparse' ' + test_when_finished "cleanup_sparse_checkout" && + setup_sparse_checkout && + + git mv --sparse folder1 sub 2>stderr && + test_must_be_empty stderr && + + test_path_is_dir sub/folder1 && + test_path_is_file sub/folder1/file1 +' + +test_expect_failure 'refuse to move out-of-cone file without --sparse' ' + test_when_finished "cleanup_sparse_checkout" && + setup_sparse_checkout && + + test_must_fail git mv folder1/file1 sub 2>stderr && + cat sparse_error_header >expect && + echo folder1/file1 >>expect && + cat sparse_hint >>expect && + test_cmp expect stderr +' + +test_expect_failure 'can move out-of-cone file with --sparse' ' + test_when_finished "cleanup_sparse_checkout" && + setup_sparse_checkout && + + git mv --sparse folder1/file1 sub 2>stderr && + test_must_be_empty stderr && + + test_path_is_file sub/file1 +' + +test_expect_failure 'refuse to move sparse file to existing destination' ' + test_when_finished "cleanup_sparse_checkout" && + mkdir folder1 && + touch folder1/file1 && + touch sub/file1 && + git add folder1 sub/file1 && + git sparse-checkout set --cone sub && + + test_must_fail git mv --sparse folder1/file1 sub 2>stderr && + echo "fatal: destination exists, source=folder1/file1, destination=sub/file1" >expect && + test_cmp expect stderr +' + +test_expect_failure 'move sparse file to existing destination with --force and --sparse' ' + test_when_finished "cleanup_sparse_checkout" && + mkdir folder1 && + touch folder1/file1 && + touch sub/file1 && + echo "overwrite" >folder1/file1 && + git add folder1 sub/file1 && + git sparse-checkout set --cone sub && + + git mv --sparse --force folder1/file1 sub 2>stderr && + test_must_be_empty stderr && + echo "overwrite" >expect && + test_cmp expect sub/file1 +' + test_done From 1143cc01b722225b041eb5db0bd733bdb4549949 Mon Sep 17 00:00:00 2001 From: Shaoxuan Yuan Date: Thu, 30 Jun 2022 10:37:31 +0800 Subject: [PATCH 2/8] t1092: mv directory from out-of-cone to in-cone Add test for "mv: add check_dir_in_index() and solve general dir check issue" in this series. This change tests the following: 1. mv as a directory on the sparse index boundary (where it would be a sparse directory in a sparse index). 2. mv as a directory which is deeper than the boundary (so the sparse index would expand in the cache_name_pos() method). These tests can be written now for correctness, but later the first case can be updated to use the 'ensure_not_expanded' helper in t1092. Suggested-by: Derrick Stolee Signed-off-by: Shaoxuan Yuan Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- t/t1092-sparse-checkout-compatibility.sh | 25 ++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index f9f8c988bb..5eef799e25 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1828,4 +1828,29 @@ test_expect_success 'checkout behaves oddly with df-conflict-2' ' test_cmp full-checkout-err sparse-index-err ' +test_expect_failure 'mv directory from out-of-cone to in-cone' ' + init_repos && + + # as a sparse directory (or SKIP_WORKTREE_DIR without enabling + # sparse index). + test_all_match git mv --sparse folder1 deep && + test_all_match git status --porcelain=v2 && + test_sparse_match git ls-files -t && + git -C sparse-checkout ls-files -t >actual && + grep -e "H deep/folder1/0/0/0" actual && + grep -e "H deep/folder1/0/1" actual && + grep -e "H deep/folder1/a" actual && + + test_all_match git reset --hard && + + # as a directory deeper than sparse index boundary (where + # sparse index will expand). + test_sparse_match git mv --sparse folder1/0 deep && + test_sparse_match git status --porcelain=v2 && + test_sparse_match git ls-files -t && + git -C sparse-checkout ls-files -t >actual && + grep -e "H deep/0/0/0" actual && + grep -e "H deep/0/1" actual +' + test_done From 707fa2f76ac77f441578943be56e5a4a3ee8c3c5 Mon Sep 17 00:00:00 2001 From: Shaoxuan Yuan Date: Thu, 30 Jun 2022 10:37:32 +0800 Subject: [PATCH 3/8] mv: update sparsity after moving from out-of-cone to in-cone Originally, "git mv" a sparse file from out-of-cone to in-cone does not update the moved file's sparsity (remove its SKIP_WORKTREE bit). And the corresponding cache entry is, unexpectedly, not checked out in the working tree. Update the behavior so that: 1. Moving from out-of-cone to in-cone removes the SKIP_WORKTREE bit from corresponding cache entry. 2. The moved cache entry is checked out in the working tree to reflect the updated sparsity. Helped-by: Victoria Dye Signed-off-by: Shaoxuan Yuan Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/mv.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/builtin/mv.c b/builtin/mv.c index 83a465ba83..5f4eeadd5d 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -13,6 +13,7 @@ #include "string-list.h" #include "parse-options.h" #include "submodule.h" +#include "entry.h" static const char * const builtin_mv_usage[] = { N_("git mv [] ... "), @@ -304,6 +305,11 @@ remove_entry: const char *src = source[i], *dst = destination[i]; enum update_mode mode = modes[i]; int pos; + struct checkout state = CHECKOUT_INIT; + state.istate = &the_index; + + if (force) + state.force = 1; if (show_only || verbose) printf(_("Renaming %s to %s\n"), src, dst); if (show_only) @@ -328,6 +334,17 @@ remove_entry: pos = cache_name_pos(src, strlen(src)); assert(pos >= 0); rename_cache_entry_at(pos, dst); + + 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; + + if (checkout_entry(active_cache[dst_pos], &state, NULL, NULL)) + die(_("cannot checkout %s"), active_cache[dst_pos]->name); + } } if (gitmodules_modified) From 7889755bae89fd792924e95d713a94578b902d11 Mon Sep 17 00:00:00 2001 From: Shaoxuan Yuan Date: Thu, 30 Jun 2022 10:37:33 +0800 Subject: [PATCH 4/8] mv: decouple if/else-if checks using goto Previous if/else-if chain are highly nested and hard to develop/extend. Refactor to decouple this if/else-if chain by using goto to jump ahead. Suggested-by: Derrick Stolee Signed-off-by: Shaoxuan Yuan Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/mv.c | 141 +++++++++++++++++++++++++++++---------------------- 1 file changed, 81 insertions(+), 60 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 5f4eeadd5d..e800da3ab8 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -187,53 +187,68 @@ int cmd_mv(int argc, const char **argv, const char *prefix) length = strlen(src); if (lstat(src, &st) < 0) { /* only error if existence is expected. */ - if (modes[i] != SPARSE) + if (modes[i] != SPARSE) { bad = _("bad source"); - } else if (!strncmp(src, dst, length) && - (dst[length] == 0 || dst[length] == '/')) { + goto act_on_entry; + } + } + if (!strncmp(src, dst, length) && + (dst[length] == 0 || dst[length] == '/')) { bad = _("can not move directory into itself"); - } else if ((src_is_dir = S_ISDIR(st.st_mode)) - && lstat(dst, &st) == 0) + goto act_on_entry; + } + if ((src_is_dir = S_ISDIR(st.st_mode)) + && lstat(dst, &st) == 0) { bad = _("cannot move directory over file"); - else if (src_is_dir) { + goto act_on_entry; + } + if (src_is_dir) { + int j, dst_len, n; int first = cache_name_pos(src, length), last; - if (first >= 0) + if (first >= 0) { prepare_move_submodule(src, first, submodule_gitfile + i); - else if (index_range_of_same_dir(src, length, - &first, &last) < 1) + goto act_on_entry; + } else if (index_range_of_same_dir(src, length, + &first, &last) < 1) { bad = _("source directory is empty"); - else { /* last - first >= 1 */ - int j, dst_len, n; - - modes[i] = WORKING_DIRECTORY; - n = argc + last - first; - REALLOC_ARRAY(source, n); - REALLOC_ARRAY(destination, n); - REALLOC_ARRAY(modes, n); - REALLOC_ARRAY(submodule_gitfile, n); - - dst = add_slash(dst); - dst_len = strlen(dst); - - for (j = 0; j < last - first; j++) { - const struct cache_entry *ce = active_cache[first + j]; - const char *path = ce->name; - source[argc + j] = path; - destination[argc + j] = - prefix_path(dst, dst_len, path + length + 1); - modes[argc + j] = ce_skip_worktree(ce) ? SPARSE : INDEX; - submodule_gitfile[argc + j] = NULL; - } - argc += last - first; + goto act_on_entry; } - } else if (!(ce = cache_file_exists(src, length, 0))) { + + /* last - first >= 1 */ + modes[i] = WORKING_DIRECTORY; + n = argc + last - first; + REALLOC_ARRAY(source, n); + REALLOC_ARRAY(destination, n); + REALLOC_ARRAY(modes, n); + REALLOC_ARRAY(submodule_gitfile, n); + + dst = add_slash(dst); + dst_len = strlen(dst); + + for (j = 0; j < last - first; j++) { + const struct cache_entry *ce = active_cache[first + j]; + const char *path = ce->name; + source[argc + j] = path; + destination[argc + j] = + prefix_path(dst, dst_len, path + length + 1); + modes[argc + j] = ce_skip_worktree(ce) ? SPARSE : INDEX; + submodule_gitfile[argc + j] = NULL; + } + argc += last - first; + goto act_on_entry; + } + if (!(ce = cache_file_exists(src, length, 0))) { bad = _("not under version control"); - } else if (ce_stage(ce)) { + goto act_on_entry; + } + if (ce_stage(ce)) { bad = _("conflicted"); - } else if (lstat(dst, &st) == 0 && - (!ignore_case || strcasecmp(src, dst))) { + goto act_on_entry; + } + if (lstat(dst, &st) == 0 && + (!ignore_case || strcasecmp(src, dst))) { bad = _("destination exists"); if (force) { /* @@ -247,34 +262,40 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } else bad = _("Cannot overwrite"); } - } else if (string_list_has_string(&src_for_dst, dst)) + goto act_on_entry; + } + if (string_list_has_string(&src_for_dst, dst)) { bad = _("multiple sources for the same target"); - else if (is_dir_sep(dst[strlen(dst) - 1])) + goto act_on_entry; + } + if (is_dir_sep(dst[strlen(dst) - 1])) { bad = _("destination directory does not exist"); - else { - /* - * We check if the paths are in the sparse-checkout - * definition as a very final check, since that - * allows us to point the user to the --sparse - * option as a way to have a successful run. - */ - if (!ignore_sparse && - !path_in_sparse_checkout(src, &the_index)) { - string_list_append(&only_match_skip_worktree, src); - skip_sparse = 1; - } - if (!ignore_sparse && - !path_in_sparse_checkout(dst, &the_index)) { - string_list_append(&only_match_skip_worktree, dst); - skip_sparse = 1; - } - - if (skip_sparse) - goto remove_entry; - - string_list_insert(&src_for_dst, dst); + goto act_on_entry; } + /* + * We check if the paths are in the sparse-checkout + * definition as a very final check, since that + * allows us to point the user to the --sparse + * option as a way to have a successful run. + */ + if (!ignore_sparse && + !path_in_sparse_checkout(src, &the_index)) { + string_list_append(&only_match_skip_worktree, src); + skip_sparse = 1; + } + if (!ignore_sparse && + !path_in_sparse_checkout(dst, &the_index)) { + string_list_append(&only_match_skip_worktree, dst); + skip_sparse = 1; + } + + if (skip_sparse) + goto remove_entry; + + string_list_insert(&src_for_dst, dst); + +act_on_entry: if (!bad) continue; if (!ignore_errors) From 6645b03ca5af33bbce1001257ee22832559ec966 Mon Sep 17 00:00:00 2001 From: Shaoxuan Yuan Date: Thu, 30 Jun 2022 10:37:34 +0800 Subject: [PATCH 5/8] mv: check if out-of-cone file exists in index with SKIP_WORKTREE bit Originally, moving a file which is not on-disk but exists in index as a SKIP_WORKTREE enabled cache entry, "giv mv" command errors out with "bad source". Change the checking logic, so that such file makes "giv mv" command warns with "advise_on_updating_sparse_paths()" instead of "bad source"; also user now can supply a "--sparse" flag so this operation can be carried out successfully. Signed-off-by: Shaoxuan Yuan Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/mv.c | 21 +++++++++++++++++++-- t/t7002-mv-sparse-checkout.sh | 4 ++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index e800da3ab8..520be85774 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -186,11 +186,28 @@ int cmd_mv(int argc, const char **argv, const char *prefix) length = strlen(src); if (lstat(src, &st) < 0) { - /* only error if existence is expected. */ - if (modes[i] != SPARSE) { + int pos; + const struct cache_entry *ce; + + pos = cache_name_pos(src, length); + if (pos < 0) { + /* only error if existence is expected. */ + if (modes[i] != SPARSE) + bad = _("bad source"); + goto act_on_entry; + } + + ce = active_cache[pos]; + if (!ce_skip_worktree(ce)) { bad = _("bad source"); goto act_on_entry; } + + if (!ignore_sparse) + string_list_append(&only_match_skip_worktree, src); + else + modes[i] = SPARSE; + goto act_on_entry; } if (!strncmp(src, dst, length) && (dst[length] == 0 || dst[length] == '/')) { diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh index 023e657c9e..1510b5ed6a 100755 --- a/t/t7002-mv-sparse-checkout.sh +++ b/t/t7002-mv-sparse-checkout.sh @@ -241,7 +241,7 @@ test_expect_failure 'can move out-of-cone directory with --sparse' ' test_path_is_file sub/folder1/file1 ' -test_expect_failure 'refuse to move out-of-cone file without --sparse' ' +test_expect_success 'refuse to move out-of-cone file without --sparse' ' test_when_finished "cleanup_sparse_checkout" && setup_sparse_checkout && @@ -252,7 +252,7 @@ test_expect_failure 'refuse to move out-of-cone file without --sparse' ' test_cmp expect stderr ' -test_expect_failure 'can move out-of-cone file with --sparse' ' +test_expect_success 'can move out-of-cone file with --sparse' ' test_when_finished "cleanup_sparse_checkout" && setup_sparse_checkout && From 8a26a3915fbe1aab9786b28c535c8541f8df2a19 Mon Sep 17 00:00:00 2001 From: Shaoxuan Yuan Date: Thu, 30 Jun 2022 10:37:35 +0800 Subject: [PATCH 6/8] mv: check if exists in index to handle overwriting Originally, moving a sparse file into cone can result in unwarned overwrite of existing entry. The expected behavior is that if the exists in the entry, user should be prompted to supply a [-f|--force] to carry out the operation, or the operation should fail. Add a check mechanism to do that. Signed-off-by: Shaoxuan Yuan Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/mv.c | 15 ++++++++++++--- t/t7002-mv-sparse-checkout.sh | 4 ++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 520be85774..7d9627938a 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -202,11 +202,20 @@ int cmd_mv(int argc, const char **argv, const char *prefix) bad = _("bad source"); goto act_on_entry; } - - if (!ignore_sparse) + if (!ignore_sparse) { string_list_append(&only_match_skip_worktree, src); - else + goto act_on_entry; + } + /* Check if dst exists in index */ + if (cache_name_pos(dst, strlen(dst)) < 0) { modes[i] = SPARSE; + goto act_on_entry; + } + if (!force) { + bad = _("destination exists"); + goto act_on_entry; + } + modes[i] = SPARSE; goto act_on_entry; } if (!strncmp(src, dst, length) && diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh index 1510b5ed6a..6d2fb4f8d2 100755 --- a/t/t7002-mv-sparse-checkout.sh +++ b/t/t7002-mv-sparse-checkout.sh @@ -262,7 +262,7 @@ test_expect_success 'can move out-of-cone file with --sparse' ' test_path_is_file sub/file1 ' -test_expect_failure 'refuse to move sparse file to existing destination' ' +test_expect_success 'refuse to move sparse file to existing destination' ' test_when_finished "cleanup_sparse_checkout" && mkdir folder1 && touch folder1/file1 && @@ -275,7 +275,7 @@ 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 && From 24ea81d9ac401731c222b28097b339aa0d2d316d Mon Sep 17 00:00:00 2001 From: Shaoxuan Yuan Date: Thu, 30 Jun 2022 10:37:36 +0800 Subject: [PATCH 7/8] mv: use flags mode for update_mode As suggested by Derrick [1], move the in-line definition of "enum update_mode" to the top 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 Helped-by: Derrick Stolee Signed-off-by: Shaoxuan Yuan Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/mv.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 7d9627938a..b805a0d0f6 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -20,6 +20,13 @@ static const char * const builtin_mv_usage[] = { NULL }; +enum update_mode { + BOTH = 0, + WORKING_DIRECTORY = (1 << 1), + INDEX = (1 << 2), + SPARSE = (1 << 3), +}; + #define DUP_BASENAME 1 #define KEEP_TRAILING_SLASH 2 @@ -130,7 +137,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) OPT_END(), }; const char **source, **destination, **dest_path, **submodule_gitfile; - enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX, SPARSE } *modes; + enum update_mode *modes; struct stat st; struct string_list src_for_dst = STRING_LIST_INIT_NODUP; struct lock_file lock_file = LOCK_INIT; @@ -192,7 +199,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) pos = cache_name_pos(src, length); if (pos < 0) { /* only error if existence is expected. */ - if (modes[i] != SPARSE) + if (!(modes[i] & SPARSE)) bad = _("bad source"); goto act_on_entry; } @@ -208,14 +215,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } /* Check if dst exists in index */ if (cache_name_pos(dst, strlen(dst)) < 0) { - modes[i] = SPARSE; + modes[i] |= SPARSE; goto act_on_entry; } if (!force) { bad = _("destination exists"); goto act_on_entry; } - modes[i] = SPARSE; + modes[i] |= SPARSE; goto act_on_entry; } if (!strncmp(src, dst, length) && @@ -243,7 +250,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } /* last - first >= 1 */ - modes[i] = WORKING_DIRECTORY; + modes[i] |= WORKING_DIRECTORY; n = argc + last - first; REALLOC_ARRAY(source, n); REALLOC_ARRAY(destination, n); @@ -259,7 +266,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix) source[argc + j] = path; destination[argc + j] = prefix_path(dst, dst_len, path + length + 1); - modes[argc + j] = ce_skip_worktree(ce) ? SPARSE : INDEX; + memset(modes + argc + j, 0, sizeof(enum update_mode)); + modes[argc + j] |= ce_skip_worktree(ce) ? SPARSE : INDEX; submodule_gitfile[argc + j] = NULL; } argc += last - first; @@ -361,7 +369,8 @@ remove_entry: printf(_("Renaming %s to %s\n"), src, dst); if (show_only) continue; - if (mode != INDEX && mode != SPARSE && rename(src, dst) < 0) { + if (!(mode & (INDEX | SPARSE)) && + rename(src, dst) < 0) { if (ignore_errors) continue; die_errno(_("renaming '%s' failed"), src); @@ -375,7 +384,7 @@ remove_entry: 1); } - if (mode == WORKING_DIRECTORY) + if (mode & (WORKING_DIRECTORY)) continue; pos = cache_name_pos(src, strlen(src)); From b91a2b6594a1da7d4d2f936f7db25bfa5a3d775e Mon Sep 17 00:00:00 2001 From: Shaoxuan Yuan Date: Thu, 30 Jun 2022 10:37:37 +0800 Subject: [PATCH 8/8] mv: add check_dir_in_index() and solve general dir check issue Originally, moving a directory which is not on-disk due to its existence outside of sparse-checkout cone, "giv mv" command errors out with "bad source". Add a helper check_dir_in_index() function to see if a directory name exists in the index. Also add a SKIP_WORKTREE_DIR bit to mark such directories. Change the checking logic, so that such directory makes "giv mv" command warns with "advise_on_updating_sparse_paths()" instead of "bad source"; also user now can supply a "--sparse" flag so this operation can be carried out successfully. Helped-by: Victoria Dye Helped-by: Derrick Stolee Signed-off-by: Shaoxuan Yuan Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/mv.c | 50 +++++++++++++++++++++--- t/t1092-sparse-checkout-compatibility.sh | 2 +- t/t7002-mv-sparse-checkout.sh | 4 +- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index b805a0d0f6..2a38e2af46 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -25,6 +25,7 @@ enum update_mode { WORKING_DIRECTORY = (1 << 1), INDEX = (1 << 2), SPARSE = (1 << 3), + SKIP_WORKTREE_DIR = (1 << 4), }; #define DUP_BASENAME 1 @@ -123,6 +124,36 @@ static int index_range_of_same_dir(const char *src, int length, return last - first; } +/* + * Check if an out-of-cone directory should be in the index. Imagine this case + * that all the files under a directory are marked with 'CE_SKIP_WORKTREE' bit + * and thus the directory is sparsified. + * + * Return 0 if such directory exist (i.e. with any of its contained files not + * 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) +{ + const char *with_slash = add_slash(name); + int length = strlen(with_slash); + + int pos = cache_name_pos(with_slash, length); + const struct cache_entry *ce; + + if (pos < 0) { + pos = -pos - 1; + if (pos >= the_index.cache_nr) + return 1; + ce = active_cache[pos]; + if (strncmp(with_slash, ce->name, length)) + return 1; + if (ce_skip_worktree(ce)) + return 0; + } + return 1; +} + int cmd_mv(int argc, const char **argv, const char *prefix) { int i, flags, gitmodules_modified = 0; @@ -184,7 +215,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) /* Checking */ for (i = 0; i < argc; i++) { const char *src = source[i], *dst = destination[i]; - int length, src_is_dir; + int length; const char *bad = NULL; int skip_sparse = 0; @@ -198,12 +229,17 @@ 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 (!path_in_sparse_checkout(src_w_slash, &the_index) && + !check_dir_in_index(src)) { + modes[i] |= SKIP_WORKTREE_DIR; + goto dir_check; + } /* only error if existence is expected. */ if (!(modes[i] & SPARSE)) bad = _("bad source"); goto act_on_entry; } - ce = active_cache[pos]; if (!ce_skip_worktree(ce)) { bad = _("bad source"); @@ -230,12 +266,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix) bad = _("can not move directory into itself"); goto act_on_entry; } - if ((src_is_dir = S_ISDIR(st.st_mode)) + if (S_ISDIR(st.st_mode) && lstat(dst, &st) == 0) { bad = _("cannot move directory over file"); goto act_on_entry; } - if (src_is_dir) { + +dir_check: + if (S_ISDIR(st.st_mode)) { int j, dst_len, n; int first = cache_name_pos(src, length), last; @@ -369,7 +407,7 @@ remove_entry: 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; @@ -384,7 +422,7 @@ remove_entry: 1); } - if (mode & (WORKING_DIRECTORY)) + if (mode & (WORKING_DIRECTORY | SKIP_WORKTREE_DIR)) continue; pos = cache_name_pos(src, strlen(src)); diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 5eef799e25..763c6cc684 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1828,7 +1828,7 @@ test_expect_success 'checkout behaves oddly with df-conflict-2' ' test_cmp full-checkout-err sparse-index-err ' -test_expect_failure 'mv directory from out-of-cone to in-cone' ' +test_expect_success 'mv directory from out-of-cone to in-cone' ' init_repos && # as a sparse directory (or SKIP_WORKTREE_DIR without enabling diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh index 6d2fb4f8d2..71fe29690f 100755 --- a/t/t7002-mv-sparse-checkout.sh +++ b/t/t7002-mv-sparse-checkout.sh @@ -219,7 +219,7 @@ test_expect_success 'refuse to move file to non-skip-worktree sparse path' ' test_cmp expect stderr ' -test_expect_failure 'refuse to move out-of-cone directory without --sparse' ' +test_expect_success 'refuse to move out-of-cone directory without --sparse' ' test_when_finished "cleanup_sparse_checkout" && setup_sparse_checkout && @@ -230,7 +230,7 @@ test_expect_failure 'refuse to move out-of-cone directory without --sparse' ' test_cmp expect stderr ' -test_expect_failure 'can move out-of-cone directory with --sparse' ' +test_expect_success 'can move out-of-cone directory with --sparse' ' test_when_finished "cleanup_sparse_checkout" && setup_sparse_checkout &&