init-db: document existing bug with core.bare in template config

The comments in create_default_files() talks about reading config from the config file in the specified `--templates` directory, which leads to the question of whether core.bare could be set in such a config file and thus whether the code is doing the right thing. It turns out, that it doesn't; it unconditionally ignores core.bare in the config file in any --templates directory. It is not clear to me that fixing it can be done within this function; it seems to occur too late: * create_default_files() is called by init_db() * init_db() is called by both builtin/{clone.c,init-db.c} * both callers of init_db() call set_git_work_tree() before init_db() and in order to actual affect whether a repository is bear, we'd need to somewhere reset these values, not just the is_bare_repository_cfg setting. I do not want to open this can of worms at this time; I'm trying to clean up some headers, for which I need to move some functions, for which I need to clean up some globals, and that's far enough down the rabbit hole. So, simply document the issue with a careful TODO comment and a few testcases. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed May 16, 2023 at 06:33 UTC 0f7443bdc7284ad36e5d0cd8b2526e2123b8aa4f
3 files changed +60 -1
builtin/init-db.c
+28 -1
@@ -231,9 +231,36 @@ static int create_default_files(const char *template_path,
231 * We must make sure command-line options continue to override any
232 * values we might have just re-read from the config.
233 */
234 - is_bare_repository_cfg = init_is_bare_repository || !work_tree;
234 if (init_shared_repository != -1)
235 set_shared_repository(init_shared_repository);
236 + /*
237 + * TODO: heed core.bare from config file in templates if no
238 + * command-line override given
239 + */
240 + is_bare_repository_cfg = init_is_bare_repository || !work_tree;
241 + /* TODO (continued):
242 + *
243 + * Unfortunately, the line above is equivalent to
244 + * is_bare_repository_cfg = !work_tree;
245 + * which ignores the config entirely even if no `--[no-]bare`
246 + * command line option was present.
247 + *
248 + * To see why, note that before this function, there was this call:
249 + * init_is_bare_repository = is_bare_repository()
250 + * expanding the right hand side:
251 + * = is_bare_repository_cfg && !get_git_work_tree()
252 + * = is_bare_repository_cfg && !work_tree
253 + * note that the last simplification above is valid because nothing
254 + * calls repo_init() or set_git_work_tree() between any of the
255 + * relevant calls in the code, and thus the !get_git_work_tree()
256 + * calls will return the same result each time. So, what we are
257 + * interested in computing is the right hand side of the line of
258 + * code just above this comment:
259 + * init_is_bare_repository || !work_tree
260 + * = is_bare_repository_cfg && !work_tree || !work_tree
261 + * = !work_tree
262 + * because "A && !B || !B == !B" for all boolean values of A & B.
263 + */
264
265 /*
266 * We would have created the above under user's umask -- under
t/t1301-shared-repo.sh
+22
@@ -52,6 +52,28 @@ test_expect_success 'shared=all' '
52 test 2 = $(git config core.sharedrepository)
53 '
54
55 +test_expect_failure 'template can set core.bare' '
56 + test_when_finished "rm -rf subdir" &&
57 + test_when_finished "rm -rf templates" &&
58 + test_config core.bare true &&
59 + umask 0022 &&
60 + mkdir -p templates/ &&
61 + cp .git/config templates/config &&
62 + git init --template=templates subdir &&
63 + test_path_exists subdir/HEAD
64 +'
65 +
66 +test_expect_success 'template can set core.bare but overridden by command line' '
67 + test_when_finished "rm -rf subdir" &&
68 + test_when_finished "rm -rf templates" &&
69 + test_config core.bare true &&
70 + umask 0022 &&
71 + mkdir -p templates/ &&
72 + cp .git/config templates/config &&
73 + git init --no-bare --template=templates subdir &&
74 + test_path_exists subdir/.git/HEAD
75 +'
76 +
77 test_expect_success POSIXPERM 'update-server-info honors core.sharedRepository' '
78 : > a1 &&
79 git add a1 &&
t/t5606-clone-options.sh
+10
@@ -120,6 +120,16 @@ test_expect_success 'prefers -c config over --template config' '
120
121 '
122
123 +test_expect_failure 'prefers --template config even for core.bare' '
124 +
125 + template="$TRASH_DIRECTORY/template-with-bare-config" &&
126 + mkdir "$template" &&
127 + git config --file "$template/config" core.bare true &&
128 + git clone "--template=$template" parent clone-bare-config &&
129 + test "$(git -C clone-bare-config config --local core.bare)" = "true" &&
130 + test_path_is_file clone-bare-config/HEAD
131 +'
132 +
133 test_expect_success 'prefers config "clone.defaultRemoteName" over default' '
134
135 test_config_global clone.defaultRemoteName from_config &&