On Mon, 26 Aug 2019 at 23:56, Eli Schwartz <eschwartz@archlinux.org> wrote:
On 8/26/19 7:29 AM, Austin Lund wrote:
There are a number of exit paths where the logpipe fifo remains in the logging directory. Remove it if it exists when cleaning up at exit.
Sounds like https://patchwork.archlinux.org/patch/1171/ again.
error_function() should always run on the errexit trap for run_function_safe, and inside run_function_safe, run_function will run to execute the function, log the logpipe, and rm the logpipe if everything succeeded. If for any reason a call in run_function fails (even the `rm "$logpipe"`) the error_function trap should definitely run, and delete the logpipe fifo.
The only way it should be left behind is if e.g. this exits without running the final rm, but does so without errexit erroring -- how???:
mkfifo "$logpipe" tee "$BUILDLOG" < "$logpipe" & local teepid=$!
$pkgfunc &>"$logpipe"
wait $teepid rm "$logpipe"
Alternatively, if the rm itself errors -- that is what the other patch is about, but why would it error?
Alternatively, makepkg is killed in such a way that it isn't allowed to try running cleanup traps. If the process is SIGKILLed, then error_function will not run, but neither will clean_up.
I would really like to know in what situations this is supposed to fail. e.g. Is it only in devtools?
I can trigger it with just basic makepkg followed by Ctrl-C to trigger SIGINT or something like makepkg & sleep 5; kill -USR1 $! .
Signed-off-by: Austin Lund <austin.lund@gmail.com> --- scripts/makepkg.sh.in | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 43484db3..0e0d3461 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -129,6 +129,8 @@ clean_up() { return 0 fi
+ [[ -p $logpipe ]] && rm "$logpipe" +
Why switch from if [[ -p $logpipe ]]; then rm "$logpipe"; fi to [[ -p $logpipe ]] && rm "$logpipe"
Seems a bit silly to add new cases of `return 1` to our cleanup function.
Indeed. I forgot the special return behaviour of 'if' constructs when the conditions are false. I will submit a revised patch.