Re: [PATCH v6 5/6] worktree: copy sparse-checkout patterns and config on add
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Feb 15, 2022, 22:23 UTC
- Message-ID
- <CAPig+cS+q-X1KLRp6fV5Lvd80wX8WZ1993OXisQUk2ra_kJ1wA@mail.gmail.com>
- In-Reply-To
- <c51cb3714e7b1d2f8c9370fe87eca9984ff4859f.1644269584.git.gitgitgadget@gmail.com>
On Mon, Feb 7, 2022 at 4:33 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 73 quoted lines
> When adding a new worktree, it is reasonable to expect that we want to
> use the current set of sparse-checkout settings for that new worktree.
> This is particularly important for repositories where the worktree would
> become too large to be useful. This is even more important when using
> partial clone as well, since we want to avoid downloading the missing
> blobs for files that should not be written to the new worktree.
> [...]
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -335,6 +335,69 @@ static int add_worktree(const char *path, const char *refname,
> + /*
> + * If the current worktree has sparse-checkout enabled, then copy
> + * the sparse-checkout patterns from the current worktree.
> + */
> + if (core_apply_sparse_checkout) {
> + char *from_file = git_pathdup("info/sparse-checkout");
> + char *to_file = xstrfmt("%s/info/sparse-checkout",
> + sb_repo.buf);
> +
> + if (file_exists(from_file)) {
> + if (safe_create_leading_directories(to_file) ||
> + copy_file(to_file, from_file, 0666))
> + error(_("failed to copy '%s' to '%s'; sparse-checkout may not work correctly"),
> + from_file, to_file);
> + }
> +
> + free(from_file);
> + free(to_file);
> + }
> +
> + /*
> + * If we are using worktree config, then copy all current config
> + * values from the current worktree into the new one, that way the
> + * new worktree behaves the same as this one.
> + */
> + if (repository_format_worktree_config) {
> + char *from_file = git_pathdup("config.worktree");
> + char *to_file = xstrfmt("%s/config.worktree",
> + sb_repo.buf);
> +
> + if (file_exists(from_file)) {
> + struct config_set cs = { { 0 } };
> + const char *core_worktree;
> + int bare;
> +
> + if (safe_create_leading_directories(to_file) ||
> + copy_file(to_file, from_file, 0666)) {
> + error(_("failed to copy worktree config from '%s' to '%s'"),
> + from_file, to_file);
> + goto worktree_copy_cleanup;
> + }
> +
> + git_configset_init(&cs);
> + git_configset_add_file(&cs, from_file);
> +
> + if (!git_configset_get_bool(&cs, "core.bare", &bare) &&
> + bare &&
> + git_config_set_multivar_in_file_gently(
> + to_file, "core.bare", NULL, "true", 0))
> + error(_("failed to unset 'core.bare' in '%s'"), to_file);
> + if (!git_configset_get_value(&cs, "core.worktree", &core_worktree) &&
> + git_config_set_in_file_gently(to_file,
> + "core.worktree", NULL))
> + error(_("failed to unset 'core.worktree' in '%s'"), to_file);
> +
> + git_configset_clear(&cs);
> + }
> +
> +worktree_copy_cleanup:
> + free(from_file);
> + free(to_file);
> + }Given that add_worktree() is already overly long, a good future cleanup would be to move these two new hunks of functionality into separate functions -- copy_sparsity() and copy_config(), perhaps -- in builtin/worktree.c. This should be simple enough since (I think) the only state that needs to be passed to them is `sb_repo`. But such cleanup needn't hold up this series.