Re: [PATCH 1/2] list-objects-filter: only parse sparse OID when 'have_git_dir'

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 28, 2019, 21:10 UTC
Message-ID: <CAPig+cQeco6-aM+UrJsbj9LXtWSe4hXn0vbrBWwVn00cbDpbzQ@mail.gmail.com>
In-Reply-To: <20190828201824.1255-2-jon@jonsimons.org>

On Wed, Aug 28, 2019 at 4:27 PM Jon Simons <jon@jonsimons.org> wrote:

Show 15 quoted lines

> Fix a bug in partial cloning with sparse filters by ensuring to check
> for 'have_git_dir' before attempting to resolve the sparse filter OID.
> [...]
> Signed-off-by: Jon Simons <jon@jonsimons.org>
> ---
> diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
> @@ -241,6 +241,29 @@ test_expect_success 'fetch what is specified on CLI even if already promised' '
> +test_expect_success 'setup src repo for sparse filter' '
> +       git init sparse-src &&
> +       git -C sparse-src config --local uploadpack.allowfilter 1 &&
> +       git -C sparse-src config --local uploadpack.allowanysha1inwant 1 &&
> +       for n in 1 2 3 4
> +       do
> +               test_commit -C sparse-src "this-is-file-$n" file.$n.txt
> +       done &&

The way this is coded, a failure of the test_commit() invocation won't fail the test overall. You need to do so manually:

    for n in 1 2 3 4
    do
        test_commit -C sparse-src "this-is-file-$n" file.$n.txt || return 1
    done &&

> +       echo "/file.1.txt" >> sparse-src/odd-files &&
> +       echo "/file.3.txt" >> sparse-src/odd-files &&
> +       echo "/file.2.txt" >> sparse-src/even-files &&
> +       echo "/file.4.txt" >> sparse-src/even-files &&

Simpler:

    test_write_lines /file1.txt /file3.txt >sparse-src/odd-files &&
    test_write_lines /file2.txt /file4.txt >sparse-src/even-files &&

> +       echo "/*" >> sparse-src/all-files &&

Style nit: drop whitespace following redirection operator.

And, using >> rather than just > here makes the test more confusing than it need be; probably best to use >.