{"thread":{"id":"63410","subject":"[PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'","startedAt":"2025-05-07T00:55:24Z","lastAt":"2025-05-20T15:07:25Z","messageCount":22,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano","Elijah Newren","Phillip Wood","Derrick Stolee","phillip.wood123@gmail.com"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"517407","messageId":"pull.1914.git.1746579320.gitgitgadget@gmail.com","threadId":"63410","inReplyTo":null,"subject":"[PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-07T00:55:17Z","receivedAt":"2025-05-07T00:55:24Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The sparse index helps make some Git commands faster when using\nsparse-checkout in cone mode. However, not all code paths are aware that the\nindex can have non-blob entries, so we are careful about rolling this\nfeature out gradually. The cost of this rollout is that some commands are\nslower with the sparse index as they need to expand a sparse index into a\nfull index in memory, which requires parsing tree objects to construct the\nfull path list.\n\nThis patch series focuses on the 'git add -p' command, which is slow with\nthe sparse index for a couple of reasons, handled in the first two patches:\n\n 1. 'git add -p' uses 'git apply' as a subcommand and 'git apply' needs\n integration with the sparse index. Luckily, we just need to add the repo\n setting and appropriate tests to confirm it behaves as expected.\n 2. The interactive modes of 'git add' ('-p' and '-i') leave cmd_add()\n before the code that sets the repo setting to allow for a sparse index.\n Patch 2 fixes this and adds appropriate tests to confirm the behavior in\n a sparse-checkout.\n\nA third patch adds a performance test to p2000-sparse-operations.sh to\nconfirm that we are getting the performance improvement we expect:\n\n BASE PATCH 1 PATCH 2\n---------------------------------------------------------\n2000.118: (full-v3) 0.80 0.84 +5.0% 0.84 +5.0%\n2000.119: (full-v4) 0.76 0.79 +3.9% 0.80 +5.3%\n2000.120: (sparse-v3) 2.09 1.39 -33.5% 0.07 -96.7%\n2000.121: (sparse-v4) 2.09 1.39 -33.5% 0.07 -96.7%\n\n\nThanks, -Stolee\n\nDerrick Stolee (3):\n apply: integrate with the sparse index\n git add: make -p/-i aware of sparse index\n p2000: add performance test for 'git add -p'\n\n builtin/add.c | 7 +-\n builtin/apply.c | 7 +-\n t/perf/p2000-sparse-operations.sh | 1 +\n t/t1092-sparse-checkout-compatibility.sh | 102 +++++++++++++++++++++++\n 4 files changed, 113 insertions(+), 4 deletions(-)\n\n\nbase-commit: 6c0bd1fc70efaf053abe4e57c976afdc72d15377\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1914%2Fderrickstolee%2Fapply-sparse-index-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1914/derrickstolee/apply-sparse-index-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1914\n-- \ngitgitgadget\n"},{"id":"517408","messageId":"0e6e199cd19ef186fb10b192529269cafc47805a.1746579321.git.gitgitgadget@gmail.com","threadId":"63410","inReplyTo":"pull.1914.git.1746579320.gitgitgadget@gmail.com","subject":"[PATCH 1/3] apply: integrate with the sparse index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-07T00:55:18Z","receivedAt":"2025-05-07T00:55:24Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe sparse index allows storing directory entries in the index, marked\nwith the skip-wortkree bit and pointing to a tree object. This may be an\nunexpected data shape for some implementation areas, so we are rolling\nit out incrementally on a builtin-per-builtin basis.\n\nThis change enables the sparse index for 'git apply'. The main\nmotivation for this change is that 'git apply' is used as a child\nprocess of 'git add -p' and expanding the sparse index for each of those\nchild processes can lead to significant performance issues.\n\nThe good news is that the actual index manipulation code used by 'git\napply' is already integrated with the sparse index, so the only product\nchange is to mark the builtin as allowing the sparse index so it isn't\ninflated on read.\n\nThe more involved part of this change is around adding tests that verify\nhow 'git apply' behaves in a sparse-checkout environment and whether or\nnot the index expands in certain operations.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/apply.c | 7 +++-\n t/t1092-sparse-checkout-compatibility.sh | 46 ++++++++++++++++++++++++\n 2 files changed, 52 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 84f1863d3ac3..a1e20c593d09 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -12,7 +12,7 @@ static const char * const apply_usage[] = {\n int cmd_apply(int argc,\n \t const char **argv,\n \t const char *prefix,\n-\t struct repository *repo UNUSED)\n+\t struct repository *repo)\n {\n \tint force_apply = 0;\n \tint options = 0;\n@@ -35,6 +35,11 @@ int cmd_apply(int argc,\n \t\t\t\t &state, &force_apply, &options,\n \t\t\t\t apply_usage);\n \n+\tif (repo) {\n+\t\tprepare_repo_settings(repo);\n+\t\trepo->settings.command_requires_full_index = 0;\n+\t}\n+\n \tif (check_apply_state(&state, force_apply))\n \t\texit(128);\n \ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex f9b448792cb4..ab8bd371eff3 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -1340,6 +1340,30 @@ test_expect_success 'submodule handling' '\n \tgrep \"160000 $(git -C initial-repo rev-parse HEAD) 0\tmodules/sub\" cache\n '\n \n+test_expect_success 'git apply functionality' '\n+\tinit_repos &&\n+\n+\ttest_all_match git checkout base &&\n+\n+\tgit -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&\n+\tgit -C full-checkout diff base..merge-right -- folder2 >patch-outside &&\n+\n+\t# Apply a patch to a file inside the sparse definition\n+\ttest_all_match git apply --index --stat ../patch-in-sparse &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\t# Apply a patch to a file outside the sparse definition\n+\ttest_sparse_match test_must_fail git apply ../patch-outside &&\n+\tgrep \"No such file or directory\" sparse-checkout-err &&\n+\n+\t# But it works with --index and --cached\n+\ttest_all_match git apply --index --stat ../patch-outside &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\ttest_all_match git reset --hard &&\n+\ttest_all_match git apply --cached --stat ../patch-outside &&\n+\ttest_all_match git status --porcelain=v2\n+'\n+\n # When working with a sparse index, some commands will need to expand the\n # index to operate properly. If those commands also write the index back\n # to disk, they need to convert the index to sparse before writing.\n@@ -2345,6 +2369,28 @@ test_expect_success 'sparse-index is not expanded: check-attr' '\n \tensure_not_expanded check-attr -a --cached -- folder1/a\n '\n \n+test_expect_success 'sparse-index is not expanded: git apply' '\n+\tinit_repos &&\n+\n+\tgit -C sparse-index checkout base &&\n+\tgit -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&\n+\tgit -C full-checkout diff base..merge-right -- folder2 >patch-outside &&\n+\n+\t# Apply a patch to a file inside the sparse definition\n+\tensure_not_expanded apply --index --stat ../patch-in-sparse &&\n+\n+\t# Apply a patch to a file outside the sparse definition\n+\t# Fails when caring about the worktree.\n+\tensure_not_expanded ! apply ../patch-outside &&\n+\n+\t# Expands when using --index.\n+\tensure_expanded apply --index ../patch-outside &&\n+\tgit -C sparse-index reset --hard &&\n+\n+\t# Does not expand when using --cached.\n+\tensure_not_expanded apply --cached ../patch-outside\n+'\n+\n test_expect_success 'advice.sparseIndexExpanded' '\n \tinit_repos &&\n \n-- \ngitgitgadget\n\n"},{"id":"517409","messageId":"63caae876343faa9609516b0797fd9ff19f7a881.1746579321.git.gitgitgadget@gmail.com","threadId":"63410","inReplyTo":"pull.1914.git.1746579320.gitgitgadget@gmail.com","subject":"[PATCH 2/3] git add: make -p/-i aware of sparse index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-07T00:55:19Z","receivedAt":"2025-05-07T00:55:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIt is slow to expand a sparse index in-memory due to parsing of trees.\nWe aim to minimize that performance cost when possible. 'git add -p'\nuses 'git apply' child processes to modify the index, but still there\nare some expansions that occur.\n\nIt turns out that control flows out of cmd_add() in the interactive\ncases before the lines that confirm that the builtin is integrated with\nthe sparse index. We need to move that earlier to ensure it prevents a\nfull index expansion on read.\n\nAdd more test cases that confirm that these interactive add options work\nwith the sparse index. One interesting aspect here is that the '-i'\noption avoids expanding the sparse index when a sparse directory exists\non disk while the '-p' option does hit the ensure_full_index() method.\nThis leaves some room for improvement, but this case should be atypical\nas users should remain within their sparse-checkout.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/add.c | 7 +--\n t/t1092-sparse-checkout-compatibility.sh | 56 ++++++++++++++++++++++++\n 2 files changed, 60 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 747511b68bc3..7c292ffdc6c2 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -390,6 +390,10 @@ int cmd_add(int argc,\n \n \targc = parse_options(argc, argv, prefix, builtin_add_options,\n \t\t\t builtin_add_usage, PARSE_OPT_KEEP_ARGV0);\n+\n+\tprepare_repo_settings(repo);\n+\trepo->settings.command_requires_full_index = 0;\n+\n \tif (patch_interactive)\n \t\tadd_interactive = 1;\n \tif (add_interactive) {\n@@ -426,9 +430,6 @@ int cmd_add(int argc,\n \tadd_new_files = !take_worktree_changes && !refresh_only && !add_renormalize;\n \trequire_pathspec = !(take_worktree_changes || (0 < addremove_explicit));\n \n-\tprepare_repo_settings(repo);\n-\trepo->settings.command_requires_full_index = 0;\n-\n \trepo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR);\n \n \t/*\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex ab8bd371eff3..0dc5dd27184d 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -384,6 +384,38 @@ test_expect_success 'add, commit, checkout' '\n \ttest_all_match git checkout -\n '\n \n+test_expect_success 'git add -p' '\n+\tinit_repos &&\n+\n+\twrite_script edit-contents <<-\\EOF &&\n+\techo text >>$1\n+\tEOF\n+\n+\t# Does not expand when edits are within sparse checkout.\n+\trun_on_all ../edit-contents deep/a &&\n+\trun_on_all ../edit-contents deep/deeper1/a &&\n+\n+\ttest_write_lines y n >in &&\n+\trun_on_all git add -p in &&\n+\trun_on_all git add -i in &&\n+\trun_on_all git add -p in &&\n+\trun_on_all git add -i sparse-index/deep/a &&\n+\techo \"new content\" >sparse-index/deep/deeper1/a &&\n+\ttest_write_lines y n >in &&\n+\tensure_not_expanded add -p sparse-index/folder1/a &&\n+\n+\t# -p does expand when edits are outside sparse checkout.\n+\ttest_write_lines y n y >in &&\n+\tensure_expanded add -p in &&\n+\tensure_not_expanded add -i \n\nThe previous two changes contributed performance improvements to 'git\napply' and 'git add -p' when using a sparse index. Add a performance\ntest to demonstrate this (and to help validate that performance remains\ngood in the future).\n\nIn the truncated test output below, we see that the full checkout\nperformance changes within noise expectations, but the sparse index\ncases improve 33% and then 96%.\n\n HEAD~3 HEAD~2 HEAD~1\n---------------------------------------------------------\n2000.118: (full-v3) 0.80 0.84 +5.0% 0.84 +5.0%\n2000.119: (full-v4) 0.76 0.79 +3.9% 0.80 +5.3%\n2000.120: (sparse-v3) 2.09 1.39 -33.5% 0.07 -96.7%\n2000.121: (sparse-v4) 2.09 1.39 -33.5% 0.07 -96.7%\n\nIt is worth noting that if our test was more involved and had multiple\nhunks to evaluate, then the time spent in 'git apply' would dominate due\nto multiple index loads and writes. As it stands, we need the sparse\nindex improvement in 'git add -p' itself to confirm this performance\nimprovement.\n\nSince the change for 'git add -i' is identical, we avoid a second test\ncase for that similar operation.\n\nSigned-off-by: Derrick Stolee \n---\n t/perf/p2000-sparse-operations.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 39e92b084143..3da6ae59c000 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -135,5 +135,6 @@ test_perf_on_all git diff-tree HEAD\n test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a\n test_perf_on_all \"git worktree add ../temp && git worktree remove ../temp\"\n test_perf_on_all git check-attr -a -- $SPARSE_CONE/a\n+test_perf_on_all 'echo >>a && test_write_lines y | git add -p'\n \n test_done\n-- \ngitgitgadget\n"},{"id":"517588","messageId":"xmqqecwz557w.fsf@gitster.g","threadId":"63410","inReplyTo":"pull.1914.git.1746579320.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-08T18:26:27Z","receivedAt":"2025-05-08T18:26:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> The sparse index helps make some Git commands faster when using\n> sparse-checkout in cone mode. However, not all code paths are aware that the\n> index can have non-blob entries, so we are careful about rolling this\n> feature out gradually. The cost of this rollout is that some commands are\n> slower with the sparse index as they need to expand a sparse index into a\n> full index in memory, which requires parsing tree objects to construct the\n> full path list.\n>\n> This patch series focuses on the 'git add -p' command, which is slow with\n> the sparse index for a couple of reasons, handled in the first two patches:\n>\n> 1. 'git add -p' uses 'git apply' as a subcommand and 'git apply' needs\n> integration with the sparse index. Luckily, we just need to add the repo\n> setting and appropriate tests to confirm it behaves as expected.\n> 2. The interactive modes of 'git add' ('-p' and '-i') leave cmd_add()\n> before the code that sets the repo setting to allow for a sparse index.\n> Patch 2 fixes this and adds appropriate tests to confirm the behavior in\n> a sparse-checkout.\n>\n> A third patch adds a performance test to p2000-sparse-operations.sh to\n> confirm that we are getting the performance improvement we expect:\n>\n> BASE PATCH 1 PATCH 2\n> ---------------------------------------------------------\n> 2000.118: (full-v3) 0.80 0.84 +5.0% 0.84 +5.0%\n> 2000.119: (full-v4) 0.76 0.79 +3.9% 0.80 +5.3%\n> 2000.120: (sparse-v3) 2.09 1.39 -33.5% 0.07 -96.7%\n> 2000.121: (sparse-v4) 2.09 1.39 -33.5% 0.07 -96.7%\n>\n>\n> Thanks, -Stolee\n\nAs always, it is delight to read a well-written cover letter that\nnaturally convinces readers why the series is worth reading ;-)\n\nThanks.\n"},{"id":"517719","messageId":"CABPp-BHkgvc0UQbhXfP4POtY8GPVpz9J8ZbX3_jyzL_V7GyBbA@mail.gmail.com","threadId":"63410","inReplyTo":"0e6e199cd19ef186fb10b192529269cafc47805a.1746579321.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] apply: integrate with the sparse index","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-05-10T03:18:35Z","receivedAt":"2025-05-10T03:18:47Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> The sparse index allows storing directory entries in the index, marked\n> with the skip-wortkree bit and pointing to a tree object. This may be an\n> unexpected data shape for some implementation areas, so we are rolling\n> it out incrementally on a builtin-per-builtin basis.\n>\n> This change enables the sparse index for 'git apply'. The main\n> motivation for this change is that 'git apply' is used as a child\n> process of 'git add -p' and expanding the sparse index for each of those\n> child processes can lead to significant performance issues.\n>\n> The good news is that the actual index manipulation code used by 'git\n> apply' is already integrated with the sparse index, so the only product\n> change is to mark the builtin as allowing the sparse index so it isn't\n> inflated on read.\n>\n> The more involved part of this change is around adding tests that verify\n> how 'git apply' behaves in a sparse-checkout environment and whether or\n> not the index expands in certain operations.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> builtin/apply.c | 7 +++-\n> t/t1092-sparse-checkout-compatibility.sh | 46 ++++++++++++++++++++++++\n> 2 files changed, 52 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/apply.c b/builtin/apply.c\n> index 84f1863d3ac3..a1e20c593d09 100644\n> --- a/builtin/apply.c\n> +++ b/builtin/apply.c\n> @@ -12,7 +12,7 @@ static const char * const apply_usage[] = {\n> int cmd_apply(int argc,\n> const char **argv,\n> const char *prefix,\n> - struct repository *repo UNUSED)\n> + struct repository *repo)\n> {\n> int force_apply = 0;\n> int options = 0;\n> @@ -35,6 +35,11 @@ int cmd_apply(int argc,\n> &state, &force_apply, &options,\n> apply_usage);\n>\n> + if (repo) {\n> + prepare_repo_settings(repo);\n> + repo->settings.command_requires_full_index = 0;\n> + }\n> +\n> if (check_apply_state(&state, force_apply))\n> exit(128);\n>\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index f9b448792cb4..ab8bd371eff3 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -1340,6 +1340,30 @@ test_expect_success 'submodule handling' '\n> grep \"160000 $(git -C initial-repo rev-parse HEAD) 0 modules/sub\" cache\n> '\n>\n> +test_expect_success 'git apply functionality' '\n> + init_repos &&\n> +\n> + test_all_match git checkout base &&\n> +\n> + git -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&\n> + git -C full-checkout diff base..merge-right -- folder2 >patch-outside &&\n> +\n> + # Apply a patch to a file inside the sparse definition\n> + test_all_match git apply --index --stat ../patch-in-sparse &&\n> + test_all_match git status --porcelain=v2 &&\n> +\n> + # Apply a patch to a file outside the sparse definition\n> + test_sparse_match test_must_fail git apply ../patch-outside &&\n> + grep \"No such file or directory\" sparse-checkout-err &&\n\nI was slightly confused by this at first, because I was thinking of\nthe case where folder2/a exists in the working directory despite not\nmatching the sparsity patterns, but you were testing a case where the\nworking directory matched the sparsity patterns, so folder2/a doesn't\nexist.\n\nSo, the check here looks good.\n\nAnd, when folder2/a does exist, then we're in the case handled by\n82386b44963f (Merge branch 'en/present-despite-skipped', 2022-03-09),\nwhich forces the directory to not be considered sparse, and so it's\njust like the ../patch-in-sparse case...meaning it's not really a\ndifferent case to test.\n\nSo, it all makes sense.\n\n> +\n> + # But it works with --index and --cached\n> + test_all_match git apply --index --stat ../patch-outside &&\n> + test_all_match git status --porcelain=v2 &&\n> + test_all_match git reset --hard &&\n> + test_all_match git apply --cached --stat ../patch-outside &&\n> + test_all_match git status --porcelain=v2\n> +'\n> +\n> # When working with a sparse index, some commands will need to expand the\n> # index to operate properly. If those commands also write the index back\n> # to disk, they need to convert the index to sparse before writing.\n> @@ -2345,6 +2369,28 @@ test_expect_success 'sparse-index is not expanded: check-attr' '\n> ensure_not_expanded check-attr -a --cached -- folder1/a\n> '\n>\n> +test_expect_success 'sparse-index is not expanded: git apply' '\n> + init_repos &&\n> +\n> + git -C sparse-index checkout base &&\n> + git -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&\n> + git -C full-checkout diff base..merge-right -- folder2 >patch-outside &&\n> +\n> + # Apply a patch to a file inside the sparse definition\n> + ensure_not_expanded apply --index --stat ../patch-in-sparse &&\n> +\n> + # Apply a patch to a file outside the sparse definition\n> + # Fails when caring about the worktree.\n> + ensure_not_expanded ! apply ../patch-outside &&\n> +\n> + # Expands when using --index.\n> + ensure_expanded apply --index ../patch-outside &&\n> + git -C sparse-index reset --hard &&\n\nAll makes sense up to here.\n\n> +\n> + # Does not expand when using --cached.\n> + ensure_not_expanded apply --cached ../patch-outside\n\nWait, what? That makes no sense.\n\nAfter some digging, I see why the test passed, but it's very\nmisleading. Just before this command, if you ran the following\ncommands from the sparse-index directory, you'd see the following:\n\n$ rm testme\n$ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside\n$ grep ensure_full_index testme\n$\n\nWhich matches what you were testing and shows why it passed for you.\nBut I'd argue the test is not correct and confusing for anyone that\nreads it, because:\n\n$ git ls-files -s --sparse | grep folder2\n040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0 folder2/0/\n100644 78981922613b2afb6025042ff6bd878ac1994e85 0 folder2/a\n\nIn other words, the index was *already* (partially) expanded by the\n`git apply --index`, and the `git reset --hard` did not fix that\ncontrary to expectations. Continuing from here we see:\n\n$ git reset --hard\nHEAD is now at 703fd3e initial commit\n$ git ls-files -s --sparse | grep folder2\n040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0 folder2/0/\n100644 78981922613b2afb6025042ff6bd878ac1994e85 0 folder2/a\n$ git sparse-checkout reapply\n$ git ls-files -s --sparse | grep folder2\n040000 123706f6fc38949628eaf0483edbf97ba21123ae 0 folder2/\n\nSo, we need to do a `git sparse-checkout reapply` to make sure we were\nactually in the expected fully sparse state. From here...\n\n$ rm testme\n$ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside\n$ grep ensure_full_index testme\n{\"event\":\"region_enter\",\"sid\":\"20250510T030856.854763Z-H8ec63c79-P0000371c\",\"thread\":\"main\",\"time\":\"2025-05-10T03:08:56.856008Z\",\"file\":\"sparse-index.c\",\"line\":372,\"repo\":1,\"nesting\":1,\"category\":\"index\",\"label\":\"ensure_full_index\"}\n{\"event\":\"region_leave\",\"sid\":\"20250510T030856.854763Z-H8ec63c79-P0000371c\",\"thread\":\"main\",\"time\":\"2025-05-10T03:08:56.856454Z\",\"file\":\"sparse-index.c\",\"line\":455,\"repo\":1,\"t_rel\":0.000446,\"nesting\":1,\"category\":\"index\",\"label\":\"ensure_full_index\"}\n{\"event\":\"region_enter\",\"sid\":\"20250510T030856.854763Z-H8ec63c79-P0000371c\",\"thread\":\"main\",\"time\":\"2025-05-10T03:08:56.857016Z\",\"file\":\"sparse-index.c\",\"line\":372,\"repo\":1,\"nesting\":1,\"category\":\"index\",\"label\":\"ensure_full_index\"}\n{\"event\":\"region_leave\",\"sid\":\"20250510T030856.854763Z-H8ec63c79-P0000371c\",\"thread\":\"main\",\"time\":\"2025-05-10T03:08:56.857135Z\",\"file\":\"sparse-index.c\",\"line\":455,\"repo\":1,\"t_rel\":0.000119,\"nesting\":1,\"category\":\"index\",\"label\":\"ensure_full_index\"}\n\nSo, indeed, `git apply --cached ../patch-outside` DOES expand the\nindex, as I expected. It has to when folder2/ is a directory in the\nindex, so that we can get a folder2/a entry that we can modify. And\nthat's just what we see:\n\n$ git ls-files -s --sparse | grep folder2\n040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0 folder2/0/\n100644 50f4ca4e6265a0497ec2ee6782648138914ad398 0 folder2/a\n\n\nCan you add a `git sparse-checkout reapply` right after your `git\nreset --hard`, and then switch the ensure_not_expanded to\nensure_expanded for the apply --cached call?\n"},{"id":"517721","messageId":"CABPp-BEmMaFQxE9NQgM8M=cgfBHY1p56vnBt7R4CfuiXnq++4Q@mail.gmail.com","threadId":"63410","inReplyTo":"63caae876343faa9609516b0797fd9ff19f7a881.1746579321.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] git add: make -p/-i aware of sparse index","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-05-10T04:38:08Z","receivedAt":"2025-05-10T04:38:20Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> It is slow to expand a sparse index in-memory due to parsing of trees.\n> We aim to minimize that performance cost when possible. 'git add -p'\n> uses 'git apply' child processes to modify the index, but still there\n> are some expansions that occur.\n\nstill there are some expansions that occur...outside of those child\nprocesses? Is that what you're trying to say, or was it something\nelse?\n\n> It turns out that control flows out of cmd_add() in the interactive\n> cases before the lines that confirm that the builtin is integrated with\n> the sparse index. We need to move that earlier to ensure it prevents a\n> full index expansion on read.\n>\n> Add more test cases that confirm that these interactive add options work\n> with the sparse index. One interesting aspect here is that the '-i'\n> option avoids expanding the sparse index when a sparse directory exists\n> on disk while the '-p' option does hit the ensure_full_index() method.\n> This leaves some room for improvement, but this case should be atypical\n> as users should remain within their sparse-checkout.\n\nIt's not clear whether this paragraph is talking about existing state\n(before the patch) or desired state (after the patch). I think based\non the context it's the former, but the last sentence sounds more like\na future work direction that makes it very unclear, to me at least.\n\n> Signed-off-by: Derrick Stolee \n> ---\n> builtin/add.c | 7 +--\n> t/t1092-sparse-checkout-compatibility.sh | 56 ++++++++++++++++++++++++\n> 2 files changed, 60 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 747511b68bc3..7c292ffdc6c2 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -390,6 +390,10 @@ int cmd_add(int argc,\n>\n> argc = parse_options(argc, argv, prefix, builtin_add_options,\n> builtin_add_usage, PARSE_OPT_KEEP_ARGV0);\n> +\n> + prepare_repo_settings(repo);\n> + repo->settings.command_requires_full_index = 0;\n> +\n> if (patch_interactive)\n> add_interactive = 1;\n> if (add_interactive) {\n> @@ -426,9 +430,6 @@ int cmd_add(int argc,\n> add_new_files = !take_worktree_changes && !refresh_only && !add_renormalize;\n> require_pathspec = !(take_worktree_changes || (0 < addremove_explicit));\n>\n> - prepare_repo_settings(repo);\n> - repo->settings.command_requires_full_index = 0;\n> -\n> repo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR);\n>\n> /*\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index ab8bd371eff3..0dc5dd27184d 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -384,6 +384,38 @@ test_expect_success 'add, commit, checkout' '\n> test_all_match git checkout -\n> '\n>\n> +test_expect_success 'git add -p' '\n> + init_repos &&\n> +\n> + write_script edit-contents <<-\\EOF &&\n> + echo text >>$1\n> + EOF\n> +\n> + # Does not expand when edits are within sparse checkout.\n> + run_on_all ../edit-contents deep/a &&\n> + run_on_all ../edit-contents deep/deeper1/a &&\n> +\n> + test_write_lines y n >in &&\n> + run_on_all git add -p + test_all_match git status --porcelain=v2 &&\n> + test_all_match git reset &&\n> +\n> + test_write_lines u 1 \"\" q >in &&\n> + run_on_all git add -i + test_all_match git status --porcelain=v2 &&\n> + test_all_match git reset --hard &&\n> +\n> + run_on_sparse mkdir -p folder1 &&\n> + run_on_all ../edit-contents folder1/a &&\n> + test_write_lines y n y >in &&\n> + run_on_all git add -p + test_sparse_match git status --porcelain=v2 &&\n> + test_sparse_match git reset &&\n> + test_write_lines u 2 3 \"\" q >in &&\n> + run_on_all git add -i + test_sparse_match git status --porcelain=v2\n> +'\n> +\n> test_expect_success 'deep changes during checkout' '\n> init_repos &&\n>\n> @@ -2391,6 +2423,30 @@ test_expect_success 'sparse-index is not expanded: git apply' '\n> ensure_not_expanded apply --cached ../patch-outside\n> '\n>\n> +test_expect_success 'sparse-index is not expanded: git add -p' '\n> + init_repos &&\n> +\n> + # Does not expand when edits are within sparse checkout.\n> + echo \"new content\" >sparse-index/deep/a &&\n> + echo \"new content\" >sparse-index/deep/deeper1/a &&\n> + test_write_lines y n >in &&\n> + ensure_not_expanded add -p + git -C sparse-index reset &&\n> + ensure_not_expanded add -i +\n> + mkdir -p sparse-index/folder1 &&\n> + echo \"new content\" >sparse-index/folder1/a &&\n> +\n> + # -p does expand when edits are outside sparse checkout.\n> + test_write_lines y n y >in &&\n> + ensure_expanded add -p +\n> + # but -i does not expand.\n> + git -C sparse-index reset &&\n> + test_write_lines u 2 3 \"\" q >in &&\n> + ensure_not_expanded add -i wrote:\n>\n> From: Derrick Stolee \n>\n> The previous two changes contributed performance improvements to 'git\n> apply' and 'git add -p' when using a sparse index. Add a performance\n> test to demonstrate this (and to help validate that performance remains\n> good in the future).\n>\n> In the truncated test output below, we see that the full checkout\n> performance changes within noise expectations, but the sparse index\n> cases improve 33% and then 96%.\n>\n> HEAD~3 HEAD~2 HEAD~1\n> ---------------------------------------------------------\n> 2000.118: (full-v3) 0.80 0.84 +5.0% 0.84 +5.0%\n> 2000.119: (full-v4) 0.76 0.79 +3.9% 0.80 +5.3%\n> 2000.120: (sparse-v3) 2.09 1.39 -33.5% 0.07 -96.7%\n> 2000.121: (sparse-v4) 2.09 1.39 -33.5% 0.07 -96.7%\n>\n> It is worth noting that if our test was more involved and had multiple\n> hunks to evaluate, then the time spent in 'git apply' would dominate due\n> to multiple index loads and writes. As it stands, we need the sparse\n> index improvement in 'git add -p' itself to confirm this performance\n> improvement.\n>\n> Since the change for 'git add -i' is identical, we avoid a second test\n> case for that similar operation.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> t/perf/p2000-sparse-operations.sh | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\n> index 39e92b084143..3da6ae59c000 100755\n> --- a/t/perf/p2000-sparse-operations.sh\n> +++ b/t/perf/p2000-sparse-operations.sh\n> @@ -135,5 +135,6 @@ test_perf_on_all git diff-tree HEAD\n> test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a\n> test_perf_on_all \"git worktree add ../temp && git worktree remove ../temp\"\n> test_perf_on_all git check-attr -a -- $SPARSE_CONE/a\n> +test_perf_on_all 'echo >>a && test_write_lines y | git add -p'\n>\n> test_done\n> --\n> gitgitgadget\n\nVery nice!\n"},{"id":"518048","messageId":"c1f7a9d6-4a44-4c00-bf26-6852819361e9@gmail.com","threadId":"63410","inReplyTo":"pull.1914.git.1746579320.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-05-14T15:16:31Z","receivedAt":"2025-05-14T15:16:37Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Stolee\n\nOn 07/05/2025 01:55, Derrick Stolee via GitGitGadget wrote:\n> The sparse index helps make some Git commands faster when using\n> sparse-checkout in cone mode. However, not all code paths are aware that the\n> index can have non-blob entries, so we are careful about rolling this\n> feature out gradually. The cost of this rollout is that some commands are\n> slower with the sparse index as they need to expand a sparse index into a\n> full index in memory, which requires parsing tree objects to construct the\n> full path list.\n> \n> This patch series focuses on the 'git add -p' command, which is slow with\n> the sparse index for a couple of reasons, handled in the first two patches:\n> \n> 1. 'git add -p' uses 'git apply' as a subcommand and 'git apply' needs\n> integration with the sparse index. Luckily, we just need to add the repo\n> setting and appropriate tests to confirm it behaves as expected.\n> 2. The interactive modes of 'git add' ('-p' and '-i') leave cmd_add()\n> before the code that sets the repo setting to allow for a sparse index.\n> Patch 2 fixes this and adds appropriate tests to confirm the behavior in\n> a sparse-checkout.\n\nThis made me wonder about the other commands that take \"--patch\" like \ncheckout and reset. Do you know how well they handle the sparse index? \nThey'll all benefit from the changes to git apply in this series but I \nwas wondering if they need any further changes.\n\nBest Wishes\n\nPhillip\n\n> A third patch adds a performance test to p2000-sparse-operations.sh to\n> confirm that we are getting the performance improvement we expect:\n> \n> BASE PATCH 1 PATCH 2\n> ---------------------------------------------------------\n> 2000.118: (full-v3) 0.80 0.84 +5.0% 0.84 +5.0%\n> 2000.119: (full-v4) 0.76 0.79 +3.9% 0.80 +5.3%\n> 2000.120: (sparse-v3) 2.09 1.39 -33.5% 0.07 -96.7%\n> 2000.121: (sparse-v4) 2.09 1.39 -33.5% 0.07 -96.7%\n> \n> \n> Thanks, -Stolee\n> \n> Derrick Stolee (3):\n> apply: integrate with the sparse index\n> git add: make -p/-i aware of sparse index\n> p2000: add performance test for 'git add -p'\n> \n> builtin/add.c | 7 +-\n> builtin/apply.c | 7 +-\n> t/perf/p2000-sparse-operations.sh | 1 +\n> t/t1092-sparse-checkout-compatibility.sh | 102 +++++++++++++++++++++++\n> 4 files changed, 113 insertions(+), 4 deletions(-)\n> \n> \n> base-commit: 6c0bd1fc70efaf053abe4e57c976afdc72d15377\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1914%2Fderrickstolee%2Fapply-sparse-index-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1914/derrickstolee/apply-sparse-index-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1914\n\n"},{"id":"518233","messageId":"3c4ef12a-3ad4-4078-a513-473f13a670fb@gmail.com","threadId":"63410","inReplyTo":"CABPp-BHkgvc0UQbhXfP4POtY8GPVpz9J8ZbX3_jyzL_V7GyBbA@mail.gmail.com","subject":"Re: [PATCH 1/3] apply: integrate with the sparse index","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-05-16T12:49:43Z","receivedAt":"2025-05-16T12:49:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/9/25 11:18 PM, Elijah Newren wrote:\n> On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget\n\n\n>> + # Expands when using --index.\n>> + ensure_expanded apply --index ../patch-outside &&\n>> + git -C sparse-index reset --hard &&\n> \n> All makes sense up to here.\n> \n>> +\n>> + # Does not expand when using --cached.\n>> + ensure_not_expanded apply --cached ../patch-outside\n> \n> Wait, what? That makes no sense.\n> \n> After some digging, I see why the test passed, but it's very\n> misleading. Just before this command, if you ran the following\n> commands from the sparse-index directory, you'd see the following:\n> \n> $ rm testme\n> $ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside\n> $ grep ensure_full_index testme\n> $\n> \n> Which matches what you were testing and shows why it passed for you.\n> But I'd argue the test is not correct and confusing for anyone that\n> reads it, because:\n> \n> $ git ls-files -s --sparse | grep folder2\n> 040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0 folder2/0/\n> 100644 78981922613b2afb6025042ff6bd878ac1994e85 0 folder2/a\n> \n> In other words, the index was *already* (partially) expanded by the\n> `git apply --index`, and the `git reset --hard` did not fix that\n> contrary to expectations. Continuing from here we see:\n> \n> $ git reset --hard\n> HEAD is now at 703fd3e initial commit\n> $ git ls-files -s --sparse | grep folder2\n> 040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0 folder2/0/\n> 100644 78981922613b2afb6025042ff6bd878ac1994e85 0 folder2/a\n> $ git sparse-checkout reapply\n> $ git ls-files -s --sparse | grep folder2\n> 040000 123706f6fc38949628eaf0483edbf97ba21123ae 0 folder2/\n> \n> So, we need to do a `git sparse-checkout reapply` to make sure we were\n> actually in the expected fully sparse state. From here...\n> \n> $ rm testme\n> $ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside\n> $ grep ensure_full_index testme\n> {\"event\":\"region_enter\",\"sid\":\"20250510T030856.854763Z-H8ec63c79-P0000371c\",\"thread\":\"main\",\"time\":\"2025-05-10T03:08:56.856008Z\",\"file\":\"sparse-index.c\",\"line\":372,\"repo\":1,\"nesting\":1,\"category\":\"index\",\"label\":\"ensure_full_index\"}\n> {\"event\":\"region_leave\",\"sid\":\"20250510T030856.854763Z-H8ec63c79-P0000371c\",\"thread\":\"main\",\"time\":\"2025-05-10T03:08:56.856454Z\",\"file\":\"sparse-index.c\",\"line\":455,\"repo\":1,\"t_rel\":0.000446,\"nesting\":1,\"category\":\"index\",\"label\":\"ensure_full_index\"}\n> {\"event\":\"region_enter\",\"sid\":\"20250510T030856.854763Z-H8ec63c79-P0000371c\",\"thread\":\"main\",\"time\":\"2025-05-10T03:08:56.857016Z\",\"file\":\"sparse-index.c\",\"line\":372,\"repo\":1,\"nesting\":1,\"category\":\"index\",\"label\":\"ensure_full_index\"}\n> {\"event\":\"region_leave\",\"sid\":\"20250510T030856.854763Z-H8ec63c79-P0000371c\",\"thread\":\"main\",\"time\":\"2025-05-10T03:08:56.857135Z\",\"file\":\"sparse-index.c\",\"line\":455,\"repo\":1,\"t_rel\":0.000119,\"nesting\":1,\"category\":\"index\",\"label\":\"ensure_full_index\"}\n> \n> So, indeed, `git apply --cached ../patch-outside` DOES expand the\n> index, as I expected. It has to when folder2/ is a directory in the\n> index, so that we can get a folder2/a entry that we can modify. And\n> that's just what we see:\n> \n> $ git ls-files -s --sparse | grep folder2\n> 040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0 folder2/0/\n> 100644 50f4ca4e6265a0497ec2ee6782648138914ad398 0 folder2/a\n> \n> \n> Can you add a `git sparse-checkout reapply` right after your `git\n> reset --hard`, and then switch the ensure_not_expanded to\n> ensure_expanded for the apply --cached call?\n\nThanks for digging in here. I'm going to augment the test to\ninclude both the ensure_expanded and ensure_not_expanded so we\nget the extra coverage for these two scenarios.\n\nThanks for the careful eye!\n-Stolee\n\n"},{"id":"518234","messageId":"bee620e4-1f45-4fff-a7d4-ecb5f99f4e0a@gmail.com","threadId":"63410","inReplyTo":"CABPp-BEmMaFQxE9NQgM8M=cgfBHY1p56vnBt7R4CfuiXnq++4Q@mail.gmail.com","subject":"Re: [PATCH 2/3] git add: make -p/-i aware of sparse index","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-05-16T12:54:10Z","receivedAt":"2025-05-16T12:54:12Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/10/25 12:38 AM, Elijah Newren wrote:\n> On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget\n> wrote:\n>>\n>> From: Derrick Stolee \n>>\n>> It is slow to expand a sparse index in-memory due to parsing of trees.\n>> We aim to minimize that performance cost when possible. 'git add -p'\n>> uses 'git apply' child processes to modify the index, but still there\n>> are some expansions that occur.\n> \n> still there are some expansions that occur...outside of those child\n> processes? Is that what you're trying to say, or was it something\n> else?\n> \n>> It turns out that control flows out of cmd_add() in the interactive\n>> cases before the lines that confirm that the builtin is integrated with\n>> the sparse index. We need to move that earlier to ensure it prevents a\n>> full index expansion on read.\n>>\n>> Add more test cases that confirm that these interactive add options work\n>> with the sparse index. One interesting aspect here is that the '-i'\n>> option avoids expanding the sparse index when a sparse directory exists\n>> on disk while the '-p' option does hit the ensure_full_index() method.\n>> This leaves some room for improvement, but this case should be atypical\n>> as users should remain within their sparse-checkout.\n> \n> It's not clear whether this paragraph is talking about existing state\n> (before the patch) or desired state (after the patch). I think based\n> on the context it's the former, but the last sentence sounds more like\n> a future work direction that makes it very unclear, to me at least.\n\nI'll try to rewrite to make this clearer.\n\n>> + # -p does expand when edits are outside sparse checkout.\n>> + test_write_lines y n y >in &&\n>> + ensure_expanded add -p > +\n>> + # but -i does not expand.\n>> + git -C sparse-index reset &&\n>> + test_write_lines u 2 3 \"\" q >in &&\n>> + ensure_not_expanded add -i \n> This has the same error as patch 1, in that you assume your reset\n> above (which wasn't even a reset --hard) will re-sparsify the index.\n> Since it doesn't, your test is misleading and only shows that when\n> already expanded to include the files of interest it doesn't expand\n> any further. To re-sparsify your index before the `add -i` call,\n> you'll need to do a `git reset --hard && git sparse-checkout reapply`\n> and then recreate folder1/a with \"new content\" again...and then run\n> your 'add -i' command.\nThanks. I didn't like that this was different. I appreciate your\nexpertise helping to clarify this issue.\n\nThanks,\n-Stolee\n\n"},{"id":"518237","messageId":"6736f2fd-a91d-4ffd-b421-6670a0e028d8@gmail.com","threadId":"63410","inReplyTo":"c1f7a9d6-4a44-4c00-bf26-6852819361e9@gmail.com","subject":"Re: [PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-05-16T13:28:39Z","receivedAt":"2025-05-16T13:28:41Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/14/25 11:16 AM, Phillip Wood wrote:\n\n> This made me wonder about the other commands that take \"--patch\" like checkout \n> and reset. Do you know how well they handle the sparse index? They'll all \n> benefit from the changes to git apply in this series but I was wondering if they \n> need any further changes.\nBy adding these two tests...\n\n test_perf_on_all 'test_write_lines y y y | git checkout --patch -'\n test_perf_on_all 'echo >>a && git add a && test_write_lines y | git reset \n--patch'\n\n...we can demonstrate that the changes to 'git apply' are sufficient\nto get the improvements we seek for 'git checkout --patch':\n\nTest HEAD~3 HEAD~2\n---------------------------------------------------------------\n... git checkout --patch - (full-v3) 1.22 1.22 +0.0%\n... git checkout --patch - (full-v4) 1.15 1.16 +0.9%\n... git checkout --patch - (sparse-v3) 1.37 0.11 -92.0%\n... git checkout --patch - (sparse-v4) 1.37 0.11 -92.0%\n... git reset --patch (full-v3) 0.82 0.81 -1.2%\n... git reset --patch (full-v4) 0.76 0.77 +1.3%\n... git reset --patch (sparse-v3) 1.57 0.91 -42.0%\n... git reset --patch (sparse-v4) 1.59 0.92 -42.1%\n\nBut 'git reset --patch' appears to not be fast _enough_. It turns\nout that it has the same issue as cmd_add(). I'll add a patch for\nthis purpose.\n\nThanks,\n-Stolee\n\n"},{"id":"518245","messageId":"pull.1914.v2.git.1747407330.gitgitgadget@gmail.com","threadId":"63410","inReplyTo":"pull.1914.git.1746579320.gitgitgadget@gmail.com","subject":"[PATCH v2 0/4] Integrate the sparse index with 'git apply' and interactive add, checkout, and reset","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-16T14:55:26Z","receivedAt":"2025-05-16T14:55:34Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The sparse index helps make some Git commands faster when using\nsparse-checkout in cone mode. However, not all code paths are aware that the\nindex can have non-blob entries, so we are careful about rolling this\nfeature out gradually. The cost of this rollout is that some commands are\nslower with the sparse index as they need to expand a sparse index into a\nfull index in memory, which requires parsing tree objects to construct the\nfull path list.\n\nThis patch series focuses on the 'git add -p' command, which is slow with\nthe sparse index for a couple of reasons, handled in the first two patches:\n\n 1. 'git add -p' uses 'git apply' as a subcommand and 'git apply' needs\n integration with the sparse index. Luckily, we just need to add the repo\n setting and appropriate tests to confirm it behaves as expected.\n 2. The interactive modes of 'git add' ('-p' and '-i') leave cmd_add()\n before the code that sets the repo setting to allow for a sparse index.\n Patch 2 fixes this and adds appropriate tests to confirm the behavior in\n a sparse-checkout.\n 3. The interactive mode of 'git reset' leaves cmd_reset() before the code\n that sets the repo setting to allow for the sparse index.\n\nA third patch adds a performance test to p2000-sparse-operations.sh to\nconfirm that we are getting the performance improvement we expect:\n\n Test BASE PATCH 1 PATCH 2 PATCH 3\n -------------------------------------------------------------------------------------\n 2000.118: ... git add -p (full-v3) 0.79 0.79 +0.0% 0.82 +3.8% 0.82 +3.8%\n 2000.119: ... git add -p (full-v4) 0.74 0.76 +2.7% 0.74 +0.0% 0.76 +2.7%\n 2000.120: ... git add -p (sparse-v3) 1.94 1.28 -34.0% 0.07 -96.4% 0.07 -96.4%\n 2000.121: ... git add -p (sparse-v4) 1.93 1.28 -33.7% 0.06 -96.9% 0.06 -96.9%\n 2000.122: ... git checkout -p (full-v3) 1.18 1.18 +0.0% 1.18 +0.0% 1.19 +0.8%\n 2000.123: ... git checkout -p (full-v4) 1.10 1.12 +1.8% 1.11 +0.9% 1.11 +0.9%\n 2000.124: ... git checkout -p (sparse-v3) 1.31 0.11 -91.6% 0.11 -91.6% 0.11 -91.6%\n 2000.125: ... git checkout -p (sparse-v4) 1.29 0.11 -91.5% 0.11 -91.5% 0.11 -91.5%\n 2000.126: ... git reset -p (full-v3) 0.81 0.80 -1.2% 0.83 +2.5% 0.83 +2.5%\n 2000.127: ... git reset -p (full-v4) 0.78 0.77 -1.3% 0.77 -1.3% 0.78 +0.0%\n 2000.128: ... git reset -p (sparse-v3) 1.58 0.92 -41.8% 0.91 -42.4% 0.07 -95.6%\n 2000.129: ... git reset -p (sparse-v4) 1.58 0.92 -41.8% 0.92 -41.8% 0.07 -95.6%\n\n\n\nUpdates in v2\n=============\n\nThanks for the careful review from Elijah and the pointer from Phillip, we\nhave these changes:\n\n 1. The tests no longer have different expansion behaviors for 'git add -p'\n and 'git add -i' due to partially-expanded indexes on disk.\n 2. We now test 'git checkout -p' and 'git reset -p'.\n 3. 'git reset -p' needed some changes to the builtin (similar to 'git add')\n to be fast.\n\nThanks, -Stolee\n\nDerrick Stolee (4):\n apply: integrate with the sparse index\n git add: make -p/-i aware of sparse index\n reset: integrate sparse index with --patch\n p2000: add performance test for patch-mode commands\n\n builtin/add.c | 7 +-\n builtin/apply.c | 7 +-\n builtin/reset.c | 6 +-\n t/perf/p2000-sparse-operations.sh | 3 +\n t/t1092-sparse-checkout-compatibility.sh | 151 +++++++++++++++++++++++\n 5 files changed, 167 insertions(+), 7 deletions(-)\n\n\nbase-commit: 6c0bd1fc70efaf053abe4e57c976afdc72d15377\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1914%2Fderrickstolee%2Fapply-sparse-index-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1914/derrickstolee/apply-sparse-index-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1914\n\nRange-diff vs v1:\n\n 1: 0e6e199cd19 ! 1: 1adf81ecb2c apply: integrate with the sparse index\n @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n\n +\n +\t# Expands when using --index.\n +\tensure_expanded apply --index ../patch-outside &&\n ++\n ++\t# Does not when index is partially expanded.\n ++\tgit -C sparse-index reset --hard &&\n ++\tensure_not_expanded apply --cached ../patch-outside &&\n ++\n ++\t# Try again with a reset and collapsed index.\n +\tgit -C sparse-index reset --hard &&\n ++\tgit -C sparse-index sparse-checkout reapply &&\n +\n -+\t# Does not expand when using --cached.\n -+\tensure_not_expanded apply --cached ../patch-outside\n ++\t# Expands when index is collapsed.\n ++\tensure_expanded apply --cached ../patch-outside\n +'\n +\n test_expect_success 'advice.sparseIndexExpanded' '\n 2: 63caae87634 ! 2: 0a2752721d0 git add: make -p/-i aware of sparse index\n @@ Commit message\n \n It turns out that control flows out of cmd_add() in the interactive\n cases before the lines that confirm that the builtin is integrated with\n - the sparse index. We need to move that earlier to ensure it prevents a\n - full index expansion on read.\n + the sparse index.\n \n - Add more test cases that confirm that these interactive add options work\n - with the sparse index. One interesting aspect here is that the '-i'\n - option avoids expanding the sparse index when a sparse directory exists\n - on disk while the '-p' option does hit the ensure_full_index() method.\n - This leaves some room for improvement, but this case should be atypical\n - as users should remain within their sparse-checkout.\n + Moving that integration point earlier in cmd_add() allows 'git add -p'\n + and 'git add -p' to operate without expanding a sparse index to a full\n + one.\n +\n + Add test cases that confirm that these interactive add options work with\n + the sparse index.\n \n Signed-off-by: Derrick Stolee \n \n @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'add, commit, chec\n \tinit_repos &&\n \n @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is not expanded: git apply' '\n - \tensure_not_expanded apply --cached ../patch-outside\n + \tensure_expanded apply --cached ../patch-outside\n '\n \n +test_expect_success 'sparse-index is not expanded: git add -p' '\n @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n\n +\tgit -C sparse-index reset &&\n +\tensure_not_expanded add -i sparse-index/folder1/a &&\n -+\n -+\t# -p does expand when edits are outside sparse checkout.\n +\ttest_write_lines y n y >in &&\n +\tensure_expanded add -p sparse-index/folder1/a &&\n +\ttest_write_lines u 2 3 \"\" q >in &&\n -+\tensure_not_expanded add -i 3: d1482a29d8f reset: integrate sparse index with --patch\n 3: 7a777281626 ! 4: a50c57f7628 p2000: add performance test for 'git add -p'\n @@ Metadata\n Author: Derrick Stolee \n \n ## Commit message ##\n - p2000: add performance test for 'git add -p'\n + p2000: add performance test for patch-mode commands\n \n - The previous two changes contributed performance improvements to 'git\n - apply' and 'git add -p' when using a sparse index. Add a performance\n - test to demonstrate this (and to help validate that performance remains\n - good in the future).\n + The previous three changes contributed performance improvements to 'git\n + apply', 'git add -p', and 'git reset -p' when using a sparse index. The\n + improvement to 'git apply' also improved 'git checkout -p'. Add\n + performance tests to demonstrate this (and to help validate that\n + performance remains good in the future).\n \n In the truncated test output below, we see that the full checkout\n performance changes within noise expectations, but the sparse index\n - cases improve 33% and then 96%.\n -\n - HEAD~3 HEAD~2 HEAD~1\n - ---------------------------------------------------------\n - 2000.118: (full-v3) 0.80 0.84 +5.0% 0.84 +5.0%\n - 2000.119: (full-v4) 0.76 0.79 +3.9% 0.80 +5.3%\n - 2000.120: (sparse-v3) 2.09 1.39 -33.5% 0.07 -96.7%\n - 2000.121: (sparse-v4) 2.09 1.39 -33.5% 0.07 -96.7%\n + cases improve 33% and then 96% for 'git add -p' and 41% and then 95% for\n + 'git reset -p'. 'git checkout -p' improves immediatley by 91% because it\n + does not need any change to its builtin.\n +\n + Test HEAD~4 HEAD~3 HEAD~2 HEAD~1\n + -------------------------------------------------------------------------------------\n + 2000.118: ... git add -p (full-v3) 0.79 0.79 +0.0% 0.82 +3.8% 0.82 +3.8%\n + 2000.119: ... git add -p (full-v4) 0.74 0.76 +2.7% 0.74 +0.0% 0.76 +2.7%\n + 2000.120: ... git add -p (sparse-v3) 1.94 1.28 -34.0% 0.07 -96.4% 0.07 -96.4%\n + 2000.121: ... git add -p (sparse-v4) 1.93 1.28 -33.7% 0.06 -96.9% 0.06 -96.9%\n + 2000.122: ... git checkout -p (full-v3) 1.18 1.18 +0.0% 1.18 +0.0% 1.19 +0.8%\n + 2000.123: ... git checkout -p (full-v4) 1.10 1.12 +1.8% 1.11 +0.9% 1.11 +0.9%\n + 2000.124: ... git checkout -p (sparse-v3) 1.31 0.11 -91.6% 0.11 -91.6% 0.11 -91.6%\n + 2000.125: ... git checkout -p (sparse-v4) 1.29 0.11 -91.5% 0.11 -91.5% 0.11 -91.5%\n + 2000.126: ... git reset -p (full-v3) 0.81 0.80 -1.2% 0.83 +2.5% 0.83 +2.5%\n + 2000.127: ... git reset -p (full-v4) 0.78 0.77 -1.3% 0.77 -1.3% 0.78 +0.0%\n + 2000.128: ... git reset -p (sparse-v3) 1.58 0.92 -41.8% 0.91 -42.4% 0.07 -95.6%\n + 2000.129: ... git reset -p (sparse-v4) 1.58 0.92 -41.8% 0.92 -41.8% 0.07 -95.6%\n \n It is worth noting that if our test was more involved and had multiple\n hunks to evaluate, then the time spent in 'git apply' would dominate due\n @@ t/perf/p2000-sparse-operations.sh: test_perf_on_all git diff-tree HEAD\n test_perf_on_all \"git worktree add ../temp && git worktree remove ../temp\"\n test_perf_on_all git check-attr -a -- $SPARSE_CONE/a\n +test_perf_on_all 'echo >>a && test_write_lines y | git add -p'\n ++test_perf_on_all 'test_write_lines y y y | git checkout --patch -'\n ++test_perf_on_all 'echo >>a && git add a && test_write_lines y | git reset --patch'\n \n test_done\n\n-- \ngitgitgadget\n"},{"id":"518246","messageId":"1adf81ecb2c45c2e90d9b5dcfb02c814ecce6d8d.1747407330.git.gitgitgadget@gmail.com","threadId":"63410","inReplyTo":"pull.1914.v2.git.1747407330.gitgitgadget@gmail.com","subject":"[PATCH v2 1/4] apply: integrate with the sparse index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-16T14:55:27Z","receivedAt":"2025-05-16T14:55:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe sparse index allows storing directory entries in the index, marked\nwith the skip-wortkree bit and pointing to a tree object. This may be an\nunexpected data shape for some implementation areas, so we are rolling\nit out incrementally on a builtin-per-builtin basis.\n\nThis change enables the sparse index for 'git apply'. The main\nmotivation for this change is that 'git apply' is used as a child\nprocess of 'git add -p' and expanding the sparse index for each of those\nchild processes can lead to significant performance issues.\n\nThe good news is that the actual index manipulation code used by 'git\napply' is already integrated with the sparse index, so the only product\nchange is to mark the builtin as allowing the sparse index so it isn't\ninflated on read.\n\nThe more involved part of this change is around adding tests that verify\nhow 'git apply' behaves in a sparse-checkout environment and whether or\nnot the index expands in certain operations.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/apply.c | 7 +++-\n t/t1092-sparse-checkout-compatibility.sh | 53 ++++++++++++++++++++++++\n 2 files changed, 59 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 84f1863d3ac3..a1e20c593d09 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -12,7 +12,7 @@ static const char * const apply_usage[] = {\n int cmd_apply(int argc,\n \t const char **argv,\n \t const char *prefix,\n-\t struct repository *repo UNUSED)\n+\t struct repository *repo)\n {\n \tint force_apply = 0;\n \tint options = 0;\n@@ -35,6 +35,11 @@ int cmd_apply(int argc,\n \t\t\t\t &state, &force_apply, &options,\n \t\t\t\t apply_usage);\n \n+\tif (repo) {\n+\t\tprepare_repo_settings(repo);\n+\t\trepo->settings.command_requires_full_index = 0;\n+\t}\n+\n \tif (check_apply_state(&state, force_apply))\n \t\texit(128);\n \ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex f9b448792cb4..83353a7dbab4 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -1340,6 +1340,30 @@ test_expect_success 'submodule handling' '\n \tgrep \"160000 $(git -C initial-repo rev-parse HEAD) 0\tmodules/sub\" cache\n '\n \n+test_expect_success 'git apply functionality' '\n+\tinit_repos &&\n+\n+\ttest_all_match git checkout base &&\n+\n+\tgit -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&\n+\tgit -C full-checkout diff base..merge-right -- folder2 >patch-outside &&\n+\n+\t# Apply a patch to a file inside the sparse definition\n+\ttest_all_match git apply --index --stat ../patch-in-sparse &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\t# Apply a patch to a file outside the sparse definition\n+\ttest_sparse_match test_must_fail git apply ../patch-outside &&\n+\tgrep \"No such file or directory\" sparse-checkout-err &&\n+\n+\t# But it works with --index and --cached\n+\ttest_all_match git apply --index --stat ../patch-outside &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\ttest_all_match git reset --hard &&\n+\ttest_all_match git apply --cached --stat ../patch-outside &&\n+\ttest_all_match git status --porcelain=v2\n+'\n+\n # When working with a sparse index, some commands will need to expand the\n # index to operate properly. If those commands also write the index back\n # to disk, they need to convert the index to sparse before writing.\n@@ -2345,6 +2369,35 @@ test_expect_success 'sparse-index is not expanded: check-attr' '\n \tensure_not_expanded check-attr -a --cached -- folder1/a\n '\n \n+test_expect_success 'sparse-index is not expanded: git apply' '\n+\tinit_repos &&\n+\n+\tgit -C sparse-index checkout base &&\n+\tgit -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&\n+\tgit -C full-checkout diff base..merge-right -- folder2 >patch-outside &&\n+\n+\t# Apply a patch to a file inside the sparse definition\n+\tensure_not_expanded apply --index --stat ../patch-in-sparse &&\n+\n+\t# Apply a patch to a file outside the sparse definition\n+\t# Fails when caring about the worktree.\n+\tensure_not_expanded ! apply ../patch-outside &&\n+\n+\t# Expands when using --index.\n+\tensure_expanded apply --index ../patch-outside &&\n+\n+\t# Does not when index is partially expanded.\n+\tgit -C sparse-index reset --hard &&\n+\tensure_not_expanded apply --cached ../patch-outside &&\n+\n+\t# Try again with a reset and collapsed index.\n+\tgit -C sparse-index reset --hard &&\n+\tgit -C sparse-index sparse-checkout reapply &&\n+\n+\t# Expands when index is collapsed.\n+\tensure_expanded apply --cached ../patch-outside\n+'\n+\n test_expect_success 'advice.sparseIndexExpanded' '\n \tinit_repos &&\n \n-- \ngitgitgadget\n\n"},{"id":"518247","messageId":"0a2752721d0292c6e2527f7a95c846b280fda2a7.1747407330.git.gitgitgadget@gmail.com","threadId":"63410","inReplyTo":"pull.1914.v2.git.1747407330.gitgitgadget@gmail.com","subject":"[PATCH v2 2/4] git add: make -p/-i aware of sparse index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-16T14:55:28Z","receivedAt":"2025-05-16T14:55:36Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIt is slow to expand a sparse index in-memory due to parsing of trees.\nWe aim to minimize that performance cost when possible. 'git add -p'\nuses 'git apply' child processes to modify the index, but still there\nare some expansions that occur.\n\nIt turns out that control flows out of cmd_add() in the interactive\ncases before the lines that confirm that the builtin is integrated with\nthe sparse index.\n\nMoving that integration point earlier in cmd_add() allows 'git add -p'\nand 'git add -p' to operate without expanding a sparse index to a full\none.\n\nAdd test cases that confirm that these interactive add options work with\nthe sparse index.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/add.c | 7 +--\n t/t1092-sparse-checkout-compatibility.sh | 60 ++++++++++++++++++++++++\n 2 files changed, 64 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 747511b68bc3..7c292ffdc6c2 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -390,6 +390,10 @@ int cmd_add(int argc,\n \n \targc = parse_options(argc, argv, prefix, builtin_add_options,\n \t\t\t builtin_add_usage, PARSE_OPT_KEEP_ARGV0);\n+\n+\tprepare_repo_settings(repo);\n+\trepo->settings.command_requires_full_index = 0;\n+\n \tif (patch_interactive)\n \t\tadd_interactive = 1;\n \tif (add_interactive) {\n@@ -426,9 +430,6 @@ int cmd_add(int argc,\n \tadd_new_files = !take_worktree_changes && !refresh_only && !add_renormalize;\n \trequire_pathspec = !(take_worktree_changes || (0 < addremove_explicit));\n \n-\tprepare_repo_settings(repo);\n-\trepo->settings.command_requires_full_index = 0;\n-\n \trepo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR);\n \n \t/*\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 83353a7dbab4..c419d8b57e84 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -384,6 +384,38 @@ test_expect_success 'add, commit, checkout' '\n \ttest_all_match git checkout -\n '\n \n+test_expect_success 'git add -p' '\n+\tinit_repos &&\n+\n+\twrite_script edit-contents <<-\\EOF &&\n+\techo text >>$1\n+\tEOF\n+\n+\t# Does not expand when edits are within sparse checkout.\n+\trun_on_all ../edit-contents deep/a &&\n+\trun_on_all ../edit-contents deep/deeper1/a &&\n+\n+\ttest_write_lines y n >in &&\n+\trun_on_all git add -p in &&\n+\trun_on_all git add -i in &&\n+\trun_on_all git add -p in &&\n+\trun_on_all git add -i sparse-index/deep/a &&\n+\techo \"new content\" >sparse-index/deep/deeper1/a &&\n+\ttest_write_lines y n >in &&\n+\tensure_not_expanded add -p sparse-index/folder1/a &&\n+\ttest_write_lines y n y >in &&\n+\tensure_expanded add -p sparse-index/folder1/a &&\n+\ttest_write_lines u 2 3 \"\" q >in &&\n+\tensure_expanded add -i \n\nSimilar to the previous change for 'git add -p', the reset builtin\nchecked for integration with the sparse index after possibly redirecting\nits logic toward the interactive logic. This means that the builtin\nwould expand the sparse index to a full one upon read.\n\nMove this check earlier within cmd_reset() to improve performance here.\n\nAdd tests to guarantee that we are not universally expanding the index.\nAdd behavior tests to check that we are doing the same operations as a\nfull index.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/reset.c | 6 ++--\n t/t1092-sparse-checkout-compatibility.sh | 42 ++++++++++++++++++++++--\n 2 files changed, 43 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 73b4537a9a56..dc50ffc1ac59 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -420,6 +420,9 @@ int cmd_reset(int argc,\n \t\toidcpy(&oid, &tree->object.oid);\n \t}\n \n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\n \tif (patch_mode) {\n \t\tif (reset_type != NONE)\n \t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--patch\", \"--{hard,mixed,soft}\");\n@@ -457,9 +460,6 @@ int cmd_reset(int argc,\n \tif (intent_to_add && reset_type != MIXED)\n \t\tdie(_(\"the option '%s' requires '%s'\"), \"-N\", \"--mixed\");\n \n-\tprepare_repo_settings(the_repository);\n-\tthe_repository->settings.command_requires_full_index = 0;\n-\n \tif (repo_read_index(the_repository) < 0)\n \t\tdie(_(\"index file corrupt\"));\n \ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex c419d8b57e84..d8101139b40a 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -384,7 +384,7 @@ test_expect_success 'add, commit, checkout' '\n \ttest_all_match git checkout -\n '\n \n-test_expect_success 'git add -p' '\n+test_expect_success 'git add, checkout, and reset with -p' '\n \tinit_repos &&\n \n \twrite_script edit-contents <<-\\EOF &&\n@@ -398,7 +398,7 @@ test_expect_success 'git add -p' '\n \ttest_write_lines y n >in &&\n \trun_on_all git add -p in &&\n \trun_on_all git add -i in &&\n \trun_on_all git add -i in &&\n+\ttest_sparse_match git checkout HEAD~1 --patch sparse-index/deep/a &&\n+\techo \"new content\" >sparse-index/deep/deeper1/a &&\n+\tgit -C sparse-index commit -a -m \"inside-changes\" &&\n+\n+\ttest_write_lines y y >in &&\n+\tensure_not_expanded checkout HEAD~1 --patch sparse-index/deep/a &&\n+\techo \"new content\" >sparse-index/deep/deeper1/a &&\n+\tgit -C sparse-index add . &&\n+\tensure_not_expanded reset --patch sparse-index/folder1/a &&\n+\tgit -C sparse-index add --sparse folder1 &&\n+\tgit -C sparse-index sparse-checkout reapply &&\n+\tensure_expanded reset --patch sparse-index/folder1/a &&\n+\tgit -C sparse-index add --sparse folder1 &&\n+\tgit -C sparse-index commit -m \"folder1 change\" &&\n+\tgit -C sparse-index sparse-checkout reapply &&\n+\tensure_expanded checkout HEAD~1 --patch \n\nThe previous three changes contributed performance improvements to 'git\napply', 'git add -p', and 'git reset -p' when using a sparse index. The\nimprovement to 'git apply' also improved 'git checkout -p'. Add\nperformance tests to demonstrate this (and to help validate that\nperformance remains good in the future).\n\nIn the truncated test output below, we see that the full checkout\nperformance changes within noise expectations, but the sparse index\ncases improve 33% and then 96% for 'git add -p' and 41% and then 95% for\n'git reset -p'. 'git checkout -p' improves immediatley by 91% because it\ndoes not need any change to its builtin.\n\n Test HEAD~4 HEAD~3 HEAD~2 HEAD~1\n -------------------------------------------------------------------------------------\n 2000.118: ... git add -p (full-v3) 0.79 0.79 +0.0% 0.82 +3.8% 0.82 +3.8%\n 2000.119: ... git add -p (full-v4) 0.74 0.76 +2.7% 0.74 +0.0% 0.76 +2.7%\n 2000.120: ... git add -p (sparse-v3) 1.94 1.28 -34.0% 0.07 -96.4% 0.07 -96.4%\n 2000.121: ... git add -p (sparse-v4) 1.93 1.28 -33.7% 0.06 -96.9% 0.06 -96.9%\n 2000.122: ... git checkout -p (full-v3) 1.18 1.18 +0.0% 1.18 +0.0% 1.19 +0.8%\n 2000.123: ... git checkout -p (full-v4) 1.10 1.12 +1.8% 1.11 +0.9% 1.11 +0.9%\n 2000.124: ... git checkout -p (sparse-v3) 1.31 0.11 -91.6% 0.11 -91.6% 0.11 -91.6%\n 2000.125: ... git checkout -p (sparse-v4) 1.29 0.11 -91.5% 0.11 -91.5% 0.11 -91.5%\n 2000.126: ... git reset -p (full-v3) 0.81 0.80 -1.2% 0.83 +2.5% 0.83 +2.5%\n 2000.127: ... git reset -p (full-v4) 0.78 0.77 -1.3% 0.77 -1.3% 0.78 +0.0%\n 2000.128: ... git reset -p (sparse-v3) 1.58 0.92 -41.8% 0.91 -42.4% 0.07 -95.6%\n 2000.129: ... git reset -p (sparse-v4) 1.58 0.92 -41.8% 0.92 -41.8% 0.07 -95.6%\n\nIt is worth noting that if our test was more involved and had multiple\nhunks to evaluate, then the time spent in 'git apply' would dominate due\nto multiple index loads and writes. As it stands, we need the sparse\nindex improvement in 'git add -p' itself to confirm this performance\nimprovement.\n\nSince the change for 'git add -i' is identical, we avoid a second test\ncase for that similar operation.\n\nSigned-off-by: Derrick Stolee \n---\n t/perf/p2000-sparse-operations.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 39e92b084143..aadf22bc2f0b 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -135,5 +135,8 @@ test_perf_on_all git diff-tree HEAD\n test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a\n test_perf_on_all \"git worktree add ../temp && git worktree remove ../temp\"\n test_perf_on_all git check-attr -a -- $SPARSE_CONE/a\n+test_perf_on_all 'echo >>a && test_write_lines y | git add -p'\n+test_perf_on_all 'test_write_lines y y y | git checkout --patch -'\n+test_perf_on_all 'echo >>a && git add a && test_write_lines y | git reset --patch'\n \n test_done\n-- \ngitgitgadget\n"},{"id":"518254","messageId":"CABPp-BFmgAsps=Y+YE=dTUxnGSKj+49jNxODdQDOnvuF7XzKZA@mail.gmail.com","threadId":"63410","inReplyTo":"pull.1914.v2.git.1747407330.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/4] Integrate the sparse index with 'git apply' and interactive add, checkout, and reset","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-05-16T15:32:11Z","receivedAt":"2025-05-16T15:32:24Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 16, 2025 at 7:55 AM Derrick Stolee via GitGitGadget\n wrote:\n>\n[...]\n> Updates in v2\n> =============\n>\n> Thanks for the careful review from Elijah and the pointer from Phillip, we\n> have these changes:\n>\n> 1. The tests no longer have different expansion behaviors for 'git add -p'\n> and 'git add -i' due to partially-expanded indexes on disk.\n> 2. We now test 'git checkout -p' and 'git reset -p'.\n> 3. 'git reset -p' needed some changes to the builtin (similar to 'git add')\n> to be fast.\n\nI haven't looked at the new patch yet, but the updated patches 1 & 2\nlook much improved (though there's still one problem with the new\ncommit message, which I comment on in the range-diff below).\n\nHowever, I think Junio already merged your v1 to next\n(https://lore.kernel.org/git/CABPp-BEukTWwsuC7MMR8D5_UAhyw-LgT=DsPKAWeR_ZmVVhjzQ@mail.gmail.com/).\nSo he'll either have to revert your v1 in next and apply the new\nseries on top, or you'll need to re-roll as fixes on top of your v1.\n\n> 2: 63caae87634 ! 2: 0a2752721d0 git add: make -p/-i aware of sparse index\n> @@ Commit message\n>\n> It turns out that control flows out of cmd_add() in the interactive\n> cases before the lines that confirm that the builtin is integrated with\n> - the sparse index. We need to move that earlier to ensure it prevents a\n> - full index expansion on read.\n> + the sparse index.\n>\n> - Add more test cases that confirm that these interactive add options work\n> - with the sparse index. One interesting aspect here is that the '-i'\n> - option avoids expanding the sparse index when a sparse directory exists\n> - on disk while the '-p' option does hit the ensure_full_index() method.\n> - This leaves some room for improvement, but this case should be atypical\n> - as users should remain within their sparse-checkout.\n> + Moving that integration point earlier in cmd_add() allows 'git add -p'\n> + and 'git add -p' to operate without expanding a sparse index to a full\n> + one.\n\nWas the second 'git add -p' meant to be 'git add -i'?\n\n> -: ----------- > 3: d1482a29d8f reset: integrate sparse index with --patch\n\nOther than the one comment above, your changes from the range-diff\nlook good to me for patches 1 & 2, and the new 4. I haven't looked at\nthis new patch 3 yet but wanted to comment on the merged-to-next\nissue.\n"},{"id":"518260","messageId":"CABPp-BGdf5emqfnCvmBGxj2WT+d2jvR66tOfkeKiuJ8Np8sh-Q@mail.gmail.com","threadId":"63410","inReplyTo":"d1482a29d8f23699c713238ba37266fe9efa9aea.1747407330.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 3/4] reset: integrate sparse index with --patch","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-05-16T16:20:08Z","receivedAt":"2025-05-16T16:20:20Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 16, 2025 at 7:55 AM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> Similar to the previous change for 'git add -p', the reset builtin\n> checked for integration with the sparse index after possibly redirecting\n> its logic toward the interactive logic. This means that the builtin\n> would expand the sparse index to a full one upon read.\n>\n> Move this check earlier within cmd_reset() to improve performance here.\n>\n> Add tests to guarantee that we are not universally expanding the index.\n> Add behavior tests to check that we are doing the same operations as a\n> full index.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> builtin/reset.c | 6 ++--\n> t/t1092-sparse-checkout-compatibility.sh | 42 ++++++++++++++++++++++--\n> 2 files changed, 43 insertions(+), 5 deletions(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 73b4537a9a56..dc50ffc1ac59 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -420,6 +420,9 @@ int cmd_reset(int argc,\n> oidcpy(&oid, &tree->object.oid);\n> }\n>\n> + prepare_repo_settings(the_repository);\n> + the_repository->settings.command_requires_full_index = 0;\n> +\n> if (patch_mode) {\n> if (reset_type != NONE)\n> die(_(\"options '%s' and '%s' cannot be used together\"), \"--patch\", \"--{hard,mixed,soft}\");\n> @@ -457,9 +460,6 @@ int cmd_reset(int argc,\n> if (intent_to_add && reset_type != MIXED)\n> die(_(\"the option '%s' requires '%s'\"), \"-N\", \"--mixed\");\n>\n> - prepare_repo_settings(the_repository);\n> - the_repository->settings.command_requires_full_index = 0;\n> -\n> if (repo_read_index(the_repository) < 0)\n> die(_(\"index file corrupt\"));\n>\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index c419d8b57e84..d8101139b40a 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -384,7 +384,7 @@ test_expect_success 'add, commit, checkout' '\n> test_all_match git checkout -\n> '\n>\n> -test_expect_success 'git add -p' '\n> +test_expect_success 'git add, checkout, and reset with -p' '\n> init_repos &&\n>\n> write_script edit-contents <<-\\EOF &&\n> @@ -398,7 +398,7 @@ test_expect_success 'git add -p' '\n> test_write_lines y n >in &&\n> run_on_all git add -p test_all_match git status --porcelain=v2 &&\n> - test_all_match git reset &&\n> + test_all_match git reset -p \n> test_write_lines u 1 \"\" q >in &&\n> run_on_all git add -i @@ -413,6 +413,12 @@ test_expect_success 'git add -p' '\n> test_sparse_match git reset &&\n> test_write_lines u 2 3 \"\" q >in &&\n> run_on_all git add -i + test_sparse_match git status --porcelain=v2 &&\n> +\n> + run_on_all git add --sparse folder1 &&\n> + run_on_all git commit -m \"take changes\" &&\n> + test_write_lines y n y >in &&\n> + test_sparse_match git checkout HEAD~1 --patch test_sparse_match git status --porcelain=v2\n> '\n>\n> @@ -2458,6 +2464,38 @@ test_expect_success 'sparse-index is not expanded: git add -p' '\n> ensure_expanded add -i '\n>\n> +test_expect_success 'sparse-index is not expanded: checkout -p, reset -p' '\n> + init_repos &&\n> +\n> + # Does not expand when edits are within sparse checkout.\n> + echo \"new content\" >sparse-index/deep/a &&\n> + echo \"new content\" >sparse-index/deep/deeper1/a &&\n> + git -C sparse-index commit -a -m \"inside-changes\" &&\n> +\n> + test_write_lines y y >in &&\n> + ensure_not_expanded checkout HEAD~1 --patch +\n> + echo \"new content\" >sparse-index/deep/a &&\n> + echo \"new content\" >sparse-index/deep/deeper1/a &&\n> + git -C sparse-index add . &&\n> + ensure_not_expanded reset --patch +\n> + # -p does expand when edits are outside sparse checkout.\n> + mkdir -p sparse-index/folder1 &&\n> + echo \"new content\" >sparse-index/folder1/a &&\n> + git -C sparse-index add --sparse folder1 &&\n> + git -C sparse-index sparse-checkout reapply &&\n> + ensure_expanded reset --patch +\n> + # Fully reset the index.\n> + mkdir -p sparse-index/folder1 &&\n> + echo \"new content\" >sparse-index/folder1/a &&\n> + git -C sparse-index add --sparse folder1 &&\n> + git -C sparse-index commit -m \"folder1 change\" &&\n> + git -C sparse-index sparse-checkout reapply &&\n> + ensure_expanded checkout HEAD~1 --patch +'\n> +\n> test_expect_success 'advice.sparseIndexExpanded' '\n> init_repos &&\n>\n> --\n> gitgitgadget\n\nPatch looks good to me.\n"},{"id":"518264","messageId":"6d26d262-612d-4231-8bea-799ae8374d69@gmail.com","threadId":"63410","inReplyTo":"CABPp-BFmgAsps=Y+YE=dTUxnGSKj+49jNxODdQDOnvuF7XzKZA@mail.gmail.com","subject":"Re: [PATCH v2 0/4] Integrate the sparse index with 'git apply' and interactive add, checkout, and reset","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-05-16T16:35:23Z","receivedAt":"2025-05-16T16:35:46Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/16/2025 11:32 AM, Elijah Newren wrote:\n> On Fri, May 16, 2025 at 7:55 AM Derrick Stolee via GitGitGadget\n> wrote:\n>>\n\n>> - Add more test cases that confirm that these interactive add options work\n>> - with the sparse index. One interesting aspect here is that the '-i'\n>> - option avoids expanding the sparse index when a sparse directory exists\n>> - on disk while the '-p' option does hit the ensure_full_index() method.\n>> - This leaves some room for improvement, but this case should be atypical\n>> - as users should remain within their sparse-checkout.\n>> + Moving that integration point earlier in cmd_add() allows 'git add -p'\n>> + and 'git add -p' to operate without expanding a sparse index to a full\n>> + one.\n> \n> Was the second 'git add -p' meant to be 'git add -i'?\n\nYou are right. The second one should be 'git add -i'.\n\nAnd regarding 'next', I do expect my v1 to be ejected from 'next' while\nreview continues on this v2. If instead it gets merged to 'master', then\nI'll prepare a new series on top that applies the learnings from this\nreview.\n\nThanks,\n-Stolee\n\n"},{"id":"518299","messageId":"xmqq34d4v10u.fsf@gitster.g","threadId":"63410","inReplyTo":"CABPp-BFmgAsps=Y+YE=dTUxnGSKj+49jNxODdQDOnvuF7XzKZA@mail.gmail.com","subject":"Re: [PATCH v2 0/4] Integrate the sparse index with 'git apply' and interactive add, checkout, and reset","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-16T18:55:13Z","receivedAt":"2025-05-16T18:55:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> However, I think Junio already merged your v1 to next\n> (https://lore.kernel.org/git/CABPp-BEukTWwsuC7MMR8D5_UAhyw-LgT=DsPKAWeR_ZmVVhjzQ@mail.gmail.com/).\n> So he'll either have to revert your v1 in next and apply the new\n> series on top, or you'll need to re-roll as fixes on top of your v1.\n\nYup, the earlier one will be reverted when I queue this round.\n\nThanks, both, for being careful.\n\n\n\n>\n>> 2: 63caae87634 ! 2: 0a2752721d0 git add: make -p/-i aware of sparse index\n>> @@ Commit message\n>>\n>> It turns out that control flows out of cmd_add() in the interactive\n>> cases before the lines that confirm that the builtin is integrated with\n>> - the sparse index. We need to move that earlier to ensure it prevents a\n>> - full index expansion on read.\n>> + the sparse index.\n>>\n>> - Add more test cases that confirm that these interactive add options work\n>> - with the sparse index. One interesting aspect here is that the '-i'\n>> - option avoids expanding the sparse index when a sparse directory exists\n>> - on disk while the '-p' option does hit the ensure_full_index() method.\n>> - This leaves some room for improvement, but this case should be atypical\n>> - as users should remain within their sparse-checkout.\n>> + Moving that integration point earlier in cmd_add() allows 'git add -p'\n>> + and 'git add -p' to operate without expanding a sparse index to a full\n>> + one.\n>\n> Was the second 'git add -p' meant to be 'git add -i'?\n\nGood eyes.\n\n>\n>> -: ----------- > 3: d1482a29d8f reset: integrate sparse index with --patch\n>\n> Other than the one comment above, your changes from the range-diff\n> look good to me for patches 1 & 2, and the new 4. I haven't looked at\n> this new patch 3 yet but wanted to comment on the merged-to-next\n> issue.\n"},{"id":"518498","messageId":"028c2c85-5b60-4ede-ad01-5faeb13898c6@gmail.com","threadId":"63410","inReplyTo":"6736f2fd-a91d-4ffd-b421-6670a0e028d8@gmail.com","subject":"Re: [PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-05-20T15:07:18Z","receivedAt":"2025-05-20T15:07:25Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 16/05/2025 14:28, Derrick Stolee wrote:\n> On 5/14/25 11:16 AM, Phillip Wood wrote:\n> \n>> This made me wonder about the other commands that take \"--patch\" like \n>> checkout and reset. Do you know how well they handle the sparse index? \n>> They'll all benefit from the changes to git apply in this series but I \n>> was wondering if they need any further changes.\n> By adding these two tests...\n> \n>   test_perf_on_all 'test_write_lines y y y | git checkout --patch -'\n>   test_perf_on_all 'echo >>a && git add a && test_write_lines y | git \n> reset --patch'\n> \n> ...we can demonstrate that the changes to 'git apply' are sufficient\n> to get the improvements we seek for 'git checkout --patch':\n> \n> Test                                   HEAD~3   HEAD~2\n> ---------------------------------------------------------------\n> ... git checkout --patch - (full-v3)     1.22   1.22  +0.0%\n> ... git checkout --patch - (full-v4)     1.15   1.16  +0.9%\n> ... git checkout --patch - (sparse-v3)   1.37   0.11 -92.0%\n> ... git checkout --patch - (sparse-v4)   1.37   0.11 -92.0%\n> ... git reset --patch (full-v3)          0.82   0.81  -1.2%\n> ... git reset --patch (full-v4)          0.76   0.77  +1.3%\n> ... git reset --patch (sparse-v3)        1.57   0.91 -42.0%\n> ... git reset --patch (sparse-v4)        1.59   0.92 -42.1%\n> \n> But 'git reset --patch' appears to not be fast _enough_. It turns\n> out that it has the same issue as cmd_add(). I'll add a patch for\n> this purpose.\n\nThat's great, I don't have anymore comments on v2 of this series.\n\nThanks\n\nPhillip\n\n"}]}