[pacman-dev] [PATCH] makepkg: Delete logpipe when cleaning up

[Eli Schwartz]

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?

> Signed-off-by: Austin Lund <austin.lund at 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.

>  	if (( (EXIT_CODE == E_OK || EXIT_CODE == E_INSTALL_FAILED) && CLEANUP )); then
>  		local pkg file
>  
> @@ -343,9 +345,6 @@ remove_deps() {
>  }
>  
>  error_function() {
> -	if [[ -p $logpipe ]]; then
> -		rm "$logpipe"
> -	fi
>  	# first exit all subshells, then print the error
>  	if (( ! BASH_SUBSHELL )); then
>  		error "$(gettext "A failure occurred in %s().")" "$1"
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1601 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/pacman-dev/attachments/20190826/7785ff37/attachment.sig>