From eae937059be0ae7b4a8e6bfbb985c8c079129419 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 10 May 2022 23:32:27 +0000 Subject: stash: expand sparse-checkout compatibility testing Add tests verifying expected 'git stash' behavior in 't1092-sparse-checkout-compatibility'. These cases establish the expected behavior of 'git stash' in a sparse-checkout and verify consistency both with and without a sparse index. Although no sparse index compatibility has been integrated into 'git stash' yet, the tests are all 'expect_success' - we don't want the cone-mode sparse-checkout behavior to change depending on whether it is using a sparse index or not. Therefore, we expect these tests to continue passing once sparse index is integrated with 'git stash'. Additionally, add performance test cases for 'git stash' both with and without untracked files. Note that, unlike the other tests in 'p2000-sparse-operations.sh', the tests added for 'stash' are combination operations. This is done to ensure the stash/unstash is not blocked by the modification of '$SPARSE_CONE/a' performed as part of 'test_perf_on_all'. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- t/perf/p2000-sparse-operations.sh | 2 ++ t/t1092-sparse-checkout-compatibility.sh | 49 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh index 382716cfca..76710cbef3 100755 --- a/t/perf/p2000-sparse-operations.sh +++ b/t/perf/p2000-sparse-operations.sh @@ -106,6 +106,8 @@ test_perf_on_all () { } test_perf_on_all git status +test_perf_on_all 'git stash && git stash pop' +test_perf_on_all 'echo >>new && git stash -u && git stash pop' test_perf_on_all git add -A test_perf_on_all git add . test_perf_on_all git commit -a -m A diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 236ab53028..86312b3044 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1034,6 +1034,55 @@ test_expect_success 'cherry-pick with conflicts' ' test_all_match test_must_fail git cherry-pick to-cherry-pick ' +test_expect_success 'stash' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + # Stash a sparse directory (folder1) + test_all_match git checkout -b test-branch rename-base && + test_all_match git reset --soft rename-out-to-out && + test_all_match git stash && + test_all_match git status --porcelain=v2 && + + # Apply the sparse directory stash without reinstating the index + test_all_match git stash apply -q && + test_all_match git status --porcelain=v2 && + + # Reset to state where stash can be applied + test_sparse_match git sparse-checkout reapply && + test_all_match git reset --hard rename-out-to-out && + + # Apply the sparse directory stash *with* reinstating the index + test_all_match git stash apply --index -q && + test_all_match git status --porcelain=v2 && + + # Reset to state where we will get a conflict applying the stash + test_sparse_match git sparse-checkout reapply && + test_all_match git reset --hard update-folder1 && + + # Apply the sparse directory stash with conflicts + test_all_match test_must_fail git stash apply --index -q && + test_all_match test_must_fail git stash apply -q && + test_all_match git status --porcelain=v2 && + + # Reset to base branch + test_sparse_match git sparse-checkout reapply && + test_all_match git reset --hard base && + + # Stash & unstash an untracked file outside of the sparse checkout + # definition. + run_on_sparse mkdir -p folder1 && + run_on_all ../edit-contents folder1/new && + test_all_match git stash -u && + test_all_match git status --porcelain=v2 && + + test_all_match git stash pop -q && + test_all_match git status --porcelain=v2 +' + test_expect_success 'checkout-index inside sparse definition' ' init_repos && -- cgit v1.3-6-g1900 From 3a58792adece081ee84e9827c4d90daf759ceb76 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 10 May 2022 23:32:28 +0000 Subject: stash: integrate with sparse index Enable sparse index in 'git stash' by disabling 'command_requires_full_index'. With sparse index enabled, some subcommands of 'stash' work without expanding the index, e.g., 'git stash', 'git stash list', 'git stash drop', etc. Others ensure the index is expanded either directly (as in the case of 'git stash [pop|apply]', where the call to 'merge_recursive_generic()' in 'do_apply_stash()' triggers the expansion), or in a command called internally by stash (e.g., 'git update-index' in 'git stash -u'). So, in addition to enabling sparse index, add tests to 't1092' demonstrating which variants of 'git stash' expand the index, and which do not. Finally, add the option to skip writing 'untracked.txt' in 'ensure_not_expanded', and use that option to successfully apply stashed untracked files without a conflict in 'untracked.txt'. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- builtin/stash.c | 3 +++ t/t1092-sparse-checkout-compatibility.sh | 29 ++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/builtin/stash.c b/builtin/stash.c index 0c7b6a9588..1bfba53204 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1770,6 +1770,9 @@ int cmd_stash(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_stash_usage, PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH); + prepare_repo_settings(the_repository); + the_repository->settings.command_requires_full_index = 0; + index_file = get_index_file(); strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file, (uintmax_t)pid); diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 86312b3044..75d844cd71 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1271,7 +1271,10 @@ test_expect_success 'index.sparse disabled inline uses full index' ' ensure_not_expanded () { rm -f trace2.txt && - echo >>sparse-index/untracked.txt && + if test -z "$WITHOUT_UNTRACKED_TXT" + then + echo >>sparse-index/untracked.txt + fi && if test "$1" = "!" then @@ -1375,6 +1378,30 @@ test_expect_success 'sparse-index is not expanded: merge conflict in cone' ' ) ' +test_expect_success 'sparse-index is not expanded: stash' ' + init_repos && + + echo >>sparse-index/a && + ensure_not_expanded stash && + ensure_not_expanded stash list && + ensure_not_expanded stash show stash@{0} && + ! ensure_not_expanded stash apply stash@{0} && + ensure_not_expanded stash drop stash@{0} && + + echo >>sparse-index/deep/new && + ! ensure_not_expanded stash -u && + ( + WITHOUT_UNTRACKED_TXT=1 && + ! ensure_not_expanded stash pop + ) && + + ensure_not_expanded stash create && + oid=$(git -C sparse-index stash create) && + ensure_not_expanded stash store -m "test" $oid && + ensure_not_expanded reset --hard && + ! ensure_not_expanded stash pop +' + test_expect_success 'sparse index is not expanded: diff' ' init_repos && -- cgit v1.3-6-g1900 From cfde4cd6ffdca7670b62a292b144425767fb1759 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 10 May 2022 23:32:29 +0000 Subject: sparse-index: expose 'is_sparse_index_allowed()' Expose 'is_sparse_index_allowed()' publicly so that it may be used by callers outside of 'sparse-index.c'. While no such callers exist yet, it will be used in a subsequent commit. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- sparse-index.c | 2 +- sparse-index.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sparse-index.c b/sparse-index.c index 8636af72de..ffbab7d35f 100644 --- a/sparse-index.c +++ b/sparse-index.c @@ -118,7 +118,7 @@ static int index_has_unmerged_entries(struct index_state *istate) return 0; } -static int is_sparse_index_allowed(struct index_state *istate, int flags) +int is_sparse_index_allowed(struct index_state *istate, int flags) { if (!core_apply_sparse_checkout || !core_sparse_checkout_cone) return 0; diff --git a/sparse-index.h b/sparse-index.h index 633d4fb7e3..f57c65d972 100644 --- a/sparse-index.h +++ b/sparse-index.h @@ -3,6 +3,7 @@ struct index_state; #define SPARSE_INDEX_MEMORY_ONLY (1 << 0) +int is_sparse_index_allowed(struct index_state *istate, int flags); int convert_to_sparse(struct index_state *istate, int flags); void ensure_correct_sparsity(struct index_state *istate); void clear_skip_worktree_from_present_files(struct index_state *istate); -- cgit v1.3-6-g1900 From 491df5f679f0381f529b967df25476ab944406ab Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 10 May 2022 23:32:30 +0000 Subject: read-cache: set sparsity when index is new When the index read in 'do_read_index()' does not exist on-disk, mark the index "sparse" if the executing command does not require a full index and sparse index is otherwise enabled. Some commands (such as 'git stash -u') implicitly create a new index (when the 'GIT_INDEX_FILE' variable points to a non-existent file) and perform some operation on it. However, when this index is created, it isn't created with the same sparsity settings as the repo index. As a result, while these indexes may be sparse during the operation, they are always expanded before being written to disk. We can avoid that expansion by defaulting the index to "sparse", in which case it will only be expanded if the full index is needed. Note that the function 'set_new_index_sparsity()' is created despite having only a single caller because additional callers will be added in a subsequent patch. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- read-cache.c | 18 +++++++++++++++++- t/t1092-sparse-checkout-compatibility.sh | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/read-cache.c b/read-cache.c index 4df97e185e..60355f5ad6 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2260,6 +2260,20 @@ static unsigned long load_cache_entries_threaded(struct index_state *istate, con return consumed; } +static void set_new_index_sparsity(struct index_state *istate) +{ + /* + * If the index's repo exists, mark it sparse according to + * repo settings. + */ + if (istate->repo) { + prepare_repo_settings(istate->repo); + if (!istate->repo->settings.command_requires_full_index && + is_sparse_index_allowed(istate, 0)) + istate->sparse_index = 1; + } +} + /* remember to discard_cache() before reading a different cache! */ int do_read_index(struct index_state *istate, const char *path, int must_exist) { @@ -2281,8 +2295,10 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist) istate->timestamp.nsec = 0; fd = open(path, O_RDONLY); if (fd < 0) { - if (!must_exist && errno == ENOENT) + if (!must_exist && errno == ENOENT) { + set_new_index_sparsity(istate); return 0; + } die_errno(_("%s: index file open failed"), path); } diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 75d844cd71..85c6a56f1b 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1389,7 +1389,7 @@ test_expect_success 'sparse-index is not expanded: stash' ' ensure_not_expanded stash drop stash@{0} && echo >>sparse-index/deep/new && - ! ensure_not_expanded stash -u && + ensure_not_expanded stash -u && ( WITHOUT_UNTRACKED_TXT=1 && ! ensure_not_expanded stash pop -- cgit v1.3-6-g1900 From 874cf2a6044462ddba7162730557354a107c3a6d Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 10 May 2022 23:32:31 +0000 Subject: stash: apply stash using 'merge_ort_nonrecursive()' Update 'stash' to use 'merge_ort_nonrecursive()' to apply a stash to the current working tree. When 'git stash apply' was converted from its shell script implementation to a builtin in 8a0fc8d19d (stash: convert apply to builtin, 2019-02-25), 'merge_recursive_generic()' was used to merge a stash into the working tree as part of 'git stash (apply|pop)'. However, with the single merge base used in 'do_apply_stash()', the commit wrapping done by 'merge_recursive_generic()' is not only unnecessary, but misleading (the *real* merge base is labeled "constructed merge base"). Therefore, a non-recursive merge of the working tree, stashed tree, and stash base tree is more appropriate. There are two options for a non-recursive merge-then-update-worktree function: 'merge_trees()' and 'merge_ort_nonrecursive()'. Use 'merge_ort_nonrecursive()' to align with the default merge strategy used by 'git merge' (6a5fb96672 (Change default merge backend from recursive to ort, 2021-08-04)) and, because merge-ort does not operate in-place on the index, avoid unnecessary index expansion. Update tests in 't1092' verifying index expansion for 'git stash' accordingly. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- builtin/stash.c | 30 ++++++++++++++++++++++++------ t/t1092-sparse-checkout-compatibility.sh | 4 ++-- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/builtin/stash.c b/builtin/stash.c index 1bfba53204..3fe549f7d3 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -7,6 +7,7 @@ #include "cache-tree.h" #include "unpack-trees.h" #include "merge-recursive.h" +#include "merge-ort-wrappers.h" #include "strvec.h" #include "run-command.h" #include "dir.h" @@ -492,13 +493,13 @@ static void unstage_changes_unless_new(struct object_id *orig_tree) static int do_apply_stash(const char *prefix, struct stash_info *info, int index, int quiet) { - int ret; + int clean, ret; int has_index = index; struct merge_options o; struct object_id c_tree; struct object_id index_tree; - struct commit *result; - const struct object_id *bases[1]; + struct tree *head, *merge, *merge_base; + struct lock_file lock = LOCK_INIT; read_cache_preload(NULL); if (refresh_and_write_cache(REFRESH_QUIET, 0, 0)) @@ -541,6 +542,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, o.branch1 = "Updated upstream"; o.branch2 = "Stashed changes"; + o.ancestor = "Stash base"; if (oideq(&info->b_tree, &c_tree)) o.branch1 = "Version stash was based on"; @@ -551,10 +553,26 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, if (o.verbosity >= 3) printf_ln(_("Merging %s with %s"), o.branch1, o.branch2); - bases[0] = &info->b_tree; + head = lookup_tree(o.repo, &c_tree); + merge = lookup_tree(o.repo, &info->w_tree); + merge_base = lookup_tree(o.repo, &info->b_tree); + + repo_hold_locked_index(o.repo, &lock, LOCK_DIE_ON_ERROR); + clean = merge_ort_nonrecursive(&o, head, merge, merge_base); + + /* + * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the + * merge was clean, and nonzero if the merge was unclean or encountered + * an error. + */ + ret = clean >= 0 ? !clean : clean; + + if (ret < 0) + rollback_lock_file(&lock); + else if (write_locked_index(o.repo->index, &lock, + COMMIT_LOCK | SKIP_IF_UNCHANGED)) + ret = error(_("could not write index")); - ret = merge_recursive_generic(&o, &c_tree, &info->w_tree, 1, bases, - &result); if (ret) { rerere(0); diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 85c6a56f1b..aaf4d880db 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1385,7 +1385,7 @@ test_expect_success 'sparse-index is not expanded: stash' ' ensure_not_expanded stash && ensure_not_expanded stash list && ensure_not_expanded stash show stash@{0} && - ! ensure_not_expanded stash apply stash@{0} && + ensure_not_expanded stash apply stash@{0} && ensure_not_expanded stash drop stash@{0} && echo >>sparse-index/deep/new && @@ -1399,7 +1399,7 @@ test_expect_success 'sparse-index is not expanded: stash' ' oid=$(git -C sparse-index stash create) && ensure_not_expanded stash store -m "test" $oid && ensure_not_expanded reset --hard && - ! ensure_not_expanded stash pop + ensure_not_expanded stash pop ' test_expect_success 'sparse index is not expanded: diff' ' -- cgit v1.3-6-g1900 From 0f329b9ae4f49d1f76cdd4cae518b2ae757e111e Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 10 May 2022 23:32:32 +0000 Subject: unpack-trees: preserve index sparsity When unpacking trees, set the default sparsity of the resultant index based on repo settings and 'is_sparse_index_allowed()'. Normally, when executing 'unpack_trees', the output index is marked sparse when (and only when) it unpacks a sparse directory. However, an index may be "sparse" even if it contains no sparse directories - when all files fall inside the sparse-checkout definition or otherwise have SKIP_WORKTREE disabled. Therefore, the output index may be marked "full" even when it is "sparse", resulting in unnecessary 'ensure_full_index' calls when writing to disk. Avoid this by setting the "default" index sparsity to match what is expected for the repository. As a consequence of this fix, the (non-merge) 'read-tree' performed when applying a stash with untracked files no longer expands the index. Update the corresponding test in 't1092'. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano --- t/t1092-sparse-checkout-compatibility.sh | 2 +- unpack-trees.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index aaf4d880db..19221c1422 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1392,7 +1392,7 @@ test_expect_success 'sparse-index is not expanded: stash' ' ensure_not_expanded stash -u && ( WITHOUT_UNTRACKED_TXT=1 && - ! ensure_not_expanded stash pop + ensure_not_expanded stash pop ) && ensure_not_expanded stash create && diff --git a/unpack-trees.c b/unpack-trees.c index 7f528d35cc..a1d0ff3a4d 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -11,6 +11,7 @@ #include "refs.h" #include "attr.h" #include "split-index.h" +#include "sparse-index.h" #include "submodule.h" #include "submodule-config.h" #include "fsmonitor.h" @@ -1839,6 +1840,11 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options o->result.fsmonitor_last_update = xstrdup_or_null(o->src_index->fsmonitor_last_update); + if (!o->src_index->initialized && + !repo->settings.command_requires_full_index && + is_sparse_index_allowed(&o->result, 0)) + o->result.sparse_index = 1; + /* * Sparse checkout loop #1: set NEW_SKIP_WORKTREE on existing entries */ -- cgit v1.3-6-g1900