Glen Choo <chooglen@xxxxxxxxxx> writes:
> We have git_config_set() that sets a config value for the_repository's
> config file, and repo_config_get* that reads config values from a struct
> repository. Thus, it seems reasonable to have to have
> repo_git_config_set() that can set values for a config file of a struct
> repository.
>
> Implement repo_config_set() and repo_config_set_gently(), which
> take struct repository. However, unlike other instances where struct
> repository is added to a repo_* function, this patch does not change the
> implementations of git_config_set() or git_config_set_gently(); those
> functions use the_repository much deeper in their call chain through
> git_pathdup(). Mark this inconsistency as NEEDSWORK.
Being able to only affect "config" in the_repository->gitdir is less
flexible than being able to affect "config" in repo->gitdir for any
repository is good. Do we need a similar thing for repo->commondir
as well?
> +/*
> + * Sets a config value in a repository.
> + */
> +int repo_config_set_gently(struct repository *r, const char *key,
> + const char *value)
> +{
> + int ret;
> + char *path;
> +
> + path = repo_git_path(r, "config");
> + ret = git_config_set_in_file_gently(path, key, value);
> + free(path);
> + return ret;
> +}
> +
> +void repo_config_set(struct repository *r, const char *key, const char *value)
> +{
> + char *path;
> +
> + path = repo_git_path(r, "config");
> + git_config_set_in_file(path, key, value);
> + free(path);
> +}
Many questions:
- What do these do for an existing key? Add another value?
Replace existing one? If the latter, what do we plan to do with
multi-valued keys?
- Don't we need an interface to remove, rename, etc.?
- If we call repo_config_set(repo, "key", "value") and then call
repo_git_config_string(repo, "key", &value) on the same
repository, do we read the value back or do we give a stale
value?
- A change like this should make existing config_set() that only
works on the_repository into a thin wrapper, e.g.
void git_config_set(const char *keyu, const char **value)
{
repo_config_set(the_repository, key, value);
}
But that is not happening here. What prevents us from doing so?
Thanks.
