[v5,2/5] worktree: create init_worktree_config()

Commit Message

Derrick Stolee Jan. 31, 2022, 3 p.m. UTC

From: Derrick Stolee <[e-mail]>

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.

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 <[e-mail]>
---
 worktree.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 worktree.h | 21 ++++++++++++++++
 2 files changed, 91 insertions(+)

Comments

Eric Sunshine Feb. 6, 2022, 9:32 a.m. UTC | #1

On Mon, Jan 31, 2022 at 10:01 AM Derrick Stolee via GitGitGadget
<[e-mail]> wrote:
> [...]
> 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.
> [...]
> Signed-off-by: Derrick Stolee <[e-mail]>
> ---
> diff --git a/worktree.c b/worktree.c
> @@ -826,3 +827,72 @@ int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath,
> +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"));

These two early returns are leaking `common_config_file` and
`main_worktree_file` which have already been allocated by xstrfmt().

> +
> +       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.
> +        */

The terminology "base worktree", which is not otherwise in the
project's lexicon, seems to be a leftover from earlier versions of
this series. Perhaps it could say instead "the repository-specific
configuration in $GIT_COMMON_DIR/worktree.config" or something?

> +       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.
> +        */

s/base worktree/main worktree/

> +       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;
> +}

Patch

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..e9e839926b0 100644
--- a/worktree.h
+++ b/worktree.h
@@ -183,4 +183,25 @@  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, then that value
+ *    is moved to the main worktree's config.worktree file.
+ *
+ * 3. If the common config file has a core.bare enabled, then that value
+ *    is 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