Re: [PATCH 0/3] [Non-critical]: sparse index vs split index fixes

From: Elijah Newren <newren@gmail.com>
Date: Jan 22, 2022, 05:44 UTC
Message-ID: <CABPp-BFXMMH2_qqWPGaWX76mqEs2Z1GFNzbyeu2rLgaFPt7JyQ@mail.gmail.com>
In-Reply-To: <pull.1119.git.1642613379.gitgitgadget@gmail.com>

On Fri, Jan 21, 2022 at 11:53 AM Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 6 quoted lines

>
> 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>

Show 19 quoted lines

>
> 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