diff --git a/builtin/reset.c b/builtin/reset.c index f63f5c440158c4..b01f5067fabf0e 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -28,7 +28,6 @@ #include "strbuf.h" #include "quote.h" #include "dir.h" -#include "entry.h" #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000) @@ -132,57 +131,39 @@ static void update_index_from_diff(struct diff_queue_struct *q, struct diff_options *opt, void *data) { int i; - int pos; int intent_to_add = *(int *)data; for (i = 0; i < q->nr; i++) { + int pos; struct diff_filespec *one = q->queue[i]->one; - struct diff_filespec *two = q->queue[i]->two; - int is_missing = !(one->mode && !is_null_oid(&one->oid)); - int was_missing = !two->mode && is_null_oid(&two->oid); + int is_in_reset_tree = one->mode && !is_null_oid(&one->oid); struct cache_entry *ce; - struct cache_entry *ceBefore; - struct checkout state = CHECKOUT_INIT; - /* - * When using the sparse-checkout feature the cache entries that are - * added here will not have the skip-worktree bit set. - * Without this code there is data that is lost because the files that - * would normally be in the working directory are not there and show as - * deleted for the next status or in the case of added files just disappear. - * We need to create the previous version of the files in the working - * directory so that they will have the right content and the next - * status call will show modified or untracked files correctly. - */ - if (core_apply_sparse_checkout && !file_exists(two->path)) - { - pos = cache_name_pos(two->path, strlen(two->path)); - if ((pos >= 0 && ce_skip_worktree(active_cache[pos])) && (is_missing || !was_missing)) - { - state.force = 1; - state.refresh_cache = 1; - state.istate = &the_index; - ceBefore = make_cache_entry(&the_index, two->mode, &two->oid, two->path, - 0, 0); - if (!ceBefore) - die(_("make_cache_entry failed for path '%s'"), - two->path); - - checkout_entry(ceBefore, &state, NULL, NULL); - } - } - - if (is_missing && !intent_to_add) { + if (!is_in_reset_tree && !intent_to_add) { remove_file_from_cache(one->path); continue; } ce = make_cache_entry(&the_index, one->mode, &one->oid, one->path, 0, 0); + + /* + * If the file 1) corresponds to an existing index entry with + * skip-worktree set, or 2) does not exist in the index but is + * outside the sparse checkout definition, add a skip-worktree bit + * to the new index entry. Note that a sparse index will be expanded + * if this entry is outside the sparse cone - this is necessary + * to properly construct the reset sparse directory. + */ + pos = cache_name_pos(one->path, strlen(one->path)); + if ((pos >= 0 && ce_skip_worktree(active_cache[pos])) || + (pos < 0 && !path_in_sparse_checkout(one->path, &the_index))) + ce->ce_flags |= CE_SKIP_WORKTREE; + if (!ce) die(_("make_cache_entry failed for path '%s'"), one->path); - if (is_missing) { + if (!is_in_reset_tree) { ce->ce_flags |= CE_INTENT_TO_ADD; set_object_name_for_intent_to_add_entry(ce); } @@ -190,13 +171,78 @@ static void update_index_from_diff(struct diff_queue_struct *q, } } +static int pathspec_needs_expanded_index(const struct pathspec *pathspec) +{ + unsigned int i, pos; + int res = 0; + char *skip_worktree_seen = NULL; + + /* + * When using a magic pathspec, assume for the sake of simplicity that + * the index needs to be expanded to match all matchable files. + */ + if (pathspec->magic) + return 1; + + for (i = 0; i < pathspec->nr; i++) { + struct pathspec_item item = pathspec->items[i]; + + /* + * If the pathspec item has a wildcard, the index should be expanded + * if the pathspec has the possibility of matching a subset of entries inside + * of a sparse directory (but not the entire directory). + * + * If the pathspec item is a literal path, the index only needs to be expanded + * if a) the pathspec isn't in the sparse checkout cone (to make sure we don't + * expand for in-cone files) and b) it doesn't match any sparse directories + * (since we can reset whole sparse directories without expanding them). + */ + if (item.nowildcard_len < item.len) { + for (pos = 0; pos < active_nr; pos++) { + struct cache_entry *ce = active_cache[pos]; + + if (!S_ISSPARSEDIR(ce->ce_mode)) + continue; + + /* + * If the pre-wildcard length is longer than the sparse + * directory name and the sparse directory is the first + * component of the pathspec, need to expand the index. + */ + if (item.nowildcard_len > ce_namelen(ce) && + !strncmp(item.original, ce->name, ce_namelen(ce))) { + res = 1; + break; + } + + /* + * If the pre-wildcard length is shorter than the sparse + * directory and the pathspec does not match the whole + * directory, need to expand the index. + */ + if (!strncmp(item.original, ce->name, item.nowildcard_len) && + wildmatch(item.original, ce->name, 0)) { + res = 1; + break; + } + } + } else if (!path_in_cone_modesparse_checkout(item.original, &the_index) && + !matches_skip_worktree(pathspec, i, &skip_worktree_seen)) + res = 1; + + if (res > 0) + break; + } + + free(skip_worktree_seen); + return res; +} + static int read_from_tree(const struct pathspec *pathspec, struct object_id *tree_oid, int intent_to_add) { struct diff_options opt; - unsigned int i; - char *skip_worktree_seen = NULL; memset(&opt, 0, sizeof(opt)); copy_pathspec(&opt.pathspec, pathspec); @@ -209,29 +255,8 @@ static int read_from_tree(const struct pathspec *pathspec, opt.change = diff_change; opt.add_remove = diff_addremove; - /* - * When pathspec is given for resetting a cone-mode sparse checkout, it may - * identify entries that are nested in sparse directories, in which case the - * index should be expanded. For the sake of efficiency, this check is - * overly-cautious: anything with a wildcard or a magic prefix requires - * expansion, as well as literal paths that aren't in the sparse checkout - * definition AND don't match any directory in the index. - */ - if (pathspec->nr && the_index.sparse_index) { - if (pathspec->magic || pathspec->has_wildcard) { - ensure_full_index(&the_index); - } else { - for (i = 0; i < pathspec->nr; i++) { - if (!path_in_cone_modesparse_checkout(pathspec->items[i].original, &the_index) && - !matches_skip_worktree(pathspec, i, &skip_worktree_seen)) { - ensure_full_index(&the_index); - break; - } - } - } - } - - free(skip_worktree_seen); + if (pathspec->nr && the_index.sparse_index && pathspec_needs_expanded_index(pathspec)) + ensure_full_index(&the_index); if (do_diff_cache(tree_oid, &opt)) return 1; diff --git a/cache-tree.c b/cache-tree.c index 2f221de68a893d..1c3f58fee17a55 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -800,15 +800,12 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state, return ret; } -static void prime_cache_tree_sparse_dir(struct repository *r, - struct cache_tree *it, - struct tree *tree, - struct strbuf *tree_path) +static void prime_cache_tree_sparse_dir(struct cache_tree *it, + struct tree *tree) { oidcpy(&it->oid, &tree->object.oid); it->entry_count = 1; - return; } static void prime_cache_tree_rec(struct repository *r, @@ -816,10 +813,10 @@ static void prime_cache_tree_rec(struct repository *r, struct tree *tree, struct strbuf *tree_path) { - struct strbuf subtree_path = STRBUF_INIT; struct tree_desc desc; struct name_entry entry; int cnt; + int base_path_len = tree_path->len; oidcpy(&it->oid, &tree->object.oid); @@ -836,30 +833,36 @@ static void prime_cache_tree_rec(struct repository *r, parse_tree(subtree); sub = cache_tree_sub(it, entry.path); sub->cache_tree = cache_tree(); - strbuf_reset(&subtree_path); - strbuf_grow(&subtree_path, tree_path->len + entry.pathlen + 1); - strbuf_addbuf(&subtree_path, tree_path); - strbuf_add(&subtree_path, entry.path, entry.pathlen); - strbuf_addch(&subtree_path, '/'); /* - * If a sparse index is in use, the directory being processed may be - * sparse. To confirm that, we can check whether an entry with that - * exact name exists in the index. If it does, the created subtree - * should be sparse. Otherwise, cache tree expansion should continue - * as normal. - */ + * Recursively-constructed subtree path is only needed when working + * in a sparse index (where it's used to determine whether the + * subtree is a sparse directory in the index). + */ + if (r->index->sparse_index) { + strbuf_setlen(tree_path, base_path_len); + strbuf_grow(tree_path, base_path_len + entry.pathlen + 1); + strbuf_add(tree_path, entry.path, entry.pathlen); + strbuf_addch(tree_path, '/'); + } + + /* + * If a sparse index is in use, the directory being processed may be + * sparse. To confirm that, we can check whether an entry with that + * exact name exists in the index. If it does, the created subtree + * should be sparse. Otherwise, cache tree expansion should continue + * as normal. + */ if (r->index->sparse_index && - index_entry_exists(r->index, subtree_path.buf, subtree_path.len)) - prime_cache_tree_sparse_dir(r, sub->cache_tree, subtree, &subtree_path); + index_entry_exists(r->index, tree_path->buf, tree_path->len)) + prime_cache_tree_sparse_dir(sub->cache_tree, subtree); else - prime_cache_tree_rec(r, sub->cache_tree, subtree, &subtree_path); + prime_cache_tree_rec(r, sub->cache_tree, subtree, tree_path); cnt += sub->cache_tree->entry_count; } } - it->entry_count = cnt; - strbuf_release(&subtree_path); + it->entry_count = cnt; } void prime_cache_tree(struct repository *r, diff --git a/read-cache.c b/read-cache.c index 564283c7e7e24c..283b3c6e7dc19c 100644 --- a/read-cache.c +++ b/read-cache.c @@ -70,6 +70,11 @@ */ #define CACHE_ENTRY_PATH_LENGTH 80 +enum index_search_mode { + NO_EXPAND_SPARSE = 0, + EXPAND_SPARSE = 1 +}; + static inline struct cache_entry *mem_pool__ce_alloc(struct mem_pool *mem_pool, size_t len) { struct cache_entry *ce; @@ -567,7 +572,7 @@ int cache_name_stage_compare(const char *name1, int len1, int stage1, const char static int index_name_stage_pos(struct index_state *istate, const char *name, int namelen, int stage, - int search_sparse) + enum index_search_mode search_mode) { int first, last; @@ -586,7 +591,7 @@ static int index_name_stage_pos(struct index_state *istate, first = next+1; } - if (search_sparse && istate->sparse_index && + if (search_mode == EXPAND_SPARSE && istate->sparse_index && first > 0) { /* Note: first <= istate->cache_nr */ struct cache_entry *ce = istate->cache[first - 1]; @@ -602,7 +607,7 @@ static int index_name_stage_pos(struct index_state *istate, ce_namelen(ce) < namelen && !strncmp(name, ce->name, ce_namelen(ce))) { ensure_full_index(istate); - return index_name_stage_pos(istate, name, namelen, stage, search_sparse); + return index_name_stage_pos(istate, name, namelen, stage, search_mode); } } @@ -611,12 +616,12 @@ static int index_name_stage_pos(struct index_state *istate, int index_name_pos(struct index_state *istate, const char *name, int namelen) { - return index_name_stage_pos(istate, name, namelen, 0, 1); + return index_name_stage_pos(istate, name, namelen, 0, EXPAND_SPARSE); } int index_entry_exists(struct index_state *istate, const char *name, int namelen) { - return index_name_stage_pos(istate, name, namelen, 0, 0) >= 0; + return index_name_stage_pos(istate, name, namelen, 0, NO_EXPAND_SPARSE) >= 0; } int remove_index_entry_at(struct index_state *istate, int pos) @@ -1243,7 +1248,7 @@ static int has_dir_name(struct index_state *istate, */ } - pos = index_name_stage_pos(istate, name, len, stage, 1); + pos = index_name_stage_pos(istate, name, len, stage, EXPAND_SPARSE); if (pos >= 0) { /* * Found one, but not so fast. This could @@ -1340,7 +1345,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) pos = index_pos_to_insert_pos(istate->cache_nr); else - pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce), 1); + pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce), EXPAND_SPARSE); /* * Cache tree path should be invalidated only after index_name_stage_pos, @@ -1382,7 +1387,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e if (!ok_to_replace) return error(_("'%s' appears as both a file and as a directory"), ce->name); - pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce), 1); + pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce), EXPAND_SPARSE); pos = -pos-1; } return pos + 1; @@ -2376,7 +2381,8 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist) if (!istate->repo) istate->repo = the_repository; prepare_repo_settings(istate->repo); - if (istate->repo->settings.command_requires_full_index) + if (!istate->repo->settings.sparse_index || + istate->repo->settings.command_requires_full_index) ensure_full_index(istate); return istate->cache_nr; diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index e501cf100ec3ed..2e2175262f4d5a 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -498,51 +498,17 @@ test_expect_success 'blame with pathspec outside sparse definition' ' test_sparse_match test_must_fail git blame deep/deeper2/deepest/a ' -# TODO: This behaves correctly in microsoft/git. Why? test_expect_success 'checkout and reset (mixed)' ' init_repos && test_all_match git checkout -b reset-test update-deep && test_all_match git reset deepest && - test_all_match git reset update-folder1 && - test_all_match git reset update-folder2 -' - -# NEEDSWORK: a sparse-checkout behaves differently from a full checkout -# in this scenario, but it shouldn't. -test_expect_success 'checkout and reset (mixed) [sparse]' ' - init_repos && - test_sparse_match git checkout -b reset-test update-deep && - test_sparse_match git reset deepest && + # Because skip-worktree is preserved, resetting to update-folder1 + # will show worktree changes for full-checkout that are not present + # in sparse-checkout or sparse-index. test_sparse_match git reset update-folder1 && - test_sparse_match git reset update-folder2 -' - -# NEEDSWORK: with mixed reset, files with differences between HEAD and -# will be added to the work tree even if outside the sparse checkout -# definition, and even if the file is modified to a state of having no local -# changes. The file is "re-ignored" if a hard reset is executed. We may want to -# change this behavior in the future and enforce that files are not written -# outside of the sparse checkout definition. -test_expect_success 'checkout and mixed reset file tracking [sparse]' ' - init_repos && - - test_all_match git checkout -b reset-test update-deep && - test_all_match git reset update-folder1 && - test_all_match git reset update-deep && - - # At this point, there are no changes in the working tree. However, - # folder1/a now exists locally (even though it is outside of the sparse - # paths). - run_on_sparse test_path_exists folder1 && - - run_on_all rm folder1/a && - test_all_match git status --porcelain=v2 && - - test_all_match git reset --hard update-deep && - run_on_sparse test_path_is_missing folder1 && - test_path_exists full-checkout/folder1 + run_on_sparse test_path_is_missing folder1 ' test_expect_success 'checkout and reset (merge)' ' @@ -599,31 +565,34 @@ test_expect_success 'reset with pathspecs inside sparse definition' ' test_all_match git status --porcelain=v2 ' -test_expect_success 'reset with sparse directory pathspec outside definition' ' +# Although the working tree differs between full and sparse checkouts after +# reset, the state of the index is the same. +test_expect_success 'reset with pathspecs outside sparse definition' ' init_repos && + test_all_match git checkout -b reset-test base && - test_all_match git checkout -b reset-test update-deep && - test_all_match git reset --hard update-folder1 && - test_all_match git reset base -- folder1 && - test_all_match git status --porcelain=v2 -' - -test_expect_success 'reset with pathspec match in sparse directory' ' - init_repos && + test_sparse_match git reset update-folder1 -- folder1 && + git -C full-checkout reset update-folder1 -- folder1 && + test_sparse_match git status --porcelain=v2 && + test_all_match git rev-parse HEAD:folder1 && - test_all_match git checkout -b reset-test update-deep && - test_all_match git reset --hard update-folder1 && - test_all_match git reset base -- folder1/a && - test_all_match git status --porcelain=v2 + test_sparse_match git reset update-folder2 -- folder2/a && + git -C full-checkout reset update-folder2 -- folder2/a && + test_sparse_match git status --porcelain=v2 && + test_all_match git rev-parse HEAD:folder2/a ' test_expect_success 'reset with wildcard pathspec' ' init_repos && test_all_match git checkout -b reset-test update-deep && - test_all_match git reset --hard update-folder1 && test_all_match git reset base -- \*/a && - test_all_match git status --porcelain=v2 + test_all_match git status --porcelain=v2 && + test_all_match git rev-parse HEAD:folder1/a && + + test_all_match git reset base -- folder\* && + test_all_match git status --porcelain=v2 && + test_all_match git rev-parse HEAD:folder2 ' # NEEDSWORK: although update-index executes without error on files outside @@ -1126,11 +1095,15 @@ test_expect_success 'submodule handling' ' grep "160000 commit $(git -C initial-repo rev-parse HEAD) modules/sub" cache ' +# When working with a sparse index, some commands will need to expand the +# index to operate properly. If those commands also write the index back +# to disk, they need to convert the index to sparse before writing. +# This test verifies that both of these events are logged in trace2 logs. test_expect_success 'sparse-index is expanded and converted back' ' init_repos && GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \ - git -C sparse-index -c core.fsmonitor="" mv a b && + git -C sparse-index reset -- folder1/a && test_region index convert_to_sparse trace2.txt && test_region index ensure_full_index trace2.txt ' @@ -1200,10 +1173,10 @@ test_expect_success 'sparse-index is not expanded' ' for ref in update-deep update-folder1 update-folder2 update-deep do echo >>sparse-index/README.md && - ensure_not_expanded reset --mixed $ref ensure_not_expanded reset --hard $ref || return 1 done && + ensure_not_expanded reset --mixed base && ensure_not_expanded reset --hard update-deep && ensure_not_expanded reset --keep base && ensure_not_expanded reset --merge update-deep && @@ -1227,6 +1200,22 @@ test_expect_success 'sparse-index is not expanded' ' ensure_not_expanded clean -fd && + ensure_not_expanded reset base -- deep/a && + ensure_not_expanded reset base -- nonexistent-file && + ensure_not_expanded reset deepest -- deep && + + # Although folder1 is outside the sparse definition, it exists as a + # directory entry in the index, so the pathspec will not force the + # index to be expanded. + ensure_not_expanded reset deepest -- folder1 && + ensure_not_expanded reset deepest -- folder1/ && + + # Wildcard identifies only in-cone files, no index expansion + ensure_not_expanded reset deepest -- deep/\* && + + # Wildcard identifies only full sparse directories, no index expansion + ensure_not_expanded reset deepest -- folder\* && + ensure_not_expanded checkout -f update-deep && ( sane_unset GIT_TEST_MERGE_ALGORITHM && diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 601b2bf97f0e7f..d05426062ec29d 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -472,6 +472,23 @@ test_expect_success '--mixed refreshes the index' ' test_cmp expect output ' +test_expect_success '--mixed preserves skip-worktree' ' + echo 123 >>file2 && + git add file2 && + git update-index --skip-worktree file2 && + git reset --mixed HEAD >output && + test_must_be_empty output && + + cat >expect <<-\EOF && + Unstaged changes after reset: + M file2 + EOF + git update-index --no-skip-worktree file2 && + git add file2 && + git reset --mixed HEAD >output && + test_cmp expect output +' + test_expect_success 'resetting specific path that is unmerged' ' git rm --cached file2 && F1=$(git rev-parse HEAD:file1) && diff --git a/t/t7114-reset-sparse-checkout.sh b/t/t7114-reset-sparse-checkout.sh deleted file mode 100755 index c46cbdb64e4ebc..00000000000000 --- a/t/t7114-reset-sparse-checkout.sh +++ /dev/null @@ -1,58 +0,0 @@ -#!/bin/sh - -test_description='reset when using a sparse-checkout' - -. ./test-lib.sh - -# reset using a sparse-checkout file - -test_expect_success 'setup' ' - test_tick && - echo "checkout file" >c && - echo "modify file" >m && - echo "delete file" >d && - git add . && - git commit -m "initial commit" && - echo "added file" >a && - echo "modification of a file" >m && - git rm d && - git add . && - git commit -m "second commit" && - git checkout -b endCommit -' - -test_expect_success 'reset when there is a sparse-checkout' ' - echo "/c" >.git/info/sparse-checkout && - test_config core.sparsecheckout true && - git checkout -B resetBranch && - test_path_is_missing m && - test_path_is_missing a && - test_path_is_missing d && - git reset HEAD~1 && - test "checkout file" = "$(cat c)" && - test "modification of a file" = "$(cat m)" && - test "added file" = "$(cat a)" && - test_path_is_missing d -' - -test_expect_success 'reset after deleting file without skip-worktree bit' ' - git checkout -f endCommit && - git clean -xdf && - echo "/c -/m" >.git/info/sparse-checkout && - test_config core.sparsecheckout true && - git checkout -B resetAfterDelete && - test_path_is_file m && - test_path_is_missing a && - test_path_is_missing d && - rm -f m && - git reset HEAD~1 && - test "checkout file" = "$(cat c)" && - test "added file" = "$(cat a)" && - test_path_is_missing m && - test_path_is_missing d -' - - - -test_done