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.