{"thread":{"id":"57267","subject":"[PATCH 0/3] [Non-critical]: sparse index vs split index fixes","startedAt":"2022-01-19T17:29:44Z","lastAt":"2022-01-22T09:32:26Z","messageCount":10,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Elijah Newren","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"446504","messageId":"pull.1119.git.1642613379.gitgitgadget@gmail.com","threadId":"57267","inReplyTo":null,"subject":"[PATCH 0/3] [Non-critical]: sparse index vs split index fixes","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-19T17:29:36Z","receivedAt":"2022-01-19T17:29:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"We noticed split/sparse index-related issues while rebasing Microsoft's fork\nof Git. These fixes are necessary for that fork's test suite to pass, but\nthey might not be super critical to get into upstream v2.35.0 (especially\nthis close to -rc2). However, the team felt that the decision should be left\nto the Git maintainer whether to take these patches into v2.35.0 or not.\n\nJohannes Schindelin (3):\n sparse-index: sparse index is disallowed when split index is active\n t1091: disable split index\n split-index: it really is incompatible with the sparse index\n\n read-cache.c | 3 ++\n sparse-index.c | 2 +-\n split-index.c | 3 ++\n t/t1091-sparse-checkout-builtin.sh | 54 ++++++++++++++----------------\n 4 files changed, 33 insertions(+), 29 deletions(-)\n\n\nbase-commit: af4e5f569bc89f356eb34a9373d7f82aca6faa8a\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1119%2Fdscho%2Fsparse-index-vs-split-index-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1119/dscho/sparse-index-vs-split-index-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1119\n-- \ngitgitgadget\n"},{"id":"446505","messageId":"d5c1440d9a9c943bf195a9d158c4badbd9a022a3.1642613380.git.gitgitgadget@gmail.com","threadId":"57267","inReplyTo":"pull.1119.git.1642613379.gitgitgadget@gmail.com","subject":"[PATCH 1/3] sparse-index: sparse index is disallowed when split index is active","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-19T17:29:37Z","receivedAt":"2022-01-19T17:29:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 6e773527b6b (sparse-index: convert from full to sparse, 2021-03-30),\nwe introduced initial support for a sparse index, and were careful to\navoid converting to a sparse index in the presence of a split index.\n\nHowever, when we _just_ read a freshly-initialized index, it might not\ncontain a split index even if _writing_ it will add one by virtue of\nbeing asked for via the `GIT_TEST_SPLIT_INDEX` variable.\n\nWe did not notice any problems with checking _only_ for `split_index`\n(and not `GIT_TEST_SPLIT_INDEX`) right until both\n`vd/sparse-sparsity-fix-on-read` _and_ `vd/sparse-reset` were merged.\n\nThose two topics' interplay triggers a bug in conjunction with running\nt1091.15 when `GIT_TEST_SPLIT_INDEX=true` in the following way:\n`vd/sparse-sparsity-fix-on-read` ensures that the index is made sparse\nright after reading, and `vd/sparse-reset` ensures that the index is\nmade non-sparse again unless running in the `--soft` mode. Since the\nsplit index feature is incompatible with the sparse index feature, we\nsee a symptom like this:\n\n\tfatal: position for replacement 4 exceeds base index size 4\n\nLet's fix this by avoiding the conversion to a sparse index when\n`GIT_TEST_SPLIT_INDEX=true`.\n\nSigned-off-by: Johannes Schindelin \n---\n sparse-index.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sparse-index.c b/sparse-index.c\nindex a1d505d50e9..08f54747bb4 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -136,7 +136,7 @@ static int is_sparse_index_allowed(struct index_state *istate, int flags)\n \t\t/*\n \t\t * The sparse index is not (yet) integrated with a split index.\n \t\t */\n-\t\tif (istate->split_index)\n+\t\tif (istate->split_index || git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0))\n \t\t\treturn 0;\n \t\t/*\n \t\t * The GIT_TEST_SPARSE_INDEX environment variable triggers the\n-- \ngitgitgadget\n\n"},{"id":"446506","messageId":"a8ec58b93244ef8f6303c06d83fff6bab5826515.1642613380.git.gitgitgadget@gmail.com","threadId":"57267","inReplyTo":"pull.1119.git.1642613379.gitgitgadget@gmail.com","subject":"[PATCH 2/3] t1091: disable split index","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-19T17:29:38Z","receivedAt":"2022-01-19T17:29:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 61feddcdf28 (tests: disable GIT_TEST_SPLIT_INDEX for sparse index\ntests, 2021-08-26), it was already called out that the split index\nfeature is incompatible with the sparse index feature, and its commit\nmessage wondered aloud whether more checks would be required to ensure\nthat the split index and sparse index features aren't enabled at the\nsame time.\n\nWe are about to introduce such additional checks, and indeed, t1091\nwould utterly fail with them. Therefore, let's preemptively disable the\nsplit index for the entirety of t1091.\n\nThis partially reverts above-mentioned patch because it covered only one\ntest case whereas we want to cover the entire test script.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t1091-sparse-checkout-builtin.sh | 54 ++++++++++++++----------------\n 1 file changed, 26 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex 42776984fe7..b229a8274d8 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -5,6 +5,9 @@ test_description='sparse checkout builtin tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+GIT_TEST_SPLIT_INDEX=false\n+export GIT_TEST_SPLIT_INDEX\n+\n . ./test-lib.sh\n \n list_files() {\n@@ -228,36 +231,31 @@ test_expect_success 'sparse-checkout disable' '\n '\n \n test_expect_success 'sparse-index enabled and disabled' '\n-\t(\n-\t\tsane_unset GIT_TEST_SPLIT_INDEX &&\n-\t\tgit -C repo update-index --no-split-index &&\n-\n-\t\tgit -C repo sparse-checkout init --cone --sparse-index &&\n-\t\ttest_cmp_config -C repo true index.sparse &&\n-\t\tgit -C repo ls-files --sparse >sparse &&\n-\t\tgit -C repo sparse-checkout disable &&\n-\t\tgit -C repo ls-files --sparse >full &&\n-\n-\t\tcat >expect <<-\\EOF &&\n-\t\t@@ -1,4 +1,7 @@\n-\t\t a\n-\t\t-deep/\n-\t\t-folder1/\n-\t\t-folder2/\n-\t\t+deep/a\n-\t\t+deep/deeper1/a\n-\t\t+deep/deeper1/deepest/a\n-\t\t+deep/deeper2/a\n-\t\t+folder1/a\n-\t\t+folder2/a\n-\t\tEOF\n+\tgit -C repo sparse-checkout init --cone --sparse-index &&\n+\ttest_cmp_config -C repo true index.sparse &&\n+\tgit -C repo ls-files --sparse >sparse &&\n+\tgit -C repo sparse-checkout disable &&\n+\tgit -C repo ls-files --sparse >full &&\n \n-\t\tdiff -u sparse full | tail -n +3 >actual &&\n-\t\ttest_cmp expect actual &&\n+\tcat >expect <<-\\EOF &&\n+\t@@ -1,4 +1,7 @@\n+\t a\n+\t-deep/\n+\t-folder1/\n+\t-folder2/\n+\t+deep/a\n+\t+deep/deeper1/a\n+\t+deep/deeper1/deepest/a\n+\t+deep/deeper2/a\n+\t+folder1/a\n+\t+folder2/a\n+\tEOF\n+\n+\tdiff -u sparse full | tail -n +3 >actual &&\n+\ttest_cmp expect actual &&\n \n-\t\tgit -C repo config --list >config &&\n-\t\t! grep index.sparse config\n-\t)\n+\tgit -C repo config --list >config &&\n+\t! grep index.sparse config\n '\n \n test_expect_success 'cone mode: init and set' '\n-- \ngitgitgadget\n\n"},{"id":"446507","messageId":"1bb57ccd61452124119bb663f5e35e9676748c82.1642613380.git.gitgitgadget@gmail.com","threadId":"57267","inReplyTo":"pull.1119.git.1642613379.gitgitgadget@gmail.com","subject":"[PATCH 3/3] split-index: it really is incompatible with the sparse index","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-19T17:29:39Z","receivedAt":"2022-01-19T17:29:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\n... at least for now. So let's error out if we are even trying to\ninitialize the split index when the index is sparse, or when trying to\nwrite the split index extension for a sparse index.\n\nSigned-off-by: Johannes Schindelin \n---\n read-cache.c | 3 +++\n split-index.c | 3 +++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex cbe73f14e5e..a932e01fc7a 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -3009,6 +3009,9 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t !is_null_oid(&istate->split_index->base_oid)) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n+\t\tif (istate->sparse_index)\n+\t\t\tdie(_(\"cannot write split index for a sparse index\"));\n+\n \t\terr = write_link_extension(&sb, istate) < 0 ||\n \t\t\twrite_index_ext_header(f, eoie_c, CACHE_EXT_LINK,\n \t\t\t\t\t sb.len) < 0;\ndiff --git a/split-index.c b/split-index.c\nindex 8e52e891c3b..9d0ccc30d00 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -5,6 +5,9 @@\n struct split_index *init_split_index(struct index_state *istate)\n {\n \tif (!istate->split_index) {\n+\t\tif (istate->sparse_index)\n+\t\t\tdie(_(\"cannot use split index with a sparse index\"));\n+\n \t\tCALLOC_ARRAY(istate->split_index, 1);\n \t\tistate->split_index->refcount = 1;\n \t}\n-- \ngitgitgadget\n"},{"id":"446661","messageId":"xmqqh79wejzo.fsf@gitster.g","threadId":"57267","inReplyTo":"1bb57ccd61452124119bb663f5e35e9676748c82.1642613380.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] split-index: it really is incompatible with the sparse index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-21T23:39:55Z","receivedAt":"2022-01-21T23:39:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> ... at least for now. So let's error out if we are even trying to\n> initialize the split index when the index is sparse, or when trying to\n> write the split index extension for a sparse index.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> read-cache.c | 3 +++\n> split-index.c | 3 +++\n> 2 files changed, 6 insertions(+)\n\nGood. Will queue.\n\n\n> diff --git a/read-cache.c b/read-cache.c\n> index cbe73f14e5e..a932e01fc7a 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -3009,6 +3009,9 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> \t !is_null_oid(&istate->split_index->base_oid)) {\n> \t\tstruct strbuf sb = STRBUF_INIT;\n> \n> +\t\tif (istate->sparse_index)\n> +\t\t\tdie(_(\"cannot write split index for a sparse index\"));\n> +\n> \t\terr = write_link_extension(&sb, istate) < 0 ||\n> \t\t\twrite_index_ext_header(f, eoie_c, CACHE_EXT_LINK,\n> \t\t\t\t\t sb.len) < 0;\n> diff --git a/split-index.c b/split-index.c\n> index 8e52e891c3b..9d0ccc30d00 100644\n> --- a/split-index.c\n> +++ b/split-index.c\n> @@ -5,6 +5,9 @@\n> struct split_index *init_split_index(struct index_state *istate)\n> {\n> \tif (!istate->split_index) {\n> +\t\tif (istate->sparse_index)\n> +\t\t\tdie(_(\"cannot use split index with a sparse index\"));\n> +\n> \t\tCALLOC_ARRAY(istate->split_index, 1);\n> \t\tistate->split_index->refcount = 1;\n> \t}\n"},{"id":"446671","messageId":"xmqqtudwbl64.fsf@gitster.g","threadId":"57267","inReplyTo":"d5c1440d9a9c943bf195a9d158c4badbd9a022a3.1642613380.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-22T01:42:43Z","receivedAt":"2022-01-22T01:42:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> In 6e773527b6b (sparse-index: convert from full to sparse, 2021-03-30),\n> we introduced initial support for a sparse index, and were careful to\n> avoid converting to a sparse index in the presence of a split index.\n>\n> However, when we _just_ read a freshly-initialized index, it might not\n> contain a split index even if _writing_ it will add one by virtue of\n> being asked for via the `GIT_TEST_SPLIT_INDEX` variable.\n>\n> We did not notice any problems with checking _only_ for `split_index`\n> (and not `GIT_TEST_SPLIT_INDEX`) right until both\n> `vd/sparse-sparsity-fix-on-read` _and_ `vd/sparse-reset` were merged.\n>\n> Those two topics' interplay triggers a bug in conjunction with running\n> t1091.15 when `GIT_TEST_SPLIT_INDEX=true` in the following way:\n> `vd/sparse-sparsity-fix-on-read` ensures that the index is made sparse\n> right after reading, and `vd/sparse-reset` ensures that the index is\n> made non-sparse again unless running in the `--soft` mode. Since the\n> split index feature is incompatible with the sparse index feature, we\n> see a symptom like this:\n>\n> \tfatal: position for replacement 4 exceeds base index size 4\n>\n> Let's fix this by avoiding the conversion to a sparse index when\n> `GIT_TEST_SPLIT_INDEX=true`.\n\nDoes [2/3] allow you to sidestep that issue entirely by skipping\n1091 altogether?\n\nThere are 4 hits of \"if (istate->split_index\" in the codebase, and\nthis patch makes me wonder why it is suffice to patch only one of\nthem.\n\nI also wondered why we test both split_index and environment\nseparately, instead of splitting the index very early when the\nenvironment variable is set, so that the rest of the runtime does\nnot have to worry about the environment, but is the reason why such\nan approach was not taken was because GIT_TEST_SPLIT_INDEX can later\nallow the index to be splitted, even if istate->split_index is still\nNULL right now when this function is called?\n\nIf that is the reason, it leads to another question. Even if we\nignore GIT_TEST_SPLIT_INDEX and concentrate only on real workload,\nif the in-core index can be NULL when this function is called and\nthen later can become split, there still must be somebody who\nnotices that sparse index is unallowed when (or after) such a split\nhappens, no? If there is no such code, then this patch would not be\nthe whole fix and there needs more change to do so, no?\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> sparse-index.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/sparse-index.c b/sparse-index.c\n> index a1d505d50e9..08f54747bb4 100644\n> --- a/sparse-index.c\n> +++ b/sparse-index.c\n> @@ -136,7 +136,7 @@ static int is_sparse_index_allowed(struct index_state *istate, int flags)\n> \t\t/*\n> \t\t * The sparse index is not (yet) integrated with a split index.\n> \t\t */\n> -\t\tif (istate->split_index)\n> +\t\tif (istate->split_index || git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0))\n> \t\t\treturn 0;\n> \t\t/*\n> \t\t * The GIT_TEST_SPARSE_INDEX environment variable triggers the\n"},{"id":"446678","messageId":"CABPp-BFXMMH2_qqWPGaWX76mqEs2Z1GFNzbyeu2rLgaFPt7JyQ@mail.gmail.com","threadId":"57267","inReplyTo":"pull.1119.git.1642613379.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] [Non-critical]: sparse index vs split index fixes","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-22T05:44:16Z","receivedAt":"2022-01-22T05:44:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 21, 2022 at 11:53 AM Johannes Schindelin via GitGitGadget\n wrote:\n>\n> We noticed split/sparse index-related issues while rebasing Microsoft's fork\n> of Git. These fixes are necessary for that fork's test suite to pass, but\n> they might not be super critical to get into upstream v2.35.0 (especially\n> this close to -rc2). However, the team felt that the decision should be left\n> to the Git maintainer whether to take these patches into v2.35.0 or not.\n\nThanks for digging into these and putting in some guardrails to\nprevent similar issues. I hit similar things with some of my changes\nand had to fight t1091 to get it to pass in CI under\nGIT_TEST_SPLIT_INDEX=1. I don't recall seeing the specific error you\nmention in patch 1, but maybe I just overlooked it. I ended up\nfinding little workarounds such as disabling sparse-checkouts at the\nend of various tests before new ones I was adding, and being extra\ncareful to not leave the sparse-index selected. I probably should\nhave dug further, but didn't. Thanks for digging in.\n\nI've read over the patch series; it looks good to me:\n\nReviewed-by: Elijah Newren \n\n\n>\n> Johannes Schindelin (3):\n> sparse-index: sparse index is disallowed when split index is active\n> t1091: disable split index\n> split-index: it really is incompatible with the sparse index\n>\n> read-cache.c | 3 ++\n> sparse-index.c | 2 +-\n> split-index.c | 3 ++\n> t/t1091-sparse-checkout-builtin.sh | 54 ++++++++++++++----------------\n> 4 files changed, 33 insertions(+), 29 deletions(-)\n>\n>\n> base-commit: af4e5f569bc89f356eb34a9373d7f82aca6faa8a\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1119%2Fdscho%2Fsparse-index-vs-split-index-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1119/dscho/sparse-index-vs-split-index-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1119\n> --\n> gitgitgadget\n"},{"id":"446685","messageId":"nycvar.QRO.7.76.6.2201221008330.2121@tvgsbejvaqbjf.bet","threadId":"57267","inReplyTo":"xmqqtudwbl64.fsf@gitster.g","subject":"Re: [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-22T09:30:55Z","receivedAt":"2022-01-22T09:31:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 21 Jan 2022, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > In 6e773527b6b (sparse-index: convert from full to sparse, 2021-03-30),\n> > we introduced initial support for a sparse index, and were careful to\n> > avoid converting to a sparse index in the presence of a split index.\n> >\n> > However, when we _just_ read a freshly-initialized index, it might not\n> > contain a split index even if _writing_ it will add one by virtue of\n> > being asked for via the `GIT_TEST_SPLIT_INDEX` variable.\n> >\n> > We did not notice any problems with checking _only_ for `split_index`\n> > (and not `GIT_TEST_SPLIT_INDEX`) right until both\n> > `vd/sparse-sparsity-fix-on-read` _and_ `vd/sparse-reset` were merged.\n> >\n> > Those two topics' interplay triggers a bug in conjunction with running\n> > t1091.15 when `GIT_TEST_SPLIT_INDEX=true` in the following way:\n> > `vd/sparse-sparsity-fix-on-read` ensures that the index is made sparse\n> > right after reading, and `vd/sparse-reset` ensures that the index is\n> > made non-sparse again unless running in the `--soft` mode. Since the\n> > split index feature is incompatible with the sparse index feature, we\n> > see a symptom like this:\n> >\n> > \tfatal: position for replacement 4 exceeds base index size 4\n> >\n> > Let's fix this by avoiding the conversion to a sparse index when\n> > `GIT_TEST_SPLIT_INDEX=true`.\n>\n> Does [2/3] allow you to sidestep that issue entirely by skipping\n> 1091 altogether?\n\nYou are right that reverting this patch after applying the next patch\n_still_ works around the issue. That's because currently only t1091\nexposes the bug where `GIT_TEST_SPLIT_INDEX` writes a split index that was\ninitially non-split (because it was just created).\n\nMy intention here was to avoid any problems in case _another_ test script\ntriggers the same condition. After all, we do have an entire CI job that\nforces `GIT_TEST_SPLIT_INDEX=1`, and in microsoft/git we enable sparse\nindex by default. The entire test suite is therefore surface for the bug\nto show.\n\n> There are 4 hits of \"if (istate->split_index\" in the codebase, and\n> this patch makes me wonder why it is suffice to patch only one of\n> them.\n\nI tried to catch only those locations where we run the danger of trying to\nmake a split index also sparse, or a sparse index also split.\n\n> I also wondered why we test both split_index and environment\n> separately, instead of splitting the index very early when the\n> environment variable is set, so that the rest of the runtime does\n> not have to worry about the environment, but is the reason why such\n> an approach was not taken was because GIT_TEST_SPLIT_INDEX can later\n> allow the index to be splitted, even if istate->split_index is still\n> NULL right now when this function is called?\n\nIndeed. A freshly created index will never be split, even if\n`GIT_TEST_SPLIT_INDEX` is set. Only when we _write_ this to disk will it\nbe turned into a split index. At this point, we might have mistakenly\nconverted the index into a sparse one, though, and this here patch wants\nto prevent that from happening.\n\n> If that is the reason, it leads to another question. Even if we\n> ignore GIT_TEST_SPLIT_INDEX and concentrate only on real workload,\n> if the in-core index can be NULL when this function is called and\n> then later can become split, there still must be somebody who\n> notices that sparse index is unallowed when (or after) such a split\n> happens, no? If there is no such code, then this patch would not be\n> the whole fix and there needs more change to do so, no?\n\nIndeed. Hence 3/3 which tries specifically to prevent the user from\nturning an already-sparse index into a split index. You will note that\n3/3 calls die() instead BUG() in such a case because it would constitute a\npilot error, not a bug in Git's code.\n\nCiao,\nDscho\n"},{"id":"446686","messageId":"nycvar.QRO.7.76.6.2201221031250.2121@tvgsbejvaqbjf.bet","threadId":"57267","inReplyTo":"CABPp-BFXMMH2_qqWPGaWX76mqEs2Z1GFNzbyeu2rLgaFPt7JyQ@mail.gmail.com","subject":"Re: [PATCH 0/3] [Non-critical]: sparse index vs split index fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-22T09:31:57Z","receivedAt":"2022-01-22T09:32:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Fri, 21 Jan 2022, Elijah Newren wrote:\n\n> On Fri, Jan 21, 2022 at 11:53 AM Johannes Schindelin via GitGitGadget\n> wrote:\n> >\n> > We noticed split/sparse index-related issues while rebasing Microsoft's fork\n> > of Git. These fixes are necessary for that fork's test suite to pass, but\n> > they might not be super critical to get into upstream v2.35.0 (especially\n> > this close to -rc2). However, the team felt that the decision should be left\n> > to the Git maintainer whether to take these patches into v2.35.0 or not.\n>\n> Thanks for digging into these and putting in some guardrails to\n> prevent similar issues. I hit similar things with some of my changes\n> and had to fight t1091 to get it to pass in CI under\n> GIT_TEST_SPLIT_INDEX=1. I don't recall seeing the specific error you\n> mention in patch 1, but maybe I just overlooked it. I ended up\n> finding little workarounds such as disabling sparse-checkouts at the\n> end of various tests before new ones I was adding, and being extra\n> careful to not leave the sparse-index selected. I probably should\n> have dug further, but didn't. Thanks for digging in.\n\nSeeing how much time it took to properly diagnose and fix these issues, I\ndo not fault you ;-)\n\n> I've read over the patch series; it looks good to me:\n>\n> Reviewed-by: Elijah Newren \n\nThank you!\nDscho\n"},{"id":"446687","messageId":"nycvar.QRO.7.76.6.2201221032060.2121@tvgsbejvaqbjf.bet","threadId":"57267","inReplyTo":"xmqqh79wejzo.fsf@gitster.g","subject":"Re: [PATCH 3/3] split-index: it really is incompatible with the sparse index","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-22T09:32:16Z","receivedAt":"2022-01-22T09:32:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 21 Jan 2022, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > ... at least for now. So let's error out if we are even trying to\n> > initialize the split index when the index is sparse, or when trying to\n> > write the split index extension for a sparse index.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > read-cache.c | 3 +++\n> > split-index.c | 3 +++\n> > 2 files changed, 6 insertions(+)\n>\n> Good. Will queue.\n\nThank you!\nDscho\n"}]}