{"thread":{"id":"62684","subject":"[PATCH] maintenance: add prune-remote-refs task","startedAt":"2024-12-23T09:36:10Z","lastAt":"2025-01-07T18:48:49Z","messageCount":15,"participants":["Shubham Kanodia via GitGitGadget","Junio C Hamano","Shubham Kanodia","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"509512","messageId":"pull.1838.git.1734946566885.gitgitgadget@gmail.com","threadId":"62684","inReplyTo":null,"subject":"[PATCH] maintenance: add prune-remote-refs task","fromName":"Shubham Kanodia via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-23T09:36:06Z","receivedAt":"2024-12-23T09:36:10Z","isPatch":true,"sender":{"key":"shubham.kanodia10@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8946207?v=4"},"body":"From: Shubham Kanodia \n\nRemote-tracking refs can accumulate in local repositories even as branches\nare deleted on remotes, impacting git performance negatively. Existing\nalternatives to keep refs pruned have a few issues — \n\n1. The `fetch.prune` config automatically cleans up remote ref on fetch,\nbut also pulls in new ref from remote which is an undesirable side-effect.\n\n2.`git remote prune` cleans up refs without adding to the existing list\nbut requires periodic user intervention.\n\nThis adds a new maintenance task 'prune-remote-refs' that runs\n'git remote prune' for each configured remote daily. This provides an\nautomated way to clean up stale remote-tracking refs — especially when\nusers may not do a full fetch.\n\nThis task is disabled by default.\n\nSigned-off-by: Shubham Kanodia \n---\n maintenance: add prune-remote-refs task\n \n As discussed previously on:\n https://lore.kernel.org/git/xmqqwmfr112w.fsf@gitster.g/T/#t\n \n Remote-tracking refs can accumulate in local repositories even as\n branches are deleted on remotes, impacting git performance negatively.\n Existing alternatives to keep refs pruned have a few issues — \n \n 1. The fetch.prune config automatically cleans up remote ref on fetch,\n but also pulls in new ref from remote which is an undesirable\n side-effect.\n \n 2.git remote prune cleans up refs without adding to the existing list\n but requires periodic user intervention.\n \n This adds a new maintenance task 'prune-remote-refs' that runs 'git\n remote prune' for each configured remote daily. This provides an\n automated way to clean up stale remote-tracking refs — especially when\n users may not do a full fetch.\n \n This task is disabled by default.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1838%2Fpastelsky%2Fsk%2Fadd-remote-prune-maintenance-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1838/pastelsky/sk/add-remote-prune-maintenance-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1838\n\n Documentation/git-maintenance.txt | 20 ++++++++++++++\n builtin/gc.c | 42 +++++++++++++++++++++++++++++\n t/t7900-maintenance.sh | 44 +++++++++++++++++++++++++++++++\n 3 files changed, 106 insertions(+)\n\ndiff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\nindex 6e6651309d3..0c8f1e01ccd 100644\n--- a/Documentation/git-maintenance.txt\n+++ b/Documentation/git-maintenance.txt\n@@ -158,6 +158,26 @@ pack-refs::\n \tneed to iterate across many references. See linkgit:git-pack-refs[1]\n \tfor more information.\n \n+prune-remote-refs::\n+\tThe `prune-remote-refs` task runs `git remote prune` on each remote\n+\trepository registered in the local repository. This task helps clean\n+\tup deleted remote branches, improving the performance of operations\n+\tthat iterate through the refs. See linkgit:git-remote[1] for more\n+\tinformation. This task is disabled by default.\n++\n+NOTE: This task is opt-in to prevent unexpected removal of remote refs\n+for users of git-maintenance. For most users, configuring `fetch.prune=true`\n+is a acceptable solution, as it will automatically clean up stale remote-tracking\n+branches during normal fetch operations. However, this task can be useful in\n+specific scenarios:\n++\n+--\n+* When using selective fetching (e.g., `git fetch origin +foo:refs/remotes/origin/foo`)\n+ where `fetch.prune` would not affect refs outside the fetched hierarchy\n+* When third-party tools might perform unexpected full fetches, and you want\n+ periodic cleanup independently of fetch operations\n+--\n+\n OPTIONS\n -------\n --auto::\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 4ae5196aedf..9acf1d29895 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -20,6 +20,7 @@\n #include \"lockfile.h\"\n #include \"parse-options.h\"\n #include \"run-command.h\"\n+#include \"remote.h\"\n #include \"sigchain.h\"\n #include \"strvec.h\"\n #include \"commit.h\"\n@@ -913,6 +914,40 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n+static int collect_remote(struct remote *remote, void *cb_data)\n+{\n+\tstruct string_list *list = cb_data;\n+\n+\tif (!remote->url.nr)\n+\t\treturn 0;\n+\n+\tstring_list_append(list, remote->name);\n+\treturn 0;\n+}\n+\n+static int maintenance_task_prune_remote(struct maintenance_run_opts *opts UNUSED,\n+\t\t\t\t\t struct gc_config *cfg UNUSED)\n+{\n+\tstruct string_list_item *item;\n+\tstruct string_list remotes_list = STRING_LIST_INIT_NODUP;\n+\tstruct child_process child = CHILD_PROCESS_INIT;\n+\tint result = 0;\n+\n+\tfor_each_remote(collect_remote, &remotes_list);\n+\n+\tfor_each_string_list_item (item, &remotes_list) {\n+\t\tconst char *remote_name = item->string;\n+\t\tchild.git_cmd = 1;\n+\t\tstrvec_pushl(&child.args, \"remote\", \"prune\", remote_name, NULL);\n+\n+\t\tif (run_command(&child))\n+\t\t\tresult = error(_(\"failed to prune '%s'\"), remote_name);\n+\t}\n+\n+\tstring_list_clear(&remotes_list, 0);\n+\treturn result;\n+}\n+\n /* Remember to update object flag allocation in object.h */\n #define SEEN\t\t(1u<<0)\n \n@@ -1375,6 +1410,7 @@ enum maintenance_task_label {\n \tTASK_GC,\n \tTASK_COMMIT_GRAPH,\n \tTASK_PACK_REFS,\n+\tTASK_PRUNE_REMOTE_REFS,\n \n \t/* Leave as final value */\n \tTASK__COUNT\n@@ -1411,6 +1447,10 @@ static struct maintenance_task tasks[] = {\n \t\tmaintenance_task_pack_refs,\n \t\tpack_refs_condition,\n \t},\n+\t[TASK_PRUNE_REMOTE_REFS] = {\n+\t\t\"prune-remote-refs\",\n+\t\tmaintenance_task_prune_remote,\n+\t},\n };\n \n static int compare_tasks_by_selection(const void *a_, const void *b_)\n@@ -1505,6 +1545,8 @@ static void initialize_maintenance_strategy(void)\n \t\ttasks[TASK_LOOSE_OBJECTS].schedule = SCHEDULE_DAILY;\n \t\ttasks[TASK_PACK_REFS].enabled = 1;\n \t\ttasks[TASK_PACK_REFS].schedule = SCHEDULE_WEEKLY;\n+\t\ttasks[TASK_PRUNE_REMOTE_REFS].enabled = 0;\n+\t\ttasks[TASK_PRUNE_REMOTE_REFS].schedule = SCHEDULE_DAILY;\n \t}\n }\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 0ce4ba1cbef..60a0c3f8353 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -446,6 +446,50 @@ test_expect_success 'pack-refs task' '\n \ttest_subcommand git pack-refs --all --prune err &&\n+\t\ttest_subcommand ! git remote prune origin err &&\n \ttest_grep \"at most one\" err\n\nbase-commit: 063bcebf0c917140ca0e705cbe0fdea127e90086\n-- \ngitgitgadget\n"},{"id":"509584","messageId":"xmqqed1tv6hu.fsf@gitster.g","threadId":"62684","inReplyTo":"pull.1838.git.1734946566885.gitgitgadget@gmail.com","subject":"Re: [PATCH] maintenance: add prune-remote-refs task","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-27T09:07:41Z","receivedAt":"2024-12-27T09:07:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks for a patch.\n\n\n\"Shubham Kanodia via GitGitGadget\" writes:\n\nYou'd want to check your procedure to tell GGG about addresses; I am\nseeing these\n\n From: \"Shubham Kanodia via GitGitGadget\" \n To: git@vger.kernel.org\n Cc: \"mailto:gitster@pobox.com\" <[gitster@pobox.com]>,\n \"mailto:ps@pks.im\" <[ps@pks.im]>,\n Shubham Kanodia ,\n Shubham Kanodia \n\nand Cc addresses in it would probably not work as-is (I've fixed\nthem up manually).\n\n> From: Shubham Kanodia \n>\n> Remote-tracking refs can accumulate in local repositories even as branches\n> are deleted on remotes, impacting git performance negatively. Existing\n> alternatives to keep refs pruned have a few issues:\n>\n> 1. The `fetch.prune` config automatically cleans up remote ref on fetch,\n> but also pulls in new ref from remote which is an undesirable side-effect.\n\nThis makes it sound as if fetch.prune configuration makes new refs\npulled, but that is not what happens and that is not what you wanted\nto hint.\n\n\tIf you run \"git fetch\" with the \"--prune\" option (or with\n\tthe fetch.prune configuration set to true) while having the\n\tdefault refspec \"+refs/heads/*:refs/remotes/$name/*\"\n\tconfigured in remote.$name.fetch, then ...\n\n> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\n> index 6e6651309d3..0c8f1e01ccd 100644\n> --- a/Documentation/git-maintenance.txt\n> +++ b/Documentation/git-maintenance.txt\n> @@ -158,6 +158,26 @@ pack-refs::\n> \tneed to iterate across many references. See linkgit:git-pack-refs[1]\n> \tfor more information.\n> \n> +prune-remote-refs::\n> +\tThe `prune-remote-refs` task runs `git remote prune` on each remote\n> +\trepository registered in the local repository. This task helps clean\n> +\tup deleted remote branches, improving the performance of operations\n> +\tthat iterate through the refs. See linkgit:git-remote[1] for more\n> +\tinformation. This task is disabled by default.\n> ++\n> +NOTE: This task is opt-in to prevent unexpected removal of remote refs\n> +for users of git-maintenance. For most users, configuring `fetch.prune=true`\n> +is a acceptable solution, as it will automatically clean up stale remote-tracking\n> +branches during normal fetch operations. However, this task can be useful in\n> +specific scenarios:\n> ++\n> +--\n> +* When using selective fetching (e.g., `git fetch origin +foo:refs/remotes/origin/foo`)\n> + where `fetch.prune` would not affect refs outside the fetched hierarchy\n\nThe word \"hierarchy\" hints that things under refs/remotes/origin/\n(which is the hierarchy 'foo' is fetched into) that went away would\nbe pruned, but that is not what happens (otherwise you would not be\nadding this feature).\n\n> +* When third-party tools might perform unexpected full fetches, and you want\n> + periodic cleanup independently of fetch operations\n\nYou'd want a full-stop after these two sentences, by the way.\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 4ae5196aedf..9acf1d29895 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -20,6 +20,7 @@\n> #include \"lockfile.h\"\n> #include \"parse-options.h\"\n> #include \"run-command.h\"\n> +#include \"remote.h\"\n> #include \"sigchain.h\"\n> #include \"strvec.h\"\n> #include \"commit.h\"\n> @@ -913,6 +914,40 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n> \treturn 0;\n> }\n> \n> +static int collect_remote(struct remote *remote, void *cb_data)\n> +{\n> +\tstruct string_list *list = cb_data;\n> +\n> +\tif (!remote->url.nr)\n> +\t\treturn 0;\n> +\n> +\tstring_list_append(list, remote->name);\n> +\treturn 0;\n> +}\n> +\n> +static int maintenance_task_prune_remote(struct maintenance_run_opts *opts UNUSED,\n> +\t\t\t\t\t struct gc_config *cfg UNUSED)\n> +{\n> +\tstruct string_list_item *item;\n> +\tstruct string_list remotes_list = STRING_LIST_INIT_NODUP;\n> +\tstruct child_process child = CHILD_PROCESS_INIT;\n> +\tint result = 0;\n> +\n> +\tfor_each_remote(collect_remote, &remotes_list);\n> +\n> +\tfor_each_string_list_item (item, &remotes_list) {\n> +\t\tconst char *remote_name = item->string;\n> +\t\tchild.git_cmd = 1;\n> +\t\tstrvec_pushl(&child.args, \"remote\", \"prune\", remote_name, NULL);\n> +\n> +\t\tif (run_command(&child))\n> +\t\t\tresult = error(_(\"failed to prune '%s'\"), remote_name);\n> +\t}\n\nHmph, is there a reason why you need two loops, instead of\nfor-each-remote calling a function that does the run_command()\nthing?\n\n\"git grep for_each_string_list_item \\*.c\" tells me that we almost\nnever write SP between the macro name and the opening parenthesis.\n\nThis loop does not stop at the first error, but returns a non-zero\nerror after noticing even a single remote fail to run prune, which\nsounds like a seneible design. Would an error percolate up the same\nway when two different tasks run and one of them fails in the\ncontrol folow in \"git maintenance\"? Just want to see if we are\nbeing consistent with the surrounding code.\n\nThanks.\n"},{"id":"509676","messageId":"CAG=Um+0a+ugf+gWUDS3htj3u2tewzOrH+xGbF+2A+w4ofjQfKg@mail.gmail.com","threadId":"62684","inReplyTo":"xmqqed1tv6hu.fsf@gitster.g","subject":"Re: [PATCH] maintenance: add prune-remote-refs task","fromName":"Shubham Kanodia","fromEmail":"shubham.kanodia10@gmail.com","sentAt":"2024-12-28T09:58:39Z","receivedAt":"2024-12-28T09:59:16Z","isPatch":true,"sender":{"key":"shubham.kanodia10@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8946207?v=4"},"body":"On Fri, Dec 27, 2024 at 2:37 PM Junio C Hamano wrote:\n>\n> Thanks for a patch.\n>\n>\n> \"Shubham Kanodia via GitGitGadget\" writes:\n>\n> You'd want to check your procedure to tell GGG about addresses; I am\n> seeing these\n>\n> From: \"Shubham Kanodia via GitGitGadget\" \n> To: git@vger.kernel.org\n> Cc: \"mailto:gitster@pobox.com\" <[gitster@pobox.com]>,\n> \"mailto:ps@pks.im\" <[ps@pks.im]>,\n> Shubham Kanodia ,\n> Shubham Kanodia \n>\n> and Cc addresses in it would probably not work as-is (I've fixed\n> them up manually).\n\nI think the GGG comment had a few formatting errors. Thanks for fixing the cc.\n\n> Hmph, is there a reason why you need two loops, instead of\n> for-each-remote calling a function that does the run_command()\n> thing?\n\nIt can be collapsed into one.\n\n> This loop does not stop at the first error, but returns a non-zero\n> error after noticing even a single remote fail to run prune, which\n> sounds like a seneible design. Would an error percolate up the same\n> way when two different tasks run and one of them fails in the\n> control folow in \"git maintenance\"? Just want to see if we are\n> being consistent with the surrounding code.\n\nFair point. I'll make the process flow identical to the prefetch refs\ntask that works similarly across remotes.\nIt returns as soon as the first remote fails (without necessarily\naffecting other tasks).\n"},{"id":"509677","messageId":"pull.1838.v2.git.1735380461980.gitgitgadget@gmail.com","threadId":"62684","inReplyTo":"pull.1838.git.1734946566885.gitgitgadget@gmail.com","subject":"[PATCH v2] maintenance: add prune-remote-refs task","fromName":"Shubham Kanodia via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-28T10:07:41Z","receivedAt":"2024-12-28T10:07:45Z","isPatch":true,"sender":{"key":"shubham.kanodia10@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8946207?v=4"},"body":"From: Shubham Kanodia \n\nRemote-tracking refs can accumulate in local repositories even as branches\nare deleted on remotes, impacting git performance negatively. Existing\nalternatives to keep refs pruned have a few issues — \n\n1. Running `git fetch` with either `--prune` or `fetch.prune=true` set will\nprune stale refs, but requires a manual operation and also pulls in new\nrefs from remote which can be an undesirable side-effect.\n\n2.`git remote prune` cleans up refs without adding to the existing list\nbut requires periodic user intervention.\n\nThis adds a new maintenance task 'prune-remote-refs' that runs\n'git remote prune' for each configured remote daily. This provides an\nautomated way to clean up stale remote-tracking refs — especially when\nusers may not do a full fetch.\n\nThis task is disabled by default.\n\nSigned-off-by: Shubham Kanodia \n---\n maintenance: add prune-remote-refs task\n \n As discussed previously on:\n https://lore.kernel.org/git/xmqqwmfr112w.fsf@gitster.g/T/#t\n \n Remote-tracking refs can accumulate in local repositories even as\n branches are deleted on remotes, impacting git performance negatively.\n Existing alternatives to keep refs pruned have a few issues — \n \n 1. The fetch.prune config automatically cleans up remote ref on fetch,\n but also pulls in new ref from remote which is an undesirable\n side-effect.\n \n 2.git remote prune cleans up refs without adding to the existing list\n but requires periodic user intervention.\n \n This adds a new maintenance task 'prune-remote-refs' that runs 'git\n remote prune' for each configured remote daily. This provides an\n automated way to clean up stale remote-tracking refs — especially when\n users may not do a full fetch.\n \n This task is disabled by default.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1838%2Fpastelsky%2Fsk%2Fadd-remote-prune-maintenance-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1838/pastelsky/sk/add-remote-prune-maintenance-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1838\n\nRange-diff vs v1:\n\n 1: 72e27d3ebe6 ! 1: 4d6c143c970 maintenance: add prune-remote-refs task\n @@ Commit message\n are deleted on remotes, impacting git performance negatively. Existing\n alternatives to keep refs pruned have a few issues — \n \n - 1. The `fetch.prune` config automatically cleans up remote ref on fetch,\n - but also pulls in new ref from remote which is an undesirable side-effect.\n + 1. Running `git fetch` with either `--prune` or `fetch.prune=true` set will\n + prune stale refs, but requires a manual operation and also pulls in new\n + refs from remote which can be an undesirable side-effect.\n \n 2.`git remote prune` cleans up refs without adding to the existing list\n but requires periodic user intervention.\n @@ Documentation/git-maintenance.txt: pack-refs::\n ++\n +--\n +* When using selective fetching (e.g., `git fetch origin +foo:refs/remotes/origin/foo`)\n -+ where `fetch.prune` would not affect refs outside the fetched hierarchy\n ++ where `fetch.prune` would only affect refs that are explicitly fetched.\n +* When third-party tools might perform unexpected full fetches, and you want\n -+ periodic cleanup independently of fetch operations\n ++ periodic cleanup independently of fetch operations.\n +--\n +\n OPTIONS\n @@ builtin/gc.c: static int maintenance_opt_schedule(const struct option *opt, cons\n \treturn 0;\n }\n \n -+static int collect_remote(struct remote *remote, void *cb_data)\n ++static int prune_remote(struct remote *remote, void *cb_data UNUSED)\n +{\n -+\tstruct string_list *list = cb_data;\n ++\tstruct child_process child = CHILD_PROCESS_INIT;\n +\n +\tif (!remote->url.nr)\n +\t\treturn 0;\n +\n -+\tstring_list_append(list, remote->name);\n -+\treturn 0;\n ++\tchild.git_cmd = 1;\n ++\tstrvec_pushl(&child.args, \"remote\", \"prune\", remote->name, NULL);\n ++\n ++\treturn !!run_command(&child);\n +}\n +\n -+static int maintenance_task_prune_remote(struct maintenance_run_opts *opts UNUSED,\n ++static int maintenance_task_prune_remote(struct maintenance_run_opts *opts,\n +\t\t\t\t\t struct gc_config *cfg UNUSED)\n +{\n -+\tstruct string_list_item *item;\n -+\tstruct string_list remotes_list = STRING_LIST_INIT_NODUP;\n -+\tstruct child_process child = CHILD_PROCESS_INIT;\n -+\tint result = 0;\n -+\n -+\tfor_each_remote(collect_remote, &remotes_list);\n -+\n -+\tfor_each_string_list_item (item, &remotes_list) {\n -+\t\tconst char *remote_name = item->string;\n -+\t\tchild.git_cmd = 1;\n -+\t\tstrvec_pushl(&child.args, \"remote\", \"prune\", remote_name, NULL);\n -+\n -+\t\tif (run_command(&child))\n -+\t\t\tresult = error(_(\"failed to prune '%s'\"), remote_name);\n ++\tif (for_each_remote(prune_remote, opts)) {\n ++\t\terror(_(\"failed to prune remotes\"));\n ++\t\treturn 1;\n +\t}\n +\n -+\tstring_list_clear(&remotes_list, 0);\n -+\treturn result;\n ++\treturn 0;\n +}\n +\n /* Remember to update object flag allocation in object.h */\n\n\n Documentation/git-maintenance.txt | 20 ++++++++++++++\n builtin/gc.c | 32 ++++++++++++++++++++++\n t/t7900-maintenance.sh | 44 +++++++++++++++++++++++++++++++\n 3 files changed, 96 insertions(+)\n\ndiff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\nindex 6e6651309d3..8b3e496c8ef 100644\n--- a/Documentation/git-maintenance.txt\n+++ b/Documentation/git-maintenance.txt\n@@ -158,6 +158,26 @@ pack-refs::\n \tneed to iterate across many references. See linkgit:git-pack-refs[1]\n \tfor more information.\n \n+prune-remote-refs::\n+\tThe `prune-remote-refs` task runs `git remote prune` on each remote\n+\trepository registered in the local repository. This task helps clean\n+\tup deleted remote branches, improving the performance of operations\n+\tthat iterate through the refs. See linkgit:git-remote[1] for more\n+\tinformation. This task is disabled by default.\n++\n+NOTE: This task is opt-in to prevent unexpected removal of remote refs\n+for users of git-maintenance. For most users, configuring `fetch.prune=true`\n+is a acceptable solution, as it will automatically clean up stale remote-tracking\n+branches during normal fetch operations. However, this task can be useful in\n+specific scenarios:\n++\n+--\n+* When using selective fetching (e.g., `git fetch origin +foo:refs/remotes/origin/foo`)\n+ where `fetch.prune` would only affect refs that are explicitly fetched.\n+* When third-party tools might perform unexpected full fetches, and you want\n+ periodic cleanup independently of fetch operations.\n+--\n+\n OPTIONS\n -------\n --auto::\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 4ae5196aedf..329c764f300 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -20,6 +20,7 @@\n #include \"lockfile.h\"\n #include \"parse-options.h\"\n #include \"run-command.h\"\n+#include \"remote.h\"\n #include \"sigchain.h\"\n #include \"strvec.h\"\n #include \"commit.h\"\n@@ -913,6 +914,30 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n+static int prune_remote(struct remote *remote, void *cb_data UNUSED)\n+{\n+\tstruct child_process child = CHILD_PROCESS_INIT;\n+\n+\tif (!remote->url.nr)\n+\t\treturn 0;\n+\n+\tchild.git_cmd = 1;\n+\tstrvec_pushl(&child.args, \"remote\", \"prune\", remote->name, NULL);\n+\n+\treturn !!run_command(&child);\n+}\n+\n+static int maintenance_task_prune_remote(struct maintenance_run_opts *opts,\n+\t\t\t\t\t struct gc_config *cfg UNUSED)\n+{\n+\tif (for_each_remote(prune_remote, opts)) {\n+\t\terror(_(\"failed to prune remotes\"));\n+\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n /* Remember to update object flag allocation in object.h */\n #define SEEN\t\t(1u<<0)\n \n@@ -1375,6 +1400,7 @@ enum maintenance_task_label {\n \tTASK_GC,\n \tTASK_COMMIT_GRAPH,\n \tTASK_PACK_REFS,\n+\tTASK_PRUNE_REMOTE_REFS,\n \n \t/* Leave as final value */\n \tTASK__COUNT\n@@ -1411,6 +1437,10 @@ static struct maintenance_task tasks[] = {\n \t\tmaintenance_task_pack_refs,\n \t\tpack_refs_condition,\n \t},\n+\t[TASK_PRUNE_REMOTE_REFS] = {\n+\t\t\"prune-remote-refs\",\n+\t\tmaintenance_task_prune_remote,\n+\t},\n };\n \n static int compare_tasks_by_selection(const void *a_, const void *b_)\n@@ -1505,6 +1535,8 @@ static void initialize_maintenance_strategy(void)\n \t\ttasks[TASK_LOOSE_OBJECTS].schedule = SCHEDULE_DAILY;\n \t\ttasks[TASK_PACK_REFS].enabled = 1;\n \t\ttasks[TASK_PACK_REFS].schedule = SCHEDULE_WEEKLY;\n+\t\ttasks[TASK_PRUNE_REMOTE_REFS].enabled = 0;\n+\t\ttasks[TASK_PRUNE_REMOTE_REFS].schedule = SCHEDULE_DAILY;\n \t}\n }\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 0ce4ba1cbef..60a0c3f8353 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -446,6 +446,50 @@ test_expect_success 'pack-refs task' '\n \ttest_subcommand git pack-refs --all --prune err &&\n+\t\ttest_subcommand ! git remote prune origin err &&\n \ttest_grep \"at most one\" err\n\nbase-commit: 063bcebf0c917140ca0e705cbe0fdea127e90086\n-- \ngitgitgadget\n"},{"id":"509684","messageId":"xmqqr05r4wu5.fsf@gitster.g","threadId":"62684","inReplyTo":"CAG=Um+0a+ugf+gWUDS3htj3u2tewzOrH+xGbF+2A+w4ofjQfKg@mail.gmail.com","subject":"Re: [PATCH] maintenance: add prune-remote-refs task","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-28T16:05:22Z","receivedAt":"2024-12-28T16:05:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shubham Kanodia writes:\n\n>> Hmph, is there a reason why you need two loops, instead of\n>> for-each-remote calling a function that does the run_command()\n>> thing?\n>\n> It can be collapsed into one.\n\nSorry, but that is not an answer, as my question was not a\nsuggestion to change anything.\n\nIt was a question asking you if there was a specific reason why the\ncode was structured the way it was written. If there is another way\nto write it, you need to answer why the alternative wasn't picked.\n\n>> This loop does not stop at the first error, but returns a non-zero\n>> error after noticing even a single remote fail to run prune, which\n>> sounds like a seneible design. Would an error percolate up the same\n>> way when two different tasks run and one of them fails in the\n>> control folow in \"git maintenance\"? Just want to see if we are\n>> being consistent with the surrounding code.\n>\n> Fair point. I'll make the process flow identical to the prefetch refs\n> task that works similarly across remotes.\n> It returns as soon as the first remote fails (without necessarily\n> affecting other tasks).\n\n... and the first failure signals the caller a failure? That would\nmatch what you did in your new feature, which is perfect.\n\nThanks.\n"},{"id":"509686","messageId":"CAG=Um+0rnj3B3QoTZFCZWNCcYMtoiLnn=nov7GuHRH-aDydARQ@mail.gmail.com","threadId":"62684","inReplyTo":"xmqqr05r4wu5.fsf@gitster.g","subject":"Re: [PATCH] maintenance: add prune-remote-refs task","fromName":"Shubham Kanodia","fromEmail":"shubham.kanodia10@gmail.com","sentAt":"2024-12-28T16:24:39Z","receivedAt":"2024-12-28T16:25:16Z","isPatch":true,"sender":{"key":"shubham.kanodia10@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8946207?v=4"},"body":"On Sat, Dec 28, 2024 at 9:35 PM Junio C Hamano wrote:\n>\n> Shubham Kanodia writes:\n>\n> >> Hmph, is there a reason why you need two loops, instead of\n> >> for-each-remote calling a function that does the run_command()\n> >> thing?\n> >\n> > It can be collapsed into one.\n>\n> Sorry, but that is not an answer, as my question was not a\n> suggestion to change anything.\n>\n> It was a question asking you if there was a specific reason why the\n> code was structured the way it was written. If there is another way\n> to write it, you need to answer why the alternative wasn't picked.\n\nThere wasn't a good reason for doing it that way. I guess I was trying\nto understand the second argument for `for_each_remote` would be best\nused if the command was called directly (while avoiding a compilation\nwarning), but looking at a few other usages of `for_each_remote` I\nrealised that it could just be marked unused in this case (since we\naren't doing anything with it).\n\nI should've probably looked deeper and learnt from existing patterns\n(e.g. `maintenance_task_prefetch`) — which I have in my last patch.\n\n> >> This loop does not stop at the first error, but returns a non-zero\n> >> error after noticing even a single remote fail to run prune, which\n> >> sounds like a seneible design. Would an error percolate up the same\n> >> way when two different tasks run and one of them fails in the\n> >> control folow in \"git maintenance\"? Just want to see if we are\n> >> being consistent with the surrounding code.\n> >\n> > Fair point. I'll make the process flow identical to the prefetch refs\n> > task that works similarly across remotes.\n> > It returns as soon as the first remote fails (without necessarily\n> > affecting other tasks).\n>\n> ... and the first failure signals the caller a failure? That would\n> match what you did in your new feature, which is perfect.\n\nExactly — the first failing remote will signal that the\n`prune-remote-refs` task has failed via an immediate `return 1`.\nThe maintenance command uses this to register the exit code of the top\nlevel command to 1, while continuing to execute all other tasks\nanyway.\n\nThanks,\nShubham K\n"},{"id":"509687","messageId":"xmqq8qrz4vwj.fsf@gitster.g","threadId":"62684","inReplyTo":"pull.1838.v2.git.1735380461980.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] maintenance: add prune-remote-refs task","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-28T16:25:32Z","receivedAt":"2024-12-28T16:25:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shubham Kanodia via GitGitGadget\" writes:\n\n> From: Shubham Kanodia \n>\n> Remote-tracking refs can accumulate in local repositories even as branches\n> are deleted on remotes, impacting git performance negatively. Existing\n> alternatives to keep refs pruned have a few issues — \n>\n> 1. Running `git fetch` with either `--prune` or `fetch.prune=true` set will\n> prune stale refs, but requires a manual operation and also pulls in new\n> refs from remote which can be an undesirable side-effect.\n\nIt is only true if you cloned without any tweaks. For example, if\nyou cloned with the single-branch option, you would not pull in new\nrefs, wouldn't you? Also \"requires a manual operation\" is not quite\na good rationale, as you could have placed such a \"git fetch\"\ninstead of \"git remote prune\", in the maintenance schedule.\n\nFor this to become an issue, the condition we saw in earlier\ndiscussion, i.e.\n\n while having the *default* refspec\n \"+refs/heads/*:refs/remotes/$name/*\" configured in\n remote.$name.fetch\n\nis crucial. Since that is the default refspec \"git clone\" gives\nyou, your \"git fetch --prune\" would give you full set of refs while\npruning, and the end result is that you have up-to-date set of\nremote-tracking branches (which you may not want).\n\n> 2.`git remote prune` cleans up refs without adding to the existing list\n> but requires periodic user intervention.\n\nYou have a SP after \"1.\" but not after \"2.\".\n\n> This adds a new maintenance task 'prune-remote-refs' that runs\n> 'git remote prune' for each configured remote daily. This provides an\n> automated way to clean up stale remote-tracking refs — especially when\n> users may not do a full fetch.\n\n\"This adds\" -> \"Add\".\n\nI'd strike the latter sentence. Regardless of what users do or do\nnot do, the automated clean-up is performed.\n\n> This task is disabled by default.\n>\n> Signed-off-by: Shubham Kanodia \n> ---\n\n> +NOTE: This task is opt-in to prevent unexpected removal of remote refs\n> +for users of git-maintenance. For most users, configuring `fetch.prune=true`\n> +is a acceptable solution, as it will automatically clean up stale remote-tracking\n\n\"a acceptable\" -> \"an acceptable\".\n\n> +branches during normal fetch operations. However, this task can be useful in\n> +specific scenarios:\n> ++\n> +--\n> +* When using selective fetching (e.g., `git fetch origin +foo:refs/remotes/origin/foo`)\n> + where `fetch.prune` would only affect refs that are explicitly fetched.\n> +* When third-party tools might perform unexpected full fetches, and you want\n> + periodic cleanup independently of fetch operations.\n> +--\n\nVery well written.\n\n> @@ -913,6 +914,30 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n> \treturn 0;\n> }\n> \n> +static int prune_remote(struct remote *remote, void *cb_data UNUSED)\n> +{\n> +\tstruct child_process child = CHILD_PROCESS_INIT;\n> +\n> +\tif (!remote->url.nr)\n> +\t\treturn 0;\n> +\n> +\tchild.git_cmd = 1;\n> +\tstrvec_pushl(&child.args, \"remote\", \"prune\", remote->name, NULL);\n> +\n> +\treturn !!run_command(&child);\n> +}\n> +\n> +static int maintenance_task_prune_remote(struct maintenance_run_opts *opts,\n> +\t\t\t\t\t struct gc_config *cfg UNUSED)\n> +{\n> +\tif (for_each_remote(prune_remote, opts)) {\n> +\t\terror(_(\"failed to prune remotes\"));\n> +\t\treturn 1;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n\nNice reuse of the program structure, which is very clean and easy to read.\n\nOverall very well written. Will queue, with attached range-diff.\nThanks.\n\n\n--- >8 ---\n1: 0ae9668b5c ! 1: 8a40f8b319 maintenance: add prune-remote-refs task\n @@ Commit message\n \n Remote-tracking refs can accumulate in local repositories even as branches\n are deleted on remotes, impacting git performance negatively. Existing\n - alternatives to keep refs pruned have a few issues — \n + alternatives to keep refs pruned have a few issues:\n \n - 1. Running `git fetch` with either `--prune` or `fetch.prune=true` set will\n - prune stale refs, but requires a manual operation and also pulls in new\n - refs from remote which can be an undesirable side-effect.\n + 1. Running `git fetch` with either `--prune` or `fetch.prune=true`\n + set, with the default refspec to copy all their branches into\n + our remote-tracking branches, will prune stale refs, but also\n + pulls in new branches from remote. That is undesirable if the\n + user wants to only work with a selected few remote branches.\n \n - 2.`git remote prune` cleans up refs without adding to the existing list\n - but requires periodic user intervention.\n + 2. `git remote prune` cleans up refs without adding to the\n + existing list but requires periodic user intervention.\n \n - This adds a new maintenance task 'prune-remote-refs' that runs\n - 'git remote prune' for each configured remote daily. This provides an\n - automated way to clean up stale remote-tracking refs — especially when\n - users may not do a full fetch.\n -\n - This task is disabled by default.\n + Add a new maintenance task 'prune-remote-refs' that runs 'git remote\n + prune' for each configured remote daily. Leave the task disabled by\n + default, as it may be unexpected to see their remote-tracking\n + branches to disappear while they are not watching for unsuspecting\n + users.\n \n Signed-off-by: Shubham Kanodia \n Signed-off-by: Junio C Hamano \n @@ Documentation/git-maintenance.txt: pack-refs::\n ++\n +NOTE: This task is opt-in to prevent unexpected removal of remote refs\n +for users of git-maintenance. For most users, configuring `fetch.prune=true`\n -+is a acceptable solution, as it will automatically clean up stale remote-tracking\n ++is an acceptable solution, as it will automatically clean up stale remote-tracking\n +branches during normal fetch operations. However, this task can be useful in\n +specific scenarios:\n ++\n\n"},{"id":"509724","messageId":"Z3JIpDQTDrQuoEN2@pks.im","threadId":"62684","inReplyTo":"pull.1838.v2.git.1735380461980.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] maintenance: add prune-remote-refs task","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-30T07:15:55Z","receivedAt":"2024-12-30T07:16:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Dec 28, 2024 at 10:07:41AM +0000, Shubham Kanodia via GitGitGadget wrote:\n> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\n> index 6e6651309d3..8b3e496c8ef 100644\n> --- a/Documentation/git-maintenance.txt\n> +++ b/Documentation/git-maintenance.txt\n> @@ -158,6 +158,26 @@ pack-refs::\n> \tneed to iterate across many references. See linkgit:git-pack-refs[1]\n> \tfor more information.\n> \n> +prune-remote-refs::\n> +\tThe `prune-remote-refs` task runs `git remote prune` on each remote\n> +\trepository registered in the local repository. This task helps clean\n> +\tup deleted remote branches, improving the performance of operations\n> +\tthat iterate through the refs. See linkgit:git-remote[1] for more\n> +\tinformation. This task is disabled by default.\n> ++\n> +NOTE: This task is opt-in to prevent unexpected removal of remote refs\n> +for users of git-maintenance. For most users, configuring `fetch.prune=true`\n\nDo we want to make this linkgit:git-maintenance[1] even though this is\nself-referential?\n\n> +is a acceptable solution, as it will automatically clean up stale remote-tracking\n> +branches during normal fetch operations. However, this task can be useful in\n> +specific scenarios:\n> ++\n> +--\n> +* When using selective fetching (e.g., `git fetch origin +foo:refs/remotes/origin/foo`)\n> + where `fetch.prune` would only affect refs that are explicitly fetched.\n> +* When third-party tools might perform unexpected full fetches, and you want\n> + periodic cleanup independently of fetch operations.\n> +--\n\nNicely explained. I wish we had more such documentation that is taking\nthe user by their hand and explains why they may or may not want to have\na specific thing.\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 4ae5196aedf..329c764f300 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -20,6 +20,7 @@\n> #include \"lockfile.h\"\n> #include \"parse-options.h\"\n> #include \"run-command.h\"\n> +#include \"remote.h\"\n> #include \"sigchain.h\"\n> #include \"strvec.h\"\n> #include \"commit.h\"\n> @@ -913,6 +914,30 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n> \treturn 0;\n> }\n> \n> +static int prune_remote(struct remote *remote, void *cb_data UNUSED)\n> +{\n> +\tstruct child_process child = CHILD_PROCESS_INIT;\n> +\n> +\tif (!remote->url.nr)\n> +\t\treturn 0;\n> +\n> +\tchild.git_cmd = 1;\n> +\tstrvec_pushl(&child.args, \"remote\", \"prune\", remote->name, NULL);\n> +\n> +\treturn !!run_command(&child);\n> +}\n> +\n> +static int maintenance_task_prune_remote(struct maintenance_run_opts *opts,\n> +\t\t\t\t\t struct gc_config *cfg UNUSED)\n> +{\n> +\tif (for_each_remote(prune_remote, opts)) {\n> +\t\terror(_(\"failed to prune remotes\"));\n> +\t\treturn 1;\n\nI wonder whether we should adapt the loop to be eager. Erroring out on\nthe first failed remote would potentially mean that none of the other\nremotes may get pruned. So if you had a now-unreachable remote as first\nremote then none of your remotes would be pruned.\n\nIf so, we may want to collect the names of failed remotes and print\nthem, as well.\n\nPatrick\n"},{"id":"509732","messageId":"xmqqh66l1d1n.fsf@gitster.g","threadId":"62684","inReplyTo":"Z3JIpDQTDrQuoEN2@pks.im","subject":"Re: [PATCH v2] maintenance: add prune-remote-refs task","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-30T14:05:40Z","receivedAt":"2024-12-30T14:05:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Sat, Dec 28, 2024 at 10:07:41AM +0000, Shubham Kanodia via GitGitGadget wrote:\n>> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\n>> index 6e6651309d3..8b3e496c8ef 100644\n>> --- a/Documentation/git-maintenance.txt\n>> +++ b/Documentation/git-maintenance.txt\n>> @@ -158,6 +158,26 @@ pack-refs::\n>> \tneed to iterate across many references. See linkgit:git-pack-refs[1]\n>> \tfor more information.\n>> \n>> +prune-remote-refs::\n>> +\tThe `prune-remote-refs` task runs `git remote prune` on each remote\n>> +\trepository registered in the local repository. This task helps clean\n>> +\tup deleted remote branches, improving the performance of operations\n>> +\tthat iterate through the refs. See linkgit:git-remote[1] for more\n>> +\tinformation. This task is disabled by default.\n>> ++\n>> +NOTE: This task is opt-in to prevent unexpected removal of remote refs\n>> +for users of git-maintenance. For most users, configuring `fetch.prune=true`\n>\n> Do we want to make this linkgit:git-maintenance[1] even though this is\n> self-referential?\n\nThat certainly is a thought---the rule could be \"whenever we refer\nto a Git command, we refer to it in a uniform way\". An alternative\nwould be \"of git-maintenance\" -> \"of this command\" to weaken it.\n\nThis refers to those users who want to use the command for other\nreasons (you use the scheduled tasks driven by 'git maintenance'\nonly because you wanted the 'gc' and 'pack-refs' tasks to run, you\ndo not necessarily want to run a new kind of task the new version of\nGit started supporting, especially when the task is destructive,\nlike this one). We might want to stress that point, perhaps? If a\nreader reads this part of the documentation, finds this task useful\nand decides to use 'git maintenance', the note would sound somewhat\nnonsensical to them---\"I thought about the ramifications, I decided\nI wanted to use the command, why would it be opt-in?\" is a plausible\nconfusion.\n\n>> +is a acceptable solution, as it will automatically clean up stale remote-tracking\n>> +branches during normal fetch operations. However, this task can be useful in\n>> +specific scenarios:\n>> ++\n>> +--\n>> +* When using selective fetching (e.g., `git fetch origin +foo:refs/remotes/origin/foo`)\n>> + where `fetch.prune` would only affect refs that are explicitly fetched.\n>> +* When third-party tools might perform unexpected full fetches, and you want\n>> + periodic cleanup independently of fetch operations.\n>> +--\n>\n> Nicely explained. I wish we had more such documentation that is taking\n> the user by their hand and explains why they may or may not want to have\n> a specific thing.\n\nYes, a configuration or an option that are not for everybody and for\nevery situation need such a guidance, and this one is done nicely.\n\n>> +static int maintenance_task_prune_remote(struct maintenance_run_opts *opts,\n>> +\t\t\t\t\t struct gc_config *cfg UNUSED)\n>> +{\n>> +\tif (for_each_remote(prune_remote, opts)) {\n>> +\t\terror(_(\"failed to prune remotes\"));\n>> +\t\treturn 1;\n>\n> I wonder whether we should adapt the loop to be eager. Erroring out on\n> the first failed remote would potentially mean that none of the other\n> remotes may get pruned. So if you had a now-unreachable remote as first\n> remote then none of your remotes would be pruned.\n\nI think the structure, hence the behaviour, is shared with an\nexisting prefetch task. I think the current way is OK-ish, but\ngiven that we are not in a hurry, we may want to correct the\nsemantics for both of them before unleashing this new task to the\nworld.\n\nFor that, we need the callback functions given to for_each_remote\n(i.e., fetch_remote and prune_remote) to always return \"success\" in\nthe sense to tell \"I am done with this remote\" to allow the loop to\ncontinue to the next remote, and convey the failure from the\nsubcommand via some other means (like flipping a bit in the cbdata).\n\nThanks.\n"},{"id":"509824","messageId":"CAG=Um+0fC3DcHB19du9YF19CdzCXkt9AYA1ndBJ5pbGAAMYREQ@mail.gmail.com","threadId":"62684","inReplyTo":"xmqqh66l1d1n.fsf@gitster.g","subject":"Re: [PATCH v2] maintenance: add prune-remote-refs task","fromName":"Shubham Kanodia","fromEmail":"shubham.kanodia10@gmail.com","sentAt":"2025-01-03T06:50:07Z","receivedAt":"2025-01-03T06:50:44Z","isPatch":true,"sender":{"key":"shubham.kanodia10@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8946207?v=4"},"body":"On Mon, Dec 30, 2024 at 7:35 PM Junio C Hamano wrote:\n>\n> Patrick Steinhardt writes:\n>\n> > On Sat, Dec 28, 2024 at 10:07:41AM +0000, Shubham Kanodia via GitGitGadget wrote:\n> >> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\n> >> index 6e6651309d3..8b3e496c8ef 100644\n> >> --- a/Documentation/git-maintenance.txt\n> >> +++ b/Documentation/git-maintenance.txt\n> >> @@ -158,6 +158,26 @@ pack-refs::\n> >> need to iterate across many references. See linkgit:git-pack-refs[1]\n> >> for more information.\n> >>\n> >> +prune-remote-refs::\n> >> + The `prune-remote-refs` task runs `git remote prune` on each remote\n> >> + repository registered in the local repository. This task helps clean\n> >> + up deleted remote branches, improving the performance of operations\n> >> + that iterate through the refs. See linkgit:git-remote[1] for more\n> >> + information. This task is disabled by default.\n> >> ++\n> >> +NOTE: This task is opt-in to prevent unexpected removal of remote refs\n> >> +for users of git-maintenance. For most users, configuring `fetch.prune=true`\n> >\n> > Do we want to make this linkgit:git-maintenance[1] even though this is\n> > self-referential?\n>\n> That certainly is a thought---the rule could be \"whenever we refer\n> to a Git command, we refer to it in a uniform way\". An alternative\n> would be \"of git-maintenance\" -> \"of this command\" to weaken it.\n>\n> This refers to those users who want to use the command for other\n> reasons (you use the scheduled tasks driven by 'git maintenance'\n> only because you wanted the 'gc' and 'pack-refs' tasks to run, you\n> do not necessarily want to run a new kind of task the new version of\n> Git started supporting, especially when the task is destructive,\n> like this one). We might want to stress that point, perhaps? If a\n> reader reads this part of the documentation, finds this task useful\n> and decides to use 'git maintenance', the note would sound somewhat\n> nonsensical to them---\"I thought about the ramifications, I decided\n> I wanted to use the command, why would it be opt-in?\" is a plausible\n> confusion.\n>\n> >> +is a acceptable solution, as it will automatically clean up stale remote-tracking\n> >> +branches during normal fetch operations. However, this task can be useful in\n> >> +specific scenarios:\n> >> ++\n> >> +--\n> >> +* When using selective fetching (e.g., `git fetch origin +foo:refs/remotes/origin/foo`)\n> >> + where `fetch.prune` would only affect refs that are explicitly fetched.\n> >> +* When third-party tools might perform unexpected full fetches, and you want\n> >> + periodic cleanup independently of fetch operations.\n> >> +--\n> >\n> > Nicely explained. I wish we had more such documentation that is taking\n> > the user by their hand and explains why they may or may not want to have\n> > a specific thing.\n>\n> Yes, a configuration or an option that are not for everybody and for\n> every situation need such a guidance, and this one is done nicely.\n>\n> >> +static int maintenance_task_prune_remote(struct maintenance_run_opts *opts,\n> >> + struct gc_config *cfg UNUSED)\n> >> +{\n> >> + if (for_each_remote(prune_remote, opts)) {\n> >> + error(_(\"failed to prune remotes\"));\n> >> + return 1;\n> >\n> > I wonder whether we should adapt the loop to be eager. Erroring out on\n> > the first failed remote would potentially mean that none of the other\n> > remotes may get pruned. So if you had a now-unreachable remote as first\n> > remote then none of your remotes would be pruned.\n>\n> I think the structure, hence the behaviour, is shared with an\n> existing prefetch task. I think the current way is OK-ish, but\n> given that we are not in a hurry, we may want to correct the\n> semantics for both of them before unleashing this new task to the\n> world.\n>\n> For that, we need the callback functions given to for_each_remote\n> (i.e., fetch_remote and prune_remote) to always return \"success\" in\n> the sense to tell \"I am done with this remote\" to allow the loop to\n> continue to the next remote, and convey the failure from the\n> subcommand via some other means (like flipping a bit in the cbdata).\n>\n> Thanks.\n\nCurious — I submitted my patches through GGG, but Junio was kind\nenough to apply a few other fixes to it.\nIs there a place I can now get the whole diff (with the range diff\npatched in) so I can pull that into GGG?\n\nThanks,\nShubham\n"},{"id":"509826","messageId":"Z3eUAHOhvOsJ26J8@pks.im","threadId":"62684","inReplyTo":"CAG=Um+0fC3DcHB19du9YF19CdzCXkt9AYA1ndBJ5pbGAAMYREQ@mail.gmail.com","subject":"Re: [PATCH v2] maintenance: add prune-remote-refs task","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-03T07:38:40Z","receivedAt":"2025-01-03T07:38:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jan 03, 2025 at 12:20:07PM +0530, Shubham Kanodia wrote:\n> Curious — I submitted my patches through GGG, but Junio was kind\n> enough to apply a few other fixes to it.\n> Is there a place I can now get the whole diff (with the range diff\n> patched in) so I can pull that into GGG?\n\nJunio publishes his branches at [1], and yours specifically is called\n\"sk/maintenance-remote-prune\". I'd typically just update my local branch\nto match what he has in there.\n\nPatrick\n\n[1]: https://github.com/gitster/git.git\n"},{"id":"509873","messageId":"pull.1838.v3.git.1735928035056.gitgitgadget@gmail.com","threadId":"62684","inReplyTo":"pull.1838.v2.git.1735380461980.gitgitgadget@gmail.com","subject":"[PATCH v3] maintenance: add prune-remote-refs task","fromName":"Shubham Kanodia via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-03T18:13:54Z","receivedAt":"2025-01-03T18:13:58Z","isPatch":true,"sender":{"key":"shubham.kanodia10@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8946207?v=4"},"body":"From: Shubham Kanodia \n\nRemote-tracking refs can accumulate in local repositories even as branches\nare deleted on remotes, impacting git performance negatively. Existing\nalternatives to keep refs pruned have a few issues:\n\n 1. Running `git fetch` with either `--prune` or `fetch.prune=true`\n set, with the default refspec to copy all their branches into\n our remote-tracking branches, will prune stale refs, but also\n pulls in new branches from remote. That is undesirable if the\n user wants to only work with a selected few remote branches.\n\n 2. `git remote prune` cleans up refs without adding to the\n existing list but requires periodic user intervention.\n\nAdd a new maintenance task 'prune-remote-refs' that runs 'git remote\nprune' for each configured remote daily. Leave the task disabled by\ndefault, as it may be unexpected to see their remote-tracking\nbranches to disappear while they are not watching for unsuspecting\nusers.\n\nSigned-off-by: Shubham Kanodia \nSigned-off-by: Junio C Hamano \n---\n maintenance: add prune-remote-refs task\n \n As discussed previously on:\n https://lore.kernel.org/git/xmqqwmfr112w.fsf@gitster.g/T/#t\n \n Remote-tracking refs can accumulate in local repositories even as\n branches are deleted on remotes, impacting git performance negatively.\n Existing alternatives to keep refs pruned have a few issues — \n \n 1. The fetch.prune config automatically cleans up remote ref on fetch,\n but also pulls in new ref from remote which is an undesirable\n side-effect.\n \n 2.git remote prune cleans up refs without adding to the existing list\n but requires periodic user intervention.\n \n This adds a new maintenance task 'prune-remote-refs' that runs 'git\n remote prune' for each configured remote daily. This provides an\n automated way to clean up stale remote-tracking refs — especially when\n users may not do a full fetch.\n \n This task is disabled by default.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1838%2Fpastelsky%2Fsk%2Fadd-remote-prune-maintenance-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1838/pastelsky/sk/add-remote-prune-maintenance-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1838\n\nRange-diff vs v2:\n\n 1: 4d6c143c970 ! 1: 7954df1009a maintenance: add prune-remote-refs task\n @@ Commit message\n \n Remote-tracking refs can accumulate in local repositories even as branches\n are deleted on remotes, impacting git performance negatively. Existing\n - alternatives to keep refs pruned have a few issues — \n + alternatives to keep refs pruned have a few issues:\n \n - 1. Running `git fetch` with either `--prune` or `fetch.prune=true` set will\n - prune stale refs, but requires a manual operation and also pulls in new\n - refs from remote which can be an undesirable side-effect.\n + 1. Running `git fetch` with either `--prune` or `fetch.prune=true`\n + set, with the default refspec to copy all their branches into\n + our remote-tracking branches, will prune stale refs, but also\n + pulls in new branches from remote. That is undesirable if the\n + user wants to only work with a selected few remote branches.\n \n - 2.`git remote prune` cleans up refs without adding to the existing list\n - but requires periodic user intervention.\n + 2. `git remote prune` cleans up refs without adding to the\n + existing list but requires periodic user intervention.\n \n - This adds a new maintenance task 'prune-remote-refs' that runs\n - 'git remote prune' for each configured remote daily. This provides an\n - automated way to clean up stale remote-tracking refs — especially when\n - users may not do a full fetch.\n -\n - This task is disabled by default.\n + Add a new maintenance task 'prune-remote-refs' that runs 'git remote\n + prune' for each configured remote daily. Leave the task disabled by\n + default, as it may be unexpected to see their remote-tracking\n + branches to disappear while they are not watching for unsuspecting\n + users.\n \n Signed-off-by: Shubham Kanodia \n + Signed-off-by: Junio C Hamano \n \n ## Documentation/git-maintenance.txt ##\n @@ Documentation/git-maintenance.txt: pack-refs::\n @@ Documentation/git-maintenance.txt: pack-refs::\n +\tinformation. This task is disabled by default.\n ++\n +NOTE: This task is opt-in to prevent unexpected removal of remote refs\n -+for users of git-maintenance. For most users, configuring `fetch.prune=true`\n -+is a acceptable solution, as it will automatically clean up stale remote-tracking\n ++for users of linkgit:git-maintenance[1]. For most users, configuring `fetch.prune=true`\n ++is an acceptable solution, as it will automatically clean up stale remote-tracking\n +branches during normal fetch operations. However, this task can be useful in\n +specific scenarios:\n ++\n @@ builtin/gc.c: static int maintenance_opt_schedule(const struct option *opt, cons\n \treturn 0;\n }\n \n -+static int prune_remote(struct remote *remote, void *cb_data UNUSED)\n ++struct remote_cb_data {\n ++\tstruct maintenance_run_opts *maintenance_opts;\n ++\tstruct string_list failed_remotes;\n ++};\n ++\n ++static void report_failed_remotes(struct string_list *failed_remotes,\n ++\t\t\t\t const char *action_name)\n ++{\n ++\tif (failed_remotes->nr) {\n ++\t\tint i;\n ++\t\tstruct strbuf msg = STRBUF_INIT;\n ++\t\tstrbuf_addf(&msg, _(\"failed to %s the following remotes: \"),\n ++\t\t\t action_name);\n ++\t\tfor (i = 0; i < failed_remotes->nr; i++) {\n ++\t\t\tif (i)\n ++\t\t\t\tstrbuf_addstr(&msg, \", \");\n ++\t\t\tstrbuf_addstr(&msg, failed_remotes->items[i].string);\n ++\t\t}\n ++\t\terror(\"%s\", msg.buf);\n ++\t\tstrbuf_release(&msg);\n ++\t}\n ++}\n ++\n ++static int prune_remote(struct remote *remote, void *cb_data)\n +{\n +\tstruct child_process child = CHILD_PROCESS_INIT;\n ++\tstruct remote_cb_data *data = cb_data;\n +\n +\tif (!remote->url.nr)\n +\t\treturn 0;\n @@ builtin/gc.c: static int maintenance_opt_schedule(const struct option *opt, cons\n +\tchild.git_cmd = 1;\n +\tstrvec_pushl(&child.args, \"remote\", \"prune\", remote->name, NULL);\n +\n -+\treturn !!run_command(&child);\n ++\tif (run_command(&child))\n ++\t\tstring_list_append(&data->failed_remotes, remote->name);\n ++\n ++\treturn 0;\n +}\n +\n +static int maintenance_task_prune_remote(struct maintenance_run_opts *opts,\n +\t\t\t\t\t struct gc_config *cfg UNUSED)\n +{\n -+\tif (for_each_remote(prune_remote, opts)) {\n -+\t\terror(_(\"failed to prune remotes\"));\n -+\t\treturn 1;\n -+\t}\n ++\tstruct remote_cb_data cbdata = { .maintenance_opts = opts,\n ++\t\t\t\t\t .failed_remotes = STRING_LIST_INIT_DUP };\n +\n -+\treturn 0;\n ++\tint result;\n ++\tresult = for_each_remote(prune_remote, &cbdata);\n ++\n ++\treport_failed_remotes(&cbdata.failed_remotes, \"prune\");\n ++\tif (cbdata.failed_remotes.nr)\n ++\t\tresult = 1;\n ++\n ++\tstring_list_clear(&cbdata.failed_remotes, 0);\n ++\treturn result;\n +}\n +\n /* Remember to update object flag allocation in object.h */\n #define SEEN\t\t(1u<<0)\n \n +@@ builtin/gc.c: static int maintenance_task_commit_graph(struct maintenance_run_opts *opts,\n + \n + static int fetch_remote(struct remote *remote, void *cbdata)\n + {\n +-\tstruct maintenance_run_opts *opts = cbdata;\n + \tstruct child_process child = CHILD_PROCESS_INIT;\n ++\tstruct remote_cb_data *data = cbdata;\n + \n + \tif (remote->skip_default_update)\n + \t\treturn 0;\n +@@ builtin/gc.c: static int fetch_remote(struct remote *remote, void *cbdata)\n + \t\t \"--no-write-fetch-head\", \"--recurse-submodules=no\",\n + \t\t NULL);\n + \n +-\tif (opts->quiet)\n ++\tif (data->maintenance_opts->quiet)\n + \t\tstrvec_push(&child.args, \"--quiet\");\n + \n +-\treturn !!run_command(&child);\n ++\tif (run_command(&child))\n ++\t\tstring_list_append(&data->failed_remotes, remote->name);\n ++\n ++\treturn 0;\n + }\n + \n + static int maintenance_task_prefetch(struct maintenance_run_opts *opts,\n + \t\t\t\t struct gc_config *cfg UNUSED)\n + {\n +-\tif (for_each_remote(fetch_remote, opts)) {\n +-\t\terror(_(\"failed to prefetch remotes\"));\n +-\t\treturn 1;\n ++\tstruct remote_cb_data cbdata = { .maintenance_opts = opts,\n ++\t\t\t\t\t .failed_remotes = STRING_LIST_INIT_DUP };\n ++\n ++\tint result = 0;\n ++\n ++\tif (for_each_remote(fetch_remote, &cbdata)) {\n ++\t\terror(_(\"failed to prefetch some remotes\"));\n ++\t\tresult = 1;\n + \t}\n + \n +-\treturn 0;\n ++\treport_failed_remotes(&cbdata.failed_remotes, \"prefetch\");\n ++\tif (cbdata.failed_remotes.nr)\n ++\t\tresult = 1;\n ++\n ++\tstring_list_clear(&cbdata.failed_remotes, 0);\n ++\treturn result;\n + }\n + \n + static int maintenance_task_gc(struct maintenance_run_opts *opts,\n @@ builtin/gc.c: enum maintenance_task_label {\n \tTASK_GC,\n \tTASK_COMMIT_GRAPH,\n @@ t/t7900-maintenance.sh: test_expect_success 'pack-refs task' '\n +\t\ttest_must_fail git rev-parse refs/remotes/remote2/side2\n +\t)\n +'\n ++\n ++test_expect_success 'prune-remote-refs task continues to prune remotes even if some fail' '\n ++\ttest_commit initial-prune-remote-refs &&\n ++\n ++\tgit clone . remote-bad1 &&\n ++\tgit clone . remote-bad2 &&\n ++\tgit clone . remote-good &&\n ++\n ++\tgit clone . prune-test-partial &&\n ++\t(\n ++\t\tcd prune-test-partial &&\n ++\t\tgit config maintenance.prune-remote-refs.enabled true &&\n ++\n ++\t\t# Add remotes in alphabetical order to ensure processing order\n ++\t\tgit remote add aaa-bad1 \"../remote-bad1\" &&\n ++\t\tgit remote add bbb-bad2 \"../remote-bad2\" &&\n ++\t\tgit remote add ccc-good \"../remote-good\" &&\n ++\n ++\t\t# Create and push branches to all remotes\n ++\t\tgit branch -f side2 HEAD &&\n ++\t\tgit push aaa-bad1 side2 &&\n ++\t\tgit push bbb-bad2 side2 &&\n ++\t\tgit push ccc-good side2 &&\n ++\n ++\t\t# Rename branch in good remote to simulate a stale branch\n ++\t\tgit -C ../remote-good branch -m side2 side3 &&\n ++\n ++\t\t# Break the bad remotes by removing their directories\n ++\t\trm -rf ../remote-bad1 ../remote-bad2 &&\n ++\n ++\t\tGIT_TRACE2_EVENT=\"$(pwd)/prune.txt\" git maintenance run --task=prune-remote-refs 2>err || true &&\n ++\n ++\t\t# Verify pruning happened for good remote despite bad remote failures\n ++\t\ttest_subcommand git remote prune ccc-good err &&\n\n\n Documentation/git-maintenance.txt | 20 +++++++\n builtin/gc.c | 92 ++++++++++++++++++++++++++++---\n t/t7900-maintenance.sh | 82 +++++++++++++++++++++++++++\n 3 files changed, 187 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\nindex 6e6651309d3..df59d43ec88 100644\n--- a/Documentation/git-maintenance.txt\n+++ b/Documentation/git-maintenance.txt\n@@ -158,6 +158,26 @@ pack-refs::\n \tneed to iterate across many references. See linkgit:git-pack-refs[1]\n \tfor more information.\n \n+prune-remote-refs::\n+\tThe `prune-remote-refs` task runs `git remote prune` on each remote\n+\trepository registered in the local repository. This task helps clean\n+\tup deleted remote branches, improving the performance of operations\n+\tthat iterate through the refs. See linkgit:git-remote[1] for more\n+\tinformation. This task is disabled by default.\n++\n+NOTE: This task is opt-in to prevent unexpected removal of remote refs\n+for users of linkgit:git-maintenance[1]. For most users, configuring `fetch.prune=true`\n+is an acceptable solution, as it will automatically clean up stale remote-tracking\n+branches during normal fetch operations. However, this task can be useful in\n+specific scenarios:\n++\n+--\n+* When using selective fetching (e.g., `git fetch origin +foo:refs/remotes/origin/foo`)\n+ where `fetch.prune` would only affect refs that are explicitly fetched.\n+* When third-party tools might perform unexpected full fetches, and you want\n+ periodic cleanup independently of fetch operations.\n+--\n+\n OPTIONS\n -------\n --auto::\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex a9b1c36de27..ae2a6762a92 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -23,6 +23,7 @@\n #include \"lockfile.h\"\n #include \"parse-options.h\"\n #include \"run-command.h\"\n+#include \"remote.h\"\n #include \"sigchain.h\"\n #include \"strvec.h\"\n #include \"commit.h\"\n@@ -916,6 +917,63 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n+struct remote_cb_data {\n+\tstruct maintenance_run_opts *maintenance_opts;\n+\tstruct string_list failed_remotes;\n+};\n+\n+static void report_failed_remotes(struct string_list *failed_remotes,\n+\t\t\t\t const char *action_name)\n+{\n+\tif (failed_remotes->nr) {\n+\t\tint i;\n+\t\tstruct strbuf msg = STRBUF_INIT;\n+\t\tstrbuf_addf(&msg, _(\"failed to %s the following remotes: \"),\n+\t\t\t action_name);\n+\t\tfor (i = 0; i < failed_remotes->nr; i++) {\n+\t\t\tif (i)\n+\t\t\t\tstrbuf_addstr(&msg, \", \");\n+\t\t\tstrbuf_addstr(&msg, failed_remotes->items[i].string);\n+\t\t}\n+\t\terror(\"%s\", msg.buf);\n+\t\tstrbuf_release(&msg);\n+\t}\n+}\n+\n+static int prune_remote(struct remote *remote, void *cb_data)\n+{\n+\tstruct child_process child = CHILD_PROCESS_INIT;\n+\tstruct remote_cb_data *data = cb_data;\n+\n+\tif (!remote->url.nr)\n+\t\treturn 0;\n+\n+\tchild.git_cmd = 1;\n+\tstrvec_pushl(&child.args, \"remote\", \"prune\", remote->name, NULL);\n+\n+\tif (run_command(&child))\n+\t\tstring_list_append(&data->failed_remotes, remote->name);\n+\n+\treturn 0;\n+}\n+\n+static int maintenance_task_prune_remote(struct maintenance_run_opts *opts,\n+\t\t\t\t\t struct gc_config *cfg UNUSED)\n+{\n+\tstruct remote_cb_data cbdata = { .maintenance_opts = opts,\n+\t\t\t\t\t .failed_remotes = STRING_LIST_INIT_DUP };\n+\n+\tint result;\n+\tresult = for_each_remote(prune_remote, &cbdata);\n+\n+\treport_failed_remotes(&cbdata.failed_remotes, \"prune\");\n+\tif (cbdata.failed_remotes.nr)\n+\t\tresult = 1;\n+\n+\tstring_list_clear(&cbdata.failed_remotes, 0);\n+\treturn result;\n+}\n+\n /* Remember to update object flag allocation in object.h */\n #define SEEN\t\t(1u<<0)\n \n@@ -1036,8 +1094,8 @@ static int maintenance_task_commit_graph(struct maintenance_run_opts *opts,\n \n static int fetch_remote(struct remote *remote, void *cbdata)\n {\n-\tstruct maintenance_run_opts *opts = cbdata;\n \tstruct child_process child = CHILD_PROCESS_INIT;\n+\tstruct remote_cb_data *data = cbdata;\n \n \tif (remote->skip_default_update)\n \t\treturn 0;\n@@ -1048,21 +1106,34 @@ static int fetch_remote(struct remote *remote, void *cbdata)\n \t\t \"--no-write-fetch-head\", \"--recurse-submodules=no\",\n \t\t NULL);\n \n-\tif (opts->quiet)\n+\tif (data->maintenance_opts->quiet)\n \t\tstrvec_push(&child.args, \"--quiet\");\n \n-\treturn !!run_command(&child);\n+\tif (run_command(&child))\n+\t\tstring_list_append(&data->failed_remotes, remote->name);\n+\n+\treturn 0;\n }\n \n static int maintenance_task_prefetch(struct maintenance_run_opts *opts,\n \t\t\t\t struct gc_config *cfg UNUSED)\n {\n-\tif (for_each_remote(fetch_remote, opts)) {\n-\t\terror(_(\"failed to prefetch remotes\"));\n-\t\treturn 1;\n+\tstruct remote_cb_data cbdata = { .maintenance_opts = opts,\n+\t\t\t\t\t .failed_remotes = STRING_LIST_INIT_DUP };\n+\n+\tint result = 0;\n+\n+\tif (for_each_remote(fetch_remote, &cbdata)) {\n+\t\terror(_(\"failed to prefetch some remotes\"));\n+\t\tresult = 1;\n \t}\n \n-\treturn 0;\n+\treport_failed_remotes(&cbdata.failed_remotes, \"prefetch\");\n+\tif (cbdata.failed_remotes.nr)\n+\t\tresult = 1;\n+\n+\tstring_list_clear(&cbdata.failed_remotes, 0);\n+\treturn result;\n }\n \n static int maintenance_task_gc(struct maintenance_run_opts *opts,\n@@ -1378,6 +1449,7 @@ enum maintenance_task_label {\n \tTASK_GC,\n \tTASK_COMMIT_GRAPH,\n \tTASK_PACK_REFS,\n+\tTASK_PRUNE_REMOTE_REFS,\n \n \t/* Leave as final value */\n \tTASK__COUNT\n@@ -1414,6 +1486,10 @@ static struct maintenance_task tasks[] = {\n \t\tmaintenance_task_pack_refs,\n \t\tpack_refs_condition,\n \t},\n+\t[TASK_PRUNE_REMOTE_REFS] = {\n+\t\t\"prune-remote-refs\",\n+\t\tmaintenance_task_prune_remote,\n+\t},\n };\n \n static int compare_tasks_by_selection(const void *a_, const void *b_)\n@@ -1508,6 +1584,8 @@ static void initialize_maintenance_strategy(void)\n \t\ttasks[TASK_LOOSE_OBJECTS].schedule = SCHEDULE_DAILY;\n \t\ttasks[TASK_PACK_REFS].enabled = 1;\n \t\ttasks[TASK_PACK_REFS].schedule = SCHEDULE_WEEKLY;\n+\t\ttasks[TASK_PRUNE_REMOTE_REFS].enabled = 0;\n+\t\ttasks[TASK_PRUNE_REMOTE_REFS].schedule = SCHEDULE_DAILY;\n \t}\n }\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 1909aed95e0..34e8fa6b5fb 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -447,6 +447,88 @@ test_expect_success 'pack-refs task' '\n \ttest_subcommand git pack-refs --all --prune err &&\n+\t\ttest_subcommand ! git remote prune origin err || true &&\n+\n+\t\t# Verify pruning happened for good remote despite bad remote failures\n+\t\ttest_subcommand git remote prune ccc-good err &&\n \ttest_grep \"at most one\" err\n\nbase-commit: 76cf4f61c87855ebf0784b88aaf737d6b09f504b\n-- \ngitgitgadget\n"},{"id":"509878","messageId":"xmqqbjwnra9u.fsf@gitster.g","threadId":"62684","inReplyTo":"pull.1838.v3.git.1735928035056.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] maintenance: add prune-remote-refs task","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-03T19:02:21Z","receivedAt":"2025-01-03T19:02:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shubham Kanodia via GitGitGadget\" writes:\n\n> From: Shubham Kanodia \n>\n> Remote-tracking refs can accumulate in local repositories even as branches\n> are deleted on remotes, impacting git performance negatively. Existing\n> alternatives to keep refs pruned have a few issues:\n>\n> 1. Running `git fetch` with either `--prune` or `fetch.prune=true`\n> set, with the default refspec to copy all their branches into\n> our remote-tracking branches, will prune stale refs, but also\n> pulls in new branches from remote. That is undesirable if the\n> user wants to only work with a selected few remote branches.\n>\n> 2. `git remote prune` cleans up refs without adding to the\n> existing list but requires periodic user intervention.\n>\n> Add a new maintenance task 'prune-remote-refs' that runs 'git remote\n> prune' for each configured remote daily. Leave the task disabled by\n> default, as it may be unexpected to see their remote-tracking\n> branches to disappear while they are not watching for unsuspecting\n> users.\n\nThere is no description on how and why the prefetch job has been\nmodified here.\n\nI haven't formed a strong opinion on the \"should we keep going after\nthe first failure?\" question yet, and if this topic is modifying the\nway how the prefetch operates, the patch(es) should be CC'ed to the\nauthor of that feature (The author of 28cb5e66 (maintenance: add\nprefetch task, 2020-09-25) CC'ed).\n\nIf it turns out to be a good idea to do so, I would expect the topic\nto consist of at least two patches:\n\n - [PATCH 1/2] to argue that it is a bug that the prefetch job stops\n at the first failed remote, and change its behaviour to prefetch\n from all remotes and then report a failure if the prefetch failed\n for any remote. With some additional tests to check the updated\n behaviour.\n\n - [PATCH 2/2] to argue the need for periodic `remote prune`, and do\n the part of this patch that relates to that new feature.\n\n> +struct remote_cb_data {\n> +\tstruct maintenance_run_opts *maintenance_opts;\n> +\tstruct string_list failed_remotes;\n> +};\n> +\n> +static void report_failed_remotes(struct string_list *failed_remotes,\n> +\t\t\t\t const char *action_name)\n> +{\n> +\tif (failed_remotes->nr) {\n> +\t\tint i;\n> +\t\tstruct strbuf msg = STRBUF_INIT;\n> +\t\tstrbuf_addf(&msg, _(\"failed to %s the following remotes: \"),\n> +\t\t\t action_name);\n> +\t\tfor (i = 0; i < failed_remotes->nr; i++) {\n> +\t\t\tif (i)\n> +\t\t\t\tstrbuf_addstr(&msg, \", \");\n> +\t\t\tstrbuf_addstr(&msg, failed_remotes->items[i].string);\n> +\t\t}\n> +\t\terror(\"%s\", msg.buf);\n> +\t\tstrbuf_release(&msg);\n> +\t}\n> +}\n\nA few comments:\n\n - The message pretends to be _(\"localizable\"), but the sentence\n logo inserts action_name that is not translated. If the\n operation failed only for a single remote, \"following remotes\" is\n grammatically incorrect.\n\n - Would it be useful to force this message to a single line, with\n multiple remote names concatenated with \",\"? Computer output of\n a listing often is more useful without \",\" if it is meant to be\n consumable for cut-and-paste users.\n\nOverall, I am fairly negative on the report this helper tries to\ngive the users. Because we are going to do the operation on all\nremotes anyway, wouldn't we have let the underlying operations (like\n\"git fetch\" or \"git remore prune\") already issue error messages to\nthe user? Do we need this extra reporting on top at all?\n\nThanks.\n"},{"id":"510101","messageId":"CAG=Um+1tD+taKyN35x6q8ynjgrp+nVPBbuff5FUm6HUp3qe+fw@mail.gmail.com","threadId":"62684","inReplyTo":"CAG=Um+1ch1sKC0H8MJoFv=6iSK3pvA=03AKXmvhm5DG=H8T1rw@mail.gmail.com","subject":"Re: [PATCH v3] maintenance: add prune-remote-refs task","fromName":"Shubham Kanodia","fromEmail":"shubham.kanodia10@gmail.com","sentAt":"2025-01-07T17:29:07Z","receivedAt":"2025-01-07T17:29:44Z","isPatch":true,"sender":{"key":"shubham.kanodia10@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8946207?v=4"},"body":"On Sat, Jan 4, 2025 at 1:23 AM Shubham Kanodia\n wrote:\n>\n>\n>\n> On Sat, 4 Jan 2025 at 12:32 AM, Junio C Hamano wrote:\n>>\n>> \"Shubham Kanodia via GitGitGadget\" writes:\n>>\n>> > From: Shubham Kanodia \n>> >\n>> > Remote-tracking refs can accumulate in local repositories even as branches\n>> > are deleted on remotes, impacting git performance negatively. Existing\n>> > alternatives to keep refs pruned have a few issues:\n>> >\n>> > 1. Running `git fetch` with either `--prune` or `fetch.prune=true`\n>> > set, with the default refspec to copy all their branches into\n>> > our remote-tracking branches, will prune stale refs, but also\n>> > pulls in new branches from remote. That is undesirable if the\n>> > user wants to only work with a selected few remote branches.\n>> >\n>> > 2. `git remote prune` cleans up refs without adding to the\n>> > existing list but requires periodic user intervention.\n>> >\n>> > Add a new maintenance task 'prune-remote-refs' that runs 'git remote\n>> > prune' for each configured remote daily. Leave the task disabled by\n>> > default, as it may be unexpected to see their remote-tracking\n>> > branches to disappear while they are not watching for unsuspecting\n>> > users.\n>>\n>> There is no description on how and why the prefetch job has been\n>> modified here.\n>>\n>> I haven't formed a strong opinion on the \"should we keep going after\n>> the first failure?\" question yet, and if this topic is modifying the\n>> way how the prefetch operates, the patch(es) should be CC'ed to the\n>> author of that feature (The author of 28cb5e66 (maintenance: add\n>> prefetch task, 2020-09-25) CC'ed).\n>>\n>> If it turns out to be a good idea to do so, I would expect the topic\n>> to consist of at least two patches:\n>>\n>> - [PATCH 1/2] to argue that it is a bug that the prefetch job stops\n>> at the first failed remote, and change its behaviour to prefetch\n>> from all remotes and then report a failure if the prefetch failed\n>> for any remote. With some additional tests to check the updated\n>> behaviour.\n>>\n>> - [PATCH 2/2] to argue the need for periodic `remote prune`, and do\n>> the part of this patch that relates to that new feature.\n>>\n>> > +struct remote_cb_data {\n>> > + struct maintenance_run_opts *maintenance_opts;\n>> > + struct string_list failed_remotes;\n>> > +};\n>> > +\n>> > +static void report_failed_remotes(struct string_list *failed_remotes,\n>> > + const char *action_name)\n>> > +{\n>> > + if (failed_remotes->nr) {\n>> > + int i;\n>> > + struct strbuf msg = STRBUF_INIT;\n>> > + strbuf_addf(&msg, _(\"failed to %s the following remotes: \"),\n>> > + action_name);\n>> > + for (i = 0; i < failed_remotes->nr; i++) {\n>> > + if (i)\n>> > + strbuf_addstr(&msg, \", \");\n>> > + strbuf_addstr(&msg, failed_remotes->items[i].string);\n>> > + }\n>> > + error(\"%s\", msg.buf);\n>> > + strbuf_release(&msg);\n>> > + }\n>> > +}\n>>\n>> A few comments:\n>>\n>> - The message pretends to be _(\"localizable\"), but the sentence\n>> logo inserts action_name that is not translated. If the\n>> operation failed only for a single remote, \"following remotes\" is\n>> grammatically incorrect.\n>>\n>> - Would it be useful to force this message to a single line, with\n>> multiple remote names concatenated with \",\"? Computer output of\n>> a listing often is more useful without \",\" if it is meant to be\n>> consumable for cut-and-paste users.\n>>\n>> Overall, I am fairly negative on the report this helper tries to\n>> give the users. Because we are going to do the operation on all\n>> remotes anyway, wouldn't we have let the underlying operations (like\n>> \"git fetch\" or \"git remore prune\") already issue error messages to\n>> the user? Do we need this extra reporting on top at all?\n>>\n>> Thanks.\n>\n>\n>\n> If it’s fine, I’d like to discuss the change to process all remotes separately from the initial change I submitted.\n>\n> I took an attempt at it in my last patch, but I don’t know if I’m really going to have the time to iterate on it as it looks more involved.\n>\n> In any case the implementation isn’t any worse off than the current maintenance command.\n>\n> Also, as a new contributor I’m unsure of recalling / patching a change that’s already in seen at the moment unless it is an unworkable solution.\n>\n> Thanks again.\n> Shubham K\n\nNoticed an update on What's Cooking —\n\n> * sk/maintenance-remote-prune (2025-01-03) 1 commit\n> - maintenance: add prune-remote-refs task\n> A new periodic maintenance task to run \"git remote prune\" has been\n introduced.\n> Expecting a reroll.\n\nJunio, what do you think about my previous suggestion — do you think\nthat changing the remote behaviours is a blocker for this change to\nmake its way to master?\n"},{"id":"510107","messageId":"xmqqmsg2sbn5.fsf@gitster.g","threadId":"62684","inReplyTo":"CAG=Um+1tD+taKyN35x6q8ynjgrp+nVPBbuff5FUm6HUp3qe+fw@mail.gmail.com","subject":"Re: [PATCH v3] maintenance: add prune-remote-refs task","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-07T18:48:46Z","receivedAt":"2025-01-07T18:48:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shubham Kanodia writes:\n\n> Junio, what do you think about my previous suggestion — do you think\n> that changing the remote behaviours is a blocker for this change to\n> make its way to master?\n\nYes, changing the behaviour of an existing command in the same patch\nthat adds a new feature, and doing so without clearly explaining why\nsuch a change is a good idea in the proposed log message, are both\nhuge blockers. Not just to 'master', but to anywhere near 'next'.\n\nThanks.\n\n"}]}