* [PATCH 0/3] [Non-critical]: sparse index vs split index fixes @ 2022-01-19 17:29 Johannes Schindelin via GitGitGadget 2022-01-19 17:29 ` [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active Johannes Schindelin via GitGitGadget ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2022-01-19 17:29 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin We noticed split/sparse index-related issues while rebasing Microsoft's fork of Git. These fixes are necessary for that fork's test suite to pass, but they might not be super critical to get into upstream v2.35.0 (especially this close to -rc2). However, the team felt that the decision should be left to the Git maintainer whether to take these patches into v2.35.0 or not. Johannes Schindelin (3): sparse-index: sparse index is disallowed when split index is active t1091: disable split index split-index: it really is incompatible with the sparse index read-cache.c | 3 ++ sparse-index.c | 2 +- split-index.c | 3 ++ t/t1091-sparse-checkout-builtin.sh | 54 ++++++++++++++---------------- 4 files changed, 33 insertions(+), 29 deletions(-) base-commit: af4e5f569bc89f356eb34a9373d7f82aca6faa8a Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1119%2Fdscho%2Fsparse-index-vs-split-index-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1119/dscho/sparse-index-vs-split-index-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1119 -- gitgitgadget ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active 2022-01-19 17:29 [PATCH 0/3] [Non-critical]: sparse index vs split index fixes Johannes Schindelin via GitGitGadget @ 2022-01-19 17:29 ` Johannes Schindelin via GitGitGadget 2022-01-22 1:42 ` Junio C Hamano 2022-01-19 17:29 ` [PATCH 2/3] t1091: disable split index Johannes Schindelin via GitGitGadget ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2022-01-19 17:29 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> In 6e773527b6b (sparse-index: convert from full to sparse, 2021-03-30), we introduced initial support for a sparse index, and were careful to avoid converting to a sparse index in the presence of a split index. However, when we _just_ read a freshly-initialized index, it might not contain a split index even if _writing_ it will add one by virtue of being asked for via the `GIT_TEST_SPLIT_INDEX` variable. We did not notice any problems with checking _only_ for `split_index` (and not `GIT_TEST_SPLIT_INDEX`) right until both `vd/sparse-sparsity-fix-on-read` _and_ `vd/sparse-reset` were merged. Those two topics' interplay triggers a bug in conjunction with running t1091.15 when `GIT_TEST_SPLIT_INDEX=true` in the following way: `vd/sparse-sparsity-fix-on-read` ensures that the index is made sparse right after reading, and `vd/sparse-reset` ensures that the index is made non-sparse again unless running in the `--soft` mode. Since the split index feature is incompatible with the sparse index feature, we see a symptom like this: fatal: position for replacement 4 exceeds base index size 4 Let's fix this by avoiding the conversion to a sparse index when `GIT_TEST_SPLIT_INDEX=true`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- sparse-index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sparse-index.c b/sparse-index.c index a1d505d50e9..08f54747bb4 100644 --- a/sparse-index.c +++ b/sparse-index.c @@ -136,7 +136,7 @@ static int is_sparse_index_allowed(struct index_state *istate, int flags) /* * The sparse index is not (yet) integrated with a split index. */ - if (istate->split_index) + if (istate->split_index || git_env_bool("GIT_TEST_SPLIT_INDEX", 0)) return 0; /* * The GIT_TEST_SPARSE_INDEX environment variable triggers the -- gitgitgadget ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active 2022-01-19 17:29 ` [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active Johannes Schindelin via GitGitGadget @ 2022-01-22 1:42 ` Junio C Hamano 2022-01-22 9:30 ` Johannes Schindelin 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2022-01-22 1:42 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > In 6e773527b6b (sparse-index: convert from full to sparse, 2021-03-30), > we introduced initial support for a sparse index, and were careful to > avoid converting to a sparse index in the presence of a split index. > > However, when we _just_ read a freshly-initialized index, it might not > contain a split index even if _writing_ it will add one by virtue of > being asked for via the `GIT_TEST_SPLIT_INDEX` variable. > > We did not notice any problems with checking _only_ for `split_index` > (and not `GIT_TEST_SPLIT_INDEX`) right until both > `vd/sparse-sparsity-fix-on-read` _and_ `vd/sparse-reset` were merged. > > Those two topics' interplay triggers a bug in conjunction with running > t1091.15 when `GIT_TEST_SPLIT_INDEX=true` in the following way: > `vd/sparse-sparsity-fix-on-read` ensures that the index is made sparse > right after reading, and `vd/sparse-reset` ensures that the index is > made non-sparse again unless running in the `--soft` mode. Since the > split index feature is incompatible with the sparse index feature, we > see a symptom like this: > > fatal: position for replacement 4 exceeds base index size 4 > > Let's fix this by avoiding the conversion to a sparse index when > `GIT_TEST_SPLIT_INDEX=true`. Does [2/3] allow you to sidestep that issue entirely by skipping 1091 altogether? There are 4 hits of "if (istate->split_index" in the codebase, and this patch makes me wonder why it is suffice to patch only one of them. I also wondered why we test both split_index and environment separately, instead of splitting the index very early when the environment variable is set, so that the rest of the runtime does not have to worry about the environment, but is the reason why such an approach was not taken was because GIT_TEST_SPLIT_INDEX can later allow the index to be splitted, even if istate->split_index is still NULL right now when this function is called? If that is the reason, it leads to another question. Even if we ignore GIT_TEST_SPLIT_INDEX and concentrate only on real workload, if the in-core index can be NULL when this function is called and then later can become split, there still must be somebody who notices that sparse index is unallowed when (or after) such a split happens, no? If there is no such code, then this patch would not be the whole fix and there needs more change to do so, no? > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > sparse-index.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sparse-index.c b/sparse-index.c > index a1d505d50e9..08f54747bb4 100644 > --- a/sparse-index.c > +++ b/sparse-index.c > @@ -136,7 +136,7 @@ static int is_sparse_index_allowed(struct index_state *istate, int flags) > /* > * The sparse index is not (yet) integrated with a split index. > */ > - if (istate->split_index) > + if (istate->split_index || git_env_bool("GIT_TEST_SPLIT_INDEX", 0)) > return 0; > /* > * The GIT_TEST_SPARSE_INDEX environment variable triggers the ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active 2022-01-22 1:42 ` Junio C Hamano @ 2022-01-22 9:30 ` Johannes Schindelin 0 siblings, 0 replies; 10+ messages in thread From: Johannes Schindelin @ 2022-01-22 9:30 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git Hi Junio, On Fri, 21 Jan 2022, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > In 6e773527b6b (sparse-index: convert from full to sparse, 2021-03-30), > > we introduced initial support for a sparse index, and were careful to > > avoid converting to a sparse index in the presence of a split index. > > > > However, when we _just_ read a freshly-initialized index, it might not > > contain a split index even if _writing_ it will add one by virtue of > > being asked for via the `GIT_TEST_SPLIT_INDEX` variable. > > > > We did not notice any problems with checking _only_ for `split_index` > > (and not `GIT_TEST_SPLIT_INDEX`) right until both > > `vd/sparse-sparsity-fix-on-read` _and_ `vd/sparse-reset` were merged. > > > > Those two topics' interplay triggers a bug in conjunction with running > > t1091.15 when `GIT_TEST_SPLIT_INDEX=true` in the following way: > > `vd/sparse-sparsity-fix-on-read` ensures that the index is made sparse > > right after reading, and `vd/sparse-reset` ensures that the index is > > made non-sparse again unless running in the `--soft` mode. Since the > > split index feature is incompatible with the sparse index feature, we > > see a symptom like this: > > > > fatal: position for replacement 4 exceeds base index size 4 > > > > Let's fix this by avoiding the conversion to a sparse index when > > `GIT_TEST_SPLIT_INDEX=true`. > > Does [2/3] allow you to sidestep that issue entirely by skipping > 1091 altogether? You are right that reverting this patch after applying the next patch _still_ works around the issue. That's because currently only t1091 exposes the bug where `GIT_TEST_SPLIT_INDEX` writes a split index that was initially non-split (because it was just created). My intention here was to avoid any problems in case _another_ test script triggers the same condition. After all, we do have an entire CI job that forces `GIT_TEST_SPLIT_INDEX=1`, and in microsoft/git we enable sparse index by default. The entire test suite is therefore surface for the bug to show. > There are 4 hits of "if (istate->split_index" in the codebase, and > this patch makes me wonder why it is suffice to patch only one of > them. I tried to catch only those locations where we run the danger of trying to make a split index also sparse, or a sparse index also split. > I also wondered why we test both split_index and environment > separately, instead of splitting the index very early when the > environment variable is set, so that the rest of the runtime does > not have to worry about the environment, but is the reason why such > an approach was not taken was because GIT_TEST_SPLIT_INDEX can later > allow the index to be splitted, even if istate->split_index is still > NULL right now when this function is called? Indeed. A freshly created index will never be split, even if `GIT_TEST_SPLIT_INDEX` is set. Only when we _write_ this to disk will it be turned into a split index. At this point, we might have mistakenly converted the index into a sparse one, though, and this here patch wants to prevent that from happening. > If that is the reason, it leads to another question. Even if we > ignore GIT_TEST_SPLIT_INDEX and concentrate only on real workload, > if the in-core index can be NULL when this function is called and > then later can become split, there still must be somebody who > notices that sparse index is unallowed when (or after) such a split > happens, no? If there is no such code, then this patch would not be > the whole fix and there needs more change to do so, no? Indeed. Hence 3/3 which tries specifically to prevent the user from turning an already-sparse index into a split index. You will note that 3/3 calls die() instead BUG() in such a case because it would constitute a pilot error, not a bug in Git's code. Ciao, Dscho ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] t1091: disable split index 2022-01-19 17:29 [PATCH 0/3] [Non-critical]: sparse index vs split index fixes Johannes Schindelin via GitGitGadget 2022-01-19 17:29 ` [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active Johannes Schindelin via GitGitGadget @ 2022-01-19 17:29 ` Johannes Schindelin via GitGitGadget 2022-01-19 17:29 ` [PATCH 3/3] split-index: it really is incompatible with the sparse index Johannes Schindelin via GitGitGadget 2022-01-22 5:44 ` [PATCH 0/3] [Non-critical]: sparse index vs split index fixes Elijah Newren 3 siblings, 0 replies; 10+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2022-01-19 17:29 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> In 61feddcdf28 (tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests, 2021-08-26), it was already called out that the split index feature is incompatible with the sparse index feature, and its commit message wondered aloud whether more checks would be required to ensure that the split index and sparse index features aren't enabled at the same time. We are about to introduce such additional checks, and indeed, t1091 would utterly fail with them. Therefore, let's preemptively disable the split index for the entirety of t1091. This partially reverts above-mentioned patch because it covered only one test case whereas we want to cover the entire test script. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t1091-sparse-checkout-builtin.sh | 54 ++++++++++++++---------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index 42776984fe7..b229a8274d8 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -5,6 +5,9 @@ test_description='sparse checkout builtin tests' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +GIT_TEST_SPLIT_INDEX=false +export GIT_TEST_SPLIT_INDEX + . ./test-lib.sh list_files() { @@ -228,36 +231,31 @@ test_expect_success 'sparse-checkout disable' ' ' test_expect_success 'sparse-index enabled and disabled' ' - ( - sane_unset GIT_TEST_SPLIT_INDEX && - git -C repo update-index --no-split-index && - - git -C repo sparse-checkout init --cone --sparse-index && - test_cmp_config -C repo true index.sparse && - git -C repo ls-files --sparse >sparse && - git -C repo sparse-checkout disable && - git -C repo ls-files --sparse >full && - - cat >expect <<-\EOF && - @@ -1,4 +1,7 @@ - a - -deep/ - -folder1/ - -folder2/ - +deep/a - +deep/deeper1/a - +deep/deeper1/deepest/a - +deep/deeper2/a - +folder1/a - +folder2/a - EOF + git -C repo sparse-checkout init --cone --sparse-index && + test_cmp_config -C repo true index.sparse && + git -C repo ls-files --sparse >sparse && + git -C repo sparse-checkout disable && + git -C repo ls-files --sparse >full && - diff -u sparse full | tail -n +3 >actual && - test_cmp expect actual && + cat >expect <<-\EOF && + @@ -1,4 +1,7 @@ + a + -deep/ + -folder1/ + -folder2/ + +deep/a + +deep/deeper1/a + +deep/deeper1/deepest/a + +deep/deeper2/a + +folder1/a + +folder2/a + EOF + + diff -u sparse full | tail -n +3 >actual && + test_cmp expect actual && - git -C repo config --list >config && - ! grep index.sparse config - ) + git -C repo config --list >config && + ! grep index.sparse config ' test_expect_success 'cone mode: init and set' ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] split-index: it really is incompatible with the sparse index 2022-01-19 17:29 [PATCH 0/3] [Non-critical]: sparse index vs split index fixes Johannes Schindelin via GitGitGadget 2022-01-19 17:29 ` [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active Johannes Schindelin via GitGitGadget 2022-01-19 17:29 ` [PATCH 2/3] t1091: disable split index Johannes Schindelin via GitGitGadget @ 2022-01-19 17:29 ` Johannes Schindelin via GitGitGadget 2022-01-21 23:39 ` Junio C Hamano 2022-01-22 5:44 ` [PATCH 0/3] [Non-critical]: sparse index vs split index fixes Elijah Newren 3 siblings, 1 reply; 10+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2022-01-19 17:29 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> ... at least for now. So let's error out if we are even trying to initialize the split index when the index is sparse, or when trying to write the split index extension for a sparse index. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- read-cache.c | 3 +++ split-index.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/read-cache.c b/read-cache.c index cbe73f14e5e..a932e01fc7a 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3009,6 +3009,9 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, !is_null_oid(&istate->split_index->base_oid)) { struct strbuf sb = STRBUF_INIT; + if (istate->sparse_index) + die(_("cannot write split index for a sparse index")); + err = write_link_extension(&sb, istate) < 0 || write_index_ext_header(f, eoie_c, CACHE_EXT_LINK, sb.len) < 0; diff --git a/split-index.c b/split-index.c index 8e52e891c3b..9d0ccc30d00 100644 --- a/split-index.c +++ b/split-index.c @@ -5,6 +5,9 @@ struct split_index *init_split_index(struct index_state *istate) { if (!istate->split_index) { + if (istate->sparse_index) + die(_("cannot use split index with a sparse index")); + CALLOC_ARRAY(istate->split_index, 1); istate->split_index->refcount = 1; } -- gitgitgadget ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] split-index: it really is incompatible with the sparse index 2022-01-19 17:29 ` [PATCH 3/3] split-index: it really is incompatible with the sparse index Johannes Schindelin via GitGitGadget @ 2022-01-21 23:39 ` Junio C Hamano 2022-01-22 9:32 ` Johannes Schindelin 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2022-01-21 23:39 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > ... at least for now. So let's error out if we are even trying to > initialize the split index when the index is sparse, or when trying to > write the split index extension for a sparse index. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > read-cache.c | 3 +++ > split-index.c | 3 +++ > 2 files changed, 6 insertions(+) Good. Will queue. > diff --git a/read-cache.c b/read-cache.c > index cbe73f14e5e..a932e01fc7a 100644 > --- a/read-cache.c > +++ b/read-cache.c > @@ -3009,6 +3009,9 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, > !is_null_oid(&istate->split_index->base_oid)) { > struct strbuf sb = STRBUF_INIT; > > + if (istate->sparse_index) > + die(_("cannot write split index for a sparse index")); > + > err = write_link_extension(&sb, istate) < 0 || > write_index_ext_header(f, eoie_c, CACHE_EXT_LINK, > sb.len) < 0; > diff --git a/split-index.c b/split-index.c > index 8e52e891c3b..9d0ccc30d00 100644 > --- a/split-index.c > +++ b/split-index.c > @@ -5,6 +5,9 @@ > struct split_index *init_split_index(struct index_state *istate) > { > if (!istate->split_index) { > + if (istate->sparse_index) > + die(_("cannot use split index with a sparse index")); > + > CALLOC_ARRAY(istate->split_index, 1); > istate->split_index->refcount = 1; > } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] split-index: it really is incompatible with the sparse index 2022-01-21 23:39 ` Junio C Hamano @ 2022-01-22 9:32 ` Johannes Schindelin 0 siblings, 0 replies; 10+ messages in thread From: Johannes Schindelin @ 2022-01-22 9:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git Hi Junio, On Fri, 21 Jan 2022, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > ... at least for now. So let's error out if we are even trying to > > initialize the split index when the index is sparse, or when trying to > > write the split index extension for a sparse index. > > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > > --- > > read-cache.c | 3 +++ > > split-index.c | 3 +++ > > 2 files changed, 6 insertions(+) > > Good. Will queue. Thank you! Dscho ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] [Non-critical]: sparse index vs split index fixes 2022-01-19 17:29 [PATCH 0/3] [Non-critical]: sparse index vs split index fixes Johannes Schindelin via GitGitGadget ` (2 preceding siblings ...) 2022-01-19 17:29 ` [PATCH 3/3] split-index: it really is incompatible with the sparse index Johannes Schindelin via GitGitGadget @ 2022-01-22 5:44 ` Elijah Newren 2022-01-22 9:31 ` Johannes Schindelin 3 siblings, 1 reply; 10+ messages in thread From: Elijah Newren @ 2022-01-22 5:44 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget Cc: Git Mailing List, Johannes Schindelin On Fri, Jan 21, 2022 at 11:53 AM Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote: > > We noticed split/sparse index-related issues while rebasing Microsoft's fork > of Git. These fixes are necessary for that fork's test suite to pass, but > they might not be super critical to get into upstream v2.35.0 (especially > this close to -rc2). However, the team felt that the decision should be left > to the Git maintainer whether to take these patches into v2.35.0 or not. Thanks for digging into these and putting in some guardrails to prevent similar issues. I hit similar things with some of my changes and had to fight t1091 to get it to pass in CI under GIT_TEST_SPLIT_INDEX=1. I don't recall seeing the specific error you mention in patch 1, but maybe I just overlooked it. I ended up finding little workarounds such as disabling sparse-checkouts at the end of various tests before new ones I was adding, and being extra careful to not leave the sparse-index selected. I probably should have dug further, but didn't. Thanks for digging in. I've read over the patch series; it looks good to me: Reviewed-by: Elijah Newren <newren@gmail.com> > > Johannes Schindelin (3): > sparse-index: sparse index is disallowed when split index is active > t1091: disable split index > split-index: it really is incompatible with the sparse index > > read-cache.c | 3 ++ > sparse-index.c | 2 +- > split-index.c | 3 ++ > t/t1091-sparse-checkout-builtin.sh | 54 ++++++++++++++---------------- > 4 files changed, 33 insertions(+), 29 deletions(-) > > > base-commit: af4e5f569bc89f356eb34a9373d7f82aca6faa8a > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1119%2Fdscho%2Fsparse-index-vs-split-index-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1119/dscho/sparse-index-vs-split-index-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1119 > -- > gitgitgadget ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] [Non-critical]: sparse index vs split index fixes 2022-01-22 5:44 ` [PATCH 0/3] [Non-critical]: sparse index vs split index fixes Elijah Newren @ 2022-01-22 9:31 ` Johannes Schindelin 0 siblings, 0 replies; 10+ messages in thread From: Johannes Schindelin @ 2022-01-22 9:31 UTC (permalink / raw) To: Elijah Newren; +Cc: Johannes Schindelin via GitGitGadget, Git Mailing List Hi Elijah, On Fri, 21 Jan 2022, Elijah Newren wrote: > On Fri, Jan 21, 2022 at 11:53 AM Johannes Schindelin via GitGitGadget > <gitgitgadget@gmail.com> wrote: > > > > We noticed split/sparse index-related issues while rebasing Microsoft's fork > > of Git. These fixes are necessary for that fork's test suite to pass, but > > they might not be super critical to get into upstream v2.35.0 (especially > > this close to -rc2). However, the team felt that the decision should be left > > to the Git maintainer whether to take these patches into v2.35.0 or not. > > Thanks for digging into these and putting in some guardrails to > prevent similar issues. I hit similar things with some of my changes > and had to fight t1091 to get it to pass in CI under > GIT_TEST_SPLIT_INDEX=1. I don't recall seeing the specific error you > mention in patch 1, but maybe I just overlooked it. I ended up > finding little workarounds such as disabling sparse-checkouts at the > end of various tests before new ones I was adding, and being extra > careful to not leave the sparse-index selected. I probably should > have dug further, but didn't. Thanks for digging in. Seeing how much time it took to properly diagnose and fix these issues, I do not fault you ;-) > I've read over the patch series; it looks good to me: > > Reviewed-by: Elijah Newren <newren@gmail.com> Thank you! Dscho ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-01-22 9:32 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-19 17:29 [PATCH 0/3] [Non-critical]: sparse index vs split index fixes Johannes Schindelin via GitGitGadget 2022-01-19 17:29 ` [PATCH 1/3] sparse-index: sparse index is disallowed when split index is active Johannes Schindelin via GitGitGadget 2022-01-22 1:42 ` Junio C Hamano 2022-01-22 9:30 ` Johannes Schindelin 2022-01-19 17:29 ` [PATCH 2/3] t1091: disable split index Johannes Schindelin via GitGitGadget 2022-01-19 17:29 ` [PATCH 3/3] split-index: it really is incompatible with the sparse index Johannes Schindelin via GitGitGadget 2022-01-21 23:39 ` Junio C Hamano 2022-01-22 9:32 ` Johannes Schindelin 2022-01-22 5:44 ` [PATCH 0/3] [Non-critical]: sparse index vs split index fixes Elijah Newren 2022-01-22 9:31 ` Johannes Schindelin
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).