From: Patrick Steinhardt Date: Fri, 19 Dec 2025 07:18:45 GMT Subject: Re: [RFC PATCH 1/1] maintenance: add config option for config-file Message-ID: In-Reply-To: <20251218184751.31209-2-matthewhughes934@gmail.com> On Thu, Dec 18, 2025 at 06:48:19PM +0000, Matthew Hughes wrote: > 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". > 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. > @@ -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=")); - 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; }