Subject: Re: [PATCH v2 6/8] maintenance: add ability to pass config options

From: Junio C Hamano <[email-masked]>
Date: Thu, 24 Feb 2022 22:57:28 -0800

"Robert Coup via GitGitGadget" <[email-masked]> writes:

> +	if (config_opts)
> +		for (i = 0; i<config_opts->nr; i++)

Style.  SP on both sides of '<'.

> +			strvec_pushl(&maint.args, "-c", config_opts->v[i], NULL);

>  	strvec_pushl(&maint.args, "maintenance", "run", "--auto", NULL);
>  	strvec_push(&maint.args, quiet ? "--quiet" : "--no-quiet");

It is unclear if it is generally a good idea to pass hardcoded set
of configuration variables to begin with, but provided if it makes
sense [*], the implementation seems OK.

	Side note.  And this is a big *IF*, as we can see in all
	other helper functions in run-commands.h, nobody has such a
	privision.  If supporting such a "feature" makes sense, we
	probably would need to do so with a common interface that
	can be used across run_command() API, not with an ad-hoc
	interface that is only usable with run_auto_maintenance(),
	which may look somewhat similar to how we have a common way
	to pass set of environment variables.

> diff --git a/run-command.h b/run-command.h
> index 07bed6c31b4..24021abd41f 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -222,8 +222,11 @@ int run_command(struct child_process *);
>  
>  /*
>   * Trigger an auto-gc
> + *
> + * config_opts is an optional list of additional config options to
> + * pass to the maintenance process in the form "some.option=value".
>   */
> -int run_auto_maintenance(int quiet);
> +int run_auto_maintenance(int quiet, const struct strvec *config_opts);
>  
>  #define RUN_COMMAND_NO_STDIN		(1<<0)
>  #define RUN_GIT_CMD			(1<<1)
n