| Message ID | d262a76b448a495f00f460e9ed9439a74946de49.1643136134.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Sparse checkout: fix bug with worktree of bare repo | expand |
On Tue, Jan 25, 2022 at 10:42 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Derrick Stolee <dstolee@microsoft.com> > > Upgrading a repository to use extensions.worktreeConfig is non-trivial. > There are several steps involved, including moving some config settings > from the common config file to the main worktree's config.worktree file. > The previous change updated the documentation with all of these details. > > Commands such as 'git sparse-checkout set' upgrade the repository to use > extensions.worktreeConfig without following these steps, causing some > user pain in some special cases. > > Create a helper method, init_worktree_config(), that will be used in a > later change to fix this behavior within 'git sparse-checkout set'. The > method is carefully documented in worktree.h. I was curious why you were only fixing `set` and not `init`, but I looked ahead and it appears you are fixing both, since both use set_config(). And I can see leaving out the mention of `init` since it's deprecated. Anyway, it's all good here, I'm basically just thinking out loud... > Note that we do _not_ upgrade the repository format version to 1 during > this process. The worktree config extension must be considered by Git > and third-party tools even if core.repositoryFormatVersion is 0 for > historical reasons documented in 11664196ac ("Revert > "check_repository_format_gently(): refuse extensions for old > repositories"", 2020-07-15). This is a special case for this extension, > and newer extensions (such as extensions.objectFormat) still need to > upgrade the repository format version. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > worktree.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > worktree.h | 19 +++++++++++++++ > 2 files changed, 89 insertions(+) > > diff --git a/worktree.c b/worktree.c > index 6f598dcfcdf..dc4ead4c8fb 100644 > --- a/worktree.c > +++ b/worktree.c > @@ -5,6 +5,7 @@ > #include "worktree.h" > #include "dir.h" > #include "wt-status.h" > +#include "config.h" > > void free_worktrees(struct worktree **worktrees) > { > @@ -826,3 +827,72 @@ int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath, > *wtpath = path; > return 0; > } > + > +static int move_config_setting(const char *key, const char *value, > + const char *from_file, const char *to_file) > +{ > + if (git_config_set_in_file_gently(to_file, key, value)) > + return error(_("unable to set %s in '%s'"), key, to_file); > + if (git_config_set_in_file_gently(from_file, key, NULL)) > + return error(_("unable to unset %s in '%s'"), key, from_file); > + return 0; > +} > + > +int init_worktree_config(struct repository *r) > +{ > + int res = 0; > + int bare = 0; > + struct config_set cs = { { 0 } }; > + const char *core_worktree; > + char *common_config_file = xstrfmt("%s/config", r->commondir); > + char *main_worktree_file = xstrfmt("%s/config.worktree", r->commondir); > + > + /* > + * If the extension is already enabled, then we can skip the > + * upgrade process. > + */ > + if (repository_format_worktree_config) > + return 0; > + if ((res = git_config_set_gently("extensions.worktreeConfig", "true"))) > + return error(_("failed to set extensions.worktreeConfig setting")); > + > + git_configset_init(&cs); > + git_configset_add_file(&cs, common_config_file); > + > + /* > + * If core.bare is true in the common config file, then we need to > + * move it to the base worktree's config file or it will break all > + * worktrees. If it is false, then leave it in place because it > + * _could_ be negating a global core.bare=true. > + */ > + if (!git_configset_get_bool(&cs, "core.bare", &bare) && bare) { > + if ((res = move_config_setting("core.bare", "true", > + common_config_file, > + main_worktree_file))) > + goto cleanup; > + } > + /* > + * If core.worktree is set, then the base worktree is located > + * somewhere different than the parent of the common Git dir. > + * Relocate that value to avoid breaking all worktrees with this > + * upgrade to worktree config. > + */ > + if (!git_configset_get_string_tmp(&cs, "core.worktree", &core_worktree)) { > + if ((res = move_config_setting("core.worktree", core_worktree, > + common_config_file, > + main_worktree_file))) > + goto cleanup; > + } > + > + /* > + * Ensure that we use worktree config for the remaining lifetime > + * of the current process. > + */ > + repository_format_worktree_config = 1; > + > +cleanup: > + git_configset_clear(&cs); > + free(common_config_file); > + free(main_worktree_file); > + return res; > +} > diff --git a/worktree.h b/worktree.h > index 9e06fcbdf3d..5ea5fcc3647 100644 > --- a/worktree.h > +++ b/worktree.h > @@ -183,4 +183,23 @@ void strbuf_worktree_ref(const struct worktree *wt, > struct strbuf *sb, > const char *refname); > > +/** > + * Enable worktree config for the first time. This will make the following > + * adjustments: > + * > + * 1. Add extensions.worktreeConfig=true in the common config file. > + * > + * 2. If the common config file has a core.worktree value or core.bare is > + * set to true, then those values are moved to the main worktree's > + * config.worktree file. This is a bit ambiguous. If core.worktree is set and core.bare is false, are both moved or only one? I'm afraid folks won't understand that it's just one from this description. > + * > + * If extensions.worktreeConfig is already true, then this method > + * terminates early without any of the above steps. The existing config > + * arrangement is assumed to be intentional. > + * > + * Returns 0 on success. Reports an error message and returns non-zero > + * if any of these steps fail. > + */ > +int init_worktree_config(struct repository *r); > + > #endif > -- Other than the ambiguity in worktree.h, this patch looks solid.
diff --git a/worktree.c b/worktree.c index 6f598dcfcdf..dc4ead4c8fb 100644 --- a/worktree.c +++ b/worktree.c @@ -5,6 +5,7 @@ #include "worktree.h" #include "dir.h" #include "wt-status.h" +#include "config.h" void free_worktrees(struct worktree **worktrees) { @@ -826,3 +827,72 @@ int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath, *wtpath = path; return 0; } + +static int move_config_setting(const char *key, const char *value, + const char *from_file, const char *to_file) +{ + if (git_config_set_in_file_gently(to_file, key, value)) + return error(_("unable to set %s in '%s'"), key, to_file); + if (git_config_set_in_file_gently(from_file, key, NULL)) + return error(_("unable to unset %s in '%s'"), key, from_file); + return 0; +} + +int init_worktree_config(struct repository *r) +{ + int res = 0; + int bare = 0; + struct config_set cs = { { 0 } }; + const char *core_worktree; + char *common_config_file = xstrfmt("%s/config", r->commondir); + char *main_worktree_file = xstrfmt("%s/config.worktree", r->commondir); + + /* + * If the extension is already enabled, then we can skip the + * upgrade process. + */ + if (repository_format_worktree_config) + return 0; + if ((res = git_config_set_gently("extensions.worktreeConfig", "true"))) + return error(_("failed to set extensions.worktreeConfig setting")); + + git_configset_init(&cs); + git_configset_add_file(&cs, common_config_file); + + /* + * If core.bare is true in the common config file, then we need to + * move it to the base worktree's config file or it will break all + * worktrees. If it is false, then leave it in place because it + * _could_ be negating a global core.bare=true. + */ + if (!git_configset_get_bool(&cs, "core.bare", &bare) && bare) { + if ((res = move_config_setting("core.bare", "true", + common_config_file, + main_worktree_file))) + goto cleanup; + } + /* + * If core.worktree is set, then the base worktree is located + * somewhere different than the parent of the common Git dir. + * Relocate that value to avoid breaking all worktrees with this + * upgrade to worktree config. + */ + if (!git_configset_get_string_tmp(&cs, "core.worktree", &core_worktree)) { + if ((res = move_config_setting("core.worktree", core_worktree, + common_config_file, + main_worktree_file))) + goto cleanup; + } + + /* + * Ensure that we use worktree config for the remaining lifetime + * of the current process. + */ + repository_format_worktree_config = 1; + +cleanup: + git_configset_clear(&cs); + free(common_config_file); + free(main_worktree_file); + return res; +} diff --git a/worktree.h b/worktree.h index 9e06fcbdf3d..5ea5fcc3647 100644 --- a/worktree.h +++ b/worktree.h @@ -183,4 +183,23 @@ void strbuf_worktree_ref(const struct worktree *wt, struct strbuf *sb, const char *refname); +/** + * Enable worktree config for the first time. This will make the following + * adjustments: + * + * 1. Add extensions.worktreeConfig=true in the common config file. + * + * 2. If the common config file has a core.worktree value or core.bare is + * set to true, then those values are moved to the main worktree's + * config.worktree file. + * + * If extensions.worktreeConfig is already true, then this method + * terminates early without any of the above steps. The existing config + * arrangement is assumed to be intentional. + * + * Returns 0 on success. Reports an error message and returns non-zero + * if any of these steps fail. + */ +int init_worktree_config(struct repository *r); + #endif