Skip to content

Commit

Permalink
config API & users: remove git_configset_get*(), use repo_*get*()
Browse files Browse the repository at this point in the history 
When the configset API was added in [1] it provided a new caching API
for config reading. Before we'd need to use e.g. the git_config_bool()
added in [2], but after we could use either git_configset_get_bool()
or git_config_get_bool().

Then later in [3] we had "repo_*()" variants added for all of these,
so now we'd also have repo_config_get_bool().

So, aside from the old functional interface we have 3x functions for
every "type" depending on whether we want to use a "configset", the
"lazy" API (which gets the configset from "the_repository"), or the
"repo" interface (ditto, but with a custom "struct repository *r").

This commit gets rid of 1/3 of these, i.e. the configset"
functions. Instead we'll use the "repo_*()" functions with a crafted
"config" member, which invariably is reading some custom file, instead
of the repo's entire config.

This is a bit of a hack in that we now need to construct this
otherwise redundant "struct repository" object.

But overall this is a big win, as we'll no longer need to maintain
this entire part of the API only for this use-case, which as it turns
out we only needed for three callers in-tree.

The resulting codepaths that read the config are also easier to
understand in cases where we'd either need to use our configset, or
"the_repository" config. See the maintenance_unregister() caller added
in [4]. Now we don't need to alter which function we call, just our
"cfg_r" object passed to repo"_config_get_*()".

1. 3c8687a (add `config_set` API for caching config-like files,
   2014-07-28)
2. 1771299 (Add ".git/config" file parser, 2005-10-10)
3. 3b25622 (config: read config from a repository object,
   2017-06-22)
4. 1f80129 (maintenance: add option to register in a specific
   config, 2022-11-09)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
  • Loading branch information
@avar
avar committed Mar 7, 2023
1 parent 0cc3ab8 commit 0233297
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 150 deletions.
11 changes: 6 additions & 5 deletions builtin/gc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1554,6 +1554,8 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
char *maintpath = get_maintpath();
const struct string_list *list;
struct config_set cs = { { 0 } };
struct repository repo;
struct repository *cfg_r = the_repository;

argc = parse_options(argc, argv, prefix, options,
builtin_maintenance_unregister_usage, 0);
Expand All @@ -1562,12 +1564,11 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
options);

if (config_file) {
git_configset_init(&cs);
git_configset_add_file(&cs, config_file);
repo_init_repo_blank_config(&repo, the_repository, &cs);
git_configset_add_file(repo.config, config_file);
cfg_r = &repo;
}
if (!(config_file
? git_configset_get_string_multi(&cs, key, &list)
: git_config_get_string_multi(key, &list)) &&
if (!repo_config_get_string_multi(cfg_r, key, &list) &&
unsorted_string_list_has_string(list, maintpath)) {
int rc;
char *user_config = NULL, *xdg_config = NULL;
Expand Down
11 changes: 6 additions & 5 deletions builtin/worktree.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir)
char *to_file = xstrfmt("%s/config.worktree", worktree_git_dir);

if (file_exists(from_file)) {
struct config_set cs = { { 0 } };
struct repository repo;
struct config_set cs;
int bare;

if (safe_create_leading_directories(to_file) ||
Expand All @@ -327,17 +328,17 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir)
from_file, to_file);
goto worktree_copy_cleanup;
}
repo_init_repo_blank_config(&repo, the_repository, &cs);

git_configset_init(&cs);
git_configset_add_file(&cs, from_file);
git_configset_add_file(repo.config, from_file);

if (!git_configset_get_bool(&cs, "core.bare", &bare) &&
if (!repo_config_get_bool(&repo, "core.bare", &bare) &&
bare &&
git_config_set_multivar_in_file_gently(
to_file, "core.bare", NULL, "true", 0))
error(_("failed to unset '%s' in '%s'"),
"core.bare", to_file);
if (!git_configset_get(&cs, "core.worktree") &&
if (!repo_config_get(&repo, "core.worktree") &&
git_config_set_in_file_gently(to_file,
"core.worktree", NULL))
error(_("failed to unset '%s' in '%s'"),
Expand Down
192 changes: 65 additions & 127 deletions config.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2467,110 +2467,6 @@ int git_configset_get_string_multi(struct config_set *cs, const char *key,
return 0;
}

int git_configset_get(struct config_set *cs, const char *key)
{
struct config_set_element *e;
int ret;

if ((ret = configset_find_element(cs, key, &e)))
return ret;
else if (!e)
return 1;
return 0;
}

int git_configset_get_string(struct config_set *cs, const char *key, char **dest)
{
const char *value;
int ret;

if ((ret = git_configset_get_value(cs, key, &value)))
return ret;
return git_config_string(dest, key, value);
}

static int git_configset_get_string_tmp(struct config_set *cs, const char *key,
const char **dest)
{
const char *value;
int ret;

if ((ret = git_configset_get_value(cs, key, &value)))
return ret;
if (!value)
return config_error_nonbool(key);
*dest = value;
return 0;
}

int git_configset_get_int(struct config_set *cs, const char *key, int *dest)
{
const char *value;
int ret;

if ((ret = git_configset_get_value(cs, key, &value)))
return ret;
*dest = git_config_int(key, value);
return 0;
}

int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest)
{
const char *value;
int ret;

if ((ret = git_configset_get_value(cs, key, &value)))
return ret;
*dest = git_config_ulong(key, value);
return 0;
}

int git_configset_get_bool(struct config_set *cs, const char *key, int *dest)
{
const char *value;
int ret;

if ((ret = git_configset_get_value(cs, key, &value)))
return ret;
*dest = git_config_bool(key, value);
return 0;
}

int git_configset_get_bool_or_int(struct config_set *cs, const char *key,
int *is_bool, int *dest)
{
const char *value;
int ret;

if ((ret = git_configset_get_value(cs, key, &value)))
return ret;
*dest = git_config_bool_or_int(key, value, is_bool);
return 0;
}

int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest)
{
const char *value;
int ret;

if ((ret = git_configset_get_value(cs, key, &value)))
return ret;
*dest = git_parse_maybe_bool(value);
if (*dest == -1)
return -1;
return 0;
}

int git_configset_get_pathname(struct config_set *cs, const char *key, char **dest)
{
const char *value;
int ret;

if ((ret = git_configset_get_value(cs, key, &value)))
return ret;
return git_config_pathname(dest, key, value);
}

/* Functions use to read configuration from a repository */
static void repo_read_config(struct repository *repo)
{
Expand Down Expand Up @@ -2624,15 +2520,24 @@ void repo_config(struct repository *repo, config_fn_t fn, void *data)

int repo_config_get(struct repository *repo, const char *key)
{
struct config_set_element *e;
int ret;

git_config_check_init(repo);
return git_configset_get(repo->config, key);

if ((ret = configset_find_element(repo->config, key, &e)))
return ret;
else if (!e)
return 1;
return 0;
}

int repo_config_get_value(struct repository *repo,
const char *key, const char **value)
{
git_config_check_init(repo);
return git_configset_get_value(repo->config, key, value);

}

int repo_config_get_value_multi(struct repository *repo, const char *key,
Expand All @@ -2652,69 +2557,102 @@ int repo_config_get_string_multi(struct repository *repo, const char *key,
int repo_config_get_string(struct repository *repo,
const char *key, char **dest)
{
const char *value;
int ret;
git_config_check_init(repo);
ret = git_configset_get_string(repo->config, key, dest);
if (ret < 0)

if ((ret = repo_config_get_value(repo, key, &value)))
return ret;
if (git_config_string(dest, key, value) < 0)
git_die_config(key, NULL);
return ret;
}

int repo_config_get_string_tmp(struct repository *repo,
const char *key, const char **dest)
{
const char *value;
int ret;
git_config_check_init(repo);
ret = git_configset_get_string_tmp(repo->config, key, dest);
if (ret < 0)

if ((ret = repo_config_get_value(repo, key, &value)))
return ret;
if (!value && (ret = config_error_nonbool(key)) < 0)
git_die_config(key, NULL);
return ret;
*dest = value;
return 0;
}

int repo_config_get_int(struct repository *repo,
const char *key, int *dest)
{
git_config_check_init(repo);
return git_configset_get_int(repo->config, key, dest);
const char *value;
int ret;

if ((ret = repo_config_get_value(repo, key, &value)))
return ret;
*dest = git_config_int(key, value);
return 0;
}

int repo_config_get_ulong(struct repository *repo,
const char *key, unsigned long *dest)
{
git_config_check_init(repo);
return git_configset_get_ulong(repo->config, key, dest);
const char *value;
int ret;

if ((ret = repo_config_get_value(repo, key, &value)))
return ret;
*dest = git_config_ulong(key, value);
return 0;
}

int repo_config_get_bool(struct repository *repo,
const char *key, int *dest)
{
git_config_check_init(repo);
return git_configset_get_bool(repo->config, key, dest);
const char *value;
int ret;

if ((ret = repo_config_get_value(repo, key, &value)))
return ret;
*dest = git_config_bool(key, value);
return 0;
}

int repo_config_get_bool_or_int(struct repository *repo,
const char *key, int *is_bool, int *dest)
{
git_config_check_init(repo);
return git_configset_get_bool_or_int(repo->config, key, is_bool, dest);
const char *value;
int ret;

if ((ret = repo_config_get_value(repo, key, &value)))
return ret;
*dest = git_config_bool_or_int(key, value, is_bool);
return 0;
}

int repo_config_get_maybe_bool(struct repository *repo,
const char *key, int *dest)
{
git_config_check_init(repo);
return git_configset_get_maybe_bool(repo->config, key, dest);

const char *value;
int ret;

if ((ret = repo_config_get_value(repo, key, &value)))
return ret;
*dest = git_parse_maybe_bool(value);
if (*dest == -1)
return -1;
return 0;
}

int repo_config_get_pathname(struct repository *repo,
const char *key, char **dest)
{
const char *value;
int ret;
git_config_check_init(repo);
ret = git_configset_get_pathname(repo->config, key, dest);
if (ret < 0)
git_die_config(key, NULL);
return ret;

if ((ret = repo_config_get_value(repo, key, &value)))
return ret;
return git_config_pathname(dest, key, value);
}

/* Read values into protected_config. */
Expand Down
8 changes: 0 additions & 8 deletions config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,14 +502,6 @@ int git_configset_get(struct config_set *cs, const char *key);
*/
int git_configset_get_value(struct config_set *cs, const char *key, const char **dest);

int git_configset_get_string(struct config_set *cs, const char *key, char **dest);
int git_configset_get_int(struct config_set *cs, const char *key, int *dest);
int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest);
int git_configset_get_bool(struct config_set *cs, const char *key, int *dest);
int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest);
int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest);
int git_configset_get_pathname(struct config_set *cs, const char *key, char **dest);

/* Functions for reading a repository's config */
struct repository;
void repo_config(struct repository *repo, config_fn_t fn, void *data);
Expand Down
9 changes: 9 additions & 0 deletions repository.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,15 @@ int repo_submodule_init(struct repository *subrepo,
return ret;
}

void repo_init_repo_blank_config(struct repository *dest,
struct repository *src,
struct config_set *config)
{
memcpy(dest, src, sizeof(*dest));
dest->config = config;
git_configset_init(dest->config);
}

static void repo_clear_path_cache(struct repo_path_cache *cache)
{
FREE_AND_NULL(cache->squash_msg);
Expand Down
30 changes: 30 additions & 0 deletions repository.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,36 @@ void initialize_the_repository(void);
RESULT_MUST_BE_USED
int repo_init(struct repository *r, const char *gitdir, const char *worktree);

/**
* Given an existing `src` repository (e.g. "the_repository") shallow
* copies it to the `dest`, except for the `config`, which will point
* to a "struct config_set" you provide, and which will be initialized
* with git_configset_init().
*
* This is a convenience helper to get a "fake" repository object with
* an alternate config, for use with the config API's
* repo_config_get_*() family of functions. E.g.:
*
*
* struct repository repo;
* struct config_set cs;
* int v;
*
* repo_init_repo_blank_config(&repo, the_repository, &cs);
* git_configset_add_file(repo.config, "custom.config");
*
* if (!repo_config_get_bool(&repo, "some.key", &v))
* ;
* git_configset_clear(&cs);
*
* When you're done with it you'll need to git_configset_clear() your
* own `config`, but must not repo_clear() or otherwise modify the
* `dest` (as we'll also hopefully clear the `src` elsewhere.
*/
void repo_init_repo_blank_config(struct repository *dest,
struct repository *src,
struct config_set *config);

/*
* Initialize the repository 'subrepo' as the submodule at the given path. If
* the submodule's gitdir cannot be found at <path>/.git, this function calls
Expand Down
Loading

0 comments on commit 0233297

Please sign in to comment.