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

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

[Matthew Hughes]

A feature request I'm submitting as an RFC since it was about the same
effort to write the code to describe the behaviour I was wanting.

An actual patch for the feature would involve some docs etc., but first
I just wanted to see if the feature seemed reasonable.

Matthew Hughes (1):
  maintenance: add config option for config-file

 builtin/gc.c           |  8 ++++++++
 t/t7900-maintenance.sh | 13 +++++++++++++
 2 files changed, 21 insertions(+)

-- 
2.52.0

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

[Matthew Hughes]

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.

Signed-off-by: Matthew Hughes <matthewhughes934@gmail.com>
---
 builtin/gc.c           |  8 ++++++++
 t/t7900-maintenance.sh | 13 +++++++++++++
 2 files changed, 21 insertions(+)

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);
+	}
+
 	/* Disable foreground maintenance */
 	repo_config_set(the_repository, "maintenance.auto", "false");
 
@@ -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);
+	}
+
 	if (config_file) {
 		git_configset_init(&cs);
 		git_configset_add_file(&cs, config_file);
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 6b36f52df7..baad960051 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -1024,6 +1024,19 @@ test_expect_success 'register and unregister' '
 	git maintenance unregister --config-file ./other --force
 '
 
+test_expect_success 'register and unregister config from maintenance.configFile' '
+	test_when_finished git config --global --unset-all maintenance.configFile &&
+
+	git config set --global maintenance.configFile ./maintenance.config &&
+	git maintenance register &&
+	pwd >>expect &&
+	git config get --file ./maintenance.config maintenance.repo >actual &&
+	test_cmp expect actual &&
+
+	git maintenance unregister &&
+	test_must_be_empty ./maintenance.config
+'
+
 test_expect_success 'register with no value for maintenance.repo' '
 	cp .git/config .git/config.orig &&
 	test_when_finished mv .git/config.orig .git/config &&
-- 
2.52.0

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

[Patrick Steinhardt]

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=<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;
 }

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

[Junio C Hamano]

Matthew Hughes <matthewhughes934@gmail.com> writes:

> This is to allow splitting out this configuration from the global config
> file, e.g.:

I cannot guess what "this" refers to in this sentence.

>     # 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

You are burdening your readers too heavily.  After reading the above
three times and then trying to guess what you are trying to do for
several minutes, what I am guessing is:

 * maintenance.configFile specifies an additional file to which
   maintenance.repo configuration items are written out when "git
   maintenance register/unregister" works.  

 * "git config" is not affected, so "git config set --global
   --append maintenance.repo foo" would still write into the
   per-user configuration file.

 * Also, the general config API does not pay maintenance.configFile
   at all, so setting it does not affect "git config list", for
   example.

 * You'd need an extra "[include] path = maintenance.config" in the
   configuration file because of the previous point.

Am I following you well so far?  Giving an explanation on your
_intent_, along with the sample configuration, would help your
readers, and I would expect something with a similar degree of
detail as above in the log message.

> 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 am not sure if singling out "maintenance" is the right approach to
solve that issue.  If we had a mechanism to have two per-user
configuration file, where one is read-only (as far as Git is
concerned) which is covered/overlayed with a separate read-write
file, not just "maintenance register/unregister" but all other
things that writes into "git config" would use that overlayed file
without touching the base configuration that is read-only.  Wouldn't
that be closer to what you want?

> Signed-off-by: Matthew Hughes <matthewhughes934@gmail.com>
> ---
>  builtin/gc.c           |  8 ++++++++
>  t/t7900-maintenance.sh | 13 +++++++++++++
>  2 files changed, 21 insertions(+)

> 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);
> +	}

 * Comparison with 0 or NULL should be spelled "if (!config_file)"
   (meaning, 'is NULL') or "if (config_file)" (meaning, 'not NULL'),
   in this project.

 * This project omits {} around a single statement block.

 * The function call is overly long. wrap to comfortably fit on
   80-column terminal after getting quoted in an e-mail review twice
   or so, which means ~72 columns is the practical width limit.

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

> +test_expect_success 'register and unregister config from maintenance.configFile' '
> +	test_when_finished git config --global --unset-all maintenance.configFile &&
> +
> +	git config set --global maintenance.configFile ./maintenance.config &&
> +	git maintenance register &&
> +	pwd >>expect &&

Readers would wonder "To what existing contents is this being
appended?  Do we have something that we care?"  If not, do not use
">>" to mislead them.

Would the output of the pwd command match what "maintenance
register" writes into the file even on Windows?  We often see
breakage between $(pwd) and $PWD and I can never get this right
without looking at past discussions.

> +	git config get --file ./maintenance.config maintenance.repo >actual &&
> +	test_cmp expect actual &&

For this particular case, would it be sufficient to ask

    git config get --all --no-includes --file maintenance.config \
	maintenance.repo

    git config get --all --no-includes --file ./git/config \
	maintenance.repo

and see if the former gives output and the latter does not, or
something?

Making sure the maintenance.repo file gets written is good, but for
your purpose, it is equally if not more important that the base
configuration file is not affected, no?

> +	git maintenance unregister &&
> +	test_must_be_empty ./maintenance.config

Ditto.

> +'
> +
>  test_expect_success 'register with no value for maintenance.repo' '
>  	cp .git/config .git/config.orig &&
>  	test_when_finished mv .git/config.orig .git/config &&

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

[Matthew Hughes]

On Fri, Dec 19, 2025 at 05:27:33PM +0900, Junio C Hamano wrote:
>  * maintenance.configFile specifies an additional file to which
>    maintenance.repo configuration items are written out when "git
>    maintenance register/unregister" works.  
> 
>  * "git config" is not affected, so "git config set --global
>    --append maintenance.repo foo" would still write into the
>    per-user configuration file.
> 
>  * Also, the general config API does not pay maintenance.configFile
>    at all, so setting it does not affect "git config list", for
>    example.
> 
>  * You'd need an extra "[include] path = maintenance.config" in the
>    configuration file because of the previous point.
> 
> Am I following you well so far? 
 
Yep, this is a good summary of what my change looks to achieve. From Patrick's
response (https://lore.kernel.org/git/aUT8Vcevf8WiQgn0@pks.im/) I understand
the requirement of the extra "include.path" setting is likely not acceptable
for a usability point of view.

> Giving an explanation on your _intent_, along with the sample configuration,
> would help your readers, and I would expect something with a similar degree
> of detail as above in the log message.

Thanks for the feedback, I'll look to be clearer with my intent in the future.

> I am not sure if singling out "maintenance" is the right approach to
> solve that issue.  If we had a mechanism to have two per-user
> configuration file, where one is read-only (as far as Git is
> concerned) which is covered/overlayed with a separate read-write
> file, not just "maintenance register/unregister" but all other
> things that writes into "git config" would use that overlayed file
> without touching the base configuration that is read-only.  Wouldn't
> that be closer to what you want?

Indeed a read-only config as you described would be a more general solution,
and a better one than focusing on single commands like this change does. I'm
now curious if a similar idea has been discussed in the past? I'll go have a
look in the history of this mailing list.

That leads me to think my proposed change is too narrow in scope, and risks
dividing functionality: where some commands are taught to consider the separate
types of configuration, while others are not.

For background: I singled out "maintenance" only because it's the first git
command that I can remember seeing that was writing to my global config
(outside of "config" itself).

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

[D. Ben Knoble]

On Mon, Dec 22, 2025 at 3:55 AM Matthew Hughes
<matthewhughes934@gmail.com> wrote:
>
> > I am not sure if singling out "maintenance" is the right approach to
> > solve that issue.  If we had a mechanism to have two per-user
> > configuration file, where one is read-only (as far as Git is
> > concerned) which is covered/overlayed with a separate read-write
> > file, not just "maintenance register/unregister" but all other
> > things that writes into "git config" would use that overlayed file
> > without touching the base configuration that is read-only.  Wouldn't
> > that be closer to what you want?
>
> Indeed a read-only config as you described would be a more general solution,
> and a better one than focusing on single commands like this change does. I'm
> now curious if a similar idea has been discussed in the past? I'll go have a
> look in the history of this mailing list.
>
> That leads me to think my proposed change is too narrow in scope, and risks
> dividing functionality: where some commands are taught to consider the separate
> types of configuration, while others are not.
>
> For background: I singled out "maintenance" only because it's the first git
> command that I can remember seeing that was writing to my global config
> (outside of "config" itself).

FWIW, I also include my gitconfig in version control, and the way I
manage this is with

    [include]
            path = ~/.gitconfig.local

If I run "git maintenance register", I then move the added
configuration lines to ~/.gitconfig.local. It's a bit of a hassle, but
not much (I don't frequently add new repos to the set).

-- 
D. Ben Knoble