Use a "best effort" strategy in scheduled maintenance

Over in https://github.com/microsoft/git/issues/623, it was pointed out that scheduled maintenance will error out when it encounters a missing repository. The scheduled maintenance should exit with an error, all right, but what about the remaining repositories for which maintenance was scheduled, and that may not be missing?

This patch series addresses this by introducing a new for-each-repo option and then using it in the command that is run via scheduled maintenance.

Johannes Schindelin (2):
for-each-repo: optionally keep going on an error
maintenance: running maintenance should not stop on errors

 Documentation/git-for-each-repo.txt |  4 ++++
 builtin/for-each-repo.c             |  8 ++++++--
 builtin/gc.c                        |  7 ++++---
 t/t0068-for-each-repo.sh            | 16 ++++++++++++++++
 t/t7900-maintenance.sh              |  6 +++---
 5 files changed, 33 insertions(+), 8 deletions(-)

base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1719%2Fdscho%2Ffor-each-repo-stop-on-error-2.44-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1719/dscho/for-each-repo-stop-on-error-2.44-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1719

gitgitgadget

From: Johannes Schindelin <[email protected]>

In https://github.com/microsoft/git/issues/623, it was reported that the regularly scheduled maintenance stops if one repo in the middle of the list was found to be missing.

This is undesirable, and points out a gap in the design of git for-each-repo: We need a mode where that command does not stop on an error, but continues to try the running the specified command with the other repositories.

Imitating the --keep-going option of GNU make, this commit teaches for-each-repo the same trick: to continue with the operation on all the remaining repositories in case there was a problem with one repository, still setting the exit code to indicate an error occurred.

---
 Documentation/git-for-each-repo.txt |  4 ++++
 builtin/for-each-repo.c             |  8 ++++++--
 t/t0068-for-each-repo.sh            | 16 ++++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

index 94bd19da263..8c18001d825 100644
@@ -42,6 +42,10 @@ These config values are loaded from system, global, and local Git config,
 as available. If `git for-each-repo` is run in a directory that is not a
 Git repository, then only the system and global config is used.
 
+--keep-going::
+	Continue with the remaining repositories if the command failed
+	on a repository. The exit code will still indicate that the
+	overall operation was not successful.
 
 SUBPROCESS BEHAVIOR
 -------------------

index 28186b30f54..74498539e9c 100644
@@ -32,6 +32,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)
 int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 {
 	static const char *config_key = NULL;
+	int keep_going = 0;
 	int i, result = 0;
 	const struct string_list *values;
 	int err;

@@ -39,6 +40,8 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	const struct option options[] = {
 		OPT_STRING(0, "config", &config_key, N_("config"),
 			   N_("config key storing a list of repository paths")),
+		OPT_BOOL(0, "keep-going", &keep_going,
+			 N_("stop at the first repository where the operation failed")),
 		OPT_END()
 	};
 

@@ -55,8 +58,9 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	else if (err)
 		return 0;
 
-	for (i = 0; !result && i < values->nr; i++)
-		result = run_command_on_repo(values->items[i].string, argc, argv);
+	for (i = 0; (keep_going || !result) && i < values->nr; i++)
+		if (run_command_on_repo(values->items[i].string, argc, argv))
+			result = 1;
 
 	return result;
 }

index 4b90b74d5d5..95019e01ed3 100755
@@ -59,4 +59,20 @@ test_expect_success 'error on NULL value for config keys' '
 	test_cmp expect actual
 '
 
+test_expect_success '--keep-going' '
+	git config keep.going non-existing &&
+	git config --add keep.going . &&
+
+	test_must_fail git for-each-repo --config=keep.going \
+		-- branch >out 2>err &&
+	test_grep "cannot change to .*non-existing" err &&
+	test_must_be_empty out &&
+
+	test_must_fail git for-each-repo --config=keep.going --keep-going \
+		-- branch >out 2>err &&
+	test_grep "cannot change to .*non-existing" err &&
+	git branch >expect &&
+	test_cmp expect out
+'
+
 test_done

gitgitgadget

From: Johannes Schindelin <[email protected]>

In https://github.com/microsoft/git/issues/623, it was reported that maintenance stops on a missing repository, omitting the remaining repositories that were scheduled for maintenance.

This is undesirable, as it should be a best effort type of operation.

It should still fail due to the missing repository, of course, but not leave the non-missing repositories in unmaintained shapes.

Let's use for-each-repo's shiny new --keep-going option that we just introduced for that very purpose.

This change will be picked up when running git maintenance start, which is run implicitly by scalar reconfigure.

---
 builtin/gc.c           | 7 ++++---
 t/t7900-maintenance.sh | 6 +++---
 2 files changed, 7 insertions(+), 6 deletions(-)

index cb80ced6cb5..b069aa49c50 100644
@@ -1858,6 +1858,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
 		   "<string>%s/git</string>\n"
 		   "<string>--exec-path=%s</string>\n"
 		   "<string>for-each-repo</string>\n"
+		   "<string>--keep-going</string>\n"
 		   "<string>--config=maintenance.repo</string>\n"
 		   "<string>maintenance</string>\n"
 		   "<string>run</string>\n"

@@ -2100,7 +2101,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
 	      "<Actions Context=\"Author\">\n"
 	      "<Exec>\n"
 	      "<Command>\"%s\\headless-git.exe\"</Command>\n"
-	      "<Arguments>--exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%s</Arguments>\n"
+	      "<Arguments>--exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s</Arguments>\n"
 	      "</Exec>\n"
 	      "</Actions>\n"
 	      "</Task>\n";

@@ -2245,7 +2246,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)
 			"# replaced in the future by a Git command.\n\n");
 
 		strbuf_addf(&line_format,
-			    "%%d %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\n",
+			    "%%d %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\n",
 			    exec_path, exec_path);
 		fprintf(cron_in, line_format.buf, minute, "1-23", "*", "hourly");
 		fprintf(cron_in, line_format.buf, minute, "0", "1-6", "daily");

@@ -2446,7 +2447,7 @@ static int systemd_timer_write_service_template(const char *exec_path)
 	       "\n"
 	       "[Service]\n"
 	       "Type=oneshot\n"
-	       "ExecStart=\"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%i\n"
+	       "ExecStart=\"%s/git\" --exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\n"
 	       "LockPersonality=yes\n"
 	       "MemoryDenyWriteExecute=yes\n"
 	       "NoNewPrivileges=yes\n"

index 0943dfa18a3..8595489cebe 100755
@@ -639,9 +639,9 @@ test_expect_success 'start from empty cron table' '
 	# start registers the repo
 	git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
 
-	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=daily" cron.txt &&
-	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=hourly" cron.txt &&
-	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
+	grep "for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=daily" cron.txt &&
+	grep "for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=hourly" cron.txt &&
+	grep "for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
 '
 
 test_expect_success 'stop from existing schedule' '

gitgitgadget

s/try the running/try running/

"Johannes Schindelin via GitGitGadget" <[email protected]>

Interesting.

As the order of the maintenance run across repositories is not controlled by the end-user, I think it is reasonable to keep going and finish the others, at least by default (if you could control the ordering, you could make an arrangement where having ran maintenance on repository A successfully would be necessary to do maintenance on repository B, but we do not give that power to the end users).

OK.

Good eyes. Thanks for carefully reading.

"Johannes Schindelin via GitGitGadget" <[email protected]>

Other than the N_("") thing Eric noticed, I didn't find anything glaringly wrong in these two patches. Nicely done.

We may want to fold overly long lines we see in the patch, but I'd prefer to see it done as a post-cleanup task (#leftoverbits), as the lines in the preimage of the patch are already overly long.

Thanks.

Over in https://github.com/microsoft/git/issues/623, it was pointed out that scheduled maintenance will error out when it encounters a missing repository. The scheduled maintenance should exit with an error, all right, but what about the remaining repositories for which maintenance was scheduled, and that may not be missing?

This patch series addresses this by introducing a new for-each-repo option and then using it in the command that is run via scheduled maintenance.

Changes since v1 (thanks Eric!):

* Changed the option's documentation to reflect the current state (instead of the original design)
* Fixed grammar issues

Johannes Schindelin (2):
for-each-repo: optionally keep going on an error
maintenance: running maintenance should not stop on errors

 Documentation/git-for-each-repo.txt |  4 ++++
 builtin/for-each-repo.c             |  8 ++++++--
 builtin/gc.c                        |  7 ++++---
 t/t0068-for-each-repo.sh            | 16 ++++++++++++++++
 t/t7900-maintenance.sh              |  6 +++---
 5 files changed, 33 insertions(+), 8 deletions(-)

base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1719%2Fdscho%2Ffor-each-repo-stop-on-error-2.44-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1719/dscho/for-each-repo-stop-on-error-2.44-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1719

Range-diff vs v1:

1: 6721e3ada5d ! 1: abd796894c8 for-each-repo: optionally keep going on an error
@@ Commit message

This is undesirable, and points out a gap in the design of git for-each-repo: We need a mode where that command does not stop on an
- error, but continues to try the running the specified command with the
- other repositories.
+ error, but continues to try running the specified command with the other
+ repositories.

Imitating the --keep-going option of GNU make, this commit teaches for-each-repo the same trick: to continue with the operation on all the remaining repositories in case there was a problem with one repository, still setting the exit code to indicate an error occurred.

+ Helped-by: Eric Sunshine <[email protected]>

## Documentation/git-for-each-repo.txt ##
@@ builtin/for-each-repo.c: int cmd_for_each_repo(int argc, const char **argv, cons
OPT_STRING(0, "config", &config_key, N_("config"),
N_("config key storing a list of repository paths")),
+ OPT_BOOL(0, "keep-going", &keep_going,
-+ N_("stop at the first repository where the operation failed")),
++ N_("keep going even if command fails in a repository")),
OPT_END()
};

2: a86bcf2e1a0 = 2: 1ae11553052 maintenance: running maintenance should not stop on errors

gitgitgadget

From: Johannes Schindelin <[email protected]>

In https://github.com/microsoft/git/issues/623, it was reported that the regularly scheduled maintenance stops if one repo in the middle of the list was found to be missing.

This is undesirable, and points out a gap in the design of git for-each-repo: We need a mode where that command does not stop on an error, but continues to try running the specified command with the other repositories.

Imitating the --keep-going option of GNU make, this commit teaches for-each-repo the same trick: to continue with the operation on all the remaining repositories in case there was a problem with one repository, still setting the exit code to indicate an error occurred.

---
 Documentation/git-for-each-repo.txt |  4 ++++
 builtin/for-each-repo.c             |  8 ++++++--
 t/t0068-for-each-repo.sh            | 16 ++++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

index 94bd19da263..8c18001d825 100644
@@ -42,6 +42,10 @@ These config values are loaded from system, global, and local Git config,
 as available. If `git for-each-repo` is run in a directory that is not a
 Git repository, then only the system and global config is used.
 
+--keep-going::
+	Continue with the remaining repositories if the command failed
+	on a repository. The exit code will still indicate that the
+	overall operation was not successful.
 
 SUBPROCESS BEHAVIOR
 -------------------

index 28186b30f54..9bdf2b34f89 100644
@@ -32,6 +32,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)
 int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 {
 	static const char *config_key = NULL;
+	int keep_going = 0;
 	int i, result = 0;
 	const struct string_list *values;
 	int err;

@@ -39,6 +40,8 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	const struct option options[] = {
 		OPT_STRING(0, "config", &config_key, N_("config"),
 			   N_("config key storing a list of repository paths")),
+		OPT_BOOL(0, "keep-going", &keep_going,
+			 N_("keep going even if command fails in a repository")),
 		OPT_END()
 	};
 

@@ -55,8 +58,9 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	else if (err)
 		return 0;
 
-	for (i = 0; !result && i < values->nr; i++)
-		result = run_command_on_repo(values->items[i].string, argc, argv);
+	for (i = 0; (keep_going || !result) && i < values->nr; i++)
+		if (run_command_on_repo(values->items[i].string, argc, argv))
+			result = 1;
 
 	return result;
 }

index 4b90b74d5d5..95019e01ed3 100755
@@ -59,4 +59,20 @@ test_expect_success 'error on NULL value for config keys' '
 	test_cmp expect actual
 '
 
+test_expect_success '--keep-going' '
+	git config keep.going non-existing &&
+	git config --add keep.going . &&
+
+	test_must_fail git for-each-repo --config=keep.going \
+		-- branch >out 2>err &&
+	test_grep "cannot change to .*non-existing" err &&
+	test_must_be_empty out &&
+
+	test_must_fail git for-each-repo --config=keep.going --keep-going \
+		-- branch >out 2>err &&
+	test_grep "cannot change to .*non-existing" err &&
+	git branch >expect &&
+	test_cmp expect out
+'
+
 test_done

gitgitgadget

From: Johannes Schindelin <[email protected]>

In https://github.com/microsoft/git/issues/623, it was reported that maintenance stops on a missing repository, omitting the remaining repositories that were scheduled for maintenance.

This is undesirable, as it should be a best effort type of operation.

It should still fail due to the missing repository, of course, but not leave the non-missing repositories in unmaintained shapes.

Let's use for-each-repo's shiny new --keep-going option that we just introduced for that very purpose.

This change will be picked up when running git maintenance start, which is run implicitly by scalar reconfigure.

---
 builtin/gc.c           | 7 ++++---
 t/t7900-maintenance.sh | 6 +++---
 2 files changed, 7 insertions(+), 6 deletions(-)

index cb80ced6cb5..b069aa49c50 100644
@@ -1858,6 +1858,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
 		   "<string>%s/git</string>\n"
 		   "<string>--exec-path=%s</string>\n"
 		   "<string>for-each-repo</string>\n"
+		   "<string>--keep-going</string>\n"
 		   "<string>--config=maintenance.repo</string>\n"
 		   "<string>maintenance</string>\n"
 		   "<string>run</string>\n"

@@ -2100,7 +2101,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
 	      "<Actions Context=\"Author\">\n"
 	      "<Exec>\n"
 	      "<Command>\"%s\\headless-git.exe\"</Command>\n"
-	      "<Arguments>--exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%s</Arguments>\n"
+	      "<Arguments>--exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s</Arguments>\n"
 	      "</Exec>\n"
 	      "</Actions>\n"
 	      "</Task>\n";

@@ -2245,7 +2246,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)
 			"# replaced in the future by a Git command.\n\n");
 
 		strbuf_addf(&line_format,
-			    "%%d %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\n",
+			    "%%d %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\n",
 			    exec_path, exec_path);
 		fprintf(cron_in, line_format.buf, minute, "1-23", "*", "hourly");
 		fprintf(cron_in, line_format.buf, minute, "0", "1-6", "daily");

@@ -2446,7 +2447,7 @@ static int systemd_timer_write_service_template(const char *exec_path)
 	       "\n"
 	       "[Service]\n"
 	       "Type=oneshot\n"
-	       "ExecStart=\"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%i\n"
+	       "ExecStart=\"%s/git\" --exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\n"
 	       "LockPersonality=yes\n"
 	       "MemoryDenyWriteExecute=yes\n"
 	       "NoNewPrivileges=yes\n"

index 0943dfa18a3..8595489cebe 100755
@@ -639,9 +639,9 @@ test_expect_success 'start from empty cron table' '
 	# start registers the repo
 	git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
 
-	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=daily" cron.txt &&
-	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=hourly" cron.txt &&
-	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
+	grep "for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=daily" cron.txt &&
+	grep "for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=hourly" cron.txt &&
+	grep "for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
 '
 
 test_expect_success 'stop from existing schedule' '

gitgitgadget

[snip]

```
for (i = 0; (keep_going || !result) && i < values->nr; i++) {
	int ret = run_command_on_repo(values->items[i].string, argc, argv);
	if (ret)
		result = keep_going ? 1 : ret;
}
```

You mean that it could be a regression that we lose the raw return value from run_command_on_repo() when !keep_going?

- git.c:handle_builtin() does exit(run_builtin(builtin, argc, argv)); In this case, builtin is set to cmd_for_each_repo.

- cmd_for_each_repo does "return result" at its end.

- result comes from run_command_on_repo(), which returns the value returned by run_command().

- run_command() returns -1 for "not found".

So if run_command() failed due to missing command, we would have exited with 255 (= (unsigned)(-1) & 0xFF), but with this change we would now exit with 1.

Passing anything outside 0..255 to exit(3) is a bad manners, and but this does change behaviour. Hmmm.

run_command() may also return the exit code of the program run. So imagine a setup like:

  git init
  git config alias.foo '!exit 123'
  git config repo.paths "$PWD"
  git for-each-repo --config=repo.paths foo
  echo $?

Before the patch we see "123" and after we see "1".

I do agree that passing -1 to exit is bad; we maybe should normalize to 127 for not found, though I think we could also see -1 for system errors like fork() failing.

-Peff

True, or when the process receives a signal, etc. With this change, we do lose information.

True, but I think that is a separate issue.

So, let's have a (hopefully final reroll to fix the error code when stopping at the first error and merge it to 'next'?

Thanks.

Over in https://github.com/microsoft/git/issues/623, it was pointed out that scheduled maintenance will error out when it encounters a missing repository. The scheduled maintenance should exit with an error, all right, but what about the remaining repositories for which maintenance was scheduled, and that may not be missing?

This patch series addresses this by introducing a new for-each-repo option and then using it in the command that is run via scheduled maintenance.

Changes since v2 (thanks Patrick, Jeff and Junio):

* When not passing the new --keep-going option, the exit code is the same as before.
* Clarified in the documentation of the --keep-going option that it is 0 for success, 1 for failure, no matter the exact exit code of the failing command invocation(s).

Changes since v1 (thanks Eric!):

* Changed the option's documentation to reflect the current state (instead of the original design)
* Fixed grammar issues

Johannes Schindelin (2):
for-each-repo: optionally keep going on an error
maintenance: running maintenance should not stop on errors

 Documentation/git-for-each-repo.txt |  9 +++++++++
 builtin/for-each-repo.c             | 13 +++++++++++--
 builtin/gc.c                        |  7 ++++---
 t/t0068-for-each-repo.sh            | 16 ++++++++++++++++
 t/t7900-maintenance.sh              |  6 +++---
 5 files changed, 43 insertions(+), 8 deletions(-)

base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1719%2Fdscho%2Ffor-each-repo-stop-on-error-2.44-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1719/dscho/for-each-repo-stop-on-error-2.44-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1719

Range-diff vs v2:

1: abd796894c8 ! 1: 39ee6386aab for-each-repo: optionally keep going on an error

     @@ Commit message
          repository, still setting the exit code to indicate an error occurred.
      
          Helped-by: Eric Sunshine <[email protected]>
     +    Helped-by: Patrick Steinhardt <[email protected]>
          Signed-off-by: Johannes Schindelin <[email protected]>
      
       ## Documentation/git-for-each-repo.txt ##
     @@ Documentation/git-for-each-repo.txt: These config values are loaded from system,
      +	Continue with the remaining repositories if the command failed
      +	on a repository. The exit code will still indicate that the
      +	overall operation was not successful.
     +++
     ++Note that the exact exit code of the failing command is not passed
     ++through as the exit code of the `for-each-repo` command: If the command
     ++failed in any of the specified repositories, the overall exit code will
     ++be 1.
       
       SUBPROCESS BEHAVIOR
       -------------------
     @@ builtin/for-each-repo.c: int cmd_for_each_repo(int argc, const char **argv, cons
       
      -	for (i = 0; !result && i < values->nr; i++)
      -		result = run_command_on_repo(values->items[i].string, argc, argv);
     -+	for (i = 0; (keep_going || !result) && i < values->nr; i++)
     -+		if (run_command_on_repo(values->items[i].string, argc, argv))
     ++	for (i = 0; i < values->nr; i++) {
     ++		int ret = run_command_on_repo(values->items[i].string, argc, argv);
     ++		if (ret) {
     ++			if (!keep_going)
     ++					return ret;
      +			result = 1;
     ++		}
     ++	}
       
       	return result;
       }

2: 1ae11553052 = 2: 540962859c5 maintenance: running maintenance should not stop on errors

gitgitgadget

From: Johannes Schindelin <[email protected]>

In https://github.com/microsoft/git/issues/623, it was reported that the regularly scheduled maintenance stops if one repo in the middle of the list was found to be missing.

This is undesirable, and points out a gap in the design of git for-each-repo: We need a mode where that command does not stop on an error, but continues to try running the specified command with the other repositories.

Imitating the --keep-going option of GNU make, this commit teaches for-each-repo the same trick: to continue with the operation on all the remaining repositories in case there was a problem with one repository, still setting the exit code to indicate an error occurred.

---
 Documentation/git-for-each-repo.txt |  9 +++++++++
 builtin/for-each-repo.c             | 13 +++++++++++--
 t/t0068-for-each-repo.sh            | 16 ++++++++++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)

index 94bd19da263..abe3527aaca 100644
@@ -42,6 +42,15 @@ These config values are loaded from system, global, and local Git config,
 as available. If `git for-each-repo` is run in a directory that is not a
 Git repository, then only the system and global config is used.
 
+--keep-going::
+	Continue with the remaining repositories if the command failed
+	on a repository. The exit code will still indicate that the
+	overall operation was not successful.
++
+Note that the exact exit code of the failing command is not passed
+through as the exit code of the `for-each-repo` command: If the command
+failed in any of the specified repositories, the overall exit code will
+be 1.
 
 SUBPROCESS BEHAVIOR
 -------------------

index 28186b30f54..c4fa41fda9f 100644
@@ -32,6 +32,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)
 int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 {
 	static const char *config_key = NULL;
+	int keep_going = 0;
 	int i, result = 0;
 	const struct string_list *values;
 	int err;

@@ -39,6 +40,8 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	const struct option options[] = {
 		OPT_STRING(0, "config", &config_key, N_("config"),
 			   N_("config key storing a list of repository paths")),
+		OPT_BOOL(0, "keep-going", &keep_going,
+			 N_("keep going even if command fails in a repository")),
 		OPT_END()
 	};
 

@@ -55,8 +58,14 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	else if (err)
 		return 0;
 
-	for (i = 0; !result && i < values->nr; i++)
-		result = run_command_on_repo(values->items[i].string, argc, argv);
+	for (i = 0; i < values->nr; i++) {
+		int ret = run_command_on_repo(values->items[i].string, argc, argv);
+		if (ret) {
+			if (!keep_going)
+					return ret;
+			result = 1;
+		}
+	}
 
 	return result;
 }

index 4b90b74d5d5..95019e01ed3 100755
@@ -59,4 +59,20 @@ test_expect_success 'error on NULL value for config keys' '
 	test_cmp expect actual
 '
 
+test_expect_success '--keep-going' '
+	git config keep.going non-existing &&
+	git config --add keep.going . &&
+
+	test_must_fail git for-each-repo --config=keep.going \
+		-- branch >out 2>err &&
+	test_grep "cannot change to .*non-existing" err &&
+	test_must_be_empty out &&
+
+	test_must_fail git for-each-repo --config=keep.going --keep-going \
+		-- branch >out 2>err &&
+	test_grep "cannot change to .*non-existing" err &&
+	git branch >expect &&
+	test_cmp expect out
+'
+
 test_done

gitgitgadget

From: Johannes Schindelin <[email protected]>

In https://github.com/microsoft/git/issues/623, it was reported that maintenance stops on a missing repository, omitting the remaining repositories that were scheduled for maintenance.

This is undesirable, as it should be a best effort type of operation.

It should still fail due to the missing repository, of course, but not leave the non-missing repositories in unmaintained shapes.

Let's use for-each-repo's shiny new --keep-going option that we just introduced for that very purpose.

This change will be picked up when running git maintenance start, which is run implicitly by scalar reconfigure.

---
 builtin/gc.c           | 7 ++++---
 t/t7900-maintenance.sh | 6 +++---
 2 files changed, 7 insertions(+), 6 deletions(-)

index cb80ced6cb5..b069aa49c50 100644
@@ -1858,6 +1858,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
 		   "<string>%s/git</string>\n"
 		   "<string>--exec-path=%s</string>\n"
 		   "<string>for-each-repo</string>\n"
+		   "<string>--keep-going</string>\n"
 		   "<string>--config=maintenance.repo</string>\n"
 		   "<string>maintenance</string>\n"
 		   "<string>run</string>\n"

@@ -2100,7 +2101,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
 	      "<Actions Context=\"Author\">\n"
 	      "<Exec>\n"
 	      "<Command>\"%s\\headless-git.exe\"</Command>\n"
-	      "<Arguments>--exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%s</Arguments>\n"
+	      "<Arguments>--exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s</Arguments>\n"
 	      "</Exec>\n"
 	      "</Actions>\n"
 	      "</Task>\n";

@@ -2245,7 +2246,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)
 			"# replaced in the future by a Git command.\n\n");
 
 		strbuf_addf(&line_format,
-			    "%%d %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\n",
+			    "%%d %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\n",
 			    exec_path, exec_path);
 		fprintf(cron_in, line_format.buf, minute, "1-23", "*", "hourly");
 		fprintf(cron_in, line_format.buf, minute, "0", "1-6", "daily");

@@ -2446,7 +2447,7 @@ static int systemd_timer_write_service_template(const char *exec_path)
 	       "\n"
 	       "[Service]\n"
 	       "Type=oneshot\n"
-	       "ExecStart=\"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%i\n"
+	       "ExecStart=\"%s/git\" --exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\n"
 	       "LockPersonality=yes\n"
 	       "MemoryDenyWriteExecute=yes\n"
 	       "NoNewPrivileges=yes\n"

index 0943dfa18a3..8595489cebe 100755
@@ -639,9 +639,9 @@ test_expect_success 'start from empty cron table' '
 	# start registers the repo
 	git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
 
-	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=daily" cron.txt &&
-	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=hourly" cron.txt &&
-	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
+	grep "for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=daily" cron.txt &&
+	grep "for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=hourly" cron.txt &&
+	grep "for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
 '
 
 test_expect_success 'stop from existing schedule' '

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

	for (i = 0; i < values->nr; i++) {
		int ret = run_command_on_repo(...);
		if (!ret)
			continue;
		if (keep_going) {
                	result = 1;			
		} else {
                	result = ret;
                        break;
		}
	}

Thanks, this version looks good to me!

Patrick