From: Phillip Wood Date: Thu, 03 Mar 2022 14:06:14 GMT Subject: Re: [PATCH 00/15] tests: don't ignore "git" exit codes Message-ID: In-Reply-To: <76a1ff22-3eb0-08fb-5aa9-e612ee5b522f@github.com> 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