{"thread":{"id":"56314","subject":"[PATCH 0/6] Fix GIT_TEST_SPLIT_INDEX","startedAt":"2021-08-17T17:49:53Z","lastAt":"2021-08-31T17:39:06Z","messageCount":23,"participants":["SZEDER Gábor","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"433005","messageId":"20210817174938.3009923-1-szeder.dev@gmail.com","threadId":"56314","inReplyTo":null,"subject":"[PATCH 0/6] Fix GIT_TEST_SPLIT_INDEX","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T17:49:32Z","receivedAt":"2021-08-17T17:49:53Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Running tests with GIT_TEST_SPLIT_INDEX=1 is supposed to turn on the\nsplit index feature and trigger index splitting (mostly) randomly.\nAlas, this has been broken for ~2.5 years, and it hasn't triggered any\nindex splitting since then.\n\nThe last patch in this series makes GIT_TEST_SPLIT_INDEX=1 work again,\nalthough it slightly changes its behavior; see its log message for all\nthe details.\n\nPatches 3 and 4 fix old tests that started to fail because\nof this slight behavior change.  Bsides the final fix, patch 4 is the\nmost interesting in this series.\n\nPatch 5 fixes new tests that were introduced while\nGIT_TEST_SPLIT_INDEX was broken.\n\nThe first two patches are just minor while-at-it test cleanups.\n\nSZEDER Gábor (6):\n  t1600-index: remove unnecessary redirection\n  t1600-index: don't run git commands upstream of a pipe\n  t1600-index: disable GIT_TEST_SPLIT_INDEX\n  read-cache: look for shared index files next to the index, too\n  tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests\n  read-cache: fix GIT_TEST_SPLIT_INDEX\n\n read-cache.c                       | 37 ++++++++++++++++------\n t/t1091-sparse-checkout-builtin.sh | 25 +++++++++------\n t/t1600-index.sh                   | 13 +++++---\n t/t1700-split-index.sh             | 34 ++++++++++++++++++++\n t/t7519-status-fsmonitor.sh        | 51 ++++++++++++++++--------------\n 5 files changed, 113 insertions(+), 47 deletions(-)\n\n-- \n2.33.0.453.gc5e41af357\n\n"},{"id":"433006","messageId":"20210817174938.3009923-2-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-1-szeder.dev@gmail.com","subject":"[PATCH 1/6] t1600-index: remove unnecessary redirection","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T17:49:33Z","receivedAt":"2021-08-17T17:49:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"In a helper function in the 't1600-index.sh' test script the stderr\nof a 'git add' command is redirected to its stdout, but its stdout is\nnot redirected anywhere.  So apparently this redirection is\nunnecessary, remove it.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t1600-index.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1600-index.sh b/t/t1600-index.sh\nindex c9b9e718b8..5d10cec67a 100755\n--- a/t/t1600-index.sh\n+++ b/t/t1600-index.sh\n@@ -79,7 +79,7 @@ test_index_version () {\n \t\telse\n \t\t\tunset GIT_INDEX_VERSION\n \t\tfi &&\n-\t\tgit add a 2>&1 &&\n+\t\tgit add a &&\n \t\techo $EXPECTED_OUTPUT_VERSION >expect &&\n \t\ttest-tool index-version <.git/index >actual &&\n \t\ttest_cmp expect actual\n-- \n2.33.0.453.gc5e41af357\n\n"},{"id":"433007","messageId":"20210817174938.3009923-3-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-1-szeder.dev@gmail.com","subject":"[PATCH 2/6] t1600-index: don't run git commands upstream of a pipe","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T17:49:34Z","receivedAt":"2021-08-17T17:50:00Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t1600-index.sh | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1600-index.sh b/t/t1600-index.sh\nindex 5d10cec67a..aa88fc30ce 100755\n--- a/t/t1600-index.sh\n+++ b/t/t1600-index.sh\n@@ -13,7 +13,8 @@ test_expect_success 'bogus GIT_INDEX_VERSION issues warning' '\n \t\trm -f .git/index &&\n \t\tGIT_INDEX_VERSION=2bogus &&\n \t\texport GIT_INDEX_VERSION &&\n-\t\tgit add a 2>&1 | sed \"s/[0-9]//\" >actual.err &&\n+\t\tgit add a 2>err &&\n+\t\tsed \"s/[0-9]//\" err >actual.err &&\n \t\tsed -e \"s/ Z$/ /\" <<-\\EOF >expect.err &&\n \t\t\twarning: GIT_INDEX_VERSION set, but the value is invalid.\n \t\t\tUsing version Z\n@@ -27,7 +28,8 @@ test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '\n \t\trm -f .git/index &&\n \t\tGIT_INDEX_VERSION=1 &&\n \t\texport GIT_INDEX_VERSION &&\n-\t\tgit add a 2>&1 | sed \"s/[0-9]//\" >actual.err &&\n+\t\tgit add a 2>err &&\n+\t\tsed \"s/[0-9]//\" err >actual.err &&\n \t\tsed -e \"s/ Z$/ /\" <<-\\EOF >expect.err &&\n \t\t\twarning: GIT_INDEX_VERSION set, but the value is invalid.\n \t\t\tUsing version Z\n@@ -50,7 +52,8 @@ test_expect_success 'out of bounds index.version issues warning' '\n \t\tsane_unset GIT_INDEX_VERSION &&\n \t\trm -f .git/index &&\n \t\tgit config --add index.version 1 &&\n-\t\tgit add a 2>&1 | sed \"s/[0-9]//\" >actual.err &&\n+\t\tgit add a 2>err &&\n+\t\tsed \"s/[0-9]//\" err >actual.err &&\n \t\tsed -e \"s/ Z$/ /\" <<-\\EOF >expect.err &&\n \t\t\twarning: index.version set, but the value is invalid.\n \t\t\tUsing version Z\n-- \n2.33.0.453.gc5e41af357\n\n"},{"id":"433008","messageId":"20210817174938.3009923-4-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-1-szeder.dev@gmail.com","subject":"[PATCH 3/6] t1600-index: disable GIT_TEST_SPLIT_INDEX","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T17:49:35Z","receivedAt":"2021-08-17T17:50:06Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Tests in 't1600-index.sh' check that various bogus index version\nvalues are recognized and an appropriate warning message is issued.\nGIT_TEST_SPLIT_INDEX=1 is supposed to trigger index splitting\nrandomly, and thus might interfere [1] with these tests: splitting the\nindex means that two index files are written (the shared base index\nand the split '.git/index'), and the same warning message is then\nissued twice, failing these tests.\n\nUnset GIT_TEST_SPLIT_INDEX in this test script to avoid such\ninterference.\n\n[1] There is no such interference at the moment, because, alas,\n    GIT_TEST_SPLIT_INDEX=1 is broken and never split the index.  There\n    was no such interference in the past (before it broke) either,\n    because the first index write with GIT_TEST_SPLIT_INDEX=1 never\n    split the index, only the second write did.  A subsequent commit\n    fixing GIT_TEST_SPLIT_INDEX will have all the details on this.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t1600-index.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/t1600-index.sh b/t/t1600-index.sh\nindex aa88fc30ce..46329c488b 100755\n--- a/t/t1600-index.sh\n+++ b/t/t1600-index.sh\n@@ -4,6 +4,8 @@ test_description='index file specific tests'\n \n . ./test-lib.sh\n \n+sane_unset GIT_TEST_SPLIT_INDEX\n+\n test_expect_success 'setup' '\n \techo 1 >a\n '\n-- \n2.33.0.453.gc5e41af357\n\n"},{"id":"433009","messageId":"20210817174938.3009923-5-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-1-szeder.dev@gmail.com","subject":"[PATCH 4/6] read-cache: look for shared index files next to the index, too","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T17:49:36Z","receivedAt":"2021-08-17T17:50:08Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"When reading a split index git always looks for its referenced shared\nbase index in the gitdir of the current repository, even when reading\nan alternate index specified via GIT_INDEX_FILE, and even when that\nalternate index file is the \"main\" '.git/index' file of an other\nrepository.  However, if that split index and its referenced shared\nindex files were written by a git command running entirely in that\nother repository, then, naturally, the shared index file is written to\nthat other repository's gitdir.  Consequently, a git command\nattempting to read that shared index file while running in a different\nrepository won't be able find it and will error out.\n\nI'm not sure in what use case it is necessary to read the index of one\nrepository by a git command running in a different repository, but it\nis certainly possible to do so, and in fact the test 'bare repository:\ncheck that --cached honors index' in 't0003-attributes.sh' does\nexactly that.  If GIT_TEST_SPLIT_INDEX=1 were to split the index in\njust the right moment [1], then this test would indeed fail, because\nthe referenced shared index file could not be found.\n\nLet's look for the referenced shared index file not only in the gitdir\nof the current directory, but, if the shared index is not there, right\nnext to the split index as well.\n\n[1] We haven't seen this issue trigger a failure in t0003 yet,\n    because:\n\n      - While GIT_TEST_SPLIT_INDEX=1 is supposed to trigger index\n        splitting randomly, the first index write has always been\n        deterministic and it has never split the index.\n\n      - That alternate index file in the other repository is written\n        only once in the entire test script, so it's never split.\n\n    However, the next patch will fix GIT_TEST_SPLIT_INDEX, and while\n    doing so it will slightly change its behavior to always split the\n    index already on the first index write, and t0003 would always\n    fail without this patch.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n read-cache.c           | 14 +++++++++++++-\n t/t1700-split-index.sh | 23 +++++++++++++++++++++++\n 2 files changed, 36 insertions(+), 1 deletion(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 9048ef9e90..fbd59886a3 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2391,9 +2391,21 @@ int read_index_from(struct index_state *istate, const char *path,\n \tbase_path = xstrfmt(\"%s/sharedindex.%s\", gitdir, base_oid_hex);\n \ttrace2_region_enter_printf(\"index\", \"shared/do_read_index\",\n \t\t\t\t   the_repository, \"%s\", base_path);\n-\tret = do_read_index(split_index->base, base_path, 1);\n+\tret = do_read_index(split_index->base, base_path, 0);\n \ttrace2_region_leave_printf(\"index\", \"shared/do_read_index\",\n \t\t\t\t   the_repository, \"%s\", base_path);\n+\tif (!ret) {\n+\t\tchar *path_copy = xstrdup(path);\n+\t\tconst char *base_path2 = xstrfmt(\"%s/sharedindex.%s\",\n+\t\t\t\t\t\t dirname(path_copy),\n+\t\t\t\t\t\t base_oid_hex);\n+\t\tfree(path_copy);\n+\t\ttrace2_region_enter_printf(\"index\", \"shared/do_read_index\",\n+\t\t\t\t\t   the_repository, \"%s\", base_path2);\n+\t\tret = do_read_index(split_index->base, base_path2, 1);\n+\t\ttrace2_region_leave_printf(\"index\", \"shared/do_read_index\",\n+\t\t\t\t\t   the_repository, \"%s\", base_path2);\n+\t}\n \tif (!oideq(&split_index->base_oid, &split_index->base->oid))\n \t\tdie(_(\"broken index, expect %s in %s, got %s\"),\n \t\t    base_oid_hex, base_path,\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex 986baa612e..e2aa0bd949 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -510,4 +510,27 @@ test_expect_success 'do not refresh null base index' '\n \t)\n '\n \n+test_expect_success 'reading split index at alternate location' '\n+\tgit init reading-alternate-location &&\n+\t(\n+\t\tcd reading-alternate-location &&\n+\t\t>file-in-alternate &&\n+\t\tgit update-index --split-index --add file-in-alternate\n+\t) &&\n+\techo file-in-alternate >expect &&\n+\n+\t# Should be able to find the shared index both right next to\n+\t# the specified split index file ...\n+\tGIT_INDEX_FILE=./reading-alternate-location/.git/index \\\n+\tgit ls-files --cached >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# ... and, for backwards compatibility, in the current GIT_DIR\n+\t# as well.\n+\tmv -v ./reading-alternate-location/.git/sharedindex.* .git &&\n+\tGIT_INDEX_FILE=./reading-alternate-location/.git/index \\\n+\tgit ls-files --cached >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.33.0.453.gc5e41af357\n\n"},{"id":"433011","messageId":"20210817174938.3009923-6-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-1-szeder.dev@gmail.com","subject":"[PATCH 5/6] tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T17:49:37Z","receivedAt":"2021-08-17T17:50:30Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The sparse index and split index features are said to be currently\nincompatible [1], and consequently GIT_TEST_SPLIT_INDEX=1 might\ninterfere with the test cases exercising the sparse index feature.\nTherefore GIT_TEST_SPLIT_INDEX is already explicitly disabled for the\nwhole of 't1092-sparse-checkout-compatibility.sh'.  There are,\nhowever, two other test cases exercising sparse index, namely\n'sparse-index enabled and disabled' in\n't1091-sparse-checkout-builtin.sh' and 'status succeeds with sparse\nindex' in 't7519-status-fsmonitor.sh', and these two could fail with\nGIT_TEST_SPLIT_INDEX=1 as well [2].\n\nUnset GIT_TEST_SPLIT_INDEX and disable the split index in these two\ntest cases to avoid such interference.\n\nNote that this is the minimal change to merely avoid failures when\nthese test cases are run with GIT_TEST_SPLIT_INDEX=1.  Interestingly,\nthough, without these changes the 'git sparse-checkout init --cone\n--sparse-index' commands still succeed even with split index, and set\nall the necessary configuration variables and create the initial\n'$GIT_DIR/info/sparse-checkout' file, but the test failures are caused\nby later sanity checks finding that the index is not in fact a sparse\nindex.  This indicates that 'git sparse-checkout init --sparse-index'\nlacks some error checking and its tests lack coverage related to split\nindex, but fixing those issues (let alone making sparse index\ncomparible with split index) is beyond the scope of this patch series.\n\n[1] https://public-inbox.org/git/48e9c3d6-407a-1843-2d91-22112410e3f8@gmail.com/\n\n[2] Neither of these test cases fail at the moment, because\n    GIT_TEST_SPLIT_INDEX=1 is broken and never splits the index, and\n    it broke long before the sparse index feature was added.\n    This patch series is about to fix GIT_TEST_SPLIT_INDEX, and then\n    both test cases mentioned above would fail.\n\n(The diff is best viewed with '--ignore-all-space')\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t1091-sparse-checkout-builtin.sh | 25 +++++++++------\n t/t7519-status-fsmonitor.sh        | 51 ++++++++++++++++--------------\n 2 files changed, 43 insertions(+), 33 deletions(-)\n\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex 38fc8340f5..3f94c41241 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -206,16 +206,21 @@ test_expect_success 'sparse-checkout disable' '\n '\n \n test_expect_success 'sparse-index enabled and disabled' '\n-\tgit -C repo sparse-checkout init --cone --sparse-index &&\n-\ttest_cmp_config -C repo true index.sparse &&\n-\ttest-tool -C repo read-cache --table >cache &&\n-\tgrep \" tree \" cache &&\n-\n-\tgit -C repo sparse-checkout disable &&\n-\ttest-tool -C repo read-cache --table >cache &&\n-\t! grep \" tree \" cache &&\n-\tgit -C repo config --list >config &&\n-\t! grep index.sparse config\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\ttest-tool -C repo read-cache --table >cache &&\n+\t\tgrep \" tree \" cache &&\n+\n+\t\tgit -C repo sparse-checkout disable &&\n+\t\ttest-tool -C repo read-cache --table >cache &&\n+\t\t! grep \" tree \" cache &&\n+\t\tgit -C repo config --list >config &&\n+\t\t! grep index.sparse config\n+\t)\n '\n \n test_expect_success 'cone mode: init and set' '\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex deea88d443..513e17065f 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -402,34 +402,39 @@ check_sparse_index_behavior () {\n }\n \n test_expect_success 'status succeeds with sparse index' '\n-\tgit reset --hard &&\n+\t(\n+\t\tsane_unset GIT_TEST_SPLIT_INDEX &&\n+\t\tgit update-index --no-split-index &&\n \n-\ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n-\tcheck_sparse_index_behavior ! &&\n+\t\tgit reset --hard &&\n \n-\twrite_script .git/hooks/fsmonitor-test<<-\\EOF &&\n-\t\tprintf \"last_update_token\\0\"\n-\tEOF\n-\tgit config core.fsmonitor .git/hooks/fsmonitor-test &&\n-\tcheck_sparse_index_behavior ! &&\n+\t\ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n+\t\tcheck_sparse_index_behavior ! &&\n \n-\twrite_script .git/hooks/fsmonitor-test<<-\\EOF &&\n-\t\tprintf \"last_update_token\\0\"\n-\t\tprintf \"dir1/modified\\0\"\n-\tEOF\n-\tcheck_sparse_index_behavior ! &&\n+\t\twrite_script .git/hooks/fsmonitor-test<<-\\EOF &&\n+\t\t\tprintf \"last_update_token\\0\"\n+\t\tEOF\n+\t\tgit config core.fsmonitor .git/hooks/fsmonitor-test &&\n+\t\tcheck_sparse_index_behavior ! &&\n \n-\tcp -r dir1 dir1a &&\n-\tgit add dir1a &&\n-\tgit commit -m \"add dir1a\" &&\n+\t\twrite_script .git/hooks/fsmonitor-test<<-\\EOF &&\n+\t\t\tprintf \"last_update_token\\0\"\n+\t\t\tprintf \"dir1/modified\\0\"\n+\t\tEOF\n+\t\tcheck_sparse_index_behavior ! &&\n \n-\t# This one modifies outside the sparse-checkout definition\n-\t# and hence we expect to expand the sparse-index.\n-\twrite_script .git/hooks/fsmonitor-test<<-\\EOF &&\n-\t\tprintf \"last_update_token\\0\"\n-\t\tprintf \"dir1a/modified\\0\"\n-\tEOF\n-\tcheck_sparse_index_behavior\n+\t\tcp -r dir1 dir1a &&\n+\t\tgit add dir1a &&\n+\t\tgit commit -m \"add dir1a\" &&\n+\n+\t\t# This one modifies outside the sparse-checkout definition\n+\t\t# and hence we expect to expand the sparse-index.\n+\t\twrite_script .git/hooks/fsmonitor-test<<-\\EOF &&\n+\t\t\tprintf \"last_update_token\\0\"\n+\t\t\tprintf \"dir1a/modified\\0\"\n+\t\tEOF\n+\t\tcheck_sparse_index_behavior\n+\t)\n '\n \n test_done\n-- \n2.33.0.453.gc5e41af357\n\n"},{"id":"433010","messageId":"20210817174938.3009923-7-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-1-szeder.dev@gmail.com","subject":"[PATCH 6/6] read-cache: fix GIT_TEST_SPLIT_INDEX","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T17:49:38Z","receivedAt":"2021-08-17T17:50:32Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Running tests with GIT_TEST_SPLIT_INDEX=1 is supposed to turn on the\nsplit index feature and trigger index splitting (mostly) randomly.\nAlas, this has been broken since 6e37c8ed3c (read-cache.c: fix writing\n\"link\" index ext with null base oid, 2019-02-13), and\nGIT_TEST_SPLIT_INDEX=1 hasn't triggered any index splitting since\nthen.\n\nThis patch makes GIT_TEST_SPLIT_INDEX work again, though it doesn't\nrestore the pre-6e37c8ed3c behavior.  To understand the bug, the fix,\nand the behavior change we first have to look at how\nGIT_TEST_SPLIT_INDEX used to work before 6e37c8ed3c:\n\n  There are two places where we check the value of\n  GIT_TEST_SPLIT_INDEX, and before 6e37c8ed3c they worked like this:\n\n    1) In the lower-level do_write_index(), where, if\n       GIT_TEST_SPLIT_INDEX is enabled, we call init_split_index().\n       This call merely allocates and zero-initializes\n       'istate->split_index', but does nothing else (i.e. doesn't fill\n       the base/shared index with cache entries, doesn't actually\n       write a shared index file, etc.).  Pertinent to this issue, the\n       hash of the base index remains all zeroed out.\n\n    2) In the higher-level write_locked_index(), but only when\n       'istate->split_index' has already been initialized.  Then, if\n       GIT_TEST_SPLIT_INDEX is enabled, it randomly sets the flag that\n       triggers index splitting later in this function.  This\n       randomness comes from the first byte of the hash of the base\n       index via an 'if ((first_byte & 15) < 6)' condition.\n\n       However, if 'istate->split_index' hasn't been initialized (i.e.\n       it is still NULL), then write_locked_index() just calls\n       do_write_locked_index(), which internally calls the above\n       mentioned do_write_index().\n\n  This means that while GIT_TEST_SPLIT_INDEX=1 usually triggered index\n  splitting randomly, the first two index writes were always\n  deterministic (though I suspect this was unintentional):\n\n    - The initial index write never splits the index.\n      During the first index write write_locked_index() is called with\n      'istate->split_index' still uninitialized, so the check in 2) is\n      not executed.  It still calls do_write_index(), though, which\n      then executes the check in 1).  The resulting all zero base\n      index hash then leads to the 'link' extension being written to\n      '.git/index', though a shared index file is not written:\n\n        $ rm .git/index\n        $ GIT_TEST_SPLIT_INDEX=1 git update-index --add file\n        $ test-tool dump-split-index .git/index\n        own c6ef71168597caec8553c83d9d0048f1ef416170\n        base 0000000000000000000000000000000000000000\n        100644 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 0 file\n        replacements:\n        deletions:\n        $ ls -l .git/sharedindex.*\n        ls: cannot access '.git/sharedindex.*': No such file or directory\n\n    - The second index write always splits the index.\n      When the index written in the previous point is read,\n      'istate->split_index' is initialized because of the presence of\n      the 'link' extension.  So during the second write\n      write_locked_index() does run the check in 2), and the first\n      byte of the all zero base index hash always fulfills the\n      randomness condition, which in turn always triggers the index\n      splitting.\n\n    - Subsequent index writes will find the 'link' extension with a\n      real non-zero base index hash, so from then on the check in 2)\n      is executed and the first byte of the base index hash is as\n      random as it gets (coming from the SHA-1 of index data including\n      timestamps and inodes...).\n\nAll this worked until 6e37c8ed3c came along, and stopped writing the\n'link' extension if the hash of the base index was all zero:\n\n  $ rm .git/index\n  $ GIT_TEST_SPLIT_INDEX=1 git update-index --add file\n  $ test-tool dump-split-index .git/index\n  own abbd6f6458d5dee73ae8e210ca15a68a390c6fd7\n  not a split index\n  $ ls -l .git/sharedindex.*\n  ls: cannot access '.git/sharedindex.*': No such file or directory\n\nSo, since the first index write with GIT_TEST_SPLIT_INDEX=1 doesn't\nwrite a 'link' extension, in the second index write\n'istate->split_index' remains uninitialized, and the check in 2) is\nnot executed, and ultimately the index is never split.\n\nFix this by modifying write_locked_index() to make sure to check\nGIT_TEST_SPLIT_INDEX even if 'istate->split_index' is still\nuninitialized, and initialize it if necessary.  The check for\nGIT_TEST_SPLIT_INDEX and separate init_split_index() call in\ndo_write_index() thus becomes unnecessary, so remove it.  Furthermore,\nadd a test to 't1700-split-index.sh' to make sure that\nGIT_TEST_SPLIT_INDEX=1 will keep working (though only check the\nindex splitting on the first index write, because after that it will\nbe random).\n\nNote that this change does not restore the pre-6e37c8ed3c behaviour,\nas it will deterministically split the index already on the first\nindex write.  Since GIT_TEST_SPLIT_INDEX is purely a developer aid,\nthere is no backwards compatibility issue here.  The new behaviour did\ntrigger test failures in 't0003-attributes.sh' and 't1600-index.sh',\nthough, which have been fixed in preparatory patches in this series.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n read-cache.c           | 23 ++++++++++++++---------\n t/t1700-split-index.sh | 11 +++++++++++\n 2 files changed, 25 insertions(+), 9 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex fbd59886a3..335242c1cf 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2824,11 +2824,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\t}\n \t}\n \n-\tif (!istate->version) {\n+\tif (!istate->version)\n \t\tistate->version = get_index_format_default(the_repository);\n-\t\tif (git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0))\n-\t\t\tinit_split_index(istate);\n-\t}\n \n \t/* demote version 3 to version 2 when the latter suffices */\n \tif (istate->version == 3 || istate->version == 2)\n@@ -3255,7 +3252,7 @@ static int too_many_not_shared_entries(struct index_state *istate)\n int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\t       unsigned flags)\n {\n-\tint new_shared_index, ret;\n+\tint new_shared_index, ret, test_split_index_env;\n \tstruct split_index *si = istate->split_index;\n \n \tif (git_env_bool(\"GIT_TEST_CHECK_CACHE_TREE\", 0))\n@@ -3270,7 +3267,10 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \tif (istate->fsmonitor_last_update)\n \t\tfill_fsmonitor_bitmap(istate);\n \n-\tif (!si || alternate_index_output ||\n+\ttest_split_index_env = git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0);\n+\n+\tif ((!si && !test_split_index_env) ||\n+\t    alternate_index_output ||\n \t    (istate->cache_changed & ~EXTMASK)) {\n \t\tif (si)\n \t\t\toidclr(&si->base_oid);\n@@ -3278,10 +3278,15 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\tgoto out;\n \t}\n \n-\tif (git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0)) {\n-\t\tint v = si->base_oid.hash[0];\n-\t\tif ((v & 15) < 6)\n+\tif (test_split_index_env) {\n+\t\tif (!si) {\n+\t\t\tsi = init_split_index(istate);\n \t\t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\n+\t\t} else {\n+\t\t\tint v = si->base_oid.hash[0];\n+\t\t\tif ((v & 15) < 6)\n+\t\t\t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\n+\t\t}\n \t}\n \tif (too_many_not_shared_entries(istate))\n \t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex e2aa0bd949..decd2527ed 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -533,4 +533,15 @@ test_expect_success 'reading split index at alternate location' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'GIT_TEST_SPLIT_INDEX works' '\n+\tgit init git-test-split-index &&\n+\t(\n+\t\tcd git-test-split-index &&\n+\t\t>file &&\n+\t\tGIT_TEST_SPLIT_INDEX=1 git update-index --add file &&\n+\t\tls -l .git/sharedindex.* >actual &&\n+\t\ttest_line_count = 1 actual\n+\t)\n+'\n+\n test_done\n-- \n2.33.0.453.gc5e41af357\n\n"},{"id":"433014","messageId":"d8d8d59a-ecac-2cd5-18a3-f1cb65e48c2e@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-2-szeder.dev@gmail.com","subject":"Re: [PATCH 1/6] t1600-index: remove unnecessary redirection","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-17T18:12:36Z","receivedAt":"2021-08-17T18:12:40Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/17/2021 1:49 PM, SZEDER Gábor wrote:\n> In a helper function in the 't1600-index.sh' test script the stderr\n> of a 'git add' command is redirected to its stdout, but its stdout is\n> not redirected anywhere.  So apparently this redirection is\n> unnecessary, remove it.\n> \n> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n> ---\n>  t/t1600-index.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t1600-index.sh b/t/t1600-index.sh\n> index c9b9e718b8..5d10cec67a 100755\n> --- a/t/t1600-index.sh\n> +++ b/t/t1600-index.sh\n> @@ -79,7 +79,7 @@ test_index_version () {\n>  \t\telse\n>  \t\t\tunset GIT_INDEX_VERSION\n>  \t\tfi &&\n> -\t\tgit add a 2>&1 &&\n> +\t\tgit add a &&\n>  \t\techo $EXPECTED_OUTPUT_VERSION >expect &&\n>  \t\ttest-tool index-version <.git/index >actual &&\n>  \t\ttest_cmp expect actual\n\nSince here we have a 'test_cmp expect actual', perhaps the\nactual mistake is that the line isn't\n\n\tgit add a 2>&1 >actual &&\n\nWhat do you think about that?\n\nThanks,\n-Stolee\n"},{"id":"433015","messageId":"6f900a58-19b4-753c-18f5-852eaebc1527@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-6-szeder.dev@gmail.com","subject":"Re: [PATCH 5/6] tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-17T18:26:23Z","receivedAt":"2021-08-17T18:26:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/17/2021 1:49 PM, SZEDER Gábor wrote:\n> The sparse index and split index features are said to be currently\n> incompatible [1], and consequently GIT_TEST_SPLIT_INDEX=1 might\n> interfere with the test cases exercising the sparse index feature.\n> Therefore GIT_TEST_SPLIT_INDEX is already explicitly disabled for the\n> whole of 't1092-sparse-checkout-compatibility.sh'.  There are,\n> however, two other test cases exercising sparse index, namely\n> 'sparse-index enabled and disabled' in\n> 't1091-sparse-checkout-builtin.sh' and 'status succeeds with sparse\n> index' in 't7519-status-fsmonitor.sh', and these two could fail with\n> GIT_TEST_SPLIT_INDEX=1 as well [2].\n> \n> Unset GIT_TEST_SPLIT_INDEX and disable the split index in these two\n> test cases to avoid such interference.\n> \n> Note that this is the minimal change to merely avoid failures when\n> these test cases are run with GIT_TEST_SPLIT_INDEX=1.  Interestingly,\n> though, without these changes the 'git sparse-checkout init --cone\n> --sparse-index' commands still succeed even with split index, and set\n> all the necessary configuration variables and create the initial\n> '$GIT_DIR/info/sparse-checkout' file, but the test failures are caused\n> by later sanity checks finding that the index is not in fact a sparse\n> index.  This indicates that 'git sparse-checkout init --sparse-index'\n> lacks some error checking and its tests lack coverage related to split\n> index, but fixing those issues (let alone making sparse index\n> comparible with split index) is beyond the scope of this patch series.\n\ns/comparible/compatible.\n\nI agree that making these two things compatible is not something to\nsolve today. I'm not sure they should _ever_ be solved because of\nthe complexity involved (what if the base index is not sparse but\nthe tip wants to be, or vice-versa?, or if a directory must be\nexpanded because of a conflict?). They use very different approaches\nto solve a similar problem: how to deal with large index files.\n\n* The split index reduces index _write_ time by only editing a diff\n  of the base index.\n\n* The sparse index reduces index _read and write_ time by writing a\n  smaller index, but only if the user is using cone mode sparse-\n  checkout.\n\n>  test_expect_success 'sparse-index enabled and disabled' '\n> -\tgit -C repo sparse-checkout init --cone --sparse-index &&\n> -\ttest_cmp_config -C repo true index.sparse &&\n> -\ttest-tool -C repo read-cache --table >cache &&\n> -\tgrep \" tree \" cache &&\n> -\n> -\tgit -C repo sparse-checkout disable &&\n> -\ttest-tool -C repo read-cache --table >cache &&\n> -\t! grep \" tree \" cache &&\n> -\tgit -C repo config --list >config &&\n> -\t! grep index.sparse config\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\ttest-tool -C repo read-cache --table >cache &&\n> +\t\tgrep \" tree \" cache &&\n> +\n> +\t\tgit -C repo sparse-checkout disable &&\n> +\t\ttest-tool -C repo read-cache --table >cache &&\n> +\t\t! grep \" tree \" cache &&\n> +\t\tgit -C repo config --list >config &&\n> +\t\t! grep index.sparse config\n> +\t)\n>  '\n\nThis test is safe for now.\n\n>  test_expect_success 'status succeeds with sparse index' '\n\nThis test is being edited in ds/sparse-index-ignored-files. v3\nof the relevant patch was just sent today [1].\n\n[1] https://lore.kernel.org/git/e66106f7a99d94145eec983ea5e72b7cf8a8a479.1629206603.git.gitgitgadget@gmail.com/\n\nYou might want to rebase on top of that topic. The edits to\nthe test are likely stable now.\n\nThanks,\n-Stolee\n"},{"id":"433016","messageId":"edd7b53e-f0e9-b56c-e13d-63a2f963b703@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-7-szeder.dev@gmail.com","subject":"Re: [PATCH 6/6] read-cache: fix GIT_TEST_SPLIT_INDEX","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-17T18:29:06Z","receivedAt":"2021-08-17T18:29:10Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/17/2021 1:49 PM, SZEDER Gábor wrote:\n\nSorry to crop all of your excellent description of why this wasn't\nworking before and why it will work now, but...\n\n> +test_expect_success 'GIT_TEST_SPLIT_INDEX works' '\n> +\tgit init git-test-split-index &&\n> +\t(\n> +\t\tcd git-test-split-index &&\n> +\t\t>file &&\n> +\t\tGIT_TEST_SPLIT_INDEX=1 git update-index --add file &&\n> +\t\tls -l .git/sharedindex.* >actual &&\n> +\t\ttest_line_count = 1 actual\n> +\t)\n> +'\n> +\n\nThis is really the critical piece to ensure we don't have a\nsimilar problem in the future. Thanks!\n\n-Stolee\n"},{"id":"433017","messageId":"20210817183917.GC2257957@szeder.dev","threadId":"56314","inReplyTo":"d8d8d59a-ecac-2cd5-18a3-f1cb65e48c2e@gmail.com","subject":"Re: [PATCH 1/6] t1600-index: remove unnecessary redirection","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T18:39:17Z","receivedAt":"2021-08-17T18:39:22Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Aug 17, 2021 at 02:12:36PM -0400, Derrick Stolee wrote:\n> On 8/17/2021 1:49 PM, SZEDER Gábor wrote:\n> > In a helper function in the 't1600-index.sh' test script the stderr\n> > of a 'git add' command is redirected to its stdout, but its stdout is\n> > not redirected anywhere.  So apparently this redirection is\n> > unnecessary, remove it.\n> > \n> > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n> > ---\n> >  t/t1600-index.sh | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/t/t1600-index.sh b/t/t1600-index.sh\n> > index c9b9e718b8..5d10cec67a 100755\n> > --- a/t/t1600-index.sh\n> > +++ b/t/t1600-index.sh\n> > @@ -79,7 +79,7 @@ test_index_version () {\n> >  \t\telse\n> >  \t\t\tunset GIT_INDEX_VERSION\n> >  \t\tfi &&\n> > -\t\tgit add a 2>&1 &&\n> > +\t\tgit add a &&\n> >  \t\techo $EXPECTED_OUTPUT_VERSION >expect &&\n> >  \t\ttest-tool index-version <.git/index >actual &&\n> >  \t\ttest_cmp expect actual\n> \n> Since here we have a 'test_cmp expect actual', perhaps the\n> actual mistake is that the line isn't\n> \n> \tgit add a 2>&1 >actual &&\n> \n> What do you think about that?\n\nThe actual file is written two lines later as:\n\n  test-tool index-version <.git/index >actual\n\nSo it seems that neither stdout nor stderr of that 'git add' command\nmatters.\n\nHowever, in most of the preceeding tests 'git add's stderr does indeed\nmatter, as they do:\n\n    git add a 2>&1 | sed \"s/[0-9]//\" >actual.err &&\n    # write expect.err\n    test_cmp expect.err actual.err\n\nI suspect that the 2>&1 redirection this patch is removing might have\nbeen a copy-paste error.\n\n"},{"id":"433018","messageId":"469a6987-15ac-82ef-b330-0802d1843f17@gmail.com","threadId":"56314","inReplyTo":"20210817183917.GC2257957@szeder.dev","subject":"Re: [PATCH 1/6] t1600-index: remove unnecessary redirection","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-17T18:48:19Z","receivedAt":"2021-08-17T18:48:22Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/17/2021 2:39 PM, SZEDER Gábor wrote:\n> On Tue, Aug 17, 2021 at 02:12:36PM -0400, Derrick Stolee wrote:\n>> On 8/17/2021 1:49 PM, SZEDER Gábor wrote:\n>>> In a helper function in the 't1600-index.sh' test script the stderr\n>>> of a 'git add' command is redirected to its stdout, but its stdout is\n>>> not redirected anywhere.  So apparently this redirection is\n>>> unnecessary, remove it.\n>>>\n>>> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n>>> ---\n>>>  t/t1600-index.sh | 2 +-\n>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>>\n>>> diff --git a/t/t1600-index.sh b/t/t1600-index.sh\n>>> index c9b9e718b8..5d10cec67a 100755\n>>> --- a/t/t1600-index.sh\n>>> +++ b/t/t1600-index.sh\n>>> @@ -79,7 +79,7 @@ test_index_version () {\n>>>  \t\telse\n>>>  \t\t\tunset GIT_INDEX_VERSION\n>>>  \t\tfi &&\n>>> -\t\tgit add a 2>&1 &&\n>>> +\t\tgit add a &&\n>>>  \t\techo $EXPECTED_OUTPUT_VERSION >expect &&\n>>>  \t\ttest-tool index-version <.git/index >actual &&\n>>>  \t\ttest_cmp expect actual\n>>\n>> Since here we have a 'test_cmp expect actual', perhaps the\n>> actual mistake is that the line isn't\n>>\n>> \tgit add a 2>&1 >actual &&\n>>\n>> What do you think about that?\n> \n> The actual file is written two lines later as:\n> \n>   test-tool index-version <.git/index >actual\n> \n> So it seems that neither stdout nor stderr of that 'git add' command\n> matters.\n\nOf course. Sorry for misreading.\n\n> However, in most of the preceeding tests 'git add's stderr does indeed\n> matter, as they do:\n> \n>     git add a 2>&1 | sed \"s/[0-9]//\" >actual.err &&\n>     # write expect.err\n>     test_cmp expect.err actual.err\n> \n> I suspect that the 2>&1 redirection this patch is removing might have\n> been a copy-paste error.\n\nThat makes more sense than an errant addition.\n\nThanks,\n-Stolee\n\n\n"},{"id":"433030","messageId":"20210817213201.GD2257957@szeder.dev","threadId":"56314","inReplyTo":"6f900a58-19b4-753c-18f5-852eaebc1527@gmail.com","subject":"Re: [PATCH 5/6] tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T21:32:01Z","receivedAt":"2021-08-17T21:32:06Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Aug 17, 2021 at 02:26:23PM -0400, Derrick Stolee wrote:\n> On 8/17/2021 1:49 PM, SZEDER Gábor wrote:\n> > The sparse index and split index features are said to be currently\n> > incompatible [1], and consequently GIT_TEST_SPLIT_INDEX=1 might\n> > interfere with the test cases exercising the sparse index feature.\n> > Therefore GIT_TEST_SPLIT_INDEX is already explicitly disabled for the\n> > whole of 't1092-sparse-checkout-compatibility.sh'.  There are,\n> > however, two other test cases exercising sparse index, namely\n> > 'sparse-index enabled and disabled' in\n> > 't1091-sparse-checkout-builtin.sh' and 'status succeeds with sparse\n> > index' in 't7519-status-fsmonitor.sh', and these two could fail with\n> > GIT_TEST_SPLIT_INDEX=1 as well [2].\n> > \n> > Unset GIT_TEST_SPLIT_INDEX and disable the split index in these two\n> > test cases to avoid such interference.\n> > \n> > Note that this is the minimal change to merely avoid failures when\n> > these test cases are run with GIT_TEST_SPLIT_INDEX=1.  Interestingly,\n> > though, without these changes the 'git sparse-checkout init --cone\n> > --sparse-index' commands still succeed even with split index, and set\n> > all the necessary configuration variables and create the initial\n> > '$GIT_DIR/info/sparse-checkout' file, but the test failures are caused\n> > by later sanity checks finding that the index is not in fact a sparse\n> > index.  This indicates that 'git sparse-checkout init --sparse-index'\n> > lacks some error checking and its tests lack coverage related to split\n> > index, but fixing those issues (let alone making sparse index\n> > comparible with split index) is beyond the scope of this patch series.\n> \n> s/comparible/compatible.\n> \n> I agree that making these two things compatible is not something to\n> solve today. I'm not sure they should _ever_ be solved because of\n> the complexity involved (what if the base index is not sparse but\n> the tip wants to be, or vice-versa?,\n\nI think that this's not an issue, because a shared index file is not\nfor forever, but it's expected that a new shared index is written\nevery once in a while anyway, see splitIndex.maxPercentChange.  So\nwhenever sparse index gets enabled/disabled we could just write a new\nshared index accordingly.  Maybe even when the sparse patterns change,\nif necessary.\n\n> or if a directory must be expanded because of a conflict?).\n\nI'm not quite up-to-date in sparse index terminology, so I'm not sure\nthat this means...  but as mentioned above we can just write a new\nshared index when deemed necessary.\n\n> They use very different approaches\n> to solve a similar problem: how to deal with large index files.\n> \n> * The split index reduces index _write_ time by only editing a diff\n>   of the base index.\n> \n> * The sparse index reduces index _read and write_ time by writing a\n>   smaller index, but only if the user is using cone mode sparse-\n>   checkout.\n\nYeah, I think that in general there is more to be gained with sparse\nindex than with split index, though the split index might further\nreduce the write time of a sparse index, because the amount of data\nwritten is proportional with the nr of changed files instead of the nr\nof all files in the sparse index.  I'm not sure that it's worth it,\neither.\n\nMy remarks about compatibility primarily stem from your remarks about\ncompatibility in response to my sparse vs. split test failure report\nin:\n\n  https://public-inbox.org/git/48e9c3d6-407a-1843-2d91-22112410e3f8@gmail.com/\n\n  \"the sparse-index is (currently) incompatible with the split-index\"\n\nI assumed that that \"(currently)\" implies that eventually the two will\nbe made compatible.\n\nAnyway, I'm fine with leaving them incompatible, but 'git\nsparse-checkout' should still learn about split index, so it won't\npretend to succeed after it couldn't do what it was asked to when\nthere is a split index.  Maybe vice-versa as well.\n\n> >  test_expect_success 'sparse-index enabled and disabled' '\n> > -\tgit -C repo sparse-checkout init --cone --sparse-index &&\n> > -\ttest_cmp_config -C repo true index.sparse &&\n> > -\ttest-tool -C repo read-cache --table >cache &&\n> > -\tgrep \" tree \" cache &&\n> > -\n> > -\tgit -C repo sparse-checkout disable &&\n> > -\ttest-tool -C repo read-cache --table >cache &&\n> > -\t! grep \" tree \" cache &&\n> > -\tgit -C repo config --list >config &&\n> > -\t! grep index.sparse config\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\ttest-tool -C repo read-cache --table >cache &&\n> > +\t\tgrep \" tree \" cache &&\n> > +\n> > +\t\tgit -C repo sparse-checkout disable &&\n> > +\t\ttest-tool -C repo read-cache --table >cache &&\n> > +\t\t! grep \" tree \" cache &&\n> > +\t\tgit -C repo config --list >config &&\n> > +\t\t! grep index.sparse config\n> > +\t)\n> >  '\n> \n> This test is safe for now.\n> \n> >  test_expect_success 'status succeeds with sparse index' '\n> \n> This test is being edited in ds/sparse-index-ignored-files. v3\n> of the relevant patch was just sent today [1].\n> \n> [1] https://lore.kernel.org/git/e66106f7a99d94145eec983ea5e72b7cf8a8a479.1629206603.git.gitgitgadget@gmail.com/\n> \n> You might want to rebase on top of that topic. The edits to\n> the test are likely stable now.\n\nOh, no :)  The test 'cone mode clears ignored subdirectories' added in\nthat patch series (as in 'seen', so not the newes version) fails with\nthe working GIT_TEST_SPLIT_INDEX=1 as well, and I don't immediately\nsee why, as it doesn't seem to use sparse index.\n\n"},{"id":"433078","messageId":"3a683988-4ee3-65be-e1f9-cb2392e8bbe1@gmail.com","threadId":"56314","inReplyTo":"20210817213201.GD2257957@szeder.dev","subject":"Re: [PATCH 5/6] tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-18T18:51:56Z","receivedAt":"2021-08-18T18:52:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/17/2021 5:32 PM, SZEDER Gábor wrote:\n> On Tue, Aug 17, 2021 at 02:26:23PM -0400, Derrick Stolee wrote:\n>> On 8/17/2021 1:49 PM, SZEDER Gábor wrote:\n>>> The sparse index and split index features are said to be currently\n>>> incompatible [1], and consequently GIT_TEST_SPLIT_INDEX=1 might\n>>> interfere with the test cases exercising the sparse index feature.\n>>> Therefore GIT_TEST_SPLIT_INDEX is already explicitly disabled for the\n>>> whole of 't1092-sparse-checkout-compatibility.sh'.  There are,\n>>> however, two other test cases exercising sparse index, namely\n>>> 'sparse-index enabled and disabled' in\n>>> 't1091-sparse-checkout-builtin.sh' and 'status succeeds with sparse\n>>> index' in 't7519-status-fsmonitor.sh', and these two could fail with\n>>> GIT_TEST_SPLIT_INDEX=1 as well [2].\n>>>\n>>> Unset GIT_TEST_SPLIT_INDEX and disable the split index in these two\n>>> test cases to avoid such interference.\n>>>\n>>> Note that this is the minimal change to merely avoid failures when\n>>> these test cases are run with GIT_TEST_SPLIT_INDEX=1.  Interestingly,\n>>> though, without these changes the 'git sparse-checkout init --cone\n>>> --sparse-index' commands still succeed even with split index, and set\n>>> all the necessary configuration variables and create the initial\n>>> '$GIT_DIR/info/sparse-checkout' file, but the test failures are caused\n>>> by later sanity checks finding that the index is not in fact a sparse\n>>> index.  This indicates that 'git sparse-checkout init --sparse-index'\n>>> lacks some error checking and its tests lack coverage related to split\n>>> index, but fixing those issues (let alone making sparse index\n>>> comparible with split index) is beyond the scope of this patch series.\n>>\n>> s/comparible/compatible.\n>>\n>> I agree that making these two things compatible is not something to\n>> solve today. I'm not sure they should _ever_ be solved because of\n>> the complexity involved (what if the base index is not sparse but\n>> the tip wants to be, or vice-versa?,\n> \n> I think that this's not an issue, because a shared index file is not\n> for forever, but it's expected that a new shared index is written\n> every once in a while anyway, see splitIndex.maxPercentChange.  So\n> whenever sparse index gets enabled/disabled we could just write a new\n> shared index accordingly.  Maybe even when the sparse patterns change,\n> if necessary.\n> \n>> or if a directory must be expanded because of a conflict?).\n> \n> I'm not quite up-to-date in sparse index terminology, so I'm not sure\n> that this means...  but as mentioned above we can just write a new\n> shared index when deemed necessary.\n> \n>> They use very different approaches\n>> to solve a similar problem: how to deal with large index files.\n>>\n>> * The split index reduces index _write_ time by only editing a diff\n>>   of the base index.\n>>\n>> * The sparse index reduces index _read and write_ time by writing a\n>>   smaller index, but only if the user is using cone mode sparse-\n>>   checkout.\n> \n> Yeah, I think that in general there is more to be gained with sparse\n> index than with split index, though the split index might further\n> reduce the write time of a sparse index, because the amount of data\n> written is proportional with the nr of changed files instead of the nr\n> of all files in the sparse index.  I'm not sure that it's worth it,\n> either.\n> \n> My remarks about compatibility primarily stem from your remarks about\n> compatibility in response to my sparse vs. split test failure report\n> in:\n> \n>   https://public-inbox.org/git/48e9c3d6-407a-1843-2d91-22112410e3f8@gmail.com/\n> \n>   \"the sparse-index is (currently) incompatible with the split-index\"\n> \n> I assumed that that \"(currently)\" implies that eventually the two will\n> be made compatible.\n\nI say \"(currently)\" because there is nothing stopping someone from\nintegrating them. I'm not sure anyone ever will.\n\n> Anyway, I'm fine with leaving them incompatible, but 'git\n> sparse-checkout' should still learn about split index, so it won't\n> pretend to succeed after it couldn't do what it was asked to when\n> there is a split index.  Maybe vice-versa as well.\n\nYou're saying that when 'git sparse-checkout init --cone --sparse-index'\ndoesn't write a sparse index because the index is already split, we\nshould actually die() saying \"split index and sparse index are\nincompatible\" so the user knows why the sparse index was not enabled?\nThat seems reasonable for now, although it is successful in setting\nthe config to enable the sparse index opportunistically in the future.\n\n>>>  test_expect_success 'sparse-index enabled and disabled' '\n>>> -\tgit -C repo sparse-checkout init --cone --sparse-index &&\n>>> -\ttest_cmp_config -C repo true index.sparse &&\n>>> -\ttest-tool -C repo read-cache --table >cache &&\n>>> -\tgrep \" tree \" cache &&\n>>> -\n>>> -\tgit -C repo sparse-checkout disable &&\n>>> -\ttest-tool -C repo read-cache --table >cache &&\n>>> -\t! grep \" tree \" cache &&\n>>> -\tgit -C repo config --list >config &&\n>>> -\t! grep index.sparse config\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\ttest-tool -C repo read-cache --table >cache &&\n>>> +\t\tgrep \" tree \" cache &&\n>>> +\n>>> +\t\tgit -C repo sparse-checkout disable &&\n>>> +\t\ttest-tool -C repo read-cache --table >cache &&\n>>> +\t\t! grep \" tree \" cache &&\n>>> +\t\tgit -C repo config --list >config &&\n>>> +\t\t! grep index.sparse config\n>>> +\t)\n>>>  '\n>>\n>> This test is safe for now.\n>>\n>>>  test_expect_success 'status succeeds with sparse index' '\n>>\n>> This test is being edited in ds/sparse-index-ignored-files. v3\n>> of the relevant patch was just sent today [1].\n>>\n>> [1] https://lore.kernel.org/git/e66106f7a99d94145eec983ea5e72b7cf8a8a479.1629206603.git.gitgitgadget@gmail.com/\n>>\n>> You might want to rebase on top of that topic. The edits to\n>> the test are likely stable now.\n> \n> Oh, no :)  The test 'cone mode clears ignored subdirectories' added in\n> that patch series (as in 'seen', so not the newes version) fails with\n> the working GIT_TEST_SPLIT_INDEX=1 as well, and I don't immediately\n> see why, as it doesn't seem to use sparse index.\n \nOh interesting! This is because the index converts to a sparse index\nin-memory solely so it can use the sparse directory entries to pick\nwhich directories need to be deleted. In the presence of a split\nindex, convert_to_sparse() fails due to these lines:\n\n\tif (istate->split_index || istate->sparse_index || !istate->cache_nr ||\n\t    !core_apply_sparse_checkout || !core_sparse_checkout_cone)\n\t\treturn 0;\n\n\tif (!istate->repo)\n\t\tistate->repo = the_repository;\n\n\tif (!(flags & SPARSE_INDEX_IGNORE_CONFIG)) {\n\t\t...\n\nWhile this if here is checking the condition for creating it in-memory.\n\nThe fix is likely to do the following instead:\n\n\tif (istate->sparse_index || !istate->cache_nr ||\n\t    !core_apply_sparse_checkout || !core_sparse_checkout_cone)\n\t\treturn 0;\n\n\tif (!istate->repo)\n\t\tistate->repo = the_repository;\n\n\tif (!(flags & SPARSE_INDEX_IGNORE_CONFIG)) {\n\t\tif (istate->split_index)\n\t\t\treturn 0;\n\t\t...\n\nand maybe even rename the flag to SPARSE_INDEX_MEMORY_ONLY or something.\n\nThanks for alerting me to this issue!\n\n-Stolee\n"},{"id":"433872","messageId":"20210826210004.672860-1-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210817174938.3009923-1-szeder.dev@gmail.com","subject":"[PATCH v2 0/6] Fix GIT_TEST_SPLIT_INDEX","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-26T20:59:58Z","receivedAt":"2021-08-26T21:00:16Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"To recap from v1's cover letter:\n\nRunning tests with GIT_TEST_SPLIT_INDEX=1 is supposed to turn on the\nsplit index feature and trigger index splitting (mostly) randomly.\nAlas, this has been broken for ~2.5 years, and it hasn't triggered any\nindex splitting since then.\n\nThe last patch in this series makes GIT_TEST_SPLIT_INDEX=1 work again,\nalthough it slightly changes its behavior; see its log message for all\nthe details.\n\nPatches 3 and 4 fix old tests that started to fail because\nof this slight behavior change.  Bsides the final fix, patch 4 is the\nmost interesting in this series.\n\nPatch 5 fixes recent sparse index tests that were introduced while\nGIT_TEST_SPLIT_INDEX was broken.\n\nThe first two patches are just minor while-at-it test cleanups.\n\n\nChanges since v1, both in patch 5:\n  - Removed a remark about making split and sparse index compatible.\n  - Stolle suggested to rebase this patch series on top of topic\n    'ds/sparse-index-ignored-files' to avoid merge conflicts in a\n    sparse index test.  Well, I didn't do that, but instead I rebased\n    this series on top of the merge of master and 86d6ca5886 (t7519:\n    rewrite sparse index test, 2021-08-10), and the latter commit:\n      - is the one responsible for said merge conflicts, and\n      - is at the bottom of 'ds/sparse-index-ignored-files', and could\n\tbe considered more as a followup fix for topic\n\t'ds/status-with-sparse-index' (already in v2.33.0) than an\n\tintegral part of 'ds/sparse-index-ignored-files'.\n    The rest of 'ds/sparse-index-ignored-files' doesn't have any\n    conflicts with this patch series that auto-merge couldn't resolve.\n\nSZEDER Gábor (6):\n  t1600-index: remove unnecessary redirection\n  t1600-index: don't run git commands upstream of a pipe\n  t1600-index: disable GIT_TEST_SPLIT_INDEX\n  read-cache: look for shared index files next to the index, too\n  tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests\n  read-cache: fix GIT_TEST_SPLIT_INDEX\n\n read-cache.c                       | 37 +++++++++++-----\n t/t1091-sparse-checkout-builtin.sh | 25 ++++++-----\n t/t1600-index.sh                   | 13 ++++--\n t/t1700-split-index.sh             | 34 +++++++++++++++\n t/t7519-status-fsmonitor.sh        | 68 ++++++++++++++++--------------\n 5 files changed, 121 insertions(+), 56 deletions(-)\n\nRange-diff against v1:\n1:  7e1d38d215 = 1:  ab555030cb t1600-index: remove unnecessary redirection\n2:  d86c91d26e = 2:  38d350b02d t1600-index: don't run git commands upstream of a pipe\n3:  792b6739e5 = 3:  57b5386aee t1600-index: disable GIT_TEST_SPLIT_INDEX\n4:  b12467bab7 = 4:  58c444bcd0 read-cache: look for shared index files next to the index, too\n5:  51e13df905 ! 5:  24b8b7738c tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests\n    @@ Commit message\n         by later sanity checks finding that the index is not in fact a sparse\n         index.  This indicates that 'git sparse-checkout init --sparse-index'\n         lacks some error checking and its tests lack coverage related to split\n    -    index, but fixing those issues (let alone making sparse index\n    -    comparible with split index) is beyond the scope of this patch series.\n    +    index, but fixing those issues is beyond the scope of this patch\n    +    series.\n     \n         [1] https://public-inbox.org/git/48e9c3d6-407a-1843-2d91-22112410e3f8@gmail.com/\n     \n    @@ t/t7519-status-fsmonitor.sh: check_sparse_index_behavior () {\n      }\n      \n      test_expect_success 'status succeeds with sparse index' '\n\n  I took the liberty to cut the part of the range-diff showing the\n  changes of changes to this test, because 86d6ca5886 totally rewrites\n  this test, while this patch changes the test's indentation,\n  resulting in an unusable range-diff.\n  Instead, to show the only relevant change in this patch here is the\n  first hunk touching this test with --ignore-all-space from v1:\n\n    @@ -402,6 +402,10 @@ check_sparse_index_behavior () {\n     }\n    \n     test_expect_success 'status succeeds with sparse index' '\n    +       (\n    +               sane_unset GIT_TEST_SPLIT_INDEX &&\n    +               git update-index --no-split-index &&\n    +\n                    git reset --hard &&\n    \n                    test_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n\n  And from v2:\n\n    @@ -399,6 +399,9 @@ check_sparse_index_behavior () {\n     }\n    \n     test_expect_success 'status succeeds with sparse index' '\n    +       (\n    +               sane_unset GIT_TEST_SPLIT_INDEX &&\n    +\n                    git clone . full &&\n                    git clone . sparse &&\n                    git -C sparse sparse-checkout init --cone --sparse-index &&\n\n6:  a5c790e622 = 6:  a43a12b0ce read-cache: fix GIT_TEST_SPLIT_INDEX\n-- \n2.33.0.358.g803110d36e\n\n"},{"id":"433873","messageId":"20210826210004.672860-2-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210826210004.672860-1-szeder.dev@gmail.com","subject":"[PATCH v2 1/6] t1600-index: remove unnecessary redirection","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-26T20:59:59Z","receivedAt":"2021-08-26T21:00:17Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"In a helper function in the 't1600-index.sh' test script the stderr\nof a 'git add' command is redirected to its stdout, but its stdout is\nnot redirected anywhere.  So apparently this redirection is\nunnecessary, remove it.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t1600-index.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1600-index.sh b/t/t1600-index.sh\nindex c9b9e718b8..5d10cec67a 100755\n--- a/t/t1600-index.sh\n+++ b/t/t1600-index.sh\n@@ -79,7 +79,7 @@ test_index_version () {\n \t\telse\n \t\t\tunset GIT_INDEX_VERSION\n \t\tfi &&\n-\t\tgit add a 2>&1 &&\n+\t\tgit add a &&\n \t\techo $EXPECTED_OUTPUT_VERSION >expect &&\n \t\ttest-tool index-version <.git/index >actual &&\n \t\ttest_cmp expect actual\n-- \n2.33.0.358.g803110d36e\n\n"},{"id":"433874","messageId":"20210826210004.672860-3-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210826210004.672860-1-szeder.dev@gmail.com","subject":"[PATCH v2 2/6] t1600-index: don't run git commands upstream of a pipe","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-26T21:00:00Z","receivedAt":"2021-08-26T21:00:18Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t1600-index.sh | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1600-index.sh b/t/t1600-index.sh\nindex 5d10cec67a..aa88fc30ce 100755\n--- a/t/t1600-index.sh\n+++ b/t/t1600-index.sh\n@@ -13,7 +13,8 @@ test_expect_success 'bogus GIT_INDEX_VERSION issues warning' '\n \t\trm -f .git/index &&\n \t\tGIT_INDEX_VERSION=2bogus &&\n \t\texport GIT_INDEX_VERSION &&\n-\t\tgit add a 2>&1 | sed \"s/[0-9]//\" >actual.err &&\n+\t\tgit add a 2>err &&\n+\t\tsed \"s/[0-9]//\" err >actual.err &&\n \t\tsed -e \"s/ Z$/ /\" <<-\\EOF >expect.err &&\n \t\t\twarning: GIT_INDEX_VERSION set, but the value is invalid.\n \t\t\tUsing version Z\n@@ -27,7 +28,8 @@ test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '\n \t\trm -f .git/index &&\n \t\tGIT_INDEX_VERSION=1 &&\n \t\texport GIT_INDEX_VERSION &&\n-\t\tgit add a 2>&1 | sed \"s/[0-9]//\" >actual.err &&\n+\t\tgit add a 2>err &&\n+\t\tsed \"s/[0-9]//\" err >actual.err &&\n \t\tsed -e \"s/ Z$/ /\" <<-\\EOF >expect.err &&\n \t\t\twarning: GIT_INDEX_VERSION set, but the value is invalid.\n \t\t\tUsing version Z\n@@ -50,7 +52,8 @@ test_expect_success 'out of bounds index.version issues warning' '\n \t\tsane_unset GIT_INDEX_VERSION &&\n \t\trm -f .git/index &&\n \t\tgit config --add index.version 1 &&\n-\t\tgit add a 2>&1 | sed \"s/[0-9]//\" >actual.err &&\n+\t\tgit add a 2>err &&\n+\t\tsed \"s/[0-9]//\" err >actual.err &&\n \t\tsed -e \"s/ Z$/ /\" <<-\\EOF >expect.err &&\n \t\t\twarning: index.version set, but the value is invalid.\n \t\t\tUsing version Z\n-- \n2.33.0.358.g803110d36e\n\n"},{"id":"433875","messageId":"20210826210004.672860-4-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210826210004.672860-1-szeder.dev@gmail.com","subject":"[PATCH v2 3/6] t1600-index: disable GIT_TEST_SPLIT_INDEX","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-26T21:00:01Z","receivedAt":"2021-08-26T21:00:20Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Tests in 't1600-index.sh' check that various bogus index version\nvalues are recognized and an appropriate warning message is issued.\nGIT_TEST_SPLIT_INDEX=1 is supposed to trigger index splitting\nrandomly, and thus might interfere [1] with these tests: splitting the\nindex means that two index files are written (the shared base index\nand the split '.git/index'), and the same warning message is then\nissued twice, failing these tests.\n\nUnset GIT_TEST_SPLIT_INDEX in this test script to avoid such\ninterference.\n\n[1] There is no such interference at the moment, because, alas,\n    GIT_TEST_SPLIT_INDEX=1 is broken and never split the index.  There\n    was no such interference in the past (before it broke) either,\n    because the first index write with GIT_TEST_SPLIT_INDEX=1 never\n    split the index, only the second write did.  A subsequent commit\n    fixing GIT_TEST_SPLIT_INDEX will have all the details on this.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t1600-index.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/t1600-index.sh b/t/t1600-index.sh\nindex aa88fc30ce..46329c488b 100755\n--- a/t/t1600-index.sh\n+++ b/t/t1600-index.sh\n@@ -4,6 +4,8 @@ test_description='index file specific tests'\n \n . ./test-lib.sh\n \n+sane_unset GIT_TEST_SPLIT_INDEX\n+\n test_expect_success 'setup' '\n \techo 1 >a\n '\n-- \n2.33.0.358.g803110d36e\n\n"},{"id":"433876","messageId":"20210826210004.672860-5-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210826210004.672860-1-szeder.dev@gmail.com","subject":"[PATCH v2 4/6] read-cache: look for shared index files next to the index, too","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-26T21:00:02Z","receivedAt":"2021-08-26T21:00:21Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"When reading a split index git always looks for its referenced shared\nbase index in the gitdir of the current repository, even when reading\nan alternate index specified via GIT_INDEX_FILE, and even when that\nalternate index file is the \"main\" '.git/index' file of an other\nrepository.  However, if that split index and its referenced shared\nindex files were written by a git command running entirely in that\nother repository, then, naturally, the shared index file is written to\nthat other repository's gitdir.  Consequently, a git command\nattempting to read that shared index file while running in a different\nrepository won't be able find it and will error out.\n\nI'm not sure in what use case it is necessary to read the index of one\nrepository by a git command running in a different repository, but it\nis certainly possible to do so, and in fact the test 'bare repository:\ncheck that --cached honors index' in 't0003-attributes.sh' does\nexactly that.  If GIT_TEST_SPLIT_INDEX=1 were to split the index in\njust the right moment [1], then this test would indeed fail, because\nthe referenced shared index file could not be found.\n\nLet's look for the referenced shared index file not only in the gitdir\nof the current directory, but, if the shared index is not there, right\nnext to the split index as well.\n\n[1] We haven't seen this issue trigger a failure in t0003 yet,\n    because:\n\n      - While GIT_TEST_SPLIT_INDEX=1 is supposed to trigger index\n        splitting randomly, the first index write has always been\n        deterministic and it has never split the index.\n\n      - That alternate index file in the other repository is written\n        only once in the entire test script, so it's never split.\n\n    However, the next patch will fix GIT_TEST_SPLIT_INDEX, and while\n    doing so it will slightly change its behavior to always split the\n    index already on the first index write, and t0003 would always\n    fail without this patch.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n read-cache.c           | 14 +++++++++++++-\n t/t1700-split-index.sh | 23 +++++++++++++++++++++++\n 2 files changed, 36 insertions(+), 1 deletion(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 9048ef9e90..fbd59886a3 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2391,9 +2391,21 @@ int read_index_from(struct index_state *istate, const char *path,\n \tbase_path = xstrfmt(\"%s/sharedindex.%s\", gitdir, base_oid_hex);\n \ttrace2_region_enter_printf(\"index\", \"shared/do_read_index\",\n \t\t\t\t   the_repository, \"%s\", base_path);\n-\tret = do_read_index(split_index->base, base_path, 1);\n+\tret = do_read_index(split_index->base, base_path, 0);\n \ttrace2_region_leave_printf(\"index\", \"shared/do_read_index\",\n \t\t\t\t   the_repository, \"%s\", base_path);\n+\tif (!ret) {\n+\t\tchar *path_copy = xstrdup(path);\n+\t\tconst char *base_path2 = xstrfmt(\"%s/sharedindex.%s\",\n+\t\t\t\t\t\t dirname(path_copy),\n+\t\t\t\t\t\t base_oid_hex);\n+\t\tfree(path_copy);\n+\t\ttrace2_region_enter_printf(\"index\", \"shared/do_read_index\",\n+\t\t\t\t\t   the_repository, \"%s\", base_path2);\n+\t\tret = do_read_index(split_index->base, base_path2, 1);\n+\t\ttrace2_region_leave_printf(\"index\", \"shared/do_read_index\",\n+\t\t\t\t\t   the_repository, \"%s\", base_path2);\n+\t}\n \tif (!oideq(&split_index->base_oid, &split_index->base->oid))\n \t\tdie(_(\"broken index, expect %s in %s, got %s\"),\n \t\t    base_oid_hex, base_path,\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex 986baa612e..e2aa0bd949 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -510,4 +510,27 @@ test_expect_success 'do not refresh null base index' '\n \t)\n '\n \n+test_expect_success 'reading split index at alternate location' '\n+\tgit init reading-alternate-location &&\n+\t(\n+\t\tcd reading-alternate-location &&\n+\t\t>file-in-alternate &&\n+\t\tgit update-index --split-index --add file-in-alternate\n+\t) &&\n+\techo file-in-alternate >expect &&\n+\n+\t# Should be able to find the shared index both right next to\n+\t# the specified split index file ...\n+\tGIT_INDEX_FILE=./reading-alternate-location/.git/index \\\n+\tgit ls-files --cached >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# ... and, for backwards compatibility, in the current GIT_DIR\n+\t# as well.\n+\tmv -v ./reading-alternate-location/.git/sharedindex.* .git &&\n+\tGIT_INDEX_FILE=./reading-alternate-location/.git/index \\\n+\tgit ls-files --cached >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.33.0.358.g803110d36e\n\n"},{"id":"433877","messageId":"20210826210004.672860-7-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210826210004.672860-1-szeder.dev@gmail.com","subject":"[PATCH v2 6/6] read-cache: fix GIT_TEST_SPLIT_INDEX","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-26T21:00:04Z","receivedAt":"2021-08-26T21:00:22Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Running tests with GIT_TEST_SPLIT_INDEX=1 is supposed to turn on the\nsplit index feature and trigger index splitting (mostly) randomly.\nAlas, this has been broken since 6e37c8ed3c (read-cache.c: fix writing\n\"link\" index ext with null base oid, 2019-02-13), and\nGIT_TEST_SPLIT_INDEX=1 hasn't triggered any index splitting since\nthen.\n\nThis patch makes GIT_TEST_SPLIT_INDEX work again, though it doesn't\nrestore the pre-6e37c8ed3c behavior.  To understand the bug, the fix,\nand the behavior change we first have to look at how\nGIT_TEST_SPLIT_INDEX used to work before 6e37c8ed3c:\n\n  There are two places where we check the value of\n  GIT_TEST_SPLIT_INDEX, and before 6e37c8ed3c they worked like this:\n\n    1) In the lower-level do_write_index(), where, if\n       GIT_TEST_SPLIT_INDEX is enabled, we call init_split_index().\n       This call merely allocates and zero-initializes\n       'istate->split_index', but does nothing else (i.e. doesn't fill\n       the base/shared index with cache entries, doesn't actually\n       write a shared index file, etc.).  Pertinent to this issue, the\n       hash of the base index remains all zeroed out.\n\n    2) In the higher-level write_locked_index(), but only when\n       'istate->split_index' has already been initialized.  Then, if\n       GIT_TEST_SPLIT_INDEX is enabled, it randomly sets the flag that\n       triggers index splitting later in this function.  This\n       randomness comes from the first byte of the hash of the base\n       index via an 'if ((first_byte & 15) < 6)' condition.\n\n       However, if 'istate->split_index' hasn't been initialized (i.e.\n       it is still NULL), then write_locked_index() just calls\n       do_write_locked_index(), which internally calls the above\n       mentioned do_write_index().\n\n  This means that while GIT_TEST_SPLIT_INDEX=1 usually triggered index\n  splitting randomly, the first two index writes were always\n  deterministic (though I suspect this was unintentional):\n\n    - The initial index write never splits the index.\n      During the first index write write_locked_index() is called with\n      'istate->split_index' still uninitialized, so the check in 2) is\n      not executed.  It still calls do_write_index(), though, which\n      then executes the check in 1).  The resulting all zero base\n      index hash then leads to the 'link' extension being written to\n      '.git/index', though a shared index file is not written:\n\n        $ rm .git/index\n        $ GIT_TEST_SPLIT_INDEX=1 git update-index --add file\n        $ test-tool dump-split-index .git/index\n        own c6ef71168597caec8553c83d9d0048f1ef416170\n        base 0000000000000000000000000000000000000000\n        100644 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 0 file\n        replacements:\n        deletions:\n        $ ls -l .git/sharedindex.*\n        ls: cannot access '.git/sharedindex.*': No such file or directory\n\n    - The second index write always splits the index.\n      When the index written in the previous point is read,\n      'istate->split_index' is initialized because of the presence of\n      the 'link' extension.  So during the second write\n      write_locked_index() does run the check in 2), and the first\n      byte of the all zero base index hash always fulfills the\n      randomness condition, which in turn always triggers the index\n      splitting.\n\n    - Subsequent index writes will find the 'link' extension with a\n      real non-zero base index hash, so from then on the check in 2)\n      is executed and the first byte of the base index hash is as\n      random as it gets (coming from the SHA-1 of index data including\n      timestamps and inodes...).\n\nAll this worked until 6e37c8ed3c came along, and stopped writing the\n'link' extension if the hash of the base index was all zero:\n\n  $ rm .git/index\n  $ GIT_TEST_SPLIT_INDEX=1 git update-index --add file\n  $ test-tool dump-split-index .git/index\n  own abbd6f6458d5dee73ae8e210ca15a68a390c6fd7\n  not a split index\n  $ ls -l .git/sharedindex.*\n  ls: cannot access '.git/sharedindex.*': No such file or directory\n\nSo, since the first index write with GIT_TEST_SPLIT_INDEX=1 doesn't\nwrite a 'link' extension, in the second index write\n'istate->split_index' remains uninitialized, and the check in 2) is\nnot executed, and ultimately the index is never split.\n\nFix this by modifying write_locked_index() to make sure to check\nGIT_TEST_SPLIT_INDEX even if 'istate->split_index' is still\nuninitialized, and initialize it if necessary.  The check for\nGIT_TEST_SPLIT_INDEX and separate init_split_index() call in\ndo_write_index() thus becomes unnecessary, so remove it.  Furthermore,\nadd a test to 't1700-split-index.sh' to make sure that\nGIT_TEST_SPLIT_INDEX=1 will keep working (though only check the\nindex splitting on the first index write, because after that it will\nbe random).\n\nNote that this change does not restore the pre-6e37c8ed3c behaviour,\nas it will deterministically split the index already on the first\nindex write.  Since GIT_TEST_SPLIT_INDEX is purely a developer aid,\nthere is no backwards compatibility issue here.  The new behaviour did\ntrigger test failures in 't0003-attributes.sh' and 't1600-index.sh',\nthough, which have been fixed in preparatory patches in this series.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n read-cache.c           | 23 ++++++++++++++---------\n t/t1700-split-index.sh | 11 +++++++++++\n 2 files changed, 25 insertions(+), 9 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex fbd59886a3..335242c1cf 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2824,11 +2824,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\t}\n \t}\n \n-\tif (!istate->version) {\n+\tif (!istate->version)\n \t\tistate->version = get_index_format_default(the_repository);\n-\t\tif (git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0))\n-\t\t\tinit_split_index(istate);\n-\t}\n \n \t/* demote version 3 to version 2 when the latter suffices */\n \tif (istate->version == 3 || istate->version == 2)\n@@ -3255,7 +3252,7 @@ static int too_many_not_shared_entries(struct index_state *istate)\n int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\t       unsigned flags)\n {\n-\tint new_shared_index, ret;\n+\tint new_shared_index, ret, test_split_index_env;\n \tstruct split_index *si = istate->split_index;\n \n \tif (git_env_bool(\"GIT_TEST_CHECK_CACHE_TREE\", 0))\n@@ -3270,7 +3267,10 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \tif (istate->fsmonitor_last_update)\n \t\tfill_fsmonitor_bitmap(istate);\n \n-\tif (!si || alternate_index_output ||\n+\ttest_split_index_env = git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0);\n+\n+\tif ((!si && !test_split_index_env) ||\n+\t    alternate_index_output ||\n \t    (istate->cache_changed & ~EXTMASK)) {\n \t\tif (si)\n \t\t\toidclr(&si->base_oid);\n@@ -3278,10 +3278,15 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\tgoto out;\n \t}\n \n-\tif (git_env_bool(\"GIT_TEST_SPLIT_INDEX\", 0)) {\n-\t\tint v = si->base_oid.hash[0];\n-\t\tif ((v & 15) < 6)\n+\tif (test_split_index_env) {\n+\t\tif (!si) {\n+\t\t\tsi = init_split_index(istate);\n \t\t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\n+\t\t} else {\n+\t\t\tint v = si->base_oid.hash[0];\n+\t\t\tif ((v & 15) < 6)\n+\t\t\t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\n+\t\t}\n \t}\n \tif (too_many_not_shared_entries(istate))\n \t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex e2aa0bd949..decd2527ed 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -533,4 +533,15 @@ test_expect_success 'reading split index at alternate location' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'GIT_TEST_SPLIT_INDEX works' '\n+\tgit init git-test-split-index &&\n+\t(\n+\t\tcd git-test-split-index &&\n+\t\t>file &&\n+\t\tGIT_TEST_SPLIT_INDEX=1 git update-index --add file &&\n+\t\tls -l .git/sharedindex.* >actual &&\n+\t\ttest_line_count = 1 actual\n+\t)\n+'\n+\n test_done\n-- \n2.33.0.358.g803110d36e\n\n"},{"id":"433878","messageId":"20210826210004.672860-6-szeder.dev@gmail.com","threadId":"56314","inReplyTo":"20210826210004.672860-1-szeder.dev@gmail.com","subject":"[PATCH v2 5/6] tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-26T21:00:03Z","receivedAt":"2021-08-26T21:00:23Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The sparse index and split index features are said to be currently\nincompatible [1], and consequently GIT_TEST_SPLIT_INDEX=1 might\ninterfere with the test cases exercising the sparse index feature.\nTherefore GIT_TEST_SPLIT_INDEX is already explicitly disabled for the\nwhole of 't1092-sparse-checkout-compatibility.sh'.  There are,\nhowever, two other test cases exercising sparse index, namely\n'sparse-index enabled and disabled' in\n't1091-sparse-checkout-builtin.sh' and 'status succeeds with sparse\nindex' in 't7519-status-fsmonitor.sh', and these two could fail with\nGIT_TEST_SPLIT_INDEX=1 as well [2].\n\nUnset GIT_TEST_SPLIT_INDEX and disable the split index in these two\ntest cases to avoid such interference.\n\nNote that this is the minimal change to merely avoid failures when\nthese test cases are run with GIT_TEST_SPLIT_INDEX=1.  Interestingly,\nthough, without these changes the 'git sparse-checkout init --cone\n--sparse-index' commands still succeed even with split index, and set\nall the necessary configuration variables and create the initial\n'$GIT_DIR/info/sparse-checkout' file, but the test failures are caused\nby later sanity checks finding that the index is not in fact a sparse\nindex.  This indicates that 'git sparse-checkout init --sparse-index'\nlacks some error checking and its tests lack coverage related to split\nindex, but fixing those issues is beyond the scope of this patch\nseries.\n\n[1] https://public-inbox.org/git/48e9c3d6-407a-1843-2d91-22112410e3f8@gmail.com/\n\n[2] Neither of these test cases fail at the moment, because\n    GIT_TEST_SPLIT_INDEX=1 is broken and never splits the index, and\n    it broke long before the sparse index feature was added.\n    This patch series is about to fix GIT_TEST_SPLIT_INDEX, and then\n    both test cases mentioned above would fail.\n\n(The diff is best viewed with '--ignore-all-space')\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t1091-sparse-checkout-builtin.sh | 25 ++++++-----\n t/t7519-status-fsmonitor.sh        | 68 ++++++++++++++++--------------\n 2 files changed, 51 insertions(+), 42 deletions(-)\n\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex 38fc8340f5..3f94c41241 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -206,16 +206,21 @@ test_expect_success 'sparse-checkout disable' '\n '\n \n test_expect_success 'sparse-index enabled and disabled' '\n-\tgit -C repo sparse-checkout init --cone --sparse-index &&\n-\ttest_cmp_config -C repo true index.sparse &&\n-\ttest-tool -C repo read-cache --table >cache &&\n-\tgrep \" tree \" cache &&\n-\n-\tgit -C repo sparse-checkout disable &&\n-\ttest-tool -C repo read-cache --table >cache &&\n-\t! grep \" tree \" cache &&\n-\tgit -C repo config --list >config &&\n-\t! grep index.sparse config\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\ttest-tool -C repo read-cache --table >cache &&\n+\t\tgrep \" tree \" cache &&\n+\n+\t\tgit -C repo sparse-checkout disable &&\n+\t\ttest-tool -C repo read-cache --table >cache &&\n+\t\t! grep \" tree \" cache &&\n+\t\tgit -C repo config --list >config &&\n+\t\t! grep index.sparse config\n+\t)\n '\n \n test_expect_success 'cone mode: init and set' '\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 6f2cf306f6..451734b9c2 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -399,41 +399,45 @@ check_sparse_index_behavior () {\n }\n \n test_expect_success 'status succeeds with sparse index' '\n-\tgit clone . full &&\n-\tgit clone . sparse &&\n-\tgit -C sparse sparse-checkout init --cone --sparse-index &&\n-\tgit -C sparse sparse-checkout set dir1 dir2 &&\n+\t(\n+\t\tsane_unset GIT_TEST_SPLIT_INDEX &&\n \n-\twrite_script .git/hooks/fsmonitor-test <<-\\EOF &&\n-\t\tprintf \"last_update_token\\0\"\n-\tEOF\n-\tgit -C full config core.fsmonitor ../.git/hooks/fsmonitor-test &&\n-\tgit -C sparse config core.fsmonitor ../.git/hooks/fsmonitor-test &&\n-\tcheck_sparse_index_behavior ! &&\n+\t\tgit clone . full &&\n+\t\tgit clone . sparse &&\n+\t\tgit -C sparse sparse-checkout init --cone --sparse-index &&\n+\t\tgit -C sparse sparse-checkout set dir1 dir2 &&\n \n-\twrite_script .git/hooks/fsmonitor-test <<-\\EOF &&\n-\t\tprintf \"last_update_token\\0\"\n-\t\tprintf \"dir1/modified\\0\"\n-\tEOF\n-\tcheck_sparse_index_behavior ! &&\n-\n-\tgit -C sparse sparse-checkout add dir1a &&\n+\t\twrite_script .git/hooks/fsmonitor-test <<-\\EOF &&\n+\t\t\tprintf \"last_update_token\\0\"\n+\t\tEOF\n+\t\tgit -C full config core.fsmonitor ../.git/hooks/fsmonitor-test &&\n+\t\tgit -C sparse config core.fsmonitor ../.git/hooks/fsmonitor-test &&\n+\t\tcheck_sparse_index_behavior ! &&\n \n-\tfor repo in full sparse\n-\tdo\n-\t\tcp -r $repo/dir1 $repo/dir1a &&\n-\t\tgit -C $repo add dir1a &&\n-\t\tgit -C $repo commit -m \"add dir1a\" || return 1\n-\tdone &&\n-\tgit -C sparse sparse-checkout set dir1 dir2 &&\n-\n-\t# This one modifies outside the sparse-checkout definition\n-\t# and hence we expect to expand the sparse-index.\n-\twrite_script .git/hooks/fsmonitor-test <<-\\EOF &&\n-\t\tprintf \"last_update_token\\0\"\n-\t\tprintf \"dir1a/modified\\0\"\n-\tEOF\n-\tcheck_sparse_index_behavior\n+\t\twrite_script .git/hooks/fsmonitor-test <<-\\EOF &&\n+\t\t\tprintf \"last_update_token\\0\"\n+\t\t\tprintf \"dir1/modified\\0\"\n+\t\tEOF\n+\t\tcheck_sparse_index_behavior ! &&\n+\n+\t\tgit -C sparse sparse-checkout add dir1a &&\n+\n+\t\tfor repo in full sparse\n+\t\tdo\n+\t\t\tcp -r $repo/dir1 $repo/dir1a &&\n+\t\t\tgit -C $repo add dir1a &&\n+\t\t\tgit -C $repo commit -m \"add dir1a\" || return 1\n+\t\tdone &&\n+\t\tgit -C sparse sparse-checkout set dir1 dir2 &&\n+\n+\t\t# This one modifies outside the sparse-checkout definition\n+\t\t# and hence we expect to expand the sparse-index.\n+\t\twrite_script .git/hooks/fsmonitor-test <<-\\EOF &&\n+\t\t\tprintf \"last_update_token\\0\"\n+\t\t\tprintf \"dir1a/modified\\0\"\n+\t\tEOF\n+\t\tcheck_sparse_index_behavior\n+\t)\n '\n \n test_done\n-- \n2.33.0.358.g803110d36e\n\n"},{"id":"434289","messageId":"885bc067-adef-6d23-b867-017cf1b72d25@gmail.com","threadId":"56314","inReplyTo":"20210826210004.672860-1-szeder.dev@gmail.com","subject":"Re: [PATCH v2 0/6] Fix GIT_TEST_SPLIT_INDEX","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-31T14:47:41Z","receivedAt":"2021-08-31T14:47:46Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/26/2021 4:59 PM, SZEDER Gábor wrote:\n> To recap from v1's cover letter:\n> \n> Running tests with GIT_TEST_SPLIT_INDEX=1 is supposed to turn on the\n> split index feature and trigger index splitting (mostly) randomly.\n> Alas, this has been broken for ~2.5 years, and it hasn't triggered any\n> index splitting since then.\n> \n> The last patch in this series makes GIT_TEST_SPLIT_INDEX=1 work again,\n> although it slightly changes its behavior; see its log message for all\n> the details.\n\nI checked the range-diff and reread the patches. This version looks\ngood to me. Thanks for pointing out the new failures that were\nhappening in my patches. I think I fixed them in my latest versions\nof the sparse-index work, so the latest 'seen' should pass with these\nchanges to GIT_TEST_SPLIT_INDEX.\n\nThanks,\n-Stolee\n"},{"id":"434309","messageId":"xmqqczptv7ik.fsf@gitster.g","threadId":"56314","inReplyTo":"885bc067-adef-6d23-b867-017cf1b72d25@gmail.com","subject":"Re: [PATCH v2 0/6] Fix GIT_TEST_SPLIT_INDEX","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-31T17:38:59Z","receivedAt":"2021-08-31T17:39:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> On 8/26/2021 4:59 PM, SZEDER Gábor wrote:\n>> To recap from v1's cover letter:\n>> \n>> Running tests with GIT_TEST_SPLIT_INDEX=1 is supposed to turn on the\n>> split index feature and trigger index splitting (mostly) randomly.\n>> Alas, this has been broken for ~2.5 years, and it hasn't triggered any\n>> index splitting since then.\n>> \n>> The last patch in this series makes GIT_TEST_SPLIT_INDEX=1 work again,\n>> although it slightly changes its behavior; see its log message for all\n>> the details.\n>\n> I checked the range-diff and reread the patches. This version looks\n> good to me. Thanks for pointing out the new failures that were\n> happening in my patches. I think I fixed them in my latest versions\n> of the sparse-index work, so the latest 'seen' should pass with these\n> changes to GIT_TEST_SPLIT_INDEX.\n>\n> Thanks,\n> -Stolee\n\n\nThis, at least the t7519-status-fsmonitor.sh part, seems to depend\non the 'ds/sparse-index-ignored-files' topic, so I'll prepare a\nmerge of the topic on top of 'master' then queue these with your\nAcked-by.\n\nThanks, both.\n"}]}