From 71471b2a7c72415a52e0c84182098e8856ceab7a Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Wed, 27 Oct 2021 14:39:17 +0000 Subject: reset: preserve skip-worktree bit in mixed reset Change `update_index_from_diff` to set `skip-worktree` when applicable for new index entries. When `git reset --mixed ` is run, entries in the index with differences between the pre-reset HEAD and reset are identified and handled with `update_index_from_diff`. For each file, a new cache entry in inserted into the index, created from the side of the reset (without changing the working tree). However, the newly-created entry must have `skip-worktree` explicitly set in either of the following scenarios: 1. the file is in the current index and has `skip-worktree` set 2. the file is not in the current index but is outside of a defined sparse checkout definition Not setting the `skip-worktree` bit leads to likely-undesirable results for a user. It causes `skip-worktree` settings to disappear on the "diff"-containing files (but *only* the diff-containing files), leading to those files now showing modifications in `git status`. For example, when running `git reset --mixed` in a sparse checkout, some file entries outside of sparse checkout could show up as deleted, despite the user never deleting anything (and not wanting them on-disk anyway). Additionally, add a test to `t7102` to ensure `skip-worktree` is preserved in a basic `git reset --mixed` scenario and update a failure-documenting test from 19a0acc (t1092: test interesting sparse-checkout scenarios, 2021-01-23) with new expected behavior. Helped-by: Junio C Hamano Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- t/t1092-sparse-checkout-compatibility.sh | 22 ++++++++-------------- t/t7102-reset.sh | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 14 deletions(-) (limited to 't') diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 886e78715f..c7449afe96 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -459,26 +459,20 @@ test_expect_failure 'blame with pathspec outside sparse definition' ' test_all_match git blame deep/deeper2/deepest/a ' -# NEEDSWORK: a sparse-checkout behaves differently from a full checkout -# in this scenario, but it shouldn't. -test_expect_failure 'checkout and reset (mixed)' ' +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 folder1/a in full-checkout, but not + # in sparse-checkout or sparse-index. + git -C full-checkout reset update-folder1 >full-checkout-out && test_sparse_match git reset update-folder1 && - test_sparse_match git reset update-folder2 + grep "M folder1/a" full-checkout-out && + ! grep "M folder1/a" sparse-checkout-out && + run_on_sparse test_path_is_missing folder1 ' test_expect_success 'merge, cherry-pick, and rebase' ' diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 601b2bf97f..d05426062e 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) && -- cgit v1.2.3 From 86609db9dae77a385ea8c156ac4ca55f8454a3f7 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Wed, 27 Oct 2021 14:39:18 +0000 Subject: sparse-index: update command for expand/collapse test In anticipation of `git reset --hard` being able to use the sparse index without expanding it, replace the command in `sparse-index is expanded and converted back` with `git reset -- folder1/a`. This command will need to expand the index to work properly, even after integrating the rest of `reset` with sparse index. Helped-by: Derrick Stolee Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- t/t1092-sparse-checkout-compatibility.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 't') diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index c7449afe96..cab6340a9d 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -634,11 +634,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="" reset --hard && + git -C sparse-index reset -- folder1/a && test_region index convert_to_sparse trace2.txt && test_region index ensure_full_index trace2.txt ' -- cgit v1.2.3 From 291d77eb3e28aee42821d500bf61211f09b4297a Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Mon, 29 Nov 2021 15:52:39 +0000 Subject: reset: expand test coverage for sparse checkouts Add new tests for `--merge` and `--keep` modes, as well as mixed reset with pathspecs. New performance test cases exercise various execution paths for `reset`. Co-authored-by: Derrick Stolee Signed-off-by: Derrick Stolee Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- t/perf/p2000-sparse-operations.sh | 3 + t/t1092-sparse-checkout-compatibility.sh | 98 ++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) (limited to 't') diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh index 597626276f..bfd332120c 100755 --- a/t/perf/p2000-sparse-operations.sh +++ b/t/perf/p2000-sparse-operations.sh @@ -110,5 +110,8 @@ test_perf_on_all git add -A test_perf_on_all git add . test_perf_on_all git commit -a -m A test_perf_on_all git checkout -f - +test_perf_on_all git reset +test_perf_on_all git reset --hard +test_perf_on_all git reset -- does-not-exist test_done diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index cab6340a9d..4125525ab8 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -475,6 +475,104 @@ test_expect_success 'checkout and reset (mixed)' ' run_on_sparse test_path_is_missing folder1 ' +test_expect_success 'checkout and reset (merge)' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + test_all_match git checkout -b reset-test update-deep && + run_on_all ../edit-contents a && + test_all_match git reset --merge deepest && + test_all_match git status --porcelain=v2 && + + test_all_match git reset --hard update-deep && + run_on_all ../edit-contents deep/a && + test_all_match test_must_fail git reset --merge deepest +' + +test_expect_success 'checkout and reset (keep)' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + test_all_match git checkout -b reset-test update-deep && + run_on_all ../edit-contents a && + test_all_match git reset --keep deepest && + test_all_match git status --porcelain=v2 && + + test_all_match git reset --hard update-deep && + run_on_all ../edit-contents deep/a && + test_all_match test_must_fail git reset --keep deepest +' + +test_expect_success 'reset with pathspecs inside sparse definition' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + test_all_match git checkout -b reset-test update-deep && + run_on_all ../edit-contents deep/a && + + test_all_match git reset base -- deep/a && + test_all_match git status --porcelain=v2 && + + test_all_match git reset base -- nonexistent-file && + test_all_match git status --porcelain=v2 && + + test_all_match git reset deepest -- deep && + test_all_match git status --porcelain=v2 +' + +# 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_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_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 reset update-deep -- deep\* && + test_all_match git ls-files -s -- deep && + + test_all_match git reset deepest -- deep\*\*\* && + test_all_match git ls-files -s -- deep && + + # The following `git reset`s result in updating the index on files with + # `skip-worktree` enabled. To avoid failing due to discrepencies in reported + # "modified" files, `test_sparse_match` reset is performed separately from + # "full-checkout" reset, then the index contents of all repos are verified. + + test_sparse_match git reset update-folder1 -- \*/a && + git -C full-checkout reset update-folder1 -- \*/a && + test_all_match git ls-files -s -- deep/a folder1/a && + + test_sparse_match git reset update-folder2 -- folder\* && + git -C full-checkout reset update-folder2 -- folder\* && + test_all_match git ls-files -s -- folder10 folder1 folder2 && + + test_sparse_match git reset base -- folder1/\* && + git -C full-checkout reset base -- folder1/\* && + test_all_match git ls-files -s -- folder1 +' + test_expect_success 'merge, cherry-pick, and rebase' ' init_repos && -- cgit v1.2.3 From 20ec2d034cda1afef15a4dcc6e275d7a69413510 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Mon, 29 Nov 2021 15:52:41 +0000 Subject: reset: make sparse-aware (except --mixed) Remove `ensure_full_index` guard on `prime_cache_tree` and update `prime_cache_tree_rec` to correctly reconstruct sparse directory entries in the cache tree. While processing a tree's entries, `prime_cache_tree_rec` must determine whether a directory entry is sparse or not by searching for it in the index (*without* expanding the index). If a matching sparse directory index entry is found, no subtrees are added to the cache tree entry and the entry count is set to 1 (representing the sparse directory itself). Otherwise, the tree is assumed to not be sparse and its subtrees are recursively added to the cache tree. Helped-by: Elijah Newren Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- t/t1092-sparse-checkout-compatibility.sh | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 4125525ab8..871cc3fcb8 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -777,9 +777,9 @@ test_expect_success 'sparse-index is not expanded' ' ensure_not_expanded checkout - && ensure_not_expanded switch rename-out-to-out && ensure_not_expanded switch - && - git -C sparse-index reset --hard && + ensure_not_expanded reset --hard && ensure_not_expanded checkout rename-out-to-out -- deep/deeper1 && - git -C sparse-index reset --hard && + ensure_not_expanded reset --hard && ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1 && echo >>sparse-index/README.md && @@ -789,6 +789,17 @@ test_expect_success 'sparse-index is not expanded' ' echo >>sparse-index/untracked.txt && ensure_not_expanded add . && + for ref in update-deep update-folder1 update-folder2 update-deep + do + echo >>sparse-index/README.md && + ensure_not_expanded reset --hard $ref || return 1 + done && + + ensure_not_expanded reset --hard update-deep && + ensure_not_expanded reset --keep base && + ensure_not_expanded reset --merge update-deep && + ensure_not_expanded reset --hard && + ensure_not_expanded checkout -f update-deep && test_config -C sparse-index pull.twohead ort && ( -- cgit v1.2.3 From 4d1cfc1351ffec47bba1318e9cd1ed13c5182951 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Mon, 29 Nov 2021 15:52:42 +0000 Subject: reset: make --mixed sparse-aware Remove the `ensure_full_index` guard on `read_from_tree` and update `git reset --mixed` to ensure it can use sparse directory index entries wherever possible. Sparse directory entries are reset using `diff_tree_oid`, which requires `change` and `add_remove` functions to process the internal contents of the sparse directory. The `recursive` diff option handles cases in which `reset --mixed` must diff/merge files that are nested multiple levels deep in a sparse directory. The use of pathspecs with `git reset --mixed` introduces scenarios in which internal contents of sparse directories may be matched by the pathspec. In order to reset *all* files in the repo that may match the pathspec, the following conditions on the pathspec require index expansion before performing the reset: * "magic" pathspecs * wildcard pathspecs that do not match only in-cone files or entire sparse directories * literal pathspecs matching something outside the sparse checkout definition Helped-by: Elijah Newren Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- t/t1092-sparse-checkout-compatibility.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 't') diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 871cc3fcb8..77e302a0ef 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -795,11 +795,28 @@ test_expect_success 'sparse-index is not expanded' ' 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 && ensure_not_expanded reset --hard && + 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 && test_config -C sparse-index pull.twohead ort && ( -- cgit v1.2.3