Re: [PATCH] builtin/init-db: handle bare clones when core.bare set to false

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, Mar 8, 2021 at 8:18 AM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> In 552955ed7f ("clone: use more conventional config/option layering",
> 2020-10-01), clone learned to read configuration options earlier in its
> execution, before creating the new repository.  However, that led to a
> problem: if the core.bare setting is set to false in the global config,
> cloning a bare repository segfaults.  This happens because the
> repository is falsely thought to be non-bare, but clone has set the work
> tree to NULL, which is then dereferenced.
> [...]
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---

Perhaps this deserves a:

    Reported-by: Joseph Vusich <jvusich@amazon.com>

> diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
> @@ -104,6 +104,13 @@ test_expect_success 'redirected clone -v does show progress' '
> +test_expect_success 'clone does not segfault with --bare and core.bare=false' '
> +       test_config_global core.bare false &&
> +       git clone --bare "file://$(pwd)/parent" clone-bare &&

Can this be done more simply as:

    git clone --bare parent clone-bare &&

or even:

    git clone --bare . clone-bare &&

without mucking about with $(pwd)?

> +       git -C clone-bare rev-parse --is-bare-repository >is-bare &&
> +       test "$(cat is-bare)" = true

These days, we'd probably say:

    echo true >expect &&
    git -C clone-bare rev-parse --is-bare-repository >actual &&
    test_cmp expect actual

but it's subjective and minor; not at all worth a re-roll.