Re: [RFC PATCH 1/1] maintenance: add config option for config-file

From: Patrick Steinhardt <ps@pks.im>
Date: Dec 19, 2025, 07:18 UTC
Message-ID: <aUT8Vcevf8WiQgn0@pks.im>
In-Reply-To: <20251218184751.31209-2-matthewhughes934@gmail.com>

On Thu, Dec 18, 2025 at 06:48:19PM +0000, Matthew Hughes wrote:

Show 19 quoted lines

> This is to allow splitting out this configuration from the global config
> file, e.g.:
> 
>     # in ~/.config/git/config
>     [include]
>         path = maintenance.config
>     [maintenance]
>         # use a separate files for reads/writes from
>         # 'git maintenance {un,}register'
>         configFile = ~/.config/git/maintenance.config
> 
>     # in ~/.config/git/maintenance.config
>     [maintenance]
>         repo = /path/to/some/repo
>         repo = /path/to/another/repo
> 
> My motivation for this is that I track my global config in git, so I'd
> like to avoid changes in there that depend on specific repos/workflows
> that I'm working with.

I think the idea is quite sensible, and I understand the need to split up static configuration from dynamic one. One part I'm unsure about though is the explicit need to use "include.path" for this. Ideally, git-maintenance(1) should by itself know to include the path that the user has configured in "maintenance.configFile".

Show 11 quoted lines

> diff --git a/builtin/gc.c b/builtin/gc.c
> index 92c6e7b954..257cceecf6 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -2124,6 +2124,10 @@ static int maintenance_register(int argc, const char **argv, const char *prefix,
>  		usage_with_options(builtin_maintenance_register_usage,
>  				   options);
>  
> +	if (config_file == NULL) {
> +		repo_config_get_pathname(the_repository, "maintenance.configFile", &config_file);
> +	}

Note that in our codebase this would typically be written as:

	if (!config_file)
		repo_config_get_pathname(the_repository, "maintenance.configFile", &config_file);

Note the missing curly braces around a single-line statement as well as the implicit check for NULL.

It's nice that we only need a two-line change here to make this work.

Show 7 quoted lines

> @@ -2194,6 +2198,10 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
>  		usage_with_options(builtin_maintenance_unregister_usage,
>  				   options);
>  
> +	if (config_file == NULL) {
> +		repo_config_get_pathname(the_repository, "maintenance.configFile", &config_file);
> +	}

Okay, this is the equivalent for unregistering maintenance. Makes senes.

So these both are straight-forward. The question is whether we can also easily teach scheduled maintenance to respect `maintenance.configFile` so that the user doesn't have to manually include the configured file. I think the answer is "not quite".

The services that scheduled maintenance writes use git-for-each-repo(1) to iterate through all repositories. We basically execute that command via `git for-each-repo --config=maintenance.repo -- git maintenance run --schedule=%%i`. But that command does not have a way to have it read the configuration from a different file.

I think adding such a feature shouldn't be that hard though. You can use the below (completely untested) patch as a starter for this. But once we had such a feature we'd only have to adapt how we write the services files to pass the new flag in case "maintenance.configFile" is set.

There's two questions in this context:

  - Do we already know to rewrite the service files in case the commands
    that Git would write change?

  - Do we need to migrate any of the preexisting keys that exist in the
    global configuration?

Also Cc'ing Stolee for input.

Thanks!

Patrick

-- >8 --

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 325a7925f1..874f98ced6 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -1,6 +1,7 @@
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "builtin.h"
+#include "abspath.h"
 #include "config.h"
 #include "gettext.h"
 #include "parse-options.h"
@@ -39,11 +40,16 @@ int cmd_for_each_repo(int argc,
 	int keep_going = 0;
 	int result = 0;
 	const struct string_list *values;
+	struct config_set configset = { 0 };
+	const char *config_file = NULL;
+	char *config_file_to_free = NULL;
 	int err;
 
 	const struct option options[] = {
 		OPT_STRING(0, "config", &config_key, N_("config"),
 			   N_("config key storing a list of repository paths")),
+		OPT_STRING(0, "file", &config_file, N_("file"),
+			   N_("use given config file")),
 		OPT_BOOL(0, "keep-going", &keep_going,
 			 N_("keep going even if command fails in a repository")),
 		OPT_END()
@@ -55,21 +61,38 @@ int cmd_for_each_repo(int argc,
 	if (!config_key)
 		die(_("missing --config=<config>"));
 
-	err = repo_config_get_string_multi(the_repository, config_key, &values);
+	if (config_file) {
+		if (!is_absolute_path(config_file) && prefix)
+			config_file = config_file_to_free = prefix_filename(prefix, config_file);
+
+		git_configset_init(&configset);
+		err = git_configset_add_file(&configset, config_file);
+		if (err < 0)
+			die_errno(_("config file could not be read: '%s'"), config_file);
+
+		err = git_configset_get_string_multi(&configset, config_key, &values);
+	} else {
+		err = repo_config_get_string_multi(the_repository, config_key, &values);
+	}
 	if (err < 0)
 		usage_msg_optf(_("got bad config --config=%s"),
 			       for_each_repo_usage, options, config_key);
 	else if (err)
-		return 0;
+		goto out;
 
 	for (size_t i = 0; i < values->nr; i++) {
 		int ret = run_command_on_repo(values->items[i].string, argc, argv);
 		if (ret) {
-			if (!keep_going)
-					return ret;
+			if (!keep_going) {
+				result = ret;
+				goto out;
+			}
 			result = 1;
 		}
 	}
 
+out:
+	git_configset_clear(&configset);
+	free(config_file_to_free);
 	return result;
 }