Re: [PATCH v2 01/27] init-db: document existing bug with core.bare in template config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 12, 2023 at 2:34 PM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> > -     is_bare_repository_cfg = init_is_bare_repository || !work_tree;
> >       if (init_shared_repository != -1)
> >               set_shared_repository(init_shared_repository);
> > +     /*
> > +      * TODO: heed core.bare from config file in templates if no
> > +      *       command-line override given
> > +      */
> > +     is_bare_repository_cfg = init_is_bare_repository || !work_tree;
>
> This patch moves this line down a few lines, but that's fine because
> set_shared_repository() doesn't modify any of the relevant variables.
>
> > +     /* TODO (continued):
> > +      *
> > +      * Unfortunately, the line above is equivalent to
> > +      *    is_bare_repository_cfg = !work_tree;
> > +      * which ignores the config entirely even if no `--[no-]bare`
> > +      * command line option was present.
> > +      *
> > +      * To see why, note that before this function, there was this call:
> > +      *    init_is_bare_repository = is_bare_repository()
>
> This is in init_db(), indeed.
>
> > +      * expanding the right hande side:
>
> s/hande/hand/

Thanks; will fix.

> > +      *                 = is_bare_repository_cfg && !get_git_work_tree()
> > +      *                 = is_bare_repository_cfg && !work_tree
> > +      * note that the last simplification above is valid because nothing
> > +      * calls repo_init() or set_git_work_tree() between any of the
> > +      * relevant calls in the code,
>
> Yes, the only calls are check_repository_format() and
> validate_hash_algorithm() (as can be seen in init_db()) before
> get_git_work_tree() is called at the start of create_default_files().
>
> > diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
> > index 1b6437ec079..c02fd64793b 100755
> > --- a/t/t1301-shared-repo.sh
> > +++ b/t/t1301-shared-repo.sh
> > @@ -52,6 +52,28 @@ test_expect_success 'shared=all' '
> >       test 2 = $(git config core.sharedrepository)
> >  '
> >
> > +test_expect_failure 'template can set core.bare' '
>
> I would have preferred a test_expect_success with the exact failing line
> documented and prepended with test_must_fail, but I can see why someone
> would prefer test_expect_failure.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux