{"thread":{"id":"61331","subject":"[PATCH 0/2] Use a \"best effort\" strategy in scheduled maintenance","startedAt":"2024-04-17T08:28:59Z","lastAt":"2024-04-25T06:36:46Z","messageCount":19,"participants":["Johannes Schindelin via GitGitGadget","Eric Sunshine","Junio C Hamano","Patrick Steinhardt","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"493078","messageId":"pull.1719.git.1713342535.gitgitgadget@gmail.com","threadId":"61331","inReplyTo":null,"subject":"[PATCH 0/2] Use a \"best effort\" strategy in scheduled maintenance","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-17T08:28:53Z","receivedAt":"2024-04-17T08:28:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Over in https://github.com/microsoft/git/issues/623, it was pointed out that\nscheduled maintenance will error out when it encounters a missing\nrepository. The scheduled maintenance should exit with an error, all right,\nbut what about the remaining repositories for which maintenance was\nscheduled, and that may not be missing?\n\nThis patch series addresses this by introducing a new for-each-repo option\nand then using it in the command that is run via scheduled maintenance.\n\nJohannes Schindelin (2):\n for-each-repo: optionally keep going on an error\n maintenance: running maintenance should not stop on errors\n\n Documentation/git-for-each-repo.txt | 4 ++++\n builtin/for-each-repo.c | 8 ++++++--\n builtin/gc.c | 7 ++++---\n t/t0068-for-each-repo.sh | 16 ++++++++++++++++\n t/t7900-maintenance.sh | 6 +++---\n 5 files changed, 33 insertions(+), 8 deletions(-)\n\n\nbase-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1719%2Fdscho%2Ffor-each-repo-stop-on-error-2.44-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1719/dscho/for-each-repo-stop-on-error-2.44-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1719\n-- \ngitgitgadget\n"},{"id":"493079","messageId":"6721e3ada5d125bd6c33561c694acb986b17b38f.1713342535.git.gitgitgadget@gmail.com","threadId":"61331","inReplyTo":"pull.1719.git.1713342535.gitgitgadget@gmail.com","subject":"[PATCH 1/2] for-each-repo: optionally keep going on an error","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-17T08:28:54Z","receivedAt":"2024-04-17T08:29:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn https://github.com/microsoft/git/issues/623, it was reported that\nthe regularly scheduled maintenance stops if one repo in the middle of\nthe list was found to be missing.\n\nThis is undesirable, and points out a gap in the design of `git\nfor-each-repo`: We need a mode where that command does not stop on an\nerror, but continues to try the running the specified command with the\nother repositories.\n\nImitating the `--keep-going` option of GNU make, this commit teaches\n`for-each-repo` the same trick: to continue with the operation on all\nthe remaining repositories in case there was a problem with one\nrepository, still setting the exit code to indicate an error occurred.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-for-each-repo.txt | 4 ++++\n builtin/for-each-repo.c | 8 ++++++--\n t/t0068-for-each-repo.sh | 16 ++++++++++++++++\n 3 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-for-each-repo.txt b/Documentation/git-for-each-repo.txt\nindex 94bd19da263..8c18001d825 100644\n--- a/Documentation/git-for-each-repo.txt\n+++ b/Documentation/git-for-each-repo.txt\n@@ -42,6 +42,10 @@ These config values are loaded from system, global, and local Git config,\n as available. If `git for-each-repo` is run in a directory that is not a\n Git repository, then only the system and global config is used.\n \n+--keep-going::\n+\tContinue with the remaining repositories if the command failed\n+\ton a repository. The exit code will still indicate that the\n+\toverall operation was not successful.\n \n SUBPROCESS BEHAVIOR\n -------------------\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 28186b30f54..74498539e9c 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -32,6 +32,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)\n int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n {\n \tstatic const char *config_key = NULL;\n+\tint keep_going = 0;\n \tint i, result = 0;\n \tconst struct string_list *values;\n \tint err;\n@@ -39,6 +40,8 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\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_BOOL(0, \"keep-going\", &keep_going,\n+\t\t\t N_(\"stop at the first repository where the operation failed\")),\n \t\tOPT_END()\n \t};\n \n@@ -55,8 +58,9 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n \telse if (err)\n \t\treturn 0;\n \n-\tfor (i = 0; !result && i < values->nr; i++)\n-\t\tresult = run_command_on_repo(values->items[i].string, argc, argv);\n+\tfor (i = 0; (keep_going || !result) && i < values->nr; i++)\n+\t\tif (run_command_on_repo(values->items[i].string, argc, argv))\n+\t\t\tresult = 1;\n \n \treturn result;\n }\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex 4b90b74d5d5..95019e01ed3 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -59,4 +59,20 @@ test_expect_success 'error on NULL value for config keys' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--keep-going' '\n+\tgit config keep.going non-existing &&\n+\tgit config --add keep.going . &&\n+\n+\ttest_must_fail git for-each-repo --config=keep.going \\\n+\t\t-- branch >out 2>err &&\n+\ttest_grep \"cannot change to .*non-existing\" err &&\n+\ttest_must_be_empty out &&\n+\n+\ttest_must_fail git for-each-repo --config=keep.going --keep-going \\\n+\t\t-- branch >out 2>err &&\n+\ttest_grep \"cannot change to .*non-existing\" err &&\n+\tgit branch >expect &&\n+\ttest_cmp expect out\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"493080","messageId":"a86bcf2e1a0aadaaaeb5bfe5b0a4a0fa12594921.1713342535.git.gitgitgadget@gmail.com","threadId":"61331","inReplyTo":"pull.1719.git.1713342535.gitgitgadget@gmail.com","subject":"[PATCH 2/2] maintenance: running maintenance should not stop on errors","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-17T08:28:55Z","receivedAt":"2024-04-17T08:29:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn https://github.com/microsoft/git/issues/623, it was reported that\nmaintenance stops on a missing repository, omitting the remaining\nrepositories that were scheduled for maintenance.\n\nThis is undesirable, as it should be a best effort type of operation.\n\nIt should still fail due to the missing repository, of course, but not\nleave the non-missing repositories in unmaintained shapes.\n\nLet's use `for-each-repo`'s shiny new `--keep-going` option that we just\nintroduced for that very purpose.\n\nThis change will be picked up when running `git maintenance start`,\nwhich is run implicitly by `scalar reconfigure`.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/gc.c | 7 ++++---\n t/t7900-maintenance.sh | 6 +++---\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex cb80ced6cb5..b069aa49c50 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1858,6 +1858,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit\n \t\t \"%s/git\\n\"\n \t\t \"--exec-path=%s\\n\"\n \t\t \"for-each-repo\\n\"\n+\t\t \"--keep-going\\n\"\n \t\t \"--config=maintenance.repo\\n\"\n \t\t \"maintenance\\n\"\n \t\t \"run\\n\"\n@@ -2100,7 +2101,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority\n \t \"\\n\"\n \t \"\\n\"\n \t \"\\\"%s\\\\headless-git.exe\\\"\\n\"\n-\t \"--exec-path=\\\"%s\\\" for-each-repo --config=maintenance.repo maintenance run --schedule=%s\\n\"\n+\t \"--exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s\\n\"\n \t \"\\n\"\n \t \"\\n\"\n \t \"\\n\";\n@@ -2245,7 +2246,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)\n \t\t\t\"# replaced in the future by a Git command.\\n\\n\");\n \n \t\tstrbuf_addf(&line_format,\n-\t\t\t \"%%d %%s * * %%s \\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\\n\",\n+\t\t\t \"%%d %%s * * %%s \\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\\n\",\n \t\t\t exec_path, exec_path);\n \t\tfprintf(cron_in, line_format.buf, minute, \"1-23\", \"*\", \"hourly\");\n \t\tfprintf(cron_in, line_format.buf, minute, \"0\", \"1-6\", \"daily\");\n@@ -2446,7 +2447,7 @@ static int systemd_timer_write_service_template(const char *exec_path)\n \t \"\\n\"\n \t \"[Service]\\n\"\n \t \"Type=oneshot\\n\"\n-\t \"ExecStart=\\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%i\\n\"\n+\t \"ExecStart=\\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\\n\"\n \t \"LockPersonality=yes\\n\"\n \t \"MemoryDenyWriteExecute=yes\\n\"\n \t \"NoNewPrivileges=yes\\n\"\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 0943dfa18a3..8595489cebe 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -639,9 +639,9 @@ test_expect_success 'start from empty cron table' '\n \t# start registers the repo\n \tgit config --get --global --fixed-value maintenance.repo \"$(pwd)\" &&\n \n-\tgrep \"for-each-repo --config=maintenance.repo maintenance run --schedule=daily\" cron.txt &&\n-\tgrep \"for-each-repo --config=maintenance.repo maintenance run --schedule=hourly\" cron.txt &&\n-\tgrep \"for-each-repo --config=maintenance.repo maintenance run --schedule=weekly\" cron.txt\n+\tgrep \"for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=daily\" cron.txt &&\n+\tgrep \"for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=hourly\" cron.txt &&\n+\tgrep \"for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=weekly\" cron.txt\n '\n \n test_expect_success 'stop from existing schedule' '\n-- \ngitgitgadget\n"},{"id":"493081","messageId":"CAPig+cSjoGe7Eeynz=jGSaNYWXQ-VkvWv7mv1NDeCXPFEtdqOA@mail.gmail.com","threadId":"61331","inReplyTo":"6721e3ada5d125bd6c33561c694acb986b17b38f.1713342535.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] for-each-repo: optionally keep going on an error","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-17T08:36:36Z","receivedAt":"2024-04-17T08:36:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 17, 2024 at 4:29 AM Johannes Schindelin via GitGitGadget\n wrote:\n> In https://github.com/microsoft/git/issues/623, it was reported that\n> the regularly scheduled maintenance stops if one repo in the middle of\n> the list was found to be missing.\n>\n> This is undesirable, and points out a gap in the design of `git\n> for-each-repo`: We need a mode where that command does not stop on an\n> error, but continues to try the running the specified command with the\n> other repositories.\n\ns/try the running/try running/\n\n> Imitating the `--keep-going` option of GNU make, this commit teaches\n> `for-each-repo` the same trick: to continue with the operation on all\n> the remaining repositories in case there was a problem with one\n> repository, still setting the exit code to indicate an error occurred.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\n> @@ -39,6 +40,8 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n> + OPT_BOOL(0, \"keep-going\", &keep_going,\n> + N_(\"stop at the first repository where the operation failed\")),\n\nIsn't this help string opposite the intended meaning? Taking a hint\nfrom GNU \"make --help\", should it instead by something like:\n\n N_(\"keep going even if command fails in a repository\")),\n"},{"id":"493095","messageId":"xmqq1q74atng.fsf@gitster.g","threadId":"61331","inReplyTo":"pull.1719.git.1713342535.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Use a \"best effort\" strategy in scheduled maintenance","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-17T15:36:03Z","receivedAt":"2024-04-17T15:36:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> Over in https://github.com/microsoft/git/issues/623, it was pointed out that\n> scheduled maintenance will error out when it encounters a missing\n> repository. The scheduled maintenance should exit with an error, all right,\n> but what about the remaining repositories for which maintenance was\n> scheduled, and that may not be missing?\n\nInteresting.\n\nAs the order of the maintenance run across repositories is not\ncontrolled by the end-user, I think it is reasonable to keep going\nand finish the others, at least by default (if you could control the\nordering, you could make an arrangement where having ran maintenance\non repository A successfully would be necessary to do maintenance on\nrepository B, but we do not give that power to the end users).\n\n> This patch series addresses this by introducing a new for-each-repo option\n> and then using it in the command that is run via scheduled maintenance.\n\nOK.\n"},{"id":"493096","messageId":"xmqqwmow9ezc.fsf@gitster.g","threadId":"61331","inReplyTo":"CAPig+cSjoGe7Eeynz=jGSaNYWXQ-VkvWv7mv1NDeCXPFEtdqOA@mail.gmail.com","subject":"Re: [PATCH 1/2] for-each-repo: optionally keep going on an error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-17T15:38:15Z","receivedAt":"2024-04-17T15:38:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> This is undesirable, and points out a gap in the design of `git\n>> for-each-repo`: We need a mode where that command does not stop on an\n>> error, but continues to try the running the specified command with the\n>> other repositories.\n>\n> s/try the running/try running/\n> ...\n>> + OPT_BOOL(0, \"keep-going\", &keep_going,\n>> + N_(\"stop at the first repository where the operation failed\")),\n>\n> Isn't this help string opposite the intended meaning? Taking a hint\n> from GNU \"make --help\", should it instead by something like:\n>\n> N_(\"keep going even if command fails in a repository\")),\n\nGood eyes. Thanks for carefully reading.\n"},{"id":"493097","messageId":"xmqqo7a89eud.fsf@gitster.g","threadId":"61331","inReplyTo":"a86bcf2e1a0aadaaaeb5bfe5b0a4a0fa12594921.1713342535.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] maintenance: running maintenance should not stop on errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-17T15:41:14Z","receivedAt":"2024-04-17T15:41:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> In https://github.com/microsoft/git/issues/623, it was reported that\n> maintenance stops on a missing repository, omitting the remaining\n> repositories that were scheduled for maintenance.\n>\n> This is undesirable, as it should be a best effort type of operation.\n>\n> It should still fail due to the missing repository, of course, but not\n> leave the non-missing repositories in unmaintained shapes.\n>\n> Let's use `for-each-repo`'s shiny new `--keep-going` option that we just\n> introduced for that very purpose.\n>\n> This change will be picked up when running `git maintenance start`,\n> which is run implicitly by `scalar reconfigure`.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin/gc.c | 7 ++++---\n> t/t7900-maintenance.sh | 6 +++---\n> 2 files changed, 7 insertions(+), 6 deletions(-)\n\nOther than the N_(\"\") thing Eric noticed, I didn't find anything\nglaringly wrong in these two patches. Nicely done.\n\nWe may want to fold overly long lines we see in the patch, but I'd\nprefer to see it done as a post-cleanup task (#leftoverbits), as the\nlines in the preimage of the patch are already overly long.\n\nThanks.\n"},{"id":"493129","messageId":"pull.1719.v2.git.1713444783.gitgitgadget@gmail.com","threadId":"61331","inReplyTo":"pull.1719.git.1713342535.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Use a \"best effort\" strategy in scheduled maintenance","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-18T12:53:01Z","receivedAt":"2024-04-18T12:53:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Over in https://github.com/microsoft/git/issues/623, it was pointed out that\nscheduled maintenance will error out when it encounters a missing\nrepository. The scheduled maintenance should exit with an error, all right,\nbut what about the remaining repositories for which maintenance was\nscheduled, and that may not be missing?\n\nThis patch series addresses this by introducing a new for-each-repo option\nand then using it in the command that is run via scheduled maintenance.\n\nChanges since v1 (thanks Eric!):\n\n * Changed the option's documentation to reflect the current state (instead\n of the original design)\n * Fixed grammar issues\n\nJohannes Schindelin (2):\n for-each-repo: optionally keep going on an error\n maintenance: running maintenance should not stop on errors\n\n Documentation/git-for-each-repo.txt | 4 ++++\n builtin/for-each-repo.c | 8 ++++++--\n builtin/gc.c | 7 ++++---\n t/t0068-for-each-repo.sh | 16 ++++++++++++++++\n t/t7900-maintenance.sh | 6 +++---\n 5 files changed, 33 insertions(+), 8 deletions(-)\n\n\nbase-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1719%2Fdscho%2Ffor-each-repo-stop-on-error-2.44-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1719/dscho/for-each-repo-stop-on-error-2.44-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1719\n\nRange-diff vs v1:\n\n 1: 6721e3ada5d ! 1: abd796894c8 for-each-repo: optionally keep going on an error\n @@ Commit message\n \n This is undesirable, and points out a gap in the design of `git\n for-each-repo`: We need a mode where that command does not stop on an\n - error, but continues to try the running the specified command with the\n - other repositories.\n + error, but continues to try running the specified command with the other\n + repositories.\n \n Imitating the `--keep-going` option of GNU make, this commit teaches\n `for-each-repo` the same trick: to continue with the operation on all\n the remaining repositories in case there was a problem with one\n repository, still setting the exit code to indicate an error occurred.\n \n + Helped-by: Eric Sunshine \n Signed-off-by: Johannes Schindelin \n \n ## Documentation/git-for-each-repo.txt ##\n @@ builtin/for-each-repo.c: int cmd_for_each_repo(int argc, const char **argv, cons\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_BOOL(0, \"keep-going\", &keep_going,\n -+\t\t\t N_(\"stop at the first repository where the operation failed\")),\n ++\t\t\t N_(\"keep going even if command fails in a repository\")),\n \t\tOPT_END()\n \t};\n \n 2: a86bcf2e1a0 = 2: 1ae11553052 maintenance: running maintenance should not stop on errors\n\n-- \ngitgitgadget\n"},{"id":"493130","messageId":"abd796894c857fc9ad96b9942089474df01f0506.1713444783.git.gitgitgadget@gmail.com","threadId":"61331","inReplyTo":"pull.1719.v2.git.1713444783.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] for-each-repo: optionally keep going on an error","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-18T12:53:02Z","receivedAt":"2024-04-18T12:53:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn https://github.com/microsoft/git/issues/623, it was reported that\nthe regularly scheduled maintenance stops if one repo in the middle of\nthe list was found to be missing.\n\nThis is undesirable, and points out a gap in the design of `git\nfor-each-repo`: We need a mode where that command does not stop on an\nerror, but continues to try running the specified command with the other\nrepositories.\n\nImitating the `--keep-going` option of GNU make, this commit teaches\n`for-each-repo` the same trick: to continue with the operation on all\nthe remaining repositories in case there was a problem with one\nrepository, still setting the exit code to indicate an error occurred.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-for-each-repo.txt | 4 ++++\n builtin/for-each-repo.c | 8 ++++++--\n t/t0068-for-each-repo.sh | 16 ++++++++++++++++\n 3 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-for-each-repo.txt b/Documentation/git-for-each-repo.txt\nindex 94bd19da263..8c18001d825 100644\n--- a/Documentation/git-for-each-repo.txt\n+++ b/Documentation/git-for-each-repo.txt\n@@ -42,6 +42,10 @@ These config values are loaded from system, global, and local Git config,\n as available. If `git for-each-repo` is run in a directory that is not a\n Git repository, then only the system and global config is used.\n \n+--keep-going::\n+\tContinue with the remaining repositories if the command failed\n+\ton a repository. The exit code will still indicate that the\n+\toverall operation was not successful.\n \n SUBPROCESS BEHAVIOR\n -------------------\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 28186b30f54..9bdf2b34f89 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -32,6 +32,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)\n int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n {\n \tstatic const char *config_key = NULL;\n+\tint keep_going = 0;\n \tint i, result = 0;\n \tconst struct string_list *values;\n \tint err;\n@@ -39,6 +40,8 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\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_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 \t};\n \n@@ -55,8 +58,9 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n \telse if (err)\n \t\treturn 0;\n \n-\tfor (i = 0; !result && i < values->nr; i++)\n-\t\tresult = run_command_on_repo(values->items[i].string, argc, argv);\n+\tfor (i = 0; (keep_going || !result) && i < values->nr; i++)\n+\t\tif (run_command_on_repo(values->items[i].string, argc, argv))\n+\t\t\tresult = 1;\n \n \treturn result;\n }\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex 4b90b74d5d5..95019e01ed3 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -59,4 +59,20 @@ test_expect_success 'error on NULL value for config keys' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--keep-going' '\n+\tgit config keep.going non-existing &&\n+\tgit config --add keep.going . &&\n+\n+\ttest_must_fail git for-each-repo --config=keep.going \\\n+\t\t-- branch >out 2>err &&\n+\ttest_grep \"cannot change to .*non-existing\" err &&\n+\ttest_must_be_empty out &&\n+\n+\ttest_must_fail git for-each-repo --config=keep.going --keep-going \\\n+\t\t-- branch >out 2>err &&\n+\ttest_grep \"cannot change to .*non-existing\" err &&\n+\tgit branch >expect &&\n+\ttest_cmp expect out\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"493131","messageId":"1ae11553052cb3f48447eb5df6cbf6f185fa81db.1713444783.git.gitgitgadget@gmail.com","threadId":"61331","inReplyTo":"pull.1719.v2.git.1713444783.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] maintenance: running maintenance should not stop on errors","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-18T12:53:03Z","receivedAt":"2024-04-18T12:53:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn https://github.com/microsoft/git/issues/623, it was reported that\nmaintenance stops on a missing repository, omitting the remaining\nrepositories that were scheduled for maintenance.\n\nThis is undesirable, as it should be a best effort type of operation.\n\nIt should still fail due to the missing repository, of course, but not\nleave the non-missing repositories in unmaintained shapes.\n\nLet's use `for-each-repo`'s shiny new `--keep-going` option that we just\nintroduced for that very purpose.\n\nThis change will be picked up when running `git maintenance start`,\nwhich is run implicitly by `scalar reconfigure`.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/gc.c | 7 ++++---\n t/t7900-maintenance.sh | 6 +++---\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex cb80ced6cb5..b069aa49c50 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1858,6 +1858,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit\n \t\t \"%s/git\\n\"\n \t\t \"--exec-path=%s\\n\"\n \t\t \"for-each-repo\\n\"\n+\t\t \"--keep-going\\n\"\n \t\t \"--config=maintenance.repo\\n\"\n \t\t \"maintenance\\n\"\n \t\t \"run\\n\"\n@@ -2100,7 +2101,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority\n \t \"\\n\"\n \t \"\\n\"\n \t \"\\\"%s\\\\headless-git.exe\\\"\\n\"\n-\t \"--exec-path=\\\"%s\\\" for-each-repo --config=maintenance.repo maintenance run --schedule=%s\\n\"\n+\t \"--exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s\\n\"\n \t \"\\n\"\n \t \"\\n\"\n \t \"\\n\";\n@@ -2245,7 +2246,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)\n \t\t\t\"# replaced in the future by a Git command.\\n\\n\");\n \n \t\tstrbuf_addf(&line_format,\n-\t\t\t \"%%d %%s * * %%s \\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\\n\",\n+\t\t\t \"%%d %%s * * %%s \\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\\n\",\n \t\t\t exec_path, exec_path);\n \t\tfprintf(cron_in, line_format.buf, minute, \"1-23\", \"*\", \"hourly\");\n \t\tfprintf(cron_in, line_format.buf, minute, \"0\", \"1-6\", \"daily\");\n@@ -2446,7 +2447,7 @@ static int systemd_timer_write_service_template(const char *exec_path)\n \t \"\\n\"\n \t \"[Service]\\n\"\n \t \"Type=oneshot\\n\"\n-\t \"ExecStart=\\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%i\\n\"\n+\t \"ExecStart=\\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\\n\"\n \t \"LockPersonality=yes\\n\"\n \t \"MemoryDenyWriteExecute=yes\\n\"\n \t \"NoNewPrivileges=yes\\n\"\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 0943dfa18a3..8595489cebe 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -639,9 +639,9 @@ test_expect_success 'start from empty cron table' '\n \t# start registers the repo\n \tgit config --get --global --fixed-value maintenance.repo \"$(pwd)\" &&\n \n-\tgrep \"for-each-repo --config=maintenance.repo maintenance run --schedule=daily\" cron.txt &&\n-\tgrep \"for-each-repo --config=maintenance.repo maintenance run --schedule=hourly\" cron.txt &&\n-\tgrep \"for-each-repo --config=maintenance.repo maintenance run --schedule=weekly\" cron.txt\n+\tgrep \"for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=daily\" cron.txt &&\n+\tgrep \"for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=hourly\" cron.txt &&\n+\tgrep \"for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=weekly\" cron.txt\n '\n \n test_expect_success 'stop from existing schedule' '\n-- \ngitgitgadget\n"},{"id":"493183","messageId":"ZiHyGFRPm_pwdGgC@tanuki","threadId":"61331","inReplyTo":"abd796894c857fc9ad96b9942089474df01f0506.1713444783.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/2] for-each-repo: optionally keep going on an error","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-04-19T04:24:56Z","receivedAt":"2024-04-19T04:25:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Apr 18, 2024 at 12:53:02PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n[snip]\n> @@ -55,8 +58,9 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n> \telse if (err)\n> \t\treturn 0;\n> \n> -\tfor (i = 0; !result && i < values->nr; i++)\n> -\t\tresult = run_command_on_repo(values->items[i].string, argc, argv);\n> +\tfor (i = 0; (keep_going || !result) && i < values->nr; i++)\n> +\t\tif (run_command_on_repo(values->items[i].string, argc, argv))\n> +\t\t\tresult = 1;\n\nOne thing that made me stop and think is whether the change in behaviour\nhere may negatively impact some usecases. Before this change we would\nerror out with the return code returned by the command that we have ran\nin repositories. It makes total sense that we don't do that anymore with\n`--keep-going`, because the result would likely be useless as all we\ncould do was to OR the result codes with each other.\n\nBut do we maybe want to make this conditional on whether or not the\n`--keep-going` flag is set? So something like this:\n\n```\nfor (i = 0; (keep_going || !result) && i < values->nr; i++) {\n\tint ret = run_command_on_repo(values->items[i].string, argc, argv);\n\tif (ret)\n\t\tresult = keep_going ? 1 : ret;\n}\n```\n\nOther than that this patch series looks good to me.\n\nPatrick\n"},{"id":"493222","messageId":"xmqqy199l4qf.fsf@gitster.g","threadId":"61331","inReplyTo":"ZiHyGFRPm_pwdGgC@tanuki","subject":"Re: [PATCH v2 1/2] for-each-repo: optionally keep going on an error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-19T16:03:20Z","receivedAt":"2024-04-19T16:03:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Thu, Apr 18, 2024 at 12:53:02PM +0000, Johannes Schindelin via GitGitGadget wrote:\n>> From: Johannes Schindelin \n> [snip]\n>> @@ -55,8 +58,9 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n>> \telse if (err)\n>> \t\treturn 0;\n>> \n>> -\tfor (i = 0; !result && i < values->nr; i++)\n>> -\t\tresult = run_command_on_repo(values->items[i].string, argc, argv);\n>> +\tfor (i = 0; (keep_going || !result) && i < values->nr; i++)\n>> +\t\tif (run_command_on_repo(values->items[i].string, argc, argv))\n>> +\t\t\tresult = 1;\n>\n> One thing that made me stop and think is whether the change in behaviour\n> here may negatively impact some usecases. Before this change we would\n> error out with the return code returned by the command that we have ran\n> in repositories. It makes total sense that we don't do that anymore with\n> `--keep-going`, because the result would likely be useless as all we\n> could do was to OR the result codes with each other.\n>\n> But do we maybe want to make this conditional on whether or not the\n> `--keep-going` flag is set? So something like this:\n>\n> ```\n> for (i = 0; (keep_going || !result) && i < values->nr; i++) {\n> \tint ret = run_command_on_repo(values->items[i].string, argc, argv);\n> \tif (ret)\n> \t\tresult = keep_going ? 1 : ret;\n> }\n> ```\n\nYou mean that it could be a regression that we lose the raw return\nvalue from run_command_on_repo() when !keep_going?\n\n - git.c:handle_builtin() does exit(run_builtin(builtin, argc, argv));\n In this case, builtin is set to cmd_for_each_repo.\n\n - cmd_for_each_repo does \"return result\" at its end.\n\n - result comes from run_command_on_repo(), which returns the value\n returned by run_command().\n\n - run_command() returns -1 for \"not found\".\n\nSo if run_command() failed due to missing command, we would have\nexited with 255 (= (unsigned)(-1) & 0xFF), but with this change we\nwould now exit with 1.\n\nPassing anything outside 0..255 to exit(3) is a bad manners, and but\nthis does change behaviour. Hmmm.\n\n\n\n\n"},{"id":"493228","messageId":"20240419175621.GB14309@coredump.intra.peff.net","threadId":"61331","inReplyTo":"xmqqy199l4qf.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] for-each-repo: optionally keep going on an error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-04-19T17:56:21Z","receivedAt":"2024-04-19T17:56:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 19, 2024 at 09:03:20AM -0700, Junio C Hamano wrote:\n\n> You mean that it could be a regression that we lose the raw return\n> value from run_command_on_repo() when !keep_going?\n> \n> - git.c:handle_builtin() does exit(run_builtin(builtin, argc, argv));\n> In this case, builtin is set to cmd_for_each_repo.\n> \n> - cmd_for_each_repo does \"return result\" at its end.\n> \n> - result comes from run_command_on_repo(), which returns the value\n> returned by run_command().\n> \n> - run_command() returns -1 for \"not found\".\n> \n> So if run_command() failed due to missing command, we would have\n> exited with 255 (= (unsigned)(-1) & 0xFF), but with this change we\n> would now exit with 1.\n> \n> Passing anything outside 0..255 to exit(3) is a bad manners, and but\n> this does change behaviour. Hmmm.\n\nrun_command() may also return the exit code of the program run. So\nimagine a setup like:\n\n git init\n git config alias.foo '!exit 123'\n git config repo.paths \"$PWD\"\n git for-each-repo --config=repo.paths foo\n echo $?\n\nBefore the patch we see \"123\" and after we see \"1\".\n\nI do agree that passing -1 to exit is bad; we maybe should normalize to\n127 for not found, though I think we could also see -1 for system errors\nlike fork() failing.\n\n-Peff\n"},{"id":"493323","messageId":"xmqqr0ext6us.fsf@gitster.g","threadId":"61331","inReplyTo":"20240419175621.GB14309@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/2] for-each-repo: optionally keep going on an error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-22T21:39:23Z","receivedAt":"2024-04-22T21:39:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> run_command() may also return the exit code of the program run. So\n> imagine a setup like:\n>\n> git init\n> git config alias.foo '!exit 123'\n> git config repo.paths \"$PWD\"\n> git for-each-repo --config=repo.paths foo\n> echo $?\n>\n> Before the patch we see \"123\" and after we see \"1\".\n\nTrue, or when the process receives a signal, etc. With this change,\nwe do lose information.\n\n> I do agree that passing -1 to exit is bad; we maybe should normalize to\n> 127 for not found, though I think we could also see -1 for system errors\n> like fork() failing.\n\nTrue, but I think that is a separate issue.\n\nSo, let's have a (hopefully final reroll to fix the error code when\nstopping at the first error and merge it to 'next'?\n\nThanks.\n"},{"id":"493412","messageId":"pull.1719.v3.git.1713975299.gitgitgadget@gmail.com","threadId":"61331","inReplyTo":"pull.1719.v2.git.1713444783.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] Use a \"best effort\" strategy in scheduled maintenance","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-24T16:14:57Z","receivedAt":"2024-04-24T16:15:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Over in https://github.com/microsoft/git/issues/623, it was pointed out that\nscheduled maintenance will error out when it encounters a missing\nrepository. The scheduled maintenance should exit with an error, all right,\nbut what about the remaining repositories for which maintenance was\nscheduled, and that may not be missing?\n\nThis patch series addresses this by introducing a new for-each-repo option\nand then using it in the command that is run via scheduled maintenance.\n\nChanges since v2 (thanks Patrick, Jeff and Junio):\n\n * When not passing the new --keep-going option, the exit code is the same\n as before.\n * Clarified in the documentation of the --keep-going option that it is 0\n for success, 1 for failure, no matter the exact exit code of the failing\n command invocation(s).\n\nChanges since v1 (thanks Eric!):\n\n * Changed the option's documentation to reflect the current state (instead\n of the original design)\n * Fixed grammar issues\n\nJohannes Schindelin (2):\n for-each-repo: optionally keep going on an error\n maintenance: running maintenance should not stop on errors\n\n Documentation/git-for-each-repo.txt | 9 +++++++++\n builtin/for-each-repo.c | 13 +++++++++++--\n builtin/gc.c | 7 ++++---\n t/t0068-for-each-repo.sh | 16 ++++++++++++++++\n t/t7900-maintenance.sh | 6 +++---\n 5 files changed, 43 insertions(+), 8 deletions(-)\n\n\nbase-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1719%2Fdscho%2Ffor-each-repo-stop-on-error-2.44-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1719/dscho/for-each-repo-stop-on-error-2.44-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1719\n\nRange-diff vs v2:\n\n 1: abd796894c8 ! 1: 39ee6386aab for-each-repo: optionally keep going on an error\n @@ Commit message\n repository, still setting the exit code to indicate an error occurred.\n \n Helped-by: Eric Sunshine \n + Helped-by: Patrick Steinhardt \n Signed-off-by: Johannes Schindelin \n \n ## Documentation/git-for-each-repo.txt ##\n @@ Documentation/git-for-each-repo.txt: These config values are loaded from system,\n +\tContinue with the remaining repositories if the command failed\n +\ton a repository. The exit code will still indicate that the\n +\toverall operation was not successful.\n +++\n ++Note that the exact exit code of the failing command is not passed\n ++through as the exit code of the `for-each-repo` command: If the command\n ++failed in any of the specified repositories, the overall exit code will\n ++be 1.\n \n SUBPROCESS BEHAVIOR\n -------------------\n @@ builtin/for-each-repo.c: int cmd_for_each_repo(int argc, const char **argv, cons\n \n -\tfor (i = 0; !result && i < values->nr; i++)\n -\t\tresult = run_command_on_repo(values->items[i].string, argc, argv);\n -+\tfor (i = 0; (keep_going || !result) && i < values->nr; i++)\n -+\t\tif (run_command_on_repo(values->items[i].string, argc, argv))\n ++\tfor (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\tresult = 1;\n ++\t\t}\n ++\t}\n \n \treturn result;\n }\n 2: 1ae11553052 = 2: 540962859c5 maintenance: running maintenance should not stop on errors\n\n-- \ngitgitgadget\n"},{"id":"493414","messageId":"39ee6386aab25f28d197a27010b2f80ccd45aab2.1713975300.git.gitgitgadget@gmail.com","threadId":"61331","inReplyTo":"pull.1719.v3.git.1713975299.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] for-each-repo: optionally keep going on an error","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-24T16:14:58Z","receivedAt":"2024-04-24T16:15:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn https://github.com/microsoft/git/issues/623, it was reported that\nthe regularly scheduled maintenance stops if one repo in the middle of\nthe list was found to be missing.\n\nThis is undesirable, and points out a gap in the design of `git\nfor-each-repo`: We need a mode where that command does not stop on an\nerror, but continues to try running the specified command with the other\nrepositories.\n\nImitating the `--keep-going` option of GNU make, this commit teaches\n`for-each-repo` the same trick: to continue with the operation on all\nthe remaining repositories in case there was a problem with one\nrepository, still setting the exit code to indicate an error occurred.\n\nHelped-by: Eric Sunshine \nHelped-by: Patrick Steinhardt \nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-for-each-repo.txt | 9 +++++++++\n builtin/for-each-repo.c | 13 +++++++++++--\n t/t0068-for-each-repo.sh | 16 ++++++++++++++++\n 3 files changed, 36 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-for-each-repo.txt b/Documentation/git-for-each-repo.txt\nindex 94bd19da263..abe3527aaca 100644\n--- a/Documentation/git-for-each-repo.txt\n+++ b/Documentation/git-for-each-repo.txt\n@@ -42,6 +42,15 @@ These config values are loaded from system, global, and local Git config,\n as available. If `git for-each-repo` is run in a directory that is not a\n Git repository, then only the system and global config is used.\n \n+--keep-going::\n+\tContinue with the remaining repositories if the command failed\n+\ton a repository. The exit code will still indicate that the\n+\toverall operation was not successful.\n++\n+Note that the exact exit code of the failing command is not passed\n+through as the exit code of the `for-each-repo` command: If the command\n+failed in any of the specified repositories, the overall exit code will\n+be 1.\n \n SUBPROCESS BEHAVIOR\n -------------------\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 28186b30f54..c4fa41fda9f 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -32,6 +32,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)\n int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n {\n \tstatic const char *config_key = NULL;\n+\tint keep_going = 0;\n \tint i, result = 0;\n \tconst struct string_list *values;\n \tint err;\n@@ -39,6 +40,8 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\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_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 \t};\n \n@@ -55,8 +58,14 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n \telse if (err)\n \t\treturn 0;\n \n-\tfor (i = 0; !result && i < values->nr; i++)\n-\t\tresult = run_command_on_repo(values->items[i].string, argc, argv);\n+\tfor (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\tresult = 1;\n+\t\t}\n+\t}\n \n \treturn result;\n }\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex 4b90b74d5d5..95019e01ed3 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -59,4 +59,20 @@ test_expect_success 'error on NULL value for config keys' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--keep-going' '\n+\tgit config keep.going non-existing &&\n+\tgit config --add keep.going . &&\n+\n+\ttest_must_fail git for-each-repo --config=keep.going \\\n+\t\t-- branch >out 2>err &&\n+\ttest_grep \"cannot change to .*non-existing\" err &&\n+\ttest_must_be_empty out &&\n+\n+\ttest_must_fail git for-each-repo --config=keep.going --keep-going \\\n+\t\t-- branch >out 2>err &&\n+\ttest_grep \"cannot change to .*non-existing\" err &&\n+\tgit branch >expect &&\n+\ttest_cmp expect out\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"493413","messageId":"540962859c599cb8896ab0860592412568e9b44b.1713975300.git.gitgitgadget@gmail.com","threadId":"61331","inReplyTo":"pull.1719.v3.git.1713975299.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] maintenance: running maintenance should not stop on errors","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-24T16:14:59Z","receivedAt":"2024-04-24T16:15:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn https://github.com/microsoft/git/issues/623, it was reported that\nmaintenance stops on a missing repository, omitting the remaining\nrepositories that were scheduled for maintenance.\n\nThis is undesirable, as it should be a best effort type of operation.\n\nIt should still fail due to the missing repository, of course, but not\nleave the non-missing repositories in unmaintained shapes.\n\nLet's use `for-each-repo`'s shiny new `--keep-going` option that we just\nintroduced for that very purpose.\n\nThis change will be picked up when running `git maintenance start`,\nwhich is run implicitly by `scalar reconfigure`.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/gc.c | 7 ++++---\n t/t7900-maintenance.sh | 6 +++---\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex cb80ced6cb5..b069aa49c50 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1858,6 +1858,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit\n \t\t \"%s/git\\n\"\n \t\t \"--exec-path=%s\\n\"\n \t\t \"for-each-repo\\n\"\n+\t\t \"--keep-going\\n\"\n \t\t \"--config=maintenance.repo\\n\"\n \t\t \"maintenance\\n\"\n \t\t \"run\\n\"\n@@ -2100,7 +2101,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority\n \t \"\\n\"\n \t \"\\n\"\n \t \"\\\"%s\\\\headless-git.exe\\\"\\n\"\n-\t \"--exec-path=\\\"%s\\\" for-each-repo --config=maintenance.repo maintenance run --schedule=%s\\n\"\n+\t \"--exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s\\n\"\n \t \"\\n\"\n \t \"\\n\"\n \t \"\\n\";\n@@ -2245,7 +2246,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)\n \t\t\t\"# replaced in the future by a Git command.\\n\\n\");\n \n \t\tstrbuf_addf(&line_format,\n-\t\t\t \"%%d %%s * * %%s \\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\\n\",\n+\t\t\t \"%%d %%s * * %%s \\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\\n\",\n \t\t\t exec_path, exec_path);\n \t\tfprintf(cron_in, line_format.buf, minute, \"1-23\", \"*\", \"hourly\");\n \t\tfprintf(cron_in, line_format.buf, minute, \"0\", \"1-6\", \"daily\");\n@@ -2446,7 +2447,7 @@ static int systemd_timer_write_service_template(const char *exec_path)\n \t \"\\n\"\n \t \"[Service]\\n\"\n \t \"Type=oneshot\\n\"\n-\t \"ExecStart=\\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%i\\n\"\n+\t \"ExecStart=\\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\\n\"\n \t \"LockPersonality=yes\\n\"\n \t \"MemoryDenyWriteExecute=yes\\n\"\n \t \"NoNewPrivileges=yes\\n\"\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 0943dfa18a3..8595489cebe 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -639,9 +639,9 @@ test_expect_success 'start from empty cron table' '\n \t# start registers the repo\n \tgit config --get --global --fixed-value maintenance.repo \"$(pwd)\" &&\n \n-\tgrep \"for-each-repo --config=maintenance.repo maintenance run --schedule=daily\" cron.txt &&\n-\tgrep \"for-each-repo --config=maintenance.repo maintenance run --schedule=hourly\" cron.txt &&\n-\tgrep \"for-each-repo --config=maintenance.repo maintenance run --schedule=weekly\" cron.txt\n+\tgrep \"for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=daily\" cron.txt &&\n+\tgrep \"for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=hourly\" cron.txt &&\n+\tgrep \"for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=weekly\" cron.txt\n '\n \n test_expect_success 'stop from existing schedule' '\n-- \ngitgitgadget\n"},{"id":"493420","messageId":"xmqqfrvazobm.fsf@gitster.g","threadId":"61331","inReplyTo":"39ee6386aab25f28d197a27010b2f80ccd45aab2.1713975300.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/2] for-each-repo: optionally keep going on an error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-24T17:02:21Z","receivedAt":"2024-04-24T17:02:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> @@ -55,8 +58,14 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)\n> \telse if (err)\n> \t\treturn 0;\n> \n> -\tfor (i = 0; !result && i < values->nr; i++)\n> -\t\tresult = run_command_on_repo(values->items[i].string, argc, argv);\n> +\tfor (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\tresult = 1;\n> +\t\t}\n> +\t}\n> \n> \treturn result;\n> }\n\nHmph, as I wish that more experienced folks to give a good structure\nto the codebase from get-go so that future developers who may be\nless experienced would avoid mistakes, with my maintainer's hat on,\nI would have expected something more like:\n\n\tfor (i = 0; i < values->nr; i++) {\n\t\tint ret = run_command_on_repo(...);\n\t\tif (!ret)\n\t\t\tcontinue;\n\t\tif (keep_going) {\n \tresult = 1;\t\t\t\n\t\t} else {\n \tresult = ret;\n break;\n\t\t}\n\t}\n\nThat way, clean-up actions, when they need to be added, can go\nbefore the single \"return result\" without structural changes,\nfuture-proofing the shape of the control flow.\n\nThe loop is simple enough that it is acceptable to leave as the\nresponsibility of future developers who wants to do something that\nrequire resource allocation and release before returning, of course\n;-).\n\n\n\n"},{"id":"493460","messageId":"Zin59zAZQyLg0iXR@tanuki","threadId":"61331","inReplyTo":"pull.1719.v3.git.1713975299.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/2] Use a \"best effort\" strategy in scheduled maintenance","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-04-25T06:36:39Z","receivedAt":"2024-04-25T06:36:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Apr 24, 2024 at 04:14:57PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> Over in https://github.com/microsoft/git/issues/623, it was pointed out that\n> scheduled maintenance will error out when it encounters a missing\n> repository. The scheduled maintenance should exit with an error, all right,\n> but what about the remaining repositories for which maintenance was\n> scheduled, and that may not be missing?\n> \n> This patch series addresses this by introducing a new for-each-repo option\n> and then using it in the command that is run via scheduled maintenance.\n> \n> Changes since v2 (thanks Patrick, Jeff and Junio):\n> \n> * When not passing the new --keep-going option, the exit code is the same\n> as before.\n> * Clarified in the documentation of the --keep-going option that it is 0\n> for success, 1 for failure, no matter the exact exit code of the failing\n> command invocation(s).\n> \n> Changes since v1 (thanks Eric!):\n> \n> * Changed the option's documentation to reflect the current state (instead\n> of the original design)\n> * Fixed grammar issues\n\nThanks, this version looks good to me!\n\nPatrick\n"}]}