On 03/03/2022 02:02, Derrick Stolee wrote:
On 3/2/2022 12:27 PM, Ævar Arnfjörð Bjarmason wrote:As an aside we still have various potential issues with hidden segfaults etc. in the test suite after this that are tricked to solve, because: * Our tests will (mostly) catch segfaults and abort(), but if we invoke a command that invokes another command it needs to ferry the exit code up to us. * run-command.c notably does not do that, so for e.g. "git push"
I'm not sure what you mean by this, the return value of run_command() already indicates which signal if any killed the child see for example run_specified_editor() which re-raises SIGINT and SIGQUIT if the editor is killed by those signals.
tests where we expect a failure and an underlying "git" command fails we won't ferry up the segfault or abort exit code.
>
Perhaps run-command.c could auto-exit for certain well-known error codes that could only happen on certain kinds of failures (segfault, for example). A simple die() might be something that is expected to be handled by the top-level command in some cases.
I think we need to be careful that run_command() does not re-raise a signal before the caller has a chance to do any cleanup. A caller to run_command() can already check the return value and choose to die based on that after doing any cleanup. If run_command() starts dying we'll end up adding more unsafe signal handlers to do the cleanup.
Best Wishes Phillip
* We have gitweb.perl and some other perl code ignoring return values from close(), i.e. ignoring exit codes from "git rev-parse" et al. * We have in-tree shellscripts like "git-merge-one-file.sh" invoking git commands, and if they fail returning "1", not ferrying up the segfault or abort() exit code.These are more involved and harder to evaluate. Add them to the pile of projects for new contributors? Thanks, -Stolee
