config: use gitdir to get worktree config

Update 'do_git_config_sequence()' to read the worktree config from 'config.worktree' in 'opts->git_dir' rather than the gitdir of 'the_repository'. The worktree config is loaded from the path returned by 'git_pathdup("config.worktree")', the 'config.worktree' relative to the gitdir of 'the_repository'. If loading the config for a submodule, this path is incorrect, since 'the_repository' is the superproject. 'opts->git_dir' is the gitdir of the submodule being configured, so the config file in that location should be read instead. To ensure the use of 'opts->git_dir' is safe, require that 'opts->git_dir' is set if-and-only-if 'opts->commondir' is set (rather than "only-if" as it is now). In all current usage of 'config_options', these values are set together, so the stricter check does not change any behavior. Finally, add tests to 't3007-ls-files-recurse-submodules.sh' to verify the corrected config is loaded. Use 'ls-files' to test this because, unlike some other '--recurse-submodules' commands, 'ls-files' parses the config of the submodule in the same process as the superproject (via 'show_submodule()' -> 'repo_read_index()' -> 'prepare_repo_settings()'). As a result, 'the_repository' points to the config of the superproject but the commondir/gitdir in the config sequence will be that of the submodule, providing the exact scenario needed to verify this patch. The first test ('--recurse-submodules parses submodule repo config') checks that the submodule's *repo* config is read when running 'ls-files' on the superproject; this confirms already-working behavior, serving as a reference for how worktree config parsing should behave. The second test ('--recurse-submodules parses submodule worktree config') tests the same scenario as the previous but instead using the *worktree* config, demonstrating the corrected behavior. The 'test_config' helper is extended for this case so that it properly applies the '--worktree' option to the configure/unconfigure operations it performs. Note that, although the submodule worktree config is now parsed instead of the superproject's, 'extensions.worktreeConfig' in the superproject still controls whether or not the worktree config is enabled at all in the submodule. This will be fixed in a later patch. Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Victoria Dye committed May 26, 2023 at 01:32 UTC 847d0027d277f5748e9073aed8deb8b81af32ee7
3 files changed +48 -11
config.c
+19 -9
@@ -2200,14 +2200,24 @@ static int do_git_config_sequence(struct config_reader *reader,
2200 char *xdg_config = NULL;
2201 char *user_config = NULL;
2202 char *repo_config;
2203 + char *worktree_config;
2204 enum config_scope prev_parsing_scope = reader->parsing_scope;
2205
2205 - if (opts->commondir)
2206 + /*
2207 + * Ensure that either:
2208 + * - the git_dir and commondir are both set, or
2209 + * - the git_dir and commondir are both NULL
2210 + */
2211 + if (!opts->git_dir != !opts->commondir)
2212 + BUG("only one of commondir and git_dir is non-NULL");
2213 +
2214 + if (opts->commondir) {
2215 repo_config = mkpathdup("%s/config", opts->commondir);
2207 - else if (opts->git_dir)
2208 - BUG("git_dir without commondir");
2209 - else
2216 + worktree_config = mkpathdup("%s/config.worktree", opts->git_dir);
2217 + } else {
2218 repo_config = NULL;
2219 + worktree_config = NULL;
2220 + }
2221
2222 config_reader_set_scope(reader, CONFIG_SCOPE_SYSTEM);
2223 if (git_config_system() && system_config &&
@@ -2230,11 +2240,10 @@ static int do_git_config_sequence(struct config_reader *reader,
2240 ret += git_config_from_file(fn, repo_config, data);
2241
2242 config_reader_set_scope(reader, CONFIG_SCOPE_WORKTREE);
2233 - if (!opts->ignore_worktree && repository_format_worktree_config) {
2234 - char *path = git_pathdup("config.worktree");
2235 - if (!access_or_die(path, R_OK, 0))
2236 - ret += git_config_from_file(fn, path, data);
2237 - free(path);
2243 + if (!opts->ignore_worktree && worktree_config &&
2244 + repository_format_worktree_config &&
2245 + !access_or_die(worktree_config, R_OK, 0)) {
2246 + ret += git_config_from_file(fn, worktree_config, data);
2247 }
2248
2249 config_reader_set_scope(reader, CONFIG_SCOPE_COMMAND);
@@ -2246,6 +2255,7 @@ static int do_git_config_sequence(struct config_reader *reader,
2255 free(xdg_config);
2256 free(user_config);
2257 free(repo_config);
2258 + free(worktree_config);
2259 return ret;
2260 }
2261
t/t3007-ls-files-recurse-submodules.sh
+18
@@ -299,6 +299,24 @@ test_expect_success '--recurse-submodules does not support --error-unmatch' '
299 test_i18ngrep "does not support --error-unmatch" actual
300 '
301
302 +test_expect_success '--recurse-submodules parses submodule repo config' '
303 + test_config -C submodule index.sparse "invalid non-boolean value" &&
304 + test_must_fail git ls-files --recurse-submodules 2>err &&
305 + grep "bad boolean config value" err
306 +'
307 +
308 +test_expect_success '--recurse-submodules parses submodule worktree config' '
309 + test_config -C submodule extensions.worktreeConfig true &&
310 + test_config -C submodule --worktree index.sparse "invalid non-boolean value" &&
311 +
312 + # NEEDSWORK: the extensions.worktreeConfig is set globally based on
313 + # superproject, so we need to enable it in the superproject.
314 + test_config extensions.worktreeConfig true &&
315 +
316 + test_must_fail git ls-files --recurse-submodules 2>err &&
317 + grep "bad boolean config value" err
318 +'
319 +
320 test_incompatible_with_recurse_submodules () {
321 test_expect_success "--recurse-submodules and $1 are incompatible" "
322 test_must_fail git ls-files --recurse-submodules $1 2>actual &&
t/test-lib-functions.sh
+11 -2
@@ -542,8 +542,17 @@ test_config () {
542 config_dir=$1
543 shift
544 fi
545 - test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} '$1'" &&
546 - git ${config_dir:+-C "$config_dir"} config "$@"
545 +
546 + # If --worktree is provided, use it to configure/unconfigure
547 + is_worktree=
548 + if test "$1" = --worktree
549 + then
550 + is_worktree=1
551 + shift
552 + fi
553 +
554 + test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} ${is_worktree:+--worktree} '$1'" &&
555 + git ${config_dir:+-C "$config_dir"} config ${is_worktree:+--worktree} "$@"
556 }
557
558 test_config_global () {