From 952de281fe63eb03e0dcc8adf773ce54cb581b83 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 16 May 2025 14:55:27 +0000 Subject: apply: integrate with the sparse index The sparse index allows storing directory entries in the index, marked with the skip-wortkree bit and pointing to a tree object. This may be an unexpected data shape for some implementation areas, so we are rolling it out incrementally on a builtin-per-builtin basis. This change enables the sparse index for 'git apply'. The main motivation for this change is that 'git apply' is used as a child process of 'git add -p' and expanding the sparse index for each of those child processes can lead to significant performance issues. The good news is that the actual index manipulation code used by 'git apply' is already integrated with the sparse index, so the only product change is to mark the builtin as allowing the sparse index so it isn't inflated on read. The more involved part of this change is around adding tests that verify how 'git apply' behaves in a sparse-checkout environment and whether or not the index expands in certain operations. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/apply.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/apply.c b/builtin/apply.c index 84f1863d3a..a1e20c593d 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -12,7 +12,7 @@ static const char * const apply_usage[] = { int cmd_apply(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { int force_apply = 0; int options = 0; @@ -35,6 +35,11 @@ int cmd_apply(int argc, &state, &force_apply, &options, apply_usage); + if (repo) { + prepare_repo_settings(repo); + repo->settings.command_requires_full_index = 0; + } + if (check_apply_state(&state, force_apply)) exit(128); -- cgit v1.3-5-g9baa From 02ed8555f68440c5f533ad3c098ac01fc8965861 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 16 May 2025 14:55:28 +0000 Subject: git add: make -p/-i aware of sparse index It is slow to expand a sparse index in-memory due to parsing of trees. We aim to minimize that performance cost when possible. 'git add -p' uses 'git apply' child processes to modify the index, but still there are some expansions that occur. It turns out that control flows out of cmd_add() in the interactive cases before the lines that confirm that the builtin is integrated with the sparse index. Moving that integration point earlier in cmd_add() allows 'git add -i' and 'git add -p' to operate without expanding a sparse index to a full one. Add test cases that confirm that these interactive add options work with the sparse index. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/add.c | 7 ++-- t/t1092-sparse-checkout-compatibility.sh | 60 ++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) (limited to 'builtin') diff --git a/builtin/add.c b/builtin/add.c index 78dfb26577..b96360dc5c 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -391,6 +391,10 @@ int cmd_add(int argc, argc = parse_options(argc, argv, prefix, builtin_add_options, builtin_add_usage, PARSE_OPT_KEEP_ARGV0); + + prepare_repo_settings(repo); + repo->settings.command_requires_full_index = 0; + if (patch_interactive) add_interactive = 1; if (add_interactive) { @@ -427,9 +431,6 @@ int cmd_add(int argc, add_new_files = !take_worktree_changes && !refresh_only && !add_renormalize; require_pathspec = !(take_worktree_changes || (0 < addremove_explicit)); - prepare_repo_settings(repo); - repo->settings.command_requires_full_index = 0; - repo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR); /* diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index fa2472010d..f47cf8fa7f 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -384,6 +384,38 @@ test_expect_success 'add, commit, checkout' ' test_all_match git checkout - ' +test_expect_success 'git add -p' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + # Does not expand when edits are within sparse checkout. + run_on_all ../edit-contents deep/a && + run_on_all ../edit-contents deep/deeper1/a && + + test_write_lines y n >in && + run_on_all git add -p in && + run_on_all git add -i in && + run_on_all git add -p in && + run_on_all git add -i sparse-index/deep/a && + echo "new content" >sparse-index/deep/deeper1/a && + test_write_lines y n >in && + ensure_not_expanded add -p sparse-index/folder1/a && + test_write_lines y n y >in && + ensure_expanded add -p sparse-index/folder1/a && + test_write_lines u 2 3 "" q >in && + ensure_expanded add -i Date: Fri, 16 May 2025 14:55:29 +0000 Subject: reset: integrate sparse index with --patch Similar to the previous change for 'git add -p', the reset builtin checked for integration with the sparse index after possibly redirecting its logic toward the interactive logic. This means that the builtin would expand the sparse index to a full one upon read. Move this check earlier within cmd_reset() to improve performance here. Add tests to guarantee that we are not universally expanding the index. Add behavior tests to check that we are doing the same operations as a full index. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/reset.c | 6 ++--- t/t1092-sparse-checkout-compatibility.sh | 42 ++++++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 5 deletions(-) (limited to 'builtin') diff --git a/builtin/reset.c b/builtin/reset.c index 73b4537a9a..dc50ffc1ac 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -420,6 +420,9 @@ int cmd_reset(int argc, oidcpy(&oid, &tree->object.oid); } + prepare_repo_settings(the_repository); + the_repository->settings.command_requires_full_index = 0; + if (patch_mode) { if (reset_type != NONE) die(_("options '%s' and '%s' cannot be used together"), "--patch", "--{hard,mixed,soft}"); @@ -457,9 +460,6 @@ int cmd_reset(int argc, if (intent_to_add && reset_type != MIXED) die(_("the option '%s' requires '%s'"), "-N", "--mixed"); - prepare_repo_settings(the_repository); - the_repository->settings.command_requires_full_index = 0; - if (repo_read_index(the_repository) < 0) die(_("index file corrupt")); diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index f47cf8fa7f..e11dfd872e 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -384,7 +384,7 @@ test_expect_success 'add, commit, checkout' ' test_all_match git checkout - ' -test_expect_success 'git add -p' ' +test_expect_success 'git add, checkout, and reset with -p' ' init_repos && write_script edit-contents <<-\EOF && @@ -398,7 +398,7 @@ test_expect_success 'git add -p' ' test_write_lines y n >in && run_on_all git add -p in && run_on_all git add -i in && run_on_all git add -i in && + test_sparse_match git checkout HEAD~1 --patch sparse-index/deep/a && + echo "new content" >sparse-index/deep/deeper1/a && + git -C sparse-index commit -a -m "inside-changes" && + + test_write_lines y y >in && + ensure_not_expanded checkout HEAD~1 --patch sparse-index/deep/a && + echo "new content" >sparse-index/deep/deeper1/a && + git -C sparse-index add . && + ensure_not_expanded reset --patch sparse-index/folder1/a && + git -C sparse-index add --sparse folder1 && + git -C sparse-index sparse-checkout reapply && + ensure_expanded reset --patch sparse-index/folder1/a && + git -C sparse-index add --sparse folder1 && + git -C sparse-index commit -m "folder1 change" && + git -C sparse-index sparse-checkout reapply && + ensure_expanded checkout HEAD~1 --patch