On Fri, Apr 19, 2024 at 09:03:20AM -0700, Junio C Hamano wrote: > 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
