Subject: Re: [PATCH v6 3/6] config: add repo_config_set_worktree_gently()

From: Junio C Hamano <[email-masked]>
Date: Tue, 08 Feb 2022 14:18:52 -0800

"Derrick Stolee via GitGitGadget" <[email-masked]> writes:

> From: Derrick Stolee <[email-masked]>
>
> Some config settings, such as those for sparse-checkout, are likely
> intended to only apply to one worktree at a time. To make this write
> easier, add a new config API method, repo_config_set_worktree_gently().
>
> This method will attempt to write to the worktree-specific config, but
> will instead write to the common config file if worktree config is not
> enabled.  The next change will introduce a consumer of this method.

Makes sense.

> +int repo_config_set_worktree_gently(struct repository *r,
> +				    const char *key, const char *value)
> +{
> +	/* Only use worktree-specific config if it is is already enabled. */
> +	if (repository_format_worktree_config) {
> +		char *file = repo_git_path(r, "config.worktree");
> +		int ret = git_config_set_multivar_in_file_gently(
> +					file, key, value, NULL, 0);
> +		free(file);
> +		return ret;
> +	}
> +	return repo_config_set_multivar_gently(r, key, value, NULL, 0);
> +}

OK.

> @@ -3181,14 +3196,28 @@ void git_config_set_multivar_in_file(const char *config_filename,
>  int git_config_set_multivar_gently(const char *key, const char *value,
>  				   const char *value_pattern, unsigned flags)
>  {
> -	return git_config_set_multivar_in_file_gently(NULL, key, value, value_pattern,
> -						      flags);
> +	return repo_config_set_multivar_gently(the_repository, key, value,
> +					       value_pattern, flags);
> +}

Is this an unrelated "morally no-op" change?

> +int repo_config_set_multivar_gently(struct repository *r, const char *key,
> +				    const char *value,
> +				    const char *value_pattern, unsigned flags)
> +{
> +	char *file = repo_git_path(r, "config");
> +	int res = git_config_set_multivar_in_file_gently(file,
> +							 key, value,
> +							 value_pattern,
> +							 flags);
> +	free(file);
> +	return res;
>  }

OK.

>  void git_config_set_multivar(const char *key, const char *value,
>  			     const char *value_pattern, unsigned flags)
>  {
> -	git_config_set_multivar_in_file(NULL, key, value, value_pattern,
> +	git_config_set_multivar_in_file(git_path("config"),
> +					key, value, value_pattern,
>  					flags);
>  }

Is this an unrelated "morally no-op" change?

It might have value to make caller more explicit by reducing the use
of "I give NULL, you use 'config' for me", but that doesn't sound
related to the addition of set_per_worktree_config_gently() helper.

> diff --git a/config.h b/config.h
> index f119de01309..1d98ad269bd 100644
> --- a/config.h
> +++ b/config.h
> @@ -253,6 +253,13 @@ void git_config_set_in_file(const char *, const char *, const char *);
>  
>  int git_config_set_gently(const char *, const char *);
>  
> +/**
> + * Write a config value that should apply to the current worktree. If
> + * extensions.worktreeConfig is enabled, then the write will happen in the
> + * current worktree's config. Otherwise, write to the common config file.
> + */
> +int repo_config_set_worktree_gently(struct repository *, const char *, const char *);
> +
>  /**
>   * write config values to `.git/config`, takes a key/value pair as parameter.
>   */
> @@ -281,6 +288,7 @@ int git_config_parse_key(const char *, char **, size_t *);
>  
>  int git_config_set_multivar_gently(const char *, const char *, const char *, unsigned);
>  void git_config_set_multivar(const char *, const char *, const char *, unsigned);
> +int repo_config_set_multivar_gently(struct repository *, const char *, const char *, const char *, unsigned);
>  int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, unsigned);
>  
>  /**