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:
Show 16 quoted lines
> 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>Show 15 quoted lines
> > 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.
Show 10 quoted lines
> > + 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
brian m. carlson <sandals@crustytoothpaste.net>