{"thread":{"id":"64652","subject":"[RFC PATCH 0/1] maintenance: add config option for config-file","startedAt":"2025-12-18T18:48:10Z","lastAt":"2025-12-22T21:51:55Z","messageCount":6,"participants":["Matthew Hughes","Patrick Steinhardt","Junio C Hamano","D. Ben Knoble"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"532496","messageId":"20251218184751.31209-1-matthewhughes934@gmail.com","threadId":"64652","inReplyTo":null,"subject":"[RFC PATCH 0/1] maintenance: add config option for config-file","fromName":"Matthew Hughes","fromEmail":"matthewhughes934@gmail.com","sentAt":"2025-12-18T18:48:07Z","receivedAt":"2025-12-18T18:48:10Z","isPatch":true,"sender":{"key":"matthewhughes934@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34972397?v=4"},"body":"A feature request I'm submitting as an RFC since it was about the same\neffort to write the code to describe the behaviour I was wanting.\n\nAn actual patch for the feature would involve some docs etc., but first\nI just wanted to see if the feature seemed reasonable.\n\nMatthew Hughes (1):\n  maintenance: add config option for config-file\n\n builtin/gc.c           |  8 ++++++++\n t/t7900-maintenance.sh | 13 +++++++++++++\n 2 files changed, 21 insertions(+)\n\n-- \n2.52.0\n\n"},{"id":"532497","messageId":"20251218184751.31209-2-matthewhughes934@gmail.com","threadId":"64652","inReplyTo":"20251218184751.31209-1-matthewhughes934@gmail.com","subject":"[RFC PATCH 1/1] maintenance: add config option for config-file","fromName":"Matthew Hughes","fromEmail":"matthewhughes934@gmail.com","sentAt":"2025-12-18T18:48:19Z","receivedAt":"2025-12-18T18:48:24Z","isPatch":true,"sender":{"key":"matthewhughes934@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34972397?v=4"},"body":"This is to allow splitting out this configuration from the global config\nfile, e.g.:\n\n    # in ~/.config/git/config\n    [include]\n        path = maintenance.config\n    [maintenance]\n        # use a separate files for reads/writes from\n        # 'git maintenance {un,}register'\n        configFile = ~/.config/git/maintenance.config\n\n    # in ~/.config/git/maintenance.config\n    [maintenance]\n        repo = /path/to/some/repo\n        repo = /path/to/another/repo\n\nMy motivation for this is that I track my global config in git, so I'd\nlike to avoid changes in there that depend on specific repos/workflows\nthat I'm working with.\n\nSigned-off-by: Matthew Hughes <matthewhughes934@gmail.com>\n---\n builtin/gc.c           |  8 ++++++++\n t/t7900-maintenance.sh | 13 +++++++++++++\n 2 files changed, 21 insertions(+)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 92c6e7b954..257cceecf6 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -2124,6 +2124,10 @@ static int maintenance_register(int argc, const char **argv, const char *prefix,\n \t\tusage_with_options(builtin_maintenance_register_usage,\n \t\t\t\t   options);\n \n+\tif (config_file == NULL) {\n+\t\trepo_config_get_pathname(the_repository, \"maintenance.configFile\", &config_file);\n+\t}\n+\n \t/* Disable foreground maintenance */\n \trepo_config_set(the_repository, \"maintenance.auto\", \"false\");\n \n@@ -2194,6 +2198,10 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \t\tusage_with_options(builtin_maintenance_unregister_usage,\n \t\t\t\t   options);\n \n+\tif (config_file == NULL) {\n+\t\trepo_config_get_pathname(the_repository, \"maintenance.configFile\", &config_file);\n+\t}\n+\n \tif (config_file) {\n \t\tgit_configset_init(&cs);\n \t\tgit_configset_add_file(&cs, config_file);\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 6b36f52df7..baad960051 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -1024,6 +1024,19 @@ test_expect_success 'register and unregister' '\n \tgit maintenance unregister --config-file ./other --force\n '\n \n+test_expect_success 'register and unregister config from maintenance.configFile' '\n+\ttest_when_finished git config --global --unset-all maintenance.configFile &&\n+\n+\tgit config set --global maintenance.configFile ./maintenance.config &&\n+\tgit maintenance register &&\n+\tpwd >>expect &&\n+\tgit config get --file ./maintenance.config maintenance.repo >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tgit maintenance unregister &&\n+\ttest_must_be_empty ./maintenance.config\n+'\n+\n test_expect_success 'register with no value for maintenance.repo' '\n \tcp .git/config .git/config.orig &&\n \ttest_when_finished mv .git/config.orig .git/config &&\n-- \n2.52.0\n\n"},{"id":"532520","messageId":"aUT8Vcevf8WiQgn0@pks.im","threadId":"64652","inReplyTo":"20251218184751.31209-2-matthewhughes934@gmail.com","subject":"Re: [RFC PATCH 1/1] maintenance: add config option for config-file","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-19T07:18:45Z","receivedAt":"2025-12-19T07:18:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Dec 18, 2025 at 06:48:19PM +0000, Matthew Hughes wrote:\n> This is to allow splitting out this configuration from the global config\n> file, e.g.:\n> \n>     # in ~/.config/git/config\n>     [include]\n>         path = maintenance.config\n>     [maintenance]\n>         # use a separate files for reads/writes from\n>         # 'git maintenance {un,}register'\n>         configFile = ~/.config/git/maintenance.config\n> \n>     # in ~/.config/git/maintenance.config\n>     [maintenance]\n>         repo = /path/to/some/repo\n>         repo = /path/to/another/repo\n> \n> My motivation for this is that I track my global config in git, so I'd\n> like to avoid changes in there that depend on specific repos/workflows\n> that I'm working with.\n\nI think the idea is quite sensible, and I understand the need to split\nup static configuration from dynamic one. One part I'm unsure about\nthough is the explicit need to use \"include.path\" for this. Ideally,\ngit-maintenance(1) should by itself know to include the path that the\nuser has configured in \"maintenance.configFile\".\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 92c6e7b954..257cceecf6 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -2124,6 +2124,10 @@ static int maintenance_register(int argc, const char **argv, const char *prefix,\n>  \t\tusage_with_options(builtin_maintenance_register_usage,\n>  \t\t\t\t   options);\n>  \n> +\tif (config_file == NULL) {\n> +\t\trepo_config_get_pathname(the_repository, \"maintenance.configFile\", &config_file);\n> +\t}\n\nNote that in our codebase this would typically be written as:\n\n\tif (!config_file)\n\t\trepo_config_get_pathname(the_repository, \"maintenance.configFile\", &config_file);\n\nNote the missing curly braces around a single-line statement as well as\nthe implicit check for NULL.\n\nIt's nice that we only need a two-line change here to make this work.\n\n> @@ -2194,6 +2198,10 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n>  \t\tusage_with_options(builtin_maintenance_unregister_usage,\n>  \t\t\t\t   options);\n>  \n> +\tif (config_file == NULL) {\n> +\t\trepo_config_get_pathname(the_repository, \"maintenance.configFile\", &config_file);\n> +\t}\n\nOkay, this is the equivalent for unregistering maintenance. Makes senes.\n\nSo these both are straight-forward. The question is whether we can also\neasily teach scheduled maintenance to respect `maintenance.configFile`\nso that the user doesn't have to manually include the configured file.\nI think the answer is \"not quite\".\n\nThe services that scheduled maintenance writes use git-for-each-repo(1)\nto iterate through all repositories. We basically execute that command\nvia `git for-each-repo --config=maintenance.repo -- git maintenance\nrun --schedule=%%i`. But that command does not have a way to have it\nread the configuration from a different file.\n\nI think adding such a feature shouldn't be that hard though. You can use\nthe below (completely untested) patch as a starter for this. But once we\nhad such a feature we'd only have to adapt how we write the services\nfiles to pass the new flag in case \"maintenance.configFile\" is set.\n\nThere's two questions in this context:\n\n  - Do we already know to rewrite the service files in case the commands\n    that Git would write change?\n\n  - Do we need to migrate any of the preexisting keys that exist in the\n    global configuration?\n\nAlso Cc'ing Stolee for input.\n\nThanks!\n\nPatrick\n\n-- >8 --\n\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 325a7925f1..874f98ced6 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -1,6 +1,7 @@\n #define USE_THE_REPOSITORY_VARIABLE\n \n #include \"builtin.h\"\n+#include \"abspath.h\"\n #include \"config.h\"\n #include \"gettext.h\"\n #include \"parse-options.h\"\n@@ -39,11 +40,16 @@ int cmd_for_each_repo(int argc,\n \tint keep_going = 0;\n \tint result = 0;\n \tconst struct string_list *values;\n+\tstruct config_set configset = { 0 };\n+\tconst char *config_file = NULL;\n+\tchar *config_file_to_free = NULL;\n \tint err;\n \n \tconst struct option options[] = {\n \t\tOPT_STRING(0, \"config\", &config_key, N_(\"config\"),\n \t\t\t   N_(\"config key storing a list of repository paths\")),\n+\t\tOPT_STRING(0, \"file\", &config_file, N_(\"file\"),\n+\t\t\t   N_(\"use given config file\")),\n \t\tOPT_BOOL(0, \"keep-going\", &keep_going,\n \t\t\t N_(\"keep going even if command fails in a repository\")),\n \t\tOPT_END()\n@@ -55,21 +61,38 @@ int cmd_for_each_repo(int argc,\n \tif (!config_key)\n \t\tdie(_(\"missing --config=<config>\"));\n \n-\terr = repo_config_get_string_multi(the_repository, config_key, &values);\n+\tif (config_file) {\n+\t\tif (!is_absolute_path(config_file) && prefix)\n+\t\t\tconfig_file = config_file_to_free = prefix_filename(prefix, config_file);\n+\n+\t\tgit_configset_init(&configset);\n+\t\terr = git_configset_add_file(&configset, config_file);\n+\t\tif (err < 0)\n+\t\t\tdie_errno(_(\"config file could not be read: '%s'\"), config_file);\n+\n+\t\terr = git_configset_get_string_multi(&configset, config_key, &values);\n+\t} else {\n+\t\terr = repo_config_get_string_multi(the_repository, config_key, &values);\n+\t}\n \tif (err < 0)\n \t\tusage_msg_optf(_(\"got bad config --config=%s\"),\n \t\t\t       for_each_repo_usage, options, config_key);\n \telse if (err)\n-\t\treturn 0;\n+\t\tgoto out;\n \n \tfor (size_t i = 0; i < values->nr; i++) {\n \t\tint ret = run_command_on_repo(values->items[i].string, argc, argv);\n \t\tif (ret) {\n-\t\t\tif (!keep_going)\n-\t\t\t\t\treturn ret;\n+\t\t\tif (!keep_going) {\n+\t\t\t\tresult = ret;\n+\t\t\t\tgoto out;\n+\t\t\t}\n \t\t\tresult = 1;\n \t\t}\n \t}\n \n+out:\n+\tgit_configset_clear(&configset);\n+\tfree(config_file_to_free);\n \treturn result;\n }\n"},{"id":"532527","messageId":"xmqqike2x4ei.fsf@gitster.g","threadId":"64652","inReplyTo":"20251218184751.31209-2-matthewhughes934@gmail.com","subject":"Re: [RFC PATCH 1/1] maintenance: add config option for config-file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-19T08:27:33Z","receivedAt":"2025-12-19T08:27:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthew Hughes <matthewhughes934@gmail.com> writes:\n\n> This is to allow splitting out this configuration from the global config\n> file, e.g.:\n\nI cannot guess what \"this\" refers to in this sentence.\n\n>     # in ~/.config/git/config\n>     [include]\n>         path = maintenance.config\n>     [maintenance]\n>         # use a separate files for reads/writes from\n>         # 'git maintenance {un,}register'\n>         configFile = ~/.config/git/maintenance.config\n>\n>     # in ~/.config/git/maintenance.config\n>     [maintenance]\n>         repo = /path/to/some/repo\n>         repo = /path/to/another/repo\n\nYou are burdening your readers too heavily.  After reading the above\nthree times and then trying to guess what you are trying to do for\nseveral minutes, what I am guessing is:\n\n * maintenance.configFile specifies an additional file to which\n   maintenance.repo configuration items are written out when \"git\n   maintenance register/unregister\" works.  \n\n * \"git config\" is not affected, so \"git config set --global\n   --append maintenance.repo foo\" would still write into the\n   per-user configuration file.\n\n * Also, the general config API does not pay maintenance.configFile\n   at all, so setting it does not affect \"git config list\", for\n   example.\n\n * You'd need an extra \"[include] path = maintenance.config\" in the\n   configuration file because of the previous point.\n\nAm I following you well so far?  Giving an explanation on your\n_intent_, along with the sample configuration, would help your\nreaders, and I would expect something with a similar degree of\ndetail as above in the log message.\n\n> My motivation for this is that I track my global config in git, so I'd\n> like to avoid changes in there that depend on specific repos/workflows\n> that I'm working with.\n\nI am not sure if singling out \"maintenance\" is the right approach to\nsolve that issue.  If we had a mechanism to have two per-user\nconfiguration file, where one is read-only (as far as Git is\nconcerned) which is covered/overlayed with a separate read-write\nfile, not just \"maintenance register/unregister\" but all other\nthings that writes into \"git config\" would use that overlayed file\nwithout touching the base configuration that is read-only.  Wouldn't\nthat be closer to what you want?\n\n> Signed-off-by: Matthew Hughes <matthewhughes934@gmail.com>\n> ---\n>  builtin/gc.c           |  8 ++++++++\n>  t/t7900-maintenance.sh | 13 +++++++++++++\n>  2 files changed, 21 insertions(+)\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 92c6e7b954..257cceecf6 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -2124,6 +2124,10 @@ static int maintenance_register(int argc, const char **argv, const char *prefix,\n>  \t\tusage_with_options(builtin_maintenance_register_usage,\n>  \t\t\t\t   options);\n>  \n> +\tif (config_file == NULL) {\n> +\t\trepo_config_get_pathname(the_repository, \"maintenance.configFile\", &config_file);\n> +\t}\n\n * Comparison with 0 or NULL should be spelled \"if (!config_file)\"\n   (meaning, 'is NULL') or \"if (config_file)\" (meaning, 'not NULL'),\n   in this project.\n\n * This project omits {} around a single statement block.\n\n * The function call is overly long. wrap to comfortably fit on\n   80-column terminal after getting quoted in an e-mail review twice\n   or so, which means ~72 columns is the practical width limit.\n\n\trepo_config_get_pathname(the_repository,\n        \t\t\t\"maintenance.configFile\", &config_file);\n\n> +test_expect_success 'register and unregister config from maintenance.configFile' '\n> +\ttest_when_finished git config --global --unset-all maintenance.configFile &&\n> +\n> +\tgit config set --global maintenance.configFile ./maintenance.config &&\n> +\tgit maintenance register &&\n> +\tpwd >>expect &&\n\nReaders would wonder \"To what existing contents is this being\nappended?  Do we have something that we care?\"  If not, do not use\n\">>\" to mislead them.\n\nWould the output of the pwd command match what \"maintenance\nregister\" writes into the file even on Windows?  We often see\nbreakage between $(pwd) and $PWD and I can never get this right\nwithout looking at past discussions.\n\n> +\tgit config get --file ./maintenance.config maintenance.repo >actual &&\n> +\ttest_cmp expect actual &&\n\nFor this particular case, would it be sufficient to ask\n\n    git config get --all --no-includes --file maintenance.config \\\n\tmaintenance.repo\n\n    git config get --all --no-includes --file ./git/config \\\n\tmaintenance.repo\n\nand see if the former gives output and the latter does not, or\nsomething?\n\nMaking sure the maintenance.repo file gets written is good, but for\nyour purpose, it is equally if not more important that the base\nconfiguration file is not affected, no?\n\n> +\tgit maintenance unregister &&\n> +\ttest_must_be_empty ./maintenance.config\n\nDitto.\n\n> +'\n> +\n>  test_expect_success 'register with no value for maintenance.repo' '\n>  \tcp .git/config .git/config.orig &&\n>  \ttest_when_finished mv .git/config.orig .git/config &&\n"},{"id":"532613","messageId":"fmj4be365s6jczb6p2ccb6a6vh64bltgfl5neshu6g7hrabzeb@twzrzmprhotf","threadId":"64652","inReplyTo":"xmqqike2x4ei.fsf@gitster.g","subject":"Re: [RFC PATCH 1/1] maintenance: add config option for config-file","fromName":"Matthew Hughes","fromEmail":"matthewhughes934@gmail.com","sentAt":"2025-12-22T08:26:51Z","receivedAt":"2025-12-22T08:26:54Z","isPatch":true,"sender":{"key":"matthewhughes934@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34972397?v=4"},"body":"On Fri, Dec 19, 2025 at 05:27:33PM +0900, Junio C Hamano wrote:\n>  * maintenance.configFile specifies an additional file to which\n>    maintenance.repo configuration items are written out when \"git\n>    maintenance register/unregister\" works.  \n> \n>  * \"git config\" is not affected, so \"git config set --global\n>    --append maintenance.repo foo\" would still write into the\n>    per-user configuration file.\n> \n>  * Also, the general config API does not pay maintenance.configFile\n>    at all, so setting it does not affect \"git config list\", for\n>    example.\n> \n>  * You'd need an extra \"[include] path = maintenance.config\" in the\n>    configuration file because of the previous point.\n> \n> Am I following you well so far? \n \nYep, this is a good summary of what my change looks to achieve. From Patrick's\nresponse (https://lore.kernel.org/git/aUT8Vcevf8WiQgn0@pks.im/) I understand\nthe requirement of the extra \"include.path\" setting is likely not acceptable\nfor a usability point of view.\n\n> Giving an explanation on your _intent_, along with the sample configuration,\n> would help your readers, and I would expect something with a similar degree\n> of detail as above in the log message.\n\nThanks for the feedback, I'll look to be clearer with my intent in the future.\n\n> I am not sure if singling out \"maintenance\" is the right approach to\n> solve that issue.  If we had a mechanism to have two per-user\n> configuration file, where one is read-only (as far as Git is\n> concerned) which is covered/overlayed with a separate read-write\n> file, not just \"maintenance register/unregister\" but all other\n> things that writes into \"git config\" would use that overlayed file\n> without touching the base configuration that is read-only.  Wouldn't\n> that be closer to what you want?\n\nIndeed a read-only config as you described would be a more general solution,\nand a better one than focusing on single commands like this change does. I'm\nnow curious if a similar idea has been discussed in the past? I'll go have a\nlook in the history of this mailing list.\n\nThat leads me to think my proposed change is too narrow in scope, and risks\ndividing functionality: where some commands are taught to consider the separate\ntypes of configuration, while others are not.\n\nFor background: I singled out \"maintenance\" only because it's the first git\ncommand that I can remember seeing that was writing to my global config\n(outside of \"config\" itself).\n\n"},{"id":"532627","messageId":"CALnO6CD7Q1-vBKkeB81G04=MT5kSH4-Hm72cSP4hBuU+fDDR6g@mail.gmail.com","threadId":"64652","inReplyTo":"fmj4be365s6jczb6p2ccb6a6vh64bltgfl5neshu6g7hrabzeb@twzrzmprhotf","subject":"Re: [RFC PATCH 1/1] maintenance: add config option for config-file","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-12-22T21:51:43Z","receivedAt":"2025-12-22T21:51:55Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Mon, Dec 22, 2025 at 3:55 AM Matthew Hughes\n<matthewhughes934@gmail.com> wrote:\n>\n> > I am not sure if singling out \"maintenance\" is the right approach to\n> > solve that issue.  If we had a mechanism to have two per-user\n> > configuration file, where one is read-only (as far as Git is\n> > concerned) which is covered/overlayed with a separate read-write\n> > file, not just \"maintenance register/unregister\" but all other\n> > things that writes into \"git config\" would use that overlayed file\n> > without touching the base configuration that is read-only.  Wouldn't\n> > that be closer to what you want?\n>\n> Indeed a read-only config as you described would be a more general solution,\n> and a better one than focusing on single commands like this change does. I'm\n> now curious if a similar idea has been discussed in the past? I'll go have a\n> look in the history of this mailing list.\n>\n> That leads me to think my proposed change is too narrow in scope, and risks\n> dividing functionality: where some commands are taught to consider the separate\n> types of configuration, while others are not.\n>\n> For background: I singled out \"maintenance\" only because it's the first git\n> command that I can remember seeing that was writing to my global config\n> (outside of \"config\" itself).\n\nFWIW, I also include my gitconfig in version control, and the way I\nmanage this is with\n\n    [include]\n            path = ~/.gitconfig.local\n\nIf I run \"git maintenance register\", I then move the added\nconfiguration lines to ~/.gitconfig.local. It's a bit of a hassle, but\nnot much (I don't frequently add new repos to the set).\n\n-- \nD. Ben Knoble\n"}]}