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

On 2021-03-08 at 16:43:58, Eric Sunshine wrote:

On Mon, Mar 8, 2021 at 8:18 AM brian m. carlson
[off-list ref] wrote:

quoted

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

Perhaps this deserves a:

    Reported-by: Joseph Vusich [off-list ref]

Good point.  Will fix.

quoted

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

Sure.  I pulled the line from the test above, but I agree that's nicer.

quoted

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

There's enough nits to warrant a v2, so I can do one.
-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US