On Tue, Dec 28, 2021 at 4:32 PM Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > [...] > To gain access to the core repository's config and config.worktree file, > we reference a repository struct's 'commondir' member. If the repository > was a submodule instead of a worktree, then this still applies > correctly. In [1], I suggested that you should be using `repository->gitdir` rather than `repository->commondir` to access the `.git/config` file. Is the above paragraph saying that my suggestion was incorrect? Or is it incorrect only in the case of submodules? Or what is it saying? > diff --git a/builtin/worktree.c b/builtin/worktree.c > @@ -1031,6 +1032,85 @@ static int repair(int ac, const char **av, const char *prefix) > +static int init_worktree_config(int ac, const char **av, const char *prefix) > +{ > + struct repository *r = the_repository; > + char *common_config_file = xstrfmt("%s/config", r->commondir); > + char *main_worktree_file = xstrfmt("%s/config.worktree", r->commondir); Specifically, in [1], I said that `common_config_file` should be using `r->gitdir` (and that the use of `r->commondir` was correct for `main_worktree_file`). [1]: https://lore.kernel.org/git/CAPig+cQrJ9yWjkc8VMu=uyx_qtrXdL3cNnxLVafoxOo6e-r9kw@xxxxxxxxxxxxxx/ > + git_configset_init(&cs); > + git_configset_add_file(&cs, common_config_file); > + > + /* > + * If the format and extension are already enabled, then we can > + * skip the upgrade process. > + */ > + if (repository_format_worktree_config) > + return 0; Rather than `return 0`, should this be `goto cleanup`... > + if (upgrade_repository_format(r, 1) < 0) { > + res = error(_("unable to upgrade repository format to enable worktreeConfig")); > + goto cleanup; > + } > + if ((res = git_config_set_gently("extensions.worktreeConfig", "true"))) { > + error(_("failed to set extensions.worktreeConfig setting")); > + goto cleanup; > + } ... as is done for these two cases? > +cleanup: > + git_configset_clear(&cs); > + free(common_config_file); > + free(main_worktree_file); > + return res; > +}