{"thread":{"id":"59521","subject":"[RFC][PATCH v1] write-tree: integrate with sparse index","startedAt":"2023-04-02T00:01:35Z","lastAt":"2023-05-08T21:27:41Z","messageCount":20,"participants":["Shuqi Liang","Junio C Hamano","Victoria Dye"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"474652","messageId":"20230402000117.313171-1-cheskaqiqi@gmail.com","threadId":"59521","inReplyTo":null,"subject":"[RFC][PATCH v1] write-tree: integrate with sparse index","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-04-02T00:01:17Z","receivedAt":"2023-04-02T00:01:35Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Update 'git write-tree' to allow using the sparse-index in memory\nwithout expanding to a full one.\n\nThe recursive algorithm for update_one() was already updated in 2de37c5\n(cache-tree: integrate with sparse directory entries, 2021-03-03) to\nhandle sparse directory entries in the index. Hence we can just set the\nrequires-full-index to false for \"write-tree\".\n\nThe `p2000` tests demonstrate a ~96% execution time reduction for 'git\nwrite-tree' using a sparse index:\n\nTest                                           before  after\n-----------------------------------------------------------------\n2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n\nSigned-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n---\n builtin/write-tree.c                     |  4 ++++\n t/perf/p2000-sparse-operations.sh        |  1 +\n t/t1092-sparse-checkout-compatibility.sh | 28 ++++++++++++++++++++++++\n 3 files changed, 33 insertions(+)\n\ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex 45d61707e7..28c45b4301 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -35,6 +35,10 @@ int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n \t};\n \n \tgit_config(git_default_config, NULL);\n+\t\n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\n \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n \t\t\t     write_tree_usage, 0);\n \ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 3242cfe91a..9924adfc26 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -125,5 +125,6 @@ test_perf_on_all git checkout-index -f --all\n test_perf_on_all git update-index --add --remove $SPARSE_CONE/a\n test_perf_on_all \"git rm -f $SPARSE_CONE/a && git checkout HEAD -- $SPARSE_CONE/a\"\n test_perf_on_all git grep --cached --sparse bogus -- \"f2/f1/f1/*\"\n+test_perf_on_all git write-tree \n \n test_done\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 801919009e..3b8191b390 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2055,4 +2055,32 @@ test_expect_success 'grep sparse directory within submodules' '\n \ttest_cmp actual expect\n '\n \n+test_expect_success 'write-tree on all' '\n+\tinit_repos &&\n+\n+\twrite_script edit-contents <<-\\EOF &&\n+\techo text >>\"$1\"\n+\tEOF\n+\n+\trun_on_all ../edit-contents deep/a &&\n+\trun_on_all git update-index deep/a &&\n+\ttest_all_match git write-tree &&\n+\n+\trun_on_all mkdir -p folder1 &&\n+\trun_on_all cp a folder1/a &&\n+\trun_on_all ../edit-contents folder1/a &&\n+\trun_on_all git update-index folder1/a &&\n+\ttest_all_match git write-tree\n+'\n+\n+test_expect_success 'sparse-index is not expanded: write-tree' '\n+\tinit_repos &&\n+\n+\tensure_not_expanded write-tree &&\n+\n+\techo \"test1\" >>sparse-index/a &&\n+\tgit -C sparse-index update-index a &&\n+\tensure_not_expanded write-tree \n+'\n+\n test_done\n-- \n2.39.0\n\n"},{"id":"474714","messageId":"xmqqilec4ra0.fsf@gitster.g","threadId":"59521","inReplyTo":"20230402000117.313171-1-cheskaqiqi@gmail.com","subject":"Re: [RFC][PATCH v1] write-tree: integrate with sparse index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-03T20:58:15Z","receivedAt":"2023-04-03T20:58:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang <cheskaqiqi@gmail.com> writes:\n\n> Update 'git write-tree' to allow using the sparse-index in memory\n> without expanding to a full one.\n>\n> The recursive algorithm for update_one() was already updated in 2de37c5\n> (cache-tree: integrate with sparse directory entries, 2021-03-03) to\n> handle sparse directory entries in the index. Hence we can just set the\n> requires-full-index to false for \"write-tree\".\n>\n> The `p2000` tests demonstrate a ~96% execution time reduction for 'git\n> write-tree' using a sparse index:\n>\n> Test                                           before  after\n> -----------------------------------------------------------------\n> 2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n> 2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n> 2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n> 2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n>\n> Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n> ---\n>  builtin/write-tree.c                     |  4 ++++\n>  t/perf/p2000-sparse-operations.sh        |  1 +\n>  t/t1092-sparse-checkout-compatibility.sh | 28 ++++++++++++++++++++++++\n>  3 files changed, 33 insertions(+)\n\nHas the test suite been exercised with this patch?  It seems to\nbreak at least t0012\n\n\n"},{"id":"474717","messageId":"CAMO4yUGnkR5Jj5m52LXb9+LQUcJyjMW_RcFM2dzALAaKa064dQ@mail.gmail.com","threadId":"59521","inReplyTo":"xmqqilec4ra0.fsf@gitster.g","subject":"Re: [RFC][PATCH v1] write-tree: integrate with sparse index","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-04-03T22:16:52Z","receivedAt":"2023-04-03T22:17:10Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"On Mon, Apr 3, 2023 at 4:58 PM Junio C Hamano <gitster@pobox.com> wrote:\n\n> Has the test suite been exercised with this patch?  It seems to\n> break at least t0012\n>\n\nHi Junio\n\nI commented out the 'test_perf_on_all git grep --cached bogus --\n\"f2/f1/f1/*\"' before\nrunning 'p2000-sparse-operations.sh'.  I did this because I found that\nwith its presence,\neven without adding any code, the tests wouldn't pass.  After commenting it out,\neverything worked well. (In the patch I submitted above I did not\ncommented it out )\n\nThanks\nShuqi\n"},{"id":"474727","messageId":"xmqq1ql04lwm.fsf@gitster.g","threadId":"59521","inReplyTo":"CAMO4yUGnkR5Jj5m52LXb9+LQUcJyjMW_RcFM2dzALAaKa064dQ@mail.gmail.com","subject":"Re: [RFC][PATCH v1] write-tree: integrate with sparse index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-03T22:54:17Z","receivedAt":"2023-04-03T22:54:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang <cheskaqiqi@gmail.com> writes:\n\n>> Has the test suite been exercised with this patch?  It seems to\n>> break at least t0012\n>>\n>\n> Hi Junio\n>\n> I commented out the 'test_perf_on_all git grep --cached bogus --\n> \"f2/f1/f1/*\"' before\n> running 'p2000-sparse-operations.sh'.\n\nSorry, but I do not see why you are bringing up p2000 performance\nmeasurement script here.\n\n"},{"id":"474732","messageId":"20230404003539.1578245-1-cheskaqiqi@gmail.com","threadId":"59521","inReplyTo":"20230402000117.313171-1-cheskaqiqi@gmail.com","subject":"[PATCH v2] write-tree: integrate with sparse index","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-04-04T00:35:39Z","receivedAt":"2023-04-04T00:35:57Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Update 'git write-tree' to allow using the sparse-index in memory\nwithout expanding to a full one.\n\nThe recursive algorithm for update_one() was already updated in 2de37c5\n(cache-tree: integrate with sparse directory entries, 2021-03-03) to\nhandle sparse directory entries in the index. Hence we can just set the\nrequires-full-index to false for \"write-tree\".\n\nThe `p2000` tests demonstrate a ~96% execution time reduction for 'git\nwrite-tree' using a sparse index:\n\nTest                                           before  after\n-----------------------------------------------------------------\n2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n\nSigned-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n---\n\n* change the position of \"settings.command_requires_full_index = 0\"\n\nRange-diff against v1:\n1:  d8a9ccd0b3 ! 1:  8873c79759 write-tree: integrate with sparse index\n    @@ Commit message\n     \n      ## builtin/write-tree.c ##\n     @@ builtin/write-tree.c: int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n    - \t};\n    - \n    - \tgit_config(git_default_config, NULL);\n    -+\t\n    -+\tprepare_repo_settings(the_repository);\n    -+\tthe_repository->settings.command_requires_full_index = 0;\n    -+\n      \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n      \t\t\t     write_tree_usage, 0);\n      \n    ++\tprepare_repo_settings(the_repository);\n    ++\tthe_repository->settings.command_requires_full_index = 0;\n    ++\t\n    + \tret = write_cache_as_tree(&oid, flags, tree_prefix);\n    + \tswitch (ret) {\n    + \tcase 0:\n     \n      ## t/perf/p2000-sparse-operations.sh ##\n     @@ t/perf/p2000-sparse-operations.sh: test_perf_on_all git checkout-index -f --all\n\n\n builtin/write-tree.c                     |  3 +++\n t/perf/p2000-sparse-operations.sh        |  1 +\n t/t1092-sparse-checkout-compatibility.sh | 28 ++++++++++++++++++++++++\n 3 files changed, 32 insertions(+)\n\ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex 45d61707e7..4492da0912 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -38,6 +38,9 @@ int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n \t\t\t     write_tree_usage, 0);\n \n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\t\n \tret = write_cache_as_tree(&oid, flags, tree_prefix);\n \tswitch (ret) {\n \tcase 0:\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 3242cfe91a..9924adfc26 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -125,5 +125,6 @@ test_perf_on_all git checkout-index -f --all\n test_perf_on_all git update-index --add --remove $SPARSE_CONE/a\n test_perf_on_all \"git rm -f $SPARSE_CONE/a && git checkout HEAD -- $SPARSE_CONE/a\"\n test_perf_on_all git grep --cached --sparse bogus -- \"f2/f1/f1/*\"\n+test_perf_on_all git write-tree \n \n test_done\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 801919009e..3b8191b390 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2055,4 +2055,32 @@ test_expect_success 'grep sparse directory within submodules' '\n \ttest_cmp actual expect\n '\n \n+test_expect_success 'write-tree on all' '\n+\tinit_repos &&\n+\n+\twrite_script edit-contents <<-\\EOF &&\n+\techo text >>\"$1\"\n+\tEOF\n+\n+\trun_on_all ../edit-contents deep/a &&\n+\trun_on_all git update-index deep/a &&\n+\ttest_all_match git write-tree &&\n+\n+\trun_on_all mkdir -p folder1 &&\n+\trun_on_all cp a folder1/a &&\n+\trun_on_all ../edit-contents folder1/a &&\n+\trun_on_all git update-index folder1/a &&\n+\ttest_all_match git write-tree\n+'\n+\n+test_expect_success 'sparse-index is not expanded: write-tree' '\n+\tinit_repos &&\n+\n+\tensure_not_expanded write-tree &&\n+\n+\techo \"test1\" >>sparse-index/a &&\n+\tgit -C sparse-index update-index a &&\n+\tensure_not_expanded write-tree \n+'\n+\n test_done\n-- \n2.39.0\n\n"},{"id":"474865","messageId":"9d0309bd-943c-dd51-97cf-59721eda78f7@github.com","threadId":"59521","inReplyTo":"20230404003539.1578245-1-cheskaqiqi@gmail.com","subject":"Re: [PATCH v2] write-tree: integrate with sparse index","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2023-04-05T17:31:36Z","receivedAt":"2023-04-05T17:31:44Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Shuqi Liang wrote:\n> Update 'git write-tree' to allow using the sparse-index in memory\n> without expanding to a full one.\n> \n> The recursive algorithm for update_one() was already updated in 2de37c5\n> (cache-tree: integrate with sparse directory entries, 2021-03-03) to\n> handle sparse directory entries in the index. Hence we can just set the\n> requires-full-index to false for \"write-tree\".\n> \n> The `p2000` tests demonstrate a ~96% execution time reduction for 'git\n> write-tree' using a sparse index:\n> \n> Test                                           before  after\n> -----------------------------------------------------------------\n> 2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n> 2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n> 2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n> 2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n> \n> Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n> ---\n> \n> * change the position of \"settings.command_requires_full_index = 0\"\n\nCould you describe why you made this change? You don't need to re-roll, but\nin the future please make sure to describe the reasoning for changes like\nthis in these version notes if the context can't be gathered from other\ndiscussions in the thread. \n\n> \n> Range-diff against v1:\n> 1:  d8a9ccd0b3 ! 1:  8873c79759 write-tree: integrate with sparse index\n>     @@ Commit message\n>      \n>       ## builtin/write-tree.c ##\n>      @@ builtin/write-tree.c: int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n>     - \t};\n>     - \n>     - \tgit_config(git_default_config, NULL);\n>     -+\t\n>     -+\tprepare_repo_settings(the_repository);\n>     -+\tthe_repository->settings.command_requires_full_index = 0;\n>     -+\n>       \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n>       \t\t\t     write_tree_usage, 0);\n>       \n>     ++\tprepare_repo_settings(the_repository);\n>     ++\tthe_repository->settings.command_requires_full_index = 0;\n>     ++\t\n>     + \tret = write_cache_as_tree(&oid, flags, tree_prefix);\n>     + \tswitch (ret) {\n>     + \tcase 0:\n>      \n>       ## t/perf/p2000-sparse-operations.sh ##\n>      @@ t/perf/p2000-sparse-operations.sh: test_perf_on_all git checkout-index -f --all\n> \n> \n>  builtin/write-tree.c                     |  3 +++\n>  t/perf/p2000-sparse-operations.sh        |  1 +\n>  t/t1092-sparse-checkout-compatibility.sh | 28 ++++++++++++++++++++++++\n>  3 files changed, 32 insertions(+)\n> \n> diff --git a/builtin/write-tree.c b/builtin/write-tree.c\n> index 45d61707e7..4492da0912 100644\n> --- a/builtin/write-tree.c\n> +++ b/builtin/write-tree.c\n> @@ -38,6 +38,9 @@ int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n>  \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n>  \t\t\t     write_tree_usage, 0);\n>  \n> +\tprepare_repo_settings(the_repository);\n> +\tthe_repository->settings.command_requires_full_index = 0;\n> +\t\n>  \tret = write_cache_as_tree(&oid, flags, tree_prefix);\n>  \tswitch (ret) {\n>  \tcase 0:\n> diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\n> index 3242cfe91a..9924adfc26 100755\n> --- a/t/perf/p2000-sparse-operations.sh\n> +++ b/t/perf/p2000-sparse-operations.sh\n> @@ -125,5 +125,6 @@ test_perf_on_all git checkout-index -f --all\n>  test_perf_on_all git update-index --add --remove $SPARSE_CONE/a\n>  test_perf_on_all \"git rm -f $SPARSE_CONE/a && git checkout HEAD -- $SPARSE_CONE/a\"\n>  test_perf_on_all git grep --cached --sparse bogus -- \"f2/f1/f1/*\"\n> +test_perf_on_all git write-tree \n>  \n>  test_done\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index 801919009e..3b8191b390 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -2055,4 +2055,32 @@ test_expect_success 'grep sparse directory within submodules' '\n>  \ttest_cmp actual expect\n>  '\n>  \n> +test_expect_success 'write-tree on all' '\n\nIt's not clear what \"on all\" means in this context. If it's \"write-tree with\nchanges both inside and outside the cone\", then please either make that\nexplicit in the test name or simplify the name to just 'write-tree' (like\n'clean').\n\n> +\tinit_repos &&\n\nIt would be nice to have a baseline 'test_all_match git write-tree' before\nmaking any changes to the index (as you do in the 'sparse-index is not\nexpanded: write-tree' test). \n\n> +\n> +\twrite_script edit-contents <<-\\EOF &&\n> +\techo text >>\"$1\"\n> +\tEOF\n> +\n> +\trun_on_all ../edit-contents deep/a &&\n> +\trun_on_all git update-index deep/a &&\n> +\ttest_all_match git write-tree &&\n\nFirst you make a change inside the sparse cone and 'write-tree'...\n\n> +\n> +\trun_on_all mkdir -p folder1 &&\n> +\trun_on_all cp a folder1/a &&\n> +\trun_on_all ../edit-contents folder1/a &&\n> +\trun_on_all git update-index folder1/a &&\n> +\ttest_all_match git write-tree\n\n...then make a change outside the cone and 'write-tree' again. Makes sense.\n\nHowever, there isn't any test of the working tree after 'write-tree' exits.\nFor example, I'd be interested in seeing a comparison of the output of 'git\nstatus --porcelain=v2', as well as ensuring that SKIP_WORKTREE files weren't\nmaterialized on disk in 'sparse-checkout' and 'sparse-index' (e.g.,\n'folder2/a' shouldn't exist).\n\nIt also wouldn't hurt to 'test_all_match' on the 'git update-index' calls,\nbut I don't feel too strongly either way.\n\n> +'\n> +\n> +test_expect_success 'sparse-index is not expanded: write-tree' '\n> +\tinit_repos &&\n> +\n> +\tensure_not_expanded write-tree &&\n> +\n> +\techo \"test1\" >>sparse-index/a &&\n> +\tgit -C sparse-index update-index a &&\n> +\tensure_not_expanded write-tree \n\nThis also looks good. \n\n> +'\n> +\n>  test_done\n\n"},{"id":"474882","messageId":"xmqqedoyun4e.fsf@gitster.g","threadId":"59521","inReplyTo":"9d0309bd-943c-dd51-97cf-59721eda78f7@github.com","subject":"Re: [PATCH v2] write-tree: integrate with sparse index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-05T19:48:01Z","receivedAt":"2023-04-05T19:48:27Z","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>> Update 'git write-tree' to allow using the sparse-index in memory\n>> without expanding to a full one.\n>> \n>> The recursive algorithm for update_one() was already updated in 2de37c5\n>> (cache-tree: integrate with sparse directory entries, 2021-03-03) to\n>> handle sparse directory entries in the index. Hence we can just set the\n>> requires-full-index to false for \"write-tree\".\n>> \n>> The `p2000` tests demonstrate a ~96% execution time reduction for 'git\n>> write-tree' using a sparse index:\n>> \n>> Test                                           before  after\n>> -----------------------------------------------------------------\n>> 2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n>> 2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n>> 2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n>> 2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n>> \n>> Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n>> ---\n>> \n>> * change the position of \"settings.command_requires_full_index = 0\"\n>\n> Could you describe why you made this change? You don't need to re-roll, but\n> in the future please make sure to describe the reasoning for changes like\n> this in these version notes if the context can't be gathered from other\n> discussions in the thread. \n\nThe reason, I think, is because previous iteration hit a BUG() when\nthe command \"git write-tree -h\" is run outside a repository.  That\nform of the help request is handled in the parse_options() machinery\nwithout any need to have a repository or a working tree.\n\nBut prepare_repo_settings() does need to be run inside a repository,\nso calling it without first checking if we are even in a repository\nis asking for trouble.\n\nI guess an alternative fix could have been to see if we are indeed\nin a repository, by doing something like\n\n\tif (the_repository->gitdir) {\n\t\tprepare_repo_settings(the_repository);\n\t\tthe_repository->settings.command_requires_full_index = 0;\n\t}\n\nlike implementations of some subcommands do.  And being explicit\nthat way, instead of relying on an implicit safety given by ordering\nof calls, would be more maintainable in the longer haul.\n\n"},{"id":"475681","messageId":"20230419072148.4297-1-cheskaqiqi@gmail.com","threadId":"59521","inReplyTo":"20230404003539.1578245-1-cheskaqiqi@gmail.com","subject":"[PATCH v3] write-tree: integrate with sparse index","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-04-19T07:21:48Z","receivedAt":"2023-04-19T07:22:07Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Update 'git write-tree' to allow using the sparse-index in memory\nwithout expanding to a full one.\n\nThe recursive algorithm for update_one() was already updated in 2de37c5\n(cache-tree: integrate with sparse directory entries, 2021-03-03) to\nhandle sparse directory entries in the index. Hence we can just set the\nrequires-full-index to false for \"write-tree\".\n\nThe `p2000` tests demonstrate a ~96% execution time reduction for 'git\nwrite-tree' using a sparse index:\n\nTest                                           before  after\n-----------------------------------------------------------------\n2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n\nSigned-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n---\n\n* Modified the code to ensure prepare_repo_settings() is called only \nwhen inside a repository.\n\n* Change 'write-tree on all' to just 'write-tree'.\n\n* Have a baseline 'test_all_match git write-tree' before making any \nchanges to the index.\n\n* Add  'git status --porcelain=v2'.\n\n* Ensuring that SKIP_WORKTREE files weren't materialized on disk by\nusing \"test_path_is_missing\".\n\n* Use 'test_all_match' on the 'git update-index'.\n\n\n\nRange-diff against v2:\n1:  8873c79759 ! 1:  cfa43c6cc7 write-tree: integrate with sparse index\n    @@ Commit message\n     \n      ## builtin/write-tree.c ##\n     @@ builtin/write-tree.c: int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n    - \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n    - \t\t\t     write_tree_usage, 0);\n    + \t};\n      \n    + \tgit_config(git_default_config, NULL);\n    ++\t\n    ++\tif (the_repository->gitdir) {\n     +\tprepare_repo_settings(the_repository);\n     +\tthe_repository->settings.command_requires_full_index = 0;\n    -+\t\n    - \tret = write_cache_as_tree(&oid, flags, tree_prefix);\n    - \tswitch (ret) {\n    - \tcase 0:\n    ++\t}\n    ++\n    + \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n    + \t\t\t     write_tree_usage, 0);\n    + \n     \n      ## t/perf/p2000-sparse-operations.sh ##\n     @@ t/perf/p2000-sparse-operations.sh: test_perf_on_all git checkout-index -f --all\n    @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'grep sparse direc\n      \ttest_cmp actual expect\n      '\n      \n    -+test_expect_success 'write-tree on all' '\n    ++test_expect_success 'write-tree' '\n     +\tinit_repos &&\n     +\n    ++\ttest_all_match git write-tree &&\n    ++\n     +\twrite_script edit-contents <<-\\EOF &&\n     +\techo text >>\"$1\"\n     +\tEOF\n     +\n    ++\t# make a change inside the sparse cone\n     +\trun_on_all ../edit-contents deep/a &&\n    -+\trun_on_all git update-index deep/a &&\n    ++\ttest_all_match git update-index deep/a &&\n     +\ttest_all_match git write-tree &&\n    ++\ttest_all_match git status --porcelain=v2 &&\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    -+\trun_on_all git update-index folder1/a &&\n    -+\ttest_all_match git write-tree\n    ++\ttest_all_match git update-index folder1/a &&\n    ++\ttest_all_match git write-tree &&\n    ++\ttest_all_match git status --porcelain=v2 &&\n    ++\t\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: write-tree' '\n\n\n\n builtin/write-tree.c                     |  6 ++++\n t/perf/p2000-sparse-operations.sh        |  1 +\n t/t1092-sparse-checkout-compatibility.sh | 38 ++++++++++++++++++++++++\n 3 files changed, 45 insertions(+)\n\ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex 45d61707e7..23d63844de 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -35,6 +35,12 @@ int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n \t};\n \n \tgit_config(git_default_config, NULL);\n+\t\n+\tif (the_repository->gitdir) {\n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\t}\n+\n \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n \t\t\t     write_tree_usage, 0);\n \ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 3242cfe91a..9924adfc26 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -125,5 +125,6 @@ test_perf_on_all git checkout-index -f --all\n test_perf_on_all git update-index --add --remove $SPARSE_CONE/a\n test_perf_on_all \"git rm -f $SPARSE_CONE/a && git checkout HEAD -- $SPARSE_CONE/a\"\n test_perf_on_all git grep --cached --sparse bogus -- \"f2/f1/f1/*\"\n+test_perf_on_all git write-tree \n \n test_done\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 801919009e..d3eb31326b 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2055,4 +2055,42 @@ test_expect_success 'grep sparse directory within submodules' '\n \ttest_cmp actual expect\n '\n \n+test_expect_success 'write-tree' '\n+\tinit_repos &&\n+\n+\ttest_all_match git write-tree &&\n+\n+\twrite_script edit-contents <<-\\EOF &&\n+\techo text >>\"$1\"\n+\tEOF\n+\n+\t# make a change inside the sparse cone\n+\trun_on_all ../edit-contents deep/a &&\n+\ttest_all_match git update-index deep/a &&\n+\ttest_all_match git write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\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 write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\t\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: write-tree' '\n+\tinit_repos &&\n+\n+\tensure_not_expanded write-tree &&\n+\n+\techo \"test1\" >>sparse-index/a &&\n+\tgit -C sparse-index update-index a &&\n+\tensure_not_expanded write-tree \n+'\n+\n test_done\n-- \n2.39.0\n\n"},{"id":"475702","messageId":"xmqqildran7n.fsf@gitster.g","threadId":"59521","inReplyTo":"20230419072148.4297-1-cheskaqiqi@gmail.com","subject":"Re: [PATCH v3] write-tree: integrate with sparse index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-19T15:47:08Z","receivedAt":"2023-04-19T15:48:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang <cheskaqiqi@gmail.com> writes:\n\n> Update 'git write-tree' to allow using the sparse-index in memory\n> without expanding to a full one.\n\nSorry, but after this exchange\n\n    https://lore.kernel.org/git/xmqqmt3bw9ir.fsf@gitster.g/\n\nI am confused what we want to do with this version.\n"},{"id":"475726","messageId":"CAMO4yUGzGzT4XC8t_LE=Z=ebERJq9Egq+wFj1K=1aUxHfPcnNA@mail.gmail.com","threadId":"59521","inReplyTo":"xmqqildran7n.fsf@gitster.g","subject":"Re: [PATCH v3] write-tree: integrate with sparse index","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-04-20T05:24:18Z","receivedAt":"2023-04-20T05:25:30Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi Junio,\n\nOn Wed, Apr 19, 2023 at 11:47 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Shuqi Liang <cheskaqiqi@gmail.com> writes:\n>\n> > Update 'git write-tree' to allow using the sparse-index in memory\n> > without expanding to a full one.\n>\n> Sorry, but after this exchange\n>\n>     https://lore.kernel.org/git/xmqqmt3bw9ir.fsf@gitster.g/\n>\n> I am confused what we want to do with this version.\n\nApologies for not noticing the patch was already merged to master.  I'll make\nthe necessary changes and submit a new patch soon.\n\nThanks\nShuqi\n"},{"id":"475738","messageId":"xmqqleim4kg9.fsf@gitster.g","threadId":"59521","inReplyTo":"CAMO4yUGzGzT4XC8t_LE=Z=ebERJq9Egq+wFj1K=1aUxHfPcnNA@mail.gmail.com","subject":"Re: [PATCH v3] write-tree: integrate with sparse index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-20T15:55:34Z","receivedAt":"2023-04-20T15:55:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang <cheskaqiqi@gmail.com> writes:\n\n>> Sorry, but after this exchange\n>>\n>>     https://lore.kernel.org/git/xmqqmt3bw9ir.fsf@gitster.g/\n>>\n>> I am confused what we want to do with this version.\n>\n> Apologies for not noticing the patch was already merged to master.  I'll make\n> the necessary changes and submit a new patch soon.\n\nNo need to apologize.  I should have been able to guess what happend\nmyself.\n\nThanks for offering to make your updates incremental.  Will look\nforward to seeing them.\n"},{"id":"475773","messageId":"20230421004108.32554-1-cheskaqiqi@gmail.com","threadId":"59521","inReplyTo":"20230419072148.4297-1-cheskaqiqi@gmail.com","subject":"[PATCH v4] write-tree: integrate with sparse index","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-04-21T00:41:08Z","receivedAt":"2023-04-21T00:41:27Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Update 'git write-tree' to allow using the sparse-index in memory\nwithout expanding to a full one.\n\nThe recursive algorithm for update_one() was already updated in 2de37c5\n(cache-tree: integrate with sparse directory entries, 2021-03-03) to\nhandle sparse directory entries in the index. Hence we can just set the\nrequires-full-index to false for \"write-tree\".\n\nThe `p2000` tests demonstrate a ~96% execution time reduction for 'git\nwrite-tree' using a sparse index:\n\nTest                                           before  after\n-----------------------------------------------------------------\n2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n\nSigned-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n---\n\n* Modified the code to ensure prepare_repo_settings() is called only \nwhen inside a repository.\n\n* Change 'write-tree on all' to just 'write-tree'.\n\n* Have a baseline 'test_all_match git write-tree' before making any \nchanges to the index.\n\n* Add 'git status --porcelain=v2'.\n\n* Ensuring that SKIP_WORKTREE files weren't materialized on disk by\nusing \"test_path_is_missing\".\n\n* Use 'test_all_match' on the 'git update-index'.\n\n\n builtin/write-tree.c                     |  9 ++++++---\n t/t1092-sparse-checkout-compatibility.sh | 20 +++++++++++++++-----\n 2 files changed, 21 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex 32e302a813..a9d5c20cde 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -38,12 +38,15 @@ int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n \t};\n \n \tgit_config(git_default_config, NULL);\n+\t\n+\tif (the_repository->gitdir) {\n+\t\tprepare_repo_settings(the_repository);\n+\t\tthe_repository->settings.command_requires_full_index = 0;\n+\t}\n+\n \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n \t\t\t     write_tree_usage, 0);\n \n-\tprepare_repo_settings(the_repository);\n-\tthe_repository->settings.command_requires_full_index = 0;\n-\n \tret = write_index_as_tree(&oid, &the_index, get_index_file(), flags,\n \t\t\t\t  tree_prefix);\n \tswitch (ret) {\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 9bbc0d646b..d3eb31326b 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2055,22 +2055,32 @@ test_expect_success 'grep sparse directory within submodules' '\n \ttest_cmp actual expect\n '\n \n-test_expect_success 'write-tree on all' '\n+test_expect_success 'write-tree' '\n \tinit_repos &&\n \n+\ttest_all_match git write-tree &&\n+\n \twrite_script edit-contents <<-\\EOF &&\n \techo text >>\"$1\"\n \tEOF\n \n+\t# make a change inside the sparse cone\n \trun_on_all ../edit-contents deep/a &&\n-\trun_on_all git update-index deep/a &&\n+\ttest_all_match git update-index deep/a &&\n \ttest_all_match git write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\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-\trun_on_all git update-index folder1/a &&\n-\ttest_all_match git write-tree\n+\ttest_all_match git update-index folder1/a &&\n+\ttest_all_match git write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\t\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: write-tree' '\n@@ -2080,7 +2090,7 @@ test_expect_success 'sparse-index is not expanded: write-tree' '\n \n \techo \"test1\" >>sparse-index/a &&\n \tgit -C sparse-index update-index a &&\n-\tensure_not_expanded write-tree\n+\tensure_not_expanded write-tree \n '\n \n test_done\n-- \n2.39.0\n\n"},{"id":"475821","messageId":"8b2a754c-6162-54d9-e9ba-fd994058066c@github.com","threadId":"59521","inReplyTo":"20230421004108.32554-1-cheskaqiqi@gmail.com","subject":"Re: [PATCH v4] write-tree: integrate with sparse index","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2023-04-21T21:42:34Z","receivedAt":"2023-04-21T21:42:40Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Shuqi Liang wrote:\n> Update 'git write-tree' to allow using the sparse-index in memory\n> without expanding to a full one.\n> \n> The recursive algorithm for update_one() was already updated in 2de37c5\n> (cache-tree: integrate with sparse directory entries, 2021-03-03) to\n> handle sparse directory entries in the index. Hence we can just set the\n> requires-full-index to false for \"write-tree\".\n> \n> The `p2000` tests demonstrate a ~96% execution time reduction for 'git\n> write-tree' using a sparse index:\n> \n> Test                                           before  after\n> -----------------------------------------------------------------\n> 2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n> 2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n> 2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n> 2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n\nPlease update your commit message to explain only the incremental updates on\ntop of 1a65b41b38a (write-tree: integrate with sparse index, 2023-04-03);\nthat patch's message (what you have here) does not accurately describe what\n_this_ patch is doing.\n\n> diff --git a/builtin/write-tree.c b/builtin/write-tree.c\n> index 32e302a813..a9d5c20cde 100644\n> --- a/builtin/write-tree.c\n> +++ b/builtin/write-tree.c\n> @@ -38,12 +38,15 @@ int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n>  \t};\n>  \n>  \tgit_config(git_default_config, NULL);\n> +\t\n> +\tif (the_repository->gitdir) {\n> +\t\tprepare_repo_settings(the_repository);\n> +\t\tthe_repository->settings.command_requires_full_index = 0;\n> +\t}\n> +\n>  \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n>  \t\t\t     write_tree_usage, 0);\n>  \n> -\tprepare_repo_settings(the_repository);\n> -\tthe_repository->settings.command_requires_full_index = 0;\n> -\n\nWhat is the functional benefit of this change? AFAICT, we don't need\n'command_requires_full_index' to be set before 'parse_options' in this case,\nso this won't have any effect on the behavior of 'write-tree'.\n\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index 9bbc0d646b..d3eb31326b 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -2055,22 +2055,32 @@ test_expect_success 'grep sparse directory within submodules' '\n>  \ttest_cmp actual expect\n>  '\n>  \n> -test_expect_success 'write-tree on all' '\n> +test_expect_success 'write-tree' '\n>  \tinit_repos &&\n>  \n> +\ttest_all_match git write-tree &&\n> +\n>  \twrite_script edit-contents <<-\\EOF &&\n>  \techo text >>\"$1\"\n>  \tEOF\n>  \n> +\t# make a change inside the sparse cone\n>  \trun_on_all ../edit-contents deep/a &&\n> -\trun_on_all git update-index deep/a &&\n> +\ttest_all_match git update-index deep/a &&\n>  \ttest_all_match git write-tree &&\n> +\ttest_all_match git status --porcelain=v2 &&\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> -\trun_on_all git update-index folder1/a &&\n> -\ttest_all_match git write-tree\n> +\ttest_all_match git update-index folder1/a &&\n> +\ttest_all_match git write-tree &&\n> +\ttest_all_match git status --porcelain=v2 &&\n> +\t\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\nTest updates look good!\n\n>  '\n>  \n>  test_expect_success 'sparse-index is not expanded: write-tree' '\n> @@ -2080,7 +2090,7 @@ test_expect_success 'sparse-index is not expanded: write-tree' '\n>  \n>  \techo \"test1\" >>sparse-index/a &&\n>  \tgit -C sparse-index update-index a &&\n> -\tensure_not_expanded write-tree\n> +\tensure_not_expanded write-tree \n\nnit: trailing whitespace should be removed\n\n>  '\n>  \n>  test_done\n\n"},{"id":"475894","messageId":"20230423071243.1863977-1-cheskaqiqi@gmail.com","threadId":"59521","inReplyTo":"20230421004108.32554-1-cheskaqiqi@gmail.com","subject":"[PATCH v5] write-tree: optimize sparse integration","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-04-23T07:12:43Z","receivedAt":"2023-04-23T07:13:06Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"'prepare_repo_settings()' needs to be run inside a repository. Ensure\nthat the code checks for the presence of a repository before calling\nthis function. 'write-tree on all' had an unclear meaning of 'on all'.\nChange the test name to simply 'write-tree'. Add a baseline\n'test_all_match git write-tree' before making any changes to the index,\nproviding a reference point for the 'write-tree' prior to any\nmodifications. Add a comparison of the output of\n'git status --porcelain=v2' to test the working tree after 'write-tree'\nexits. Ensure SKIP_WORKTREE files weren't materialized on disk by using\n\"test_path_is_missing\".\n\nSigned-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n---\n\n* Update commit message.\n\n* 'command_requires_full_index' to be set after 'parse_options'.\n\n* Remove trailing whitespace.\n\n\nRange-diff against v4:\n1:  07f9bbd3c4 ! 1:  df470c2d61 write-tree: integrate with sparse index\n    @@ Metadata\n     Author: Shuqi Liang <cheskaqiqi@gmail.com>\n     \n      ## Commit message ##\n    -    write-tree: integrate with sparse index\n    +    write-tree: optimize sparse integration\n     \n    -    Update 'git write-tree' to allow using the sparse-index in memory\n    -    without expanding to a full one.\n    -\n    -    The recursive algorithm for update_one() was already updated in 2de37c5\n    -    (cache-tree: integrate with sparse directory entries, 2021-03-03) to\n    -    handle sparse directory entries in the index. Hence we can just set the\n    -    requires-full-index to false for \"write-tree\".\n    -\n    -    The `p2000` tests demonstrate a ~96% execution time reduction for 'git\n    -    write-tree' using a sparse index:\n    -\n    -    Test                                           before  after\n    -    -----------------------------------------------------------------\n    -    2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n    -    2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n    -    2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n    -    2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n    +    'prepare_repo_settings()' needs to be run inside a repository. Ensure\n    +    that the code checks for the presence of a repository before calling\n    +    this function. 'write-tree on all' had an unclear meaning of 'on all'.\n    +    Change the test name to simply 'write-tree'. Add a baseline\n    +    'test_all_match git write-tree' before making any changes to the index,\n    +    providing a reference point for the 'write-tree' prior to any\n    +    modifications. Add a comparison of the output of\n    +    'git status --porcelain=v2' to test the working tree after 'write-tree'\n    +    exits. Ensure SKIP_WORKTREE files weren't materialized on disk by using\n    +    \"test_path_is_missing\".\n     \n         Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n     \n      ## builtin/write-tree.c ##\n     @@ builtin/write-tree.c: int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n    - \t};\n    + \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n    + \t\t\t     write_tree_usage, 0);\n      \n    - \tgit_config(git_default_config, NULL);\n    -+\t\n    +-\tprepare_repo_settings(the_repository);\n    +-\tthe_repository->settings.command_requires_full_index = 0;\n     +\tif (the_repository->gitdir) {\n     +\t\tprepare_repo_settings(the_repository);\n     +\t\tthe_repository->settings.command_requires_full_index = 0;\n     +\t}\n    -+\n    - \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n    - \t\t\t     write_tree_usage, 0);\n      \n    --\tprepare_repo_settings(the_repository);\n    --\tthe_repository->settings.command_requires_full_index = 0;\n    --\n      \tret = write_index_as_tree(&oid, &the_index, get_index_file(), flags,\n      \t\t\t\t  tree_prefix);\n    - \tswitch (ret) {\n     \n      ## t/t1092-sparse-checkout-compatibility.sh ##\n     @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'grep sparse directory within submodules' '\n    @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'grep sparse direc\n      '\n      \n      test_expect_success 'sparse-index is not expanded: write-tree' '\n    -@@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is not expanded: write-tree' '\n    - \n    - \techo \"test1\" >>sparse-index/a &&\n    - \tgit -C sparse-index update-index a &&\n    --\tensure_not_expanded write-tree\n    -+\tensure_not_expanded write-tree \n    - '\n    - \n    - test_done\n\n builtin/write-tree.c                     |  6 ++++--\n t/t1092-sparse-checkout-compatibility.sh | 18 ++++++++++++++----\n 2 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex 32e302a813..52caa096a8 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -41,8 +41,10 @@ int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n \t\t\t     write_tree_usage, 0);\n \n-\tprepare_repo_settings(the_repository);\n-\tthe_repository->settings.command_requires_full_index = 0;\n+\tif (the_repository->gitdir) {\n+\t\tprepare_repo_settings(the_repository);\n+\t\tthe_repository->settings.command_requires_full_index = 0;\n+\t}\n \n \tret = write_index_as_tree(&oid, &the_index, get_index_file(), flags,\n \t\t\t\t  tree_prefix);\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 0c784813f1..2a467e4b31 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2080,22 +2080,32 @@ test_expect_success 'grep sparse directory within submodules' '\n \ttest_cmp actual expect\n '\n \n-test_expect_success 'write-tree on all' '\n+test_expect_success 'write-tree' '\n \tinit_repos &&\n \n+\ttest_all_match git write-tree &&\n+\n \twrite_script edit-contents <<-\\EOF &&\n \techo text >>\"$1\"\n \tEOF\n \n+\t# make a change inside the sparse cone\n \trun_on_all ../edit-contents deep/a &&\n-\trun_on_all git update-index deep/a &&\n+\ttest_all_match git update-index deep/a &&\n \ttest_all_match git write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\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-\trun_on_all git update-index folder1/a &&\n-\ttest_all_match git write-tree\n+\ttest_all_match git update-index folder1/a &&\n+\ttest_all_match git write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\t\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: write-tree' '\n-- \n2.39.0\n\n"},{"id":"475921","messageId":"xmqq7cu1ia7m.fsf@gitster.g","threadId":"59521","inReplyTo":"8b2a754c-6162-54d9-e9ba-fd994058066c@github.com","subject":"Re: [PATCH v4] write-tree: integrate with sparse index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-24T15:14:21Z","receivedAt":"2023-04-24T15:14:25Z","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>> Update 'git write-tree' to allow using the sparse-index in memory\n>> without expanding to a full one.\n>> \n>> The recursive algorithm for update_one() was already updated in 2de37c5\n>> (cache-tree: integrate with sparse directory entries, 2021-03-03) to\n>> handle sparse directory entries in the index. Hence we can just set the\n>> requires-full-index to false for \"write-tree\".\n>> \n>> The `p2000` tests demonstrate a ~96% execution time reduction for 'git\n>> write-tree' using a sparse index:\n>> \n>> Test                                           before  after\n>> -----------------------------------------------------------------\n>> 2000.78: git write-tree (full-v3)              0.34    0.33 -2.9%\n>> 2000.79: git write-tree (full-v4)              0.32    0.30 -6.3%\n>> 2000.80: git write-tree (sparse-v3)            0.47    0.02 -95.8%\n>> 2000.81: git write-tree (sparse-v4)            0.45    0.02 -95.6%\n>\n> Please update your commit message to explain only the incremental updates on\n> top of 1a65b41b38a (write-tree: integrate with sparse index, 2023-04-03);\n> that patch's message (what you have here) does not accurately describe what\n> _this_ patch is doing.\n\nGood point.\n\nIn addition, as this is the first iteration of a follow-up topic,\n\"v4\" on the subject line is a bit misleading.  Let's treat it as a\nnew and separate topic that build on top of the previous achievement.\n\nThanks for working on this topic and mentoring a new contributor.\n"},{"id":"475927","messageId":"xmqqleihgtho.fsf@gitster.g","threadId":"59521","inReplyTo":"20230423071243.1863977-1-cheskaqiqi@gmail.com","subject":"Re: [PATCH v5] write-tree: optimize sparse integration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-24T16:00:51Z","receivedAt":"2023-04-24T16:00:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang <cheskaqiqi@gmail.com> writes:\n\n> 'prepare_repo_settings()' needs to be run inside a repository. Ensure\n> that the code checks for the presence of a repository before calling\n> this function.\n\nCan you explain why this change is needed?\n\nThat is, if the caller made sure if this codepath is entered only\nwhen inside a repository, such a \"we need to refrain from doing\nthis\" becomes unnecessary.  Describe under what condition the\ncontrol passes this section with !the_repository->gitdir, e.g. \"When\nthe command is run in such and such way outside a repository, the\ncontrol reaches this position but prepare_repo_settings() cannot be\nblindly called\".\n\nI suspect that it is a bug if the control reaches this point without\nhaving a repository, as the call to write_index_as_tree() would be\nalready failing if we were not in a repository, but there is no such\na bug, and you did not introduce one with your previous changes to\nthis codepath that you need to fix here.  You can observe a few\nthings:\n\n - \"write-tree\" in the git.c::commands[] table has RUN_SETUP.\n\n - git.c::run_builtin() is repsonsible for calling cmd_write_tree();\n   what happens before it calls the function?  For a command with\n   RUN_SETUP set, unless the command line argument is \"-h\" (that is,\n   \"git write-tree -h\" is run), setup_git_directory() is called.\n\n - setup_git_directory() dies unless run in a repository.\n\n - git.c::run_builtin() calls setup_git_directory_gently() when the\n   command line argument is \"-h\" and it does not die even run\n   outside a repository.  However, before the code you touched,\n   there is a call to parse_options().\n\n - parse_options() called for the command line argument \"-h\" shows a\n   short help and then exits.\n\nSo...?\n\nAlso when starting to talk about totally different things (like, you\nwere discussing the change to write_tree.c to avoid segfaulting when\nrun outside a repository, but now you are going to talk about the\ntitle of one test piece), please make sure it is clear for readers.\nA blank line here may be appropriate.\n\n> 'write-tree on all' had an unclear meaning of 'on all'.\n> Change the test name to simply 'write-tree'. Add a baseline\n> 'test_all_match git write-tree' before making any changes to the index,\n> providing a reference point for the 'write-tree' prior to any\n> modifications. Add a comparison of the output of\n> 'git status --porcelain=v2' to test the working tree after 'write-tree'\n> exits. Ensure SKIP_WORKTREE files weren't materialized on disk by using\n> \"test_path_is_missing\".\n\nAll of the above may be easier to read in a bulletted list form,\ne.g.\n\n * 'on all' in the title of the test 'write-tree on all' was\n   unclear; remove it.\n\n * test the baseline test_all_match git write-tree\" before doing\n   anything else.\n\n ...\n\n\n\n> Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n> ---\n>\n> * Update commit message.\n\nOK.\n\n> * 'command_requires_full_index' to be set after 'parse_options'.\n\nThis does not match what we see in this patch.\n\n> * Remove trailing whitespace.\n\nOK.  But there is a new line with a trailing whitespace before the\nline that says # check that SKIP_WORKTREE files are not materialized\"\nin the test.\n\nThanks.\n"},{"id":"476806","messageId":"20230508200508.462423-1-cheskaqiqi@gmail.com","threadId":"59521","inReplyTo":"20230423071243.1863977-1-cheskaqiqi@gmail.com","subject":"[PATCH v6] write-tree: optimize sparse integration","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-05-08T20:05:08Z","receivedAt":"2023-05-08T20:05:25Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"* Remove 'on all' from the test title 'write-tree on all', making it\n'write-tree'.\n\n* Add a baseline 'test_all_match git write-tree' before making any\nchanges to the index, providing a reference point for the 'write-tree'\nprior to any modifications.\n\n* Add a comparison of the output of 'git status --porcelain=v2' to test\nthe working tree after 'write-tree' exits.\n\n* Ensure SKIP_WORKTREE files weren't materialized on disk by using\n\"test_path_is_missing\".\n\nSigned-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n---\n\nchange sine V5:\n\n* We not need to check for the presence of a repository before calling\n'prepare_repo_settings()', as the control flow should not reach this\npoint without a repository. This is because 'setup_git_directory()' is\ncalled for commands with RUN_SETUP set, except when the command line\nargument is \"-h\", in which case 'parse_options()' takes over and exits\nthe program.\n\n* Change the commit message to make it easier to read.\n\n* Remove whitespace before the line that says # check that SKIP_WORKTREE\nfiles are not materialized\".\n\n\n\nRange-diff against v5:\n1:  df470c2d61 ! 1:  0510b08c96 write-tree: optimize sparse integration\n    @@ Metadata\n      ## Commit message ##\n         write-tree: optimize sparse integration\n     \n    -    'prepare_repo_settings()' needs to be run inside a repository. Ensure\n    -    that the code checks for the presence of a repository before calling\n    -    this function. 'write-tree on all' had an unclear meaning of 'on all'.\n    -    Change the test name to simply 'write-tree'. Add a baseline\n    -    'test_all_match git write-tree' before making any changes to the index,\n    -    providing a reference point for the 'write-tree' prior to any\n    -    modifications. Add a comparison of the output of\n    -    'git status --porcelain=v2' to test the working tree after 'write-tree'\n    -    exits. Ensure SKIP_WORKTREE files weren't materialized on disk by using\n    +    * Remove 'on all' from the test title 'write-tree on all', making it\n    +    'write-tree'.\n    +\n    +    * Add a baseline 'test_all_match git write-tree' before making any\n    +    changes to the index, providing a reference point for the 'write-tree'\n    +    prior to any modifications.\n    +\n    +    * Add a comparison of the output of 'git status --porcelain=v2' to test\n    +    the working tree after 'write-tree' exits.\n    +\n    +    * Ensure SKIP_WORKTREE files weren't materialized on disk by using\n         \"test_path_is_missing\".\n     \n         Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n     \n    - ## builtin/write-tree.c ##\n    -@@ builtin/write-tree.c: int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n    - \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n    - \t\t\t     write_tree_usage, 0);\n    - \n    --\tprepare_repo_settings(the_repository);\n    --\tthe_repository->settings.command_requires_full_index = 0;\n    -+\tif (the_repository->gitdir) {\n    -+\t\tprepare_repo_settings(the_repository);\n    -+\t\tthe_repository->settings.command_requires_full_index = 0;\n    -+\t}\n    - \n    - \tret = write_index_as_tree(&oid, &the_index, get_index_file(), flags,\n    - \t\t\t\t  tree_prefix);\n    -\n      ## t/t1092-sparse-checkout-compatibility.sh ##\n     @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'grep sparse directory within submodules' '\n      \ttest_cmp actual expect\n    @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'grep sparse direc\n     +\ttest_all_match git update-index folder1/a &&\n     +\ttest_all_match git write-tree &&\n     +\ttest_all_match git status --porcelain=v2 &&\n    -+\t\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 t/t1092-sparse-checkout-compatibility.sh | 18 ++++++++++++++----\n 1 file changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 0c784813f1..3aa6356a85 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2080,22 +2080,32 @@ test_expect_success 'grep sparse directory within submodules' '\n \ttest_cmp actual expect\n '\n \n-test_expect_success 'write-tree on all' '\n+test_expect_success 'write-tree' '\n \tinit_repos &&\n \n+\ttest_all_match git write-tree &&\n+\n \twrite_script edit-contents <<-\\EOF &&\n \techo text >>\"$1\"\n \tEOF\n \n+\t# make a change inside the sparse cone\n \trun_on_all ../edit-contents deep/a &&\n-\trun_on_all git update-index deep/a &&\n+\ttest_all_match git update-index deep/a &&\n \ttest_all_match git write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\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-\trun_on_all git update-index folder1/a &&\n-\ttest_all_match git write-tree\n+\ttest_all_match git update-index folder1/a &&\n+\ttest_all_match git write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\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: write-tree' '\n-- \n2.39.0\n\n"},{"id":"476807","messageId":"20230508202140.464363-1-cheskaqiqi@gmail.com","threadId":"59521","inReplyTo":"20230508200508.462423-1-cheskaqiqi@gmail.com","subject":"[PATCH v6] write-tree: optimize sparse integration","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-05-08T20:21:40Z","receivedAt":"2023-05-08T20:21:57Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"* 'on all' in the title of the test 'write-tree on all' was unclear;\nremove it.\n\n* Add a baseline 'test_all_match git write-tree' before making any\nchanges to the index, providing a reference point for the 'write-tree'\nprior to any modifications.\n\n* Add a comparison of the output of 'git status --porcelain=v2' to test\nthe working tree after 'write-tree' exits.\n\n* Ensure SKIP_WORKTREE files weren't materialized on disk by using\n\"test_path_is_missing\".\n\nSigned-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n---\n\nMy apologies, please ignore the previous v6 iteration.\n\nchange sine V5:\n\n* We not need to check for the presence of a repository before calling\n'prepare_repo_settings()', as the control flow should not reach this\npoint without a repository. This is because 'setup_git_directory()' is\ncalled for commands with RUN_SETUP set, except when the command line\nargument is \"-h\", in which case 'parse_options()' takes over and exits\nthe program.\n\n* Change the commit message to make it easier to read.\n\n* Remove whitespace before the line that says # check that SKIP_WORKTREE\nfiles are not materialized\".\n\nRange-diff against v5:\n1:  df470c2d61 ! 1:  e6c21ec6b8 write-tree: optimize sparse integration\n    @@ Metadata\n      ## Commit message ##\n         write-tree: optimize sparse integration\n     \n    -    'prepare_repo_settings()' needs to be run inside a repository. Ensure\n    -    that the code checks for the presence of a repository before calling\n    -    this function. 'write-tree on all' had an unclear meaning of 'on all'.\n    -    Change the test name to simply 'write-tree'. Add a baseline\n    -    'test_all_match git write-tree' before making any changes to the index,\n    -    providing a reference point for the 'write-tree' prior to any\n    -    modifications. Add a comparison of the output of\n    -    'git status --porcelain=v2' to test the working tree after 'write-tree'\n    -    exits. Ensure SKIP_WORKTREE files weren't materialized on disk by using\n    +    * 'on all' in the title of the test 'write-tree on all' was unclear;\n    +    remove it.\n    +\n    +    * Add a baseline 'test_all_match git write-tree' before making any\n    +    changes to the index, providing a reference point for the 'write-tree'\n    +    prior to any modifications.\n    +\n    +    * Add a comparison of the output of 'git status --porcelain=v2' to test\n    +    the working tree after 'write-tree' exits.\n    +\n    +    * Ensure SKIP_WORKTREE files weren't materialized on disk by using\n         \"test_path_is_missing\".\n     \n         Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n     \n    - ## builtin/write-tree.c ##\n    -@@ builtin/write-tree.c: int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)\n    - \targc = parse_options(argc, argv, cmd_prefix, write_tree_options,\n    - \t\t\t     write_tree_usage, 0);\n    - \n    --\tprepare_repo_settings(the_repository);\n    --\tthe_repository->settings.command_requires_full_index = 0;\n    -+\tif (the_repository->gitdir) {\n    -+\t\tprepare_repo_settings(the_repository);\n    -+\t\tthe_repository->settings.command_requires_full_index = 0;\n    -+\t}\n    - \n    - \tret = write_index_as_tree(&oid, &the_index, get_index_file(), flags,\n    - \t\t\t\t  tree_prefix);\n    -\n      ## t/t1092-sparse-checkout-compatibility.sh ##\n     @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'grep sparse directory within submodules' '\n      \ttest_cmp actual expect\n    @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'grep sparse direc\n     +\ttest_all_match git update-index folder1/a &&\n     +\ttest_all_match git write-tree &&\n     +\ttest_all_match git status --porcelain=v2 &&\n    -+\t\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\n t/t1092-sparse-checkout-compatibility.sh | 18 ++++++++++++++----\n 1 file changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 0c784813f1..3aa6356a85 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2080,22 +2080,32 @@ test_expect_success 'grep sparse directory within submodules' '\n \ttest_cmp actual expect\n '\n \n-test_expect_success 'write-tree on all' '\n+test_expect_success 'write-tree' '\n \tinit_repos &&\n \n+\ttest_all_match git write-tree &&\n+\n \twrite_script edit-contents <<-\\EOF &&\n \techo text >>\"$1\"\n \tEOF\n \n+\t# make a change inside the sparse cone\n \trun_on_all ../edit-contents deep/a &&\n-\trun_on_all git update-index deep/a &&\n+\ttest_all_match git update-index deep/a &&\n \ttest_all_match git write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\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-\trun_on_all git update-index folder1/a &&\n-\ttest_all_match git write-tree\n+\ttest_all_match git update-index folder1/a &&\n+\ttest_all_match git write-tree &&\n+\ttest_all_match git status --porcelain=v2 &&\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: write-tree' '\n-- \n2.39.0\n\n"},{"id":"476813","messageId":"xmqqednqmswx.fsf@gitster.g","threadId":"59521","inReplyTo":"20230508202140.464363-1-cheskaqiqi@gmail.com","subject":"Re: [PATCH v6] write-tree: optimize sparse integration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-08T21:09:50Z","receivedAt":"2023-05-08T21:09:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang <cheskaqiqi@gmail.com> writes:\n\n> * 'on all' in the title of the test 'write-tree on all' was unclear;\n> remove it.\n>\n> * Add a baseline 'test_all_match git write-tree' before making any\n> changes to the index, providing a reference point for the 'write-tree'\n> prior to any modifications.\n>\n> * Add a comparison of the output of 'git status --porcelain=v2' to test\n> the working tree after 'write-tree' exits.\n>\n> * Ensure SKIP_WORKTREE files weren't materialized on disk by using\n> \"test_path_is_missing\".\n>\n> Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n> ---\n>\n\nAs we have lost the change to the code, the title has become stale.\nHow about I retitle it like so after queuing the patch?\n\n    Subject: t1092: update write-tree test\n\nThe changes to the test seem to match what Victoria already gave a\nthums-up in her review of v4; I see nothing surprising or unexpected\nthere.\n\nThanks.  Will queue.\n"},{"id":"476814","messageId":"CAMO4yUEmJM1-VYRePn6tjYHXmhEhj5-wkZ4VrX9EaS9=kSX-3w@mail.gmail.com","threadId":"59521","inReplyTo":"xmqqednqmswx.fsf@gitster.g","subject":"Re: [PATCH v6] write-tree: optimize sparse integration","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-05-08T21:27:07Z","receivedAt":"2023-05-08T21:27:41Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi Junio,\n\nOn Mon, May 8, 2023 at 5:09 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Shuqi Liang <cheskaqiqi@gmail.com> writes:\n>\n> > * 'on all' in the title of the test 'write-tree on all' was unclear;\n> > remove it.\n> >\n> > * Add a baseline 'test_all_match git write-tree' before making any\n> > changes to the index, providing a reference point for the 'write-tree'\n> > prior to any modifications.\n> >\n> > * Add a comparison of the output of 'git status --porcelain=v2' to test\n> > the working tree after 'write-tree' exits.\n> >\n> > * Ensure SKIP_WORKTREE files weren't materialized on disk by using\n> > \"test_path_is_missing\".\n> >\n> > Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>\n> > ---\n> >\n>\n> As we have lost the change to the code, the title has become stale.\n> How about I retitle it like so after queuing the patch?\n>\n>     Subject: t1092: update write-tree test\n\nI think it's a good idea to retitle the patch， as it better reflects the\ncurrent changes in the test.\n\n> The changes to the test seem to match what Victoria already gave a\n> thums-up in her review of v4; I see nothing surprising or unexpected\n> there.\n>\n> Thanks.  Will queue.\n\nI really appreciate your and Victoria's continuous support and\nguidance throughout\nthe review process :)\n\nThanks!\nShuqi\n"}]}