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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 29, 2019, 22:45 UTC
Message-ID: <CAPig+cQDPsXvOYa3HYo_cYeVq4PhJE_2RGw9n+92=t3BA0-yaw@mail.gmail.com>
In-Reply-To: <20190829223811.12072-2-jon@jonsimons.org>

On Thu, Aug 29, 2019 at 6:38 PM Jon Simons <jon@jonsimons.org> wrote:

Show 18 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,27 @@ 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 || return 1
> +       done &&
> +       test_write_lines /file1.txt /file3.txt >sparse-src/odd-files &&
> +       test_write_lines /file2.txt /file4.txt >sparse-src/even-files &&
> +       test_write_lines /* >sparse-src/all-files &&

Hmm, does this work correctly? I would expect the /* to expand to all names at the root of your filesystem, which isn't what you want. You want the literal string "/*", which means you should quote it (with double quotes inside the test body). I'd also suggest using the simple 'echo' for this one as you did in v1 since it's more obvious that you're writing just a single line to the file, whereas using test_write_lines() has the potential to confuses readers.

> +       git -C sparse-src add odd-files even-files all-files &&
> +       git -C sparse-src commit -m "some sparse checkout files"
> +'