Skip to content

Commit

Permalink
config: add ctx arg to config_fn_t
Browse files Browse the repository at this point in the history
Add a new "const struct config_context *ctx" arg to config_fn_t to hold
additional information about the config iteration operation.
config_context has a "struct key_value_info kvi" member that holds
metadata about the config source being read (e.g. what kind of config
source it is, the filename, etc). In this series, we're only interested
in .kvi, so we could have just used "struct key_value_info" as an arg,
but config_context makes it possible to add/adjust members in the future
without changing the config_fn_t signature. We could also consider other
ways of organizing the args (e.g. moving the config name and value into
config_context or key_value_info), but in my experiments, the
incremental benefit doesn't justify the added complexity (e.g. a
config_fn_t will sometimes invoke another config_fn_t but with a
different config value).

In subsequent commits, the .kvi member will replace the global "struct
config_reader" in config.c, making config iteration a global-free
operation. It requires much more work for the machinery to provide
meaningful values of .kvi, so for now, merely change the signature and
call sites, pass NULL as a placeholder value, and don't rely on the arg
in any meaningful way.

Most of the changes are performed by
contrib/coccinelle/config_fn_ctx.pending.cocci, which, for every
config_fn_t:

- Modifies the signature to accept "const struct config_context *ctx"
- Passes "ctx" to any inner config_fn_t, if needed
- Adds UNUSED attributes to "ctx", if needed

Most config_fn_t instances are easily identified by seeing if they are
called by the various config functions. Most of the remaining ones are
manually named in the .cocci patch. Manual cleanups are still needed,
but the majority of it is trivial; it's either adjusting config_fn_t
that the .cocci patch didn't catch, or adding forward declarations of
"struct config_context ctx" to make the signatures make sense.

The non-trivial changes are in cases where we are invoking a config_fn_t
outside of config machinery, and we now need to decide what value of
"ctx" to pass. These cases are:

- trace2/tr2_cfg.c:tr2_cfg_set_fl()

  This is indirectly called by git_config_set() so that the trace2
  machinery can notice the new config values and update its settings
  using the tr2 config parsing function, i.e. tr2_cfg_cb().

- builtin/checkout.c:checkout_main()

  This calls git_xmerge_config() as a shorthand for parsing a CLI arg.
  This might be worth refactoring away in the future, since
  git_xmerge_config() can call git_default_config(), which can do much
  more than just parsing.

Handle them by creating a KVI_INIT macro that initializes "struct
key_value_info" to a reasonable default, and use that to construct the
"ctx" arg.

Signed-off-by: Glen Choo <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
chooglen authored and gitster committed Jun 28, 2023
1 parent e0f9a51 commit a4e7e31
Show file tree
Hide file tree
Showing 91 changed files with 515 additions and 181 deletions.
3 changes: 2 additions & 1 deletion alias.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ struct config_alias_data {
struct string_list *list;
};

static int config_alias_cb(const char *key, const char *value, void *d)
static int config_alias_cb(const char *key, const char *value,
const struct config_context *ctx UNUSED, void *d)
{
struct config_alias_data *data = d;
const char *p;
Expand Down
3 changes: 2 additions & 1 deletion archive-tar.c
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ static int tar_filter_config(const char *var, const char *value,
return 0;
}

static int git_tar_config(const char *var, const char *value, void *cb)
static int git_tar_config(const char *var, const char *value,
const struct config_context *ctx UNUSED, void *cb)
{
if (!strcmp(var, "tar.umask")) {
if (value && !strcmp(value, "user")) {
Expand Down
1 change: 1 addition & 0 deletions archive-zip.c
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,7 @@ static void dos_time(timestamp_t *timestamp, int *dos_date, int *dos_time)
}

static int archive_zip_config(const char *var, const char *value,
const struct config_context *ctx UNUSED,
void *data UNUSED)
{
return userdiff_config(var, value);
Expand Down
5 changes: 3 additions & 2 deletions builtin/add.c
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,8 @@ static struct option builtin_add_options[] = {
OPT_END(),
};

static int add_config(const char *var, const char *value, void *cb)
static int add_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
if (!strcmp(var, "add.ignoreerrors") ||
!strcmp(var, "add.ignore-errors")) {
Expand All @@ -368,7 +369,7 @@ static int add_config(const char *var, const char *value, void *cb)
if (git_color_config(var, value, cb) < 0)
return -1;

return git_default_config(var, value, cb);
return git_default_config(var, value, ctx, cb);
}

static const char embedded_advice[] = N_(
Expand Down
5 changes: 3 additions & 2 deletions builtin/blame.c
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,8 @@ static const char *add_prefix(const char *prefix, const char *path)
return prefix_path(prefix, prefix ? strlen(prefix) : 0, path);
}

static int git_blame_config(const char *var, const char *value, void *cb)
static int git_blame_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
if (!strcmp(var, "blame.showroot")) {
show_root = git_config_bool(var, value);
Expand Down Expand Up @@ -767,7 +768,7 @@ static int git_blame_config(const char *var, const char *value, void *cb)
if (userdiff_config(var, value) < 0)
return -1;

return git_default_config(var, value, cb);
return git_default_config(var, value, ctx, cb);
}

static int blame_copy_callback(const struct option *option, const char *arg, int unset)
Expand Down
5 changes: 3 additions & 2 deletions builtin/branch.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ static unsigned int colopts;

define_list_config_array(color_branch_slots);

static int git_branch_config(const char *var, const char *value, void *cb)
static int git_branch_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
const char *slot_name;

Expand Down Expand Up @@ -120,7 +121,7 @@ static int git_branch_config(const char *var, const char *value, void *cb)
if (git_color_config(var, value, cb) < 0)
return -1;

return git_default_config(var, value, cb);
return git_default_config(var, value, ctx, cb);
}

static const char *branch_get_color(enum color_branch ix)
Expand Down
5 changes: 3 additions & 2 deletions builtin/cat-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -873,12 +873,13 @@ static int batch_objects(struct batch_options *opt)
return retval;
}

static int git_cat_file_config(const char *var, const char *value, void *cb)
static int git_cat_file_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
if (userdiff_config(var, value) < 0)
return -1;

return git_default_config(var, value, cb);
return git_default_config(var, value, ctx, cb);
}

static int batch_option_callback(const struct option *opt,
Expand Down
12 changes: 9 additions & 3 deletions builtin/checkout.c
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,8 @@ static int switch_branches(const struct checkout_opts *opts,
return ret || writeout_error;
}

static int git_checkout_config(const char *var, const char *value, void *cb)
static int git_checkout_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
struct checkout_opts *opts = cb;

Expand All @@ -1202,7 +1203,7 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
if (starts_with(var, "submodule."))
return git_default_submodule_config(var, value, NULL);

return git_xmerge_config(var, value, NULL);
return git_xmerge_config(var, value, ctx, NULL);
}

static void setup_new_branch_info_and_source_tree(
Expand Down Expand Up @@ -1689,8 +1690,13 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
}

if (opts->conflict_style) {
struct key_value_info kvi = KVI_INIT;
struct config_context ctx = {
.kvi = &kvi,
};
opts->merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", opts->conflict_style, NULL);
git_xmerge_config("merge.conflictstyle", opts->conflict_style,
&ctx, NULL);
}
if (opts->force) {
opts->discard_changes = 1;
Expand Down
5 changes: 3 additions & 2 deletions builtin/clean.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ struct menu_stuff {

define_list_config_array(color_interactive_slots);

static int git_clean_config(const char *var, const char *value, void *cb)
static int git_clean_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
const char *slot_name;

Expand Down Expand Up @@ -133,7 +134,7 @@ static int git_clean_config(const char *var, const char *value, void *cb)
if (git_color_config(var, value, cb) < 0)
return -1;

return git_default_config(var, value, cb);
return git_default_config(var, value, ctx, cb);
}

static const char *clean_get_color(enum color_clean ix)
Expand Down
11 changes: 7 additions & 4 deletions builtin/clone.c
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,8 @@ static int checkout(int submodule_progress, int filter_submodules)
return err;
}

static int git_clone_config(const char *k, const char *v, void *cb)
static int git_clone_config(const char *k, const char *v,
const struct config_context *ctx, void *cb)
{
if (!strcmp(k, "clone.defaultremotename")) {
free(remote_name);
Expand All @@ -801,17 +802,19 @@ static int git_clone_config(const char *k, const char *v, void *cb)
if (!strcmp(k, "clone.filtersubmodules"))
config_filter_submodules = git_config_bool(k, v);

return git_default_config(k, v, cb);
return git_default_config(k, v, ctx, cb);
}

static int write_one_config(const char *key, const char *value, void *data)
static int write_one_config(const char *key, const char *value,
const struct config_context *ctx,
void *data)
{
/*
* give git_clone_config a chance to write config values back to the
* environment, since git_config_set_multivar_gently only deals with
* config-file writes
*/
int apply_failed = git_clone_config(key, value, data);
int apply_failed = git_clone_config(key, value, ctx, data);
if (apply_failed)
return apply_failed;

Expand Down
3 changes: 2 additions & 1 deletion builtin/column.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ static const char * const builtin_column_usage[] = {
};
static unsigned int colopts;

static int column_config(const char *var, const char *value, void *cb)
static int column_config(const char *var, const char *value,
const struct config_context *ctx UNUSED, void *cb)
{
return git_column_config(var, value, cb, &colopts);
}
Expand Down
1 change: 1 addition & 0 deletions builtin/commit-graph.c
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ static int write_option_max_new_filters(const struct option *opt,
}

static int git_commit_graph_write_config(const char *var, const char *value,
const struct config_context *ctx UNUSED,
void *cb UNUSED)
{
if (!strcmp(var, "commitgraph.maxnewfilters"))
Expand Down
10 changes: 6 additions & 4 deletions builtin/commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -1405,7 +1405,8 @@ static int parse_status_slot(const char *slot)
return LOOKUP_CONFIG(color_status_slots, slot);
}

static int git_status_config(const char *k, const char *v, void *cb)
static int git_status_config(const char *k, const char *v,
const struct config_context *ctx, void *cb)
{
struct wt_status *s = cb;
const char *slot_name;
Expand Down Expand Up @@ -1490,7 +1491,7 @@ static int git_status_config(const char *k, const char *v, void *cb)
s->detect_rename = git_config_rename(k, v);
return 0;
}
return git_diff_ui_config(k, v, NULL);
return git_diff_ui_config(k, v, ctx, NULL);
}

int cmd_status(int argc, const char **argv, const char *prefix)
Expand Down Expand Up @@ -1605,7 +1606,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
return 0;
}

static int git_commit_config(const char *k, const char *v, void *cb)
static int git_commit_config(const char *k, const char *v,
const struct config_context *ctx, void *cb)
{
struct wt_status *s = cb;

Expand All @@ -1627,7 +1629,7 @@ static int git_commit_config(const char *k, const char *v, void *cb)
return 0;
}

return git_status_config(k, v, s);
return git_status_config(k, v, ctx, s);
}

int cmd_commit(int argc, const char **argv, const char *prefix)
Expand Down
10 changes: 8 additions & 2 deletions builtin/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ static void show_config_scope(struct strbuf *buf)
}

static int show_all_config(const char *key_, const char *value_,
const struct config_context *ctx UNUSED,
void *cb UNUSED)
{
if (show_origin || show_scope) {
Expand Down Expand Up @@ -301,7 +302,8 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value
return 0;
}

static int collect_config(const char *key_, const char *value_, void *cb)
static int collect_config(const char *key_, const char *value_,
const struct config_context *ctx UNUSED, void *cb)
{
struct strbuf_list *values = cb;

Expand Down Expand Up @@ -470,6 +472,7 @@ static const char *get_colorbool_slot;
static char parsed_color[COLOR_MAXLEN];

static int git_get_color_config(const char *var, const char *value,
const struct config_context *ctx UNUSED,
void *cb UNUSED)
{
if (!strcmp(var, get_color_slot)) {
Expand Down Expand Up @@ -503,6 +506,7 @@ static int get_colorbool_found;
static int get_diff_color_found;
static int get_color_ui_found;
static int git_get_colorbool_config(const char *var, const char *value,
const struct config_context *ctx UNUSED,
void *data UNUSED)
{
if (!strcmp(var, get_colorbool_slot))
Expand Down Expand Up @@ -561,7 +565,9 @@ struct urlmatch_current_candidate_value {
struct strbuf value;
};

static int urlmatch_collect_fn(const char *var, const char *value, void *cb)
static int urlmatch_collect_fn(const char *var, const char *value,
const struct config_context *ctx UNUSED,
void *cb)
{
struct string_list *values = cb;
struct string_list_item *item = string_list_insert(values, var);
Expand Down
5 changes: 3 additions & 2 deletions builtin/difftool.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,15 @@ static const char *const builtin_difftool_usage[] = {
NULL
};

static int difftool_config(const char *var, const char *value, void *cb)
static int difftool_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
if (!strcmp(var, "difftool.trustexitcode")) {
trust_exit_code = git_config_bool(var, value);
return 0;
}

return git_default_config(var, value, cb);
return git_default_config(var, value, ctx, cb);
}

static int print_tool_help(void)
Expand Down
9 changes: 6 additions & 3 deletions builtin/fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ struct fetch_config {
int submodule_fetch_jobs;
};

static int git_fetch_config(const char *k, const char *v, void *cb)
static int git_fetch_config(const char *k, const char *v,
const struct config_context *ctx, void *cb)
{
struct fetch_config *fetch_config = cb;

Expand Down Expand Up @@ -164,7 +165,7 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
"fetch.output", v);
}

return git_default_config(k, v, cb);
return git_default_config(k, v, ctx, cb);
}

static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
Expand Down Expand Up @@ -1799,7 +1800,9 @@ struct remote_group_data {
struct string_list *list;
};

static int get_remote_group(const char *key, const char *value, void *priv)
static int get_remote_group(const char *key, const char *value,
const struct config_context *ctx UNUSED,
void *priv)
{
struct remote_group_data *g = priv;

Expand Down
5 changes: 3 additions & 2 deletions builtin/fsmonitor--daemon.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ static int fsmonitor__start_timeout_sec = 60;
#define FSMONITOR__ANNOUNCE_STARTUP "fsmonitor.announcestartup"
static int fsmonitor__announce_startup = 0;

static int fsmonitor_config(const char *var, const char *value, void *cb)
static int fsmonitor_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
if (!strcmp(var, FSMONITOR__IPC_THREADS)) {
int i = git_config_int(var, value);
Expand Down Expand Up @@ -67,7 +68,7 @@ static int fsmonitor_config(const char *var, const char *value, void *cb)
return 0;
}

return git_default_config(var, value, cb);
return git_default_config(var, value, ctx, cb);
}

/*
Expand Down
7 changes: 4 additions & 3 deletions builtin/grep.c
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,14 @@ static int wait_all(void)
return hit;
}

static int grep_cmd_config(const char *var, const char *value, void *cb)
static int grep_cmd_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
int st = grep_config(var, value, cb);
int st = grep_config(var, value, ctx, cb);

if (git_color_config(var, value, cb) < 0)
st = -1;
else if (git_default_config(var, value, cb) < 0)
else if (git_default_config(var, value, ctx, cb) < 0)
st = -1;

if (!strcmp(var, "grep.threads")) {
Expand Down
5 changes: 3 additions & 2 deletions builtin/help.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,8 @@ static int add_man_viewer_info(const char *var, const char *value)
return 0;
}

static int git_help_config(const char *var, const char *value, void *cb)
static int git_help_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
if (!strcmp(var, "help.format")) {
if (!value)
Expand All @@ -421,7 +422,7 @@ static int git_help_config(const char *var, const char *value, void *cb)
if (starts_with(var, "man."))
return add_man_viewer_info(var, value);

return git_default_config(var, value, cb);
return git_default_config(var, value, ctx, cb);
}

static struct cmdnames main_cmds, other_cmds;
Expand Down
Loading

0 comments on commit a4e7e31

Please sign in to comment.