{"thread":{"id":"59745","subject":"[RFC][PATCH V1] diff-tree: integrate with sparse index","startedAt":"2023-05-15T19:19:22Z","lastAt":"2023-05-23T04:38:42Z","messageCount":7,"participants":["Shuqi Liang","Victoria Dye","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"477292","messageId":"20230515191836.674234-1-cheskaqiqi@gmail.com","threadId":"59745","inReplyTo":null,"subject":"[RFC][PATCH V1] diff-tree: integrate with sparse index","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-05-15T19:18:36Z","receivedAt":"2023-05-15T19:19:22Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Remove full index requirement for `git diff-tree`. Add tests that verify\nthat 'git diff-tree' behaves correctly when the sparse index is enabled\nand test to ensure the index is not expanded.\n\nThe `p2000` tests demonstrate a ~98% execution time reduction for\n'git diff-tree' using a sparse index:\n\nTest                                                before  after\n------------------------------------------------------------------------\n2000.94: git diff-tree HEAD (full-v3)                0.05   0.04 -20.0%\n2000.95: git diff-tree HEAD (full-v4)                0.06   0.05 -16.7%\n2000.96: git diff-tree HEAD (sparse-v3)              0.59   0.01 -98.3%\n2000.97: git diff-tree HEAD (sparse-v4)              0.61   0.01 -98.4%\n2000.98: git diff-tree HEAD -- f2/f4/a (full-v3)     0.05   0.05 +0.0%\n2000.99: git diff-tree HEAD -- f2/f4/a (full-v4)     0.05   0.04 -20.0%\n2000.100: git diff-tree HEAD -- f2/f4/a (sparse-v3)  0.58   0.01 -98.3%\n2000.101: git diff-tree HEAD -- f2/f4/a (sparse-v4)  0.55   0.01 -98.2%\n\nSigned-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n---\n builtin/diff-tree.c                      |  4 ++\n t/perf/p2000-sparse-operations.sh        |  2 +\n t/t1092-sparse-checkout-compatibility.sh | 62 ++++++++++++++++++++++++\n 3 files changed, 68 insertions(+)\n\ndiff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\nindex 385c2d0230..c5d5730ebf 100644\n--- a/builtin/diff-tree.c\n+++ b/builtin/diff-tree.c\n@@ -121,6 +121,10 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \t\tusage(diff_tree_usage);\n \n \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n+\n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\n \trepo_init_revisions(the_repository, opt, prefix);\n \tif (repo_read_index(the_repository) < 0)\n \t\tdie(_(\"index file corrupt\"));\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 60d1de0662..14caf01718 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -129,5 +129,7 @@ test_perf_on_all git grep --cached bogus -- \"f2/f1/f1/*\"\n test_perf_on_all git write-tree\n test_perf_on_all git describe --dirty\n test_perf_on_all 'echo >>new && git describe --dirty'\n+test_perf_on_all git diff-tree HEAD\n+test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a\n \n test_done\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 0c784813f1..f08edcbf8e 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2108,4 +2108,66 @@ test_expect_success 'sparse-index is not expanded: write-tree' '\n \tensure_not_expanded write-tree\n '\n \n+test_expect_success 'diff-tree' '\n+\tinit_repos &&\n+\n+\twrite_script edit-contents <<-\\EOF &&\n+\techo text >>\"$1\"\n+\tEOF\n+\n+\t# Get the tree SHA for the current HEAD\n+\ttree1=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n+\n+\t# make a change inside the sparse cone\n+\trun_on_all ../edit-contents deep/a &&\n+\ttest_all_match git add deep/a &&\n+\ttest_all_match git commit -m \"Change deep/a\" &&\n+\n+\t# Get the tree SHA for the new HEAD\n+\ttree2=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n+\n+\n+\ttest_all_match git diff-tree $tree1 $tree2 &&\n+\ttest_all_match git diff-tree HEAD &&\n+\ttest_all_match git diff-tree HEAD -- deep/a &&\n+\n+\t# make a change outside the sparse cone\n+\trun_on_all mkdir -p folder1 &&\n+\trun_on_all cp a folder1/a &&\n+\trun_on_all ../edit-contents folder1/a &&\n+\ttest_all_match git update-index folder1/a &&\n+\ttest_all_match git commit -m \"Change folder1/a\" &&\n+\n+\t# Get the tree SHA for the new HEAD\n+\ttree3=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n+\n+\ttest_all_match git diff-tree $tree1 $tree3 &&\n+\ttest_all_match git diff-tree $tree1 $tree3 -- folder1/a &&\n+\ttest_all_match git diff-tree HEAD &&\n+\ttest_all_match git diff-tree HEAD -- folder1/a &&\n+\n+\t# check that SKIP_WORKTREE files are not materialized\n+\ttest_path_is_missing sparse-checkout/folder2/a &&\n+\ttest_path_is_missing sparse-index/folder2/a\n+'\n+\n+test_expect_success 'sparse-index is not expanded: diff-tree' '\n+\tinit_repos &&\n+\n+\t# Get the tree SHA for the current HEAD\n+\ttree1=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n+\n+\techo \"test1\" >>sparse-index/deep/a &&\n+\tgit -C sparse-index add deep/a &&\n+\tgit -C sparse-index commit -m \"Change deep/a\" &&\n+\n+\t# Get the tree SHA for the new HEAD\n+\ttree2=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n+\n+\tensure_not_expanded diff-tree $tree1 $tree2 &&\n+\tensure_not_expanded diff-tree $tree1 $tree2 -- deep/a &&\n+\tensure_not_expanded diff-tree HEAD &&\n+\tensure_not_expanded diff-tree HEAD -- deep/a\n+'\n+\n test_done\n-- \n2.39.0\n\n"},{"id":"477415","messageId":"fa24482c-7c48-9b7f-5d97-3dbf9822728c@github.com","threadId":"59745","inReplyTo":"20230515191836.674234-1-cheskaqiqi@gmail.com","subject":"Re: [RFC][PATCH V1] diff-tree: integrate with sparse index","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2023-05-16T21:19:04Z","receivedAt":"2023-05-16T21:19:11Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Shuqi Liang wrote:\n> Remove full index requirement for `git diff-tree`. Add tests that verify\n> that 'git diff-tree' behaves correctly when the sparse index is enabled\n> and test to ensure the index is not expanded.\n> \n> The `p2000` tests demonstrate a ~98% execution time reduction for\n> 'git diff-tree' using a sparse index:\n> \n> Test                                                before  after\n> ------------------------------------------------------------------------\n> 2000.94: git diff-tree HEAD (full-v3)                0.05   0.04 -20.0%\n> 2000.95: git diff-tree HEAD (full-v4)                0.06   0.05 -16.7%\n> 2000.96: git diff-tree HEAD (sparse-v3)              0.59   0.01 -98.3%\n> 2000.97: git diff-tree HEAD (sparse-v4)              0.61   0.01 -98.4%\n> 2000.98: git diff-tree HEAD -- f2/f4/a (full-v3)     0.05   0.05 +0.0%\n> 2000.99: git diff-tree HEAD -- f2/f4/a (full-v4)     0.05   0.04 -20.0%\n> 2000.100: git diff-tree HEAD -- f2/f4/a (sparse-v3)  0.58   0.01 -98.3%\n> 2000.101: git diff-tree HEAD -- f2/f4/a (sparse-v4)  0.55   0.01 -98.2%\n\nThese performance results look great! This is generally what we'd expect,\ntoo, since 'diff-tree' should be fast enough that index expansion is the\nmajority of its runtime.\n\n> \n> Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n> ---\n>  builtin/diff-tree.c                      |  4 ++\n>  t/perf/p2000-sparse-operations.sh        |  2 +\n>  t/t1092-sparse-checkout-compatibility.sh | 62 ++++++++++++++++++++++++\n>  3 files changed, 68 insertions(+)\n> \n> diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\n> index 385c2d0230..c5d5730ebf 100644\n> --- a/builtin/diff-tree.c\n> +++ b/builtin/diff-tree.c\n> @@ -121,6 +121,10 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n>  \t\tusage(diff_tree_usage);\n>  \n>  \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n> +\n> +\tprepare_repo_settings(the_repository);\n> +\tthe_repository->settings.command_requires_full_index = 0;\n\ntl;dr: this does appear to be all you need to integrate the sparse index\nwith 'diff-tree', although there's an opportunity to clean up some\n(seemingly) unused code to make that clearer.\n \nLonger version: \n\nLooking at the documentation for 'diff-tree', it's not immediately obvious\nwhy we'd need the index at all - it \"[c]ompares the content and mode of\nblobs found via two tree objects,\" per the command documentation. However,\nthe index is read in 'cmd_diff_tree' at two points, so to be reasonably\ncertain that this sparse index integration is correct we should verify that\nthe intended usage associated with those two reads will still work with a\nsparse index.\n\nReading the index for attributes\n================================\nThe first index read was added in fd66bcc31ff (diff-tree: read the index so\nattribute checks work in bare repositories, 2017-12-06) to deal with reading\n'.gitattributes' content. Per the 'gitattributes.txt' documentation:\n\n\"Git consults `$GIT_DIR/info/attributes` file (which has the highest\nprecedence), `.gitattributes` file in the same directory as the path in\nquestion, and its parent directories up to the toplevel of the work\ntree...When the `.gitattributes` file is missing from the work tree, the\npath in the index is used as a fall-back.\"\n\nHowever, 77efbb366ab (attr: be careful about sparse directories, 2021-09-08)\nestablished that, in a sparse index, we do _not_ try to load a\n'.gitattributes' file from within a sparse directory. Therefore, we don't\nneed to expand the index or change anything about reading attributes in\n'diff-tree'. Good!\n\nReading the index for rename detection(?)\n=========================================\nThe second one is read only on the condition that we're reading from stdin:\n\nif (opt->diffopt.detect_rename) {\n\tif (!the_index.cache)\n\t\trepo_read_index(the_repository);\n\topt->diffopt.setup |= DIFF_SETUP_USE_SIZE_CACHE;\n}\n\nThis was initially added in f0c6b2a2fd9 ([PATCH] Optimize diff-tree -[CM]\n--stdin, 2005-05-27), where 'setup' was set to 'DIFF_SETUP_USE_SIZE_CACHE |\nDIFF_SETUP_USE_CACHE'. That assignment was later modified to drop the\n'DIFF_SETUP_USE_CACHE' in ff7fe37b053 (diff.c: move read_index() code back\nto the caller, 2018-08-13). \n\nHowever, 'DIFF_SETUP_USE_SIZE_CACHE' seems to be unused as of 6e0b8ed6d35\n(diff.c: do not use a separate \"size cache\"., 2007-05-07) and nothing about\n'detect_rename' otherwise indicates index usage, so AFAICT that whole\ncondition can be dropped (along with DIFF_SETUP_USE_SIZE_CACHE,\nDIFF_SETUP_REVERSE, and diff_options.setup). Note that, if you want to make\nthat change in this series, it should be done in a separate patch _before_\nthis one (since dropping the deprecated setup infrastructure isn't really\npart of the sparse index integration).\n\n> +\n>  \trepo_init_revisions(the_repository, opt, prefix);\n>  \tif (repo_read_index(the_repository) < 0)\n>  \t\tdie(_(\"index file corrupt\"));\n> diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\n> index 60d1de0662..14caf01718 100755\n> --- a/t/perf/p2000-sparse-operations.sh\n> +++ b/t/perf/p2000-sparse-operations.sh\n> @@ -129,5 +129,7 @@ test_perf_on_all git grep --cached bogus -- \"f2/f1/f1/*\"\n>  test_perf_on_all git write-tree\n>  test_perf_on_all git describe --dirty\n>  test_perf_on_all 'echo >>new && git describe --dirty'\n> +test_perf_on_all git diff-tree HEAD\n> +test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a\n\nThese tests cover both the whole tree & a specific pathspec, looks good.\n\n>  \n>  test_done\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index 0c784813f1..f08edcbf8e 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -2108,4 +2108,66 @@ test_expect_success 'sparse-index is not expanded: write-tree' '\n>  \tensure_not_expanded write-tree\n>  '\n>  \n> +test_expect_success 'diff-tree' '\n> +\tinit_repos &&\n> +\n> +\twrite_script edit-contents <<-\\EOF &&\n> +\techo text >>\"$1\"\n> +\tEOF\n> +\n> +\t# Get the tree SHA for the current HEAD\n> +\ttree1=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n> +\n> +\t# make a change inside the sparse cone\n> +\trun_on_all ../edit-contents deep/a &&\n> +\ttest_all_match git add deep/a &&\n> +\ttest_all_match git commit -m \"Change deep/a\" &&\n> +\n> +\t# Get the tree SHA for the new HEAD\n> +\ttree2=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n\nCommits with changes similar to what you create here here already exist in\nthe test repo; you can simplify the test by using them:\n\ntest_expect_success 'diff-tree' '\n        init_repos &&\n\n        # Test change inside sparse cone\n        test_all_match git diff-tree HEAD update-deep &&\n        test_all_match git diff-tree HEAD update-deep -- deep/a &&\n\n        # Test change outside sparse cone\n        test_all_match git diff-tree HEAD update-folder1 &&\n        test_all_match git diff-tree HEAD update-folder1 -- folder1/a &&\n\n\t# Check that SKIP_WORKTREE files are not materialized\n\ttest_path_is_missing sparse-checkout/folder1/a &&\n\ttest_path_is_missing sparse-index/folder1/a\n'\n\nThis also has the benefit of avoiding the creation of 'folder1/a' on disk in\nthe sparse-checkout test repos.\n\n> +\n> +\n\nnit: extra newline should be removed\n\n> +\ttest_all_match git diff-tree $tree1 $tree2 &&\n> +\ttest_all_match git diff-tree HEAD &&\n> +\ttest_all_match git diff-tree HEAD -- deep/a &&\n\nYou don't have a wildcard pathspec tested here, but I think that's okay in\nthis case; unlike e.g. 'git grep', there's no sparse index-related behavior\nhere that depends on the pathspec's contents.\n\n> +\n> +\t# make a change outside the sparse cone\n> +\trun_on_all mkdir -p folder1 &&\n> +\trun_on_all cp a folder1/a &&\n> +\trun_on_all ../edit-contents folder1/a &&\n> +\ttest_all_match git update-index folder1/a &&\n\n'update-index' will work here, but I think using the more porcelain command\n'add --sparse' might be better for demonstrating typical user behavior.\n\n> +\ttest_all_match git commit -m \"Change folder1/a\" &&\n> +\n> +\t# Get the tree SHA for the new HEAD\n> +\ttree3=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n> +\n> +\ttest_all_match git diff-tree $tree1 $tree3 &&\n> +\ttest_all_match git diff-tree $tree1 $tree3 -- folder1/a &&\n> +\ttest_all_match git diff-tree HEAD &&\n> +\ttest_all_match git diff-tree HEAD -- folder1/a &&\n> +\n> +\t# check that SKIP_WORKTREE files are not materialized\n> +\ttest_path_is_missing sparse-checkout/folder2/a &&\n> +\ttest_path_is_missing sparse-index/folder2/a\n\nAt first I wasn't sure about whether these checks were necessary (we don't\n'diff-tree' on any 'folder2/' pathspec), but this check verifies that we\ndon't materialize the files in the case with no pathspec. While that's\nunlikely to happen, it doesn't hurt to have this test to be sure.\n\n> +'\n> +\n> +test_expect_success 'sparse-index is not expanded: diff-tree' '\n> +\tinit_repos &&\n> +\n> +\t# Get the tree SHA for the current HEAD\n> +\ttree1=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n\nAs with the previous test, you can use the 'update-deep' and\n'update-folder1' branches to simplify here.\n\n> +\n> +\techo \"test1\" >>sparse-index/deep/a &&\n> +\tgit -C sparse-index add deep/a &&\n> +\tgit -C sparse-index commit -m \"Change deep/a\" &&\n> +\n> +\t# Get the tree SHA for the new HEAD\n> +\ttree2=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n> +\n> +\tensure_not_expanded diff-tree $tree1 $tree2 &&\n> +\tensure_not_expanded diff-tree $tree1 $tree2 -- deep/a &&\n> +\tensure_not_expanded diff-tree HEAD &&\n> +\tensure_not_expanded diff-tree HEAD -- deep/a\n\nSince the index won't expand regardless of whether you 'diff-tree'\na file inside or outside the cone, it'd be nice to have a test like:\n\nensure_not_expanded diff-tree update-folder1 &&\nensure_not_expanded diff-tree update-folder1 -- folder1/a\n\n> +'\n> +\n>  test_done\n\n"},{"id":"477422","messageId":"xmqqsfbvswci.fsf@gitster.g","threadId":"59745","inReplyTo":"fa24482c-7c48-9b7f-5d97-3dbf9822728c@github.com","subject":"Re: [RFC][PATCH V1] diff-tree: integrate with sparse index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-16T23:14:05Z","receivedAt":"2023-05-16T23:14:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Victoria Dye <vdye@github.com> writes:\n\n> Longer version: \n\nThanks, as usual, for a great review.  A lot of the stuff you wrote\nshould inspire and result in an improved log message that explains\nwhy this change is sufficient to teach diff-tree to take advantage\nof the sparse index.\n\n> However, 'DIFF_SETUP_USE_SIZE_CACHE' seems to be unused as of 6e0b8ed6d35\n> (diff.c: do not use a separate \"size cache\"., 2007-05-07) and nothing about\n> 'detect_rename' otherwise indicates index usage, so AFAICT that whole\n> condition can be dropped (along with DIFF_SETUP_USE_SIZE_CACHE,\n> DIFF_SETUP_REVERSE, and diff_options.setup).\n\nTrue.  The size cache does not exist anymore.  6b5ee137 (Diff\nclean-up., 2005-09-21) restructured the command line option parsing\nquite a bit, and we lost DIFF_SETUP_REVERSE, which is a bit that\ngets OR'ed in to a file-scope diff_setup_opt static of each of the\ncommand in the diff family.  The bit and the diff_setup_opt variable\ngot replaced with members of \"struct diff_options\", and I should\nhave removed the macro at the same time.\n\n> Note that, if you want to make\n> that change in this series, it should be done in a separate patch _before_\n> this one (since dropping the deprecated setup infrastructure isn't really\n> part of the sparse index integration).\n\nOr after this one, perhaps?  I agree that the clean-up opportunity\nyou found is very much unrelated to the work to teach diff-tree to\ntake advantage of the sparse index.\n\nTHanks.\n"},{"id":"477472","messageId":"2d99fbed-3074-be22-2b8c-a75dd22bda65@github.com","threadId":"59745","inReplyTo":"xmqqsfbvswci.fsf@gitster.g","subject":"Re: [RFC][PATCH V1] diff-tree: integrate with sparse index","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2023-05-17T18:47:03Z","receivedAt":"2023-05-17T18:49:39Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Junio C Hamano wrote:\n> Victoria Dye <vdye@github.com> writes:\n> \n>> Note that, if you want to make\n>> that change in this series, it should be done in a separate patch _before_\n>> this one (since dropping the deprecated setup infrastructure isn't really\n>> part of the sparse index integration).\n> \n> Or after this one, perhaps?  I agree that the clean-up opportunity\n> you found is very much unrelated to the work to teach diff-tree to\n> take advantage of the sparse index.\n\nYou're right, it doesn't need to come before this patch (or belong to this\nseries). *If* the cleanup was done in this series, my thought was that it\nwould be (subjectively) better to end the series on the sparse index\nintegration. However, it doesn't really make a practical difference whether\nthe cleanup is done before or after this patch, since it's functionally\nunrelated to the sparse index work. \n\nThanks for the clarification!\n\n"},{"id":"477526","messageId":"20230518154454.475487-1-cheskaqiqi@gmail.com","threadId":"59745","inReplyTo":"20230515191836.674234-1-cheskaqiqi@gmail.com","subject":"[PATCH v2] diff-tree: integrate with sparse index","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-05-18T15:44:54Z","receivedAt":"2023-05-18T15:45:24Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"The index is read in 'cmd_diff_tree' at two points:\n\n1. The first index read was added in fd66bcc31ff (diff-tree: read the\nindex so attribute checks work in bare repositories, 2017-12-06) to deal\nwith reading '.gitattributes' content. 77efbb366ab (attr: be careful\nabout sparse directories, 2021-09-08) established that, in a sparse\nindex, we do _not_ try to load a '.gitattributes' file from within a\nsparse directory.\n\n2. The second index access point is involved in rename detection,\nspecifically when reading from stdin.This was initially added in\nf0c6b2a2fd9 ([PATCH] Optimize diff-tree -[CM]--stdin, 2005-05-27), where\n'setup' was set to 'DIFF_SETUP_USE_SIZE_CACHE |DIFF_SETUP_USE_CACHE'.\nThat assignment was later modified to drop the'DIFF_SETUP_USE_CACHE' in\nff7fe37b053 (diff.c: move read_index() code back to the caller,\n2018-08-13).However, 'DIFF_SETUP_USE_SIZE_CACHE' seems to be unused as\nof 6e0b8ed6d35 (diff.c: do not use a separate \"size cache\"., 2007-05-07)\nand nothing about 'detect_rename' otherwise indicates index usage.\n\nHence we can just set the requires-full-index to false for \"diff-tree\".\n\nAdd tests that verify that 'git diff-tree' behaves correctly when the\nsparse index is enabled and test to ensure the index is not expanded.\n\nThe `p2000` tests demonstrate a ~98% execution time reduction for\n'git diff-tree' using a sparse index:\n\nTest                                                before  after\n-----------------------------------------------------------------------\n2000.94: git diff-tree HEAD (full-v3)                0.05   0.04 -20.0%\n2000.95: git diff-tree HEAD (full-v4)                0.06   0.05 -16.7%\n2000.96: git diff-tree HEAD (sparse-v3)              0.59   0.01 -98.3%\n2000.97: git diff-tree HEAD (sparse-v4)              0.61   0.01 -98.4%\n2000.98: git diff-tree HEAD -- f2/f4/a (full-v3)     0.05   0.05 +0.0%\n2000.99: git diff-tree HEAD -- f2/f4/a (full-v4)     0.05   0.04 -20.0%\n2000.100: git diff-tree HEAD -- f2/f4/a (sparse-v3)  0.58   0.01 -98.3%\n2000.101: git diff-tree HEAD -- f2/f4/a (sparse-v4)  0.55   0.01 -98.2%\n\nHelped-by: Victoria Dye <vdye@github.com>\nSigned-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n---\nChange since v1:\n\n* Update commit message.\n* Use existing test repo to simplify the test.\n* Add test to ensure index won't expand regardless of 'diff-tree' a file\ninside or outside the cone\n\n\nRange-diff against v1:\n1:  47049834d1 < -:  ---------- diff-tree: integrate with sparse index\n-:  ---------- > 1:  a24605f579 diff-tree: integrate with sparse index\n\n builtin/diff-tree.c                      |  4 +++\n t/perf/p2000-sparse-operations.sh        |  2 ++\n t/t1092-sparse-checkout-compatibility.sh | 42 ++++++++++++++++++++++++\n 3 files changed, 48 insertions(+)\n\ndiff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\nindex 0b02c62b85..c0540317fb 100644\n--- a/builtin/diff-tree.c\n+++ b/builtin/diff-tree.c\n@@ -122,6 +122,10 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \t\tusage(diff_tree_usage);\n \n \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n+\n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\n \trepo_init_revisions(the_repository, opt, prefix);\n \tif (repo_read_index(the_repository) < 0)\n \t\tdie(_(\"index file corrupt\"));\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 901cc493ef..5a11910189 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -131,5 +131,7 @@ test_perf_on_all git describe --dirty\n test_perf_on_all 'echo >>new && git describe --dirty'\n test_perf_on_all git diff-files\n test_perf_on_all git diff-files -- $SPARSE_CONE/a\n+test_perf_on_all git diff-tree HEAD\n+test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a\n \n test_done\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex e58bfbfcb4..90f827ffe9 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2170,4 +2170,46 @@ test_expect_success 'sparse index is not expanded: diff-files' '\n \tensure_not_expanded diff-files -- \"deep/*\"\n '\n \n+test_expect_success 'diff-tree' '\n+\tinit_repos &&\n+\n+\t# Test change inside sparse cone\n+\ttree1=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n+\ttree2=$(git -C sparse-index rev-parse update-deep^{tree}) &&\n+\ttest_all_match git diff-tree $tree1 $tree2 &&\n+\ttest_all_match git diff-tree $tree1 $tree2 -- deep/a &&\n+\ttest_all_match git diff-tree HEAD update-deep &&\n+\ttest_all_match git diff-tree HEAD update-deep -- deep/a &&\n+\n+\t# Test change outside sparse cone\n+\ttree3=$(git -C sparse-index rev-parse update-folder1^{tree}) &&\n+\ttest_all_match git diff-tree $tree1 $tree3 &&\n+\ttest_all_match git diff-tree $tree1 $tree3 -- folder1/a &&\n+\ttest_all_match git diff-tree HEAD update-folder1 &&\n+\ttest_all_match git diff-tree HEAD update-folder1 -- folder1/a &&\n+\n+\t# Check that SKIP_WORKTREE files are not materialized\n+\ttest_path_is_missing sparse-checkout/folder1/a &&\n+\ttest_path_is_missing sparse-index/folder1/a &&\n+\ttest_path_is_missing sparse-checkout/folder2/a &&\n+\ttest_path_is_missing sparse-index/folder2/a\n+'\n+\n+test_expect_success 'sparse-index is not expanded: diff-tree' '\n+\tinit_repos &&\n+\n+\ttree1=$(git -C sparse-index rev-parse HEAD^{tree}) &&\n+\ttree2=$(git -C sparse-index rev-parse update-deep^{tree}) &&\n+\ttree3=$(git -C sparse-index rev-parse update-folder1^{tree}) &&\n+\n+\tensure_not_expanded diff-tree $tree1 $tree2 &&\n+\tensure_not_expanded diff-tree $tree1 $tree2 -- deep/a &&\n+\tensure_not_expanded diff-tree HEAD update-deep &&\n+\tensure_not_expanded diff-tree HEAD update-deep -- deep/a &&\n+\tensure_not_expanded diff-tree $tree1 $tree3 &&\n+\tensure_not_expanded diff-tree $tree1 $tree3 -- folder1/a &&\n+\tensure_not_expanded diff-tree HEAD update-folder1 &&\n+\tensure_not_expanded diff-tree HEAD update-folder1 -- folder1/a\n+'\n+\n test_done\n-- \n2.39.0\n\n"},{"id":"477642","messageId":"2a2b7223-bb5d-65f9-95bb-9be45d329c87@github.com","threadId":"59745","inReplyTo":"20230518154454.475487-1-cheskaqiqi@gmail.com","subject":"Re: [PATCH v2] diff-tree: integrate with sparse index","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2023-05-22T19:07:23Z","receivedAt":"2023-05-22T19:07:30Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Shuqi Liang wrote:\n> Change since v1:\n> \n> * Update commit message.\n> * Use existing test repo to simplify the test.\n> * Add test to ensure index won't expand regardless of 'diff-tree' a file\n> inside or outside the cone\n\nThanks for these updates! This version looks ready for 'next' to me.\n\n"},{"id":"477659","messageId":"xmqqy1lfirw2.fsf@gitster.g","threadId":"59745","inReplyTo":"2a2b7223-bb5d-65f9-95bb-9be45d329c87@github.com","subject":"Re: [PATCH v2] diff-tree: integrate with sparse index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-23T04:38:37Z","receivedAt":"2023-05-23T04:38:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Victoria Dye <vdye@github.com> writes:\n\n> Shuqi Liang wrote:\n>> Change since v1:\n>> \n>> * Update commit message.\n>> * Use existing test repo to simplify the test.\n>> * Add test to ensure index won't expand regardless of 'diff-tree' a file\n>> inside or outside the cone\n>\n> Thanks for these updates! This version looks ready for 'next' to me.\n\nThanks, both.  Will do so (when I get back to the keyboard---I am on\nhalf-vacation).\n\n"}]}