[pacman-dev] [PATCH] tolerate broken logpipe
Sometimes makechrootpkg fails with: rm: cannot remove '/logdest/logpipe.xxxxxxxx': No such file or directory This shouldn't cause the whole script to fail, so let's tolerate a missing pipe Signed-off-by: Yardena Cohen <yardenack@gmail.com> --- scripts/makepkg.sh.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index aa03e9d9..9f8c9096 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -344,7 +344,7 @@ remove_deps() { error_function() { if [[ -p $logpipe ]]; then - rm "$logpipe" + rm -f "$logpipe" fi # first exit all subshells, then print the error if (( ! BASH_SUBSHELL )); then @@ -428,7 +428,7 @@ run_function() { $pkgfunc &>"$logpipe" wait $teepid - rm "$logpipe" + rm -f "$logpipe" else "$pkgfunc" fi -- 2.22.0
On 7/9/19 9:57 PM, Yardena Cohen wrote:
Sometimes makechrootpkg fails with:
rm: cannot remove '/logdest/logpipe.xxxxxxxx': No such file or directory
This shouldn't cause the whole script to fail, so let's tolerate a missing pipe
As a matter of curiosity, when does this happen? I don't recall offhand seeing it. I'm not saying it's fundamentally wrong to make this change, but I wouldn't have expected this to be a problem.
Signed-off-by: Yardena Cohen <yardenack@gmail.com> --- scripts/makepkg.sh.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index aa03e9d9..9f8c9096 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -344,7 +344,7 @@ remove_deps() {
error_function() { if [[ -p $logpipe ]]; then - rm "$logpipe" + rm -f "$logpipe"
If we just checked using [[ -p namedpipe ]] then it seems like the window for a TOCTOU is pretty narrow... Aside for which, if we've reached this point then we're already erroring out, so -f would only silence a warning, not make anything succeed. So this should not be what is causing the whole script to fail.
fi # first exit all subshells, then print the error if (( ! BASH_SUBSHELL )); then @@ -428,7 +428,7 @@ run_function() { $pkgfunc &>"$logpipe"
wait $teepid - rm "$logpipe" + rm -f "$logpipe"
I'm curious under what situation we could tee to a file, wait until tee exits, and then try but fail to delete the file. IIRC the only times we use run_function() are inside run_function_safe() which sets up errexit, so if this fails then that would explain why the script would abort, but I don't get how the file is supposed to disappear in the first place.
else "$pkgfunc" fi
-- Eli Schwartz Bug Wrangler and Trusted User
On Tue, Jul 9, 2019 at 7:52 PM Eli Schwartz <eschwartz@archlinux.org> wrote:
As a matter of curiosity, when does this happen? I don't recall offhand seeing it.
I find it every so often, most recently building yuzu-git. I don't know why and don't see any pattern. Googling the error message tells me I'm not alone: https://bbs.archlinux.org/viewtopic.php?pid=1789264#p1789264 https://github.com/AladW/aurutils/issues/19#issuecomment-194565520 I usually just solve it by patching $chroot/usr/bin/makepkg when it happens, but I'm getting tired of doing that. :)
If we just checked using [[ -p namedpipe ]] then it seems like the window for a TOCTOU is pretty narrow...
Aside for which, if we've reached this point then we're already erroring out, so -f would only silence a warning, not make anything succeed. So this should not be what is causing the whole script to fail.
Good point. I was overzealous replacing both instances. Do you want a new patch with only the second part?
On 10/7/19 12:52 pm, Eli Schwartz wrote:
On 7/9/19 9:57 PM, Yardena Cohen wrote:
Sometimes makechrootpkg fails with:
rm: cannot remove '/logdest/logpipe.xxxxxxxx': No such file or directory
This shouldn't cause the whole script to fail, so let's tolerate a missing pipe
As a matter of curiosity, when does this happen? I don't recall offhand seeing it.
I'm not saying it's fundamentally wrong to make this change, but I wouldn't have expected this to be a problem.
Signed-off-by: Yardena Cohen <yardenack@gmail.com> --- scripts/makepkg.sh.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index aa03e9d9..9f8c9096 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -344,7 +344,7 @@ remove_deps() {
error_function() { if [[ -p $logpipe ]]; then - rm "$logpipe" + rm -f "$logpipe"
If we just checked using [[ -p namedpipe ]] then it seems like the window for a TOCTOU is pretty narrow...
Aside for which, if we've reached this point then we're already erroring out, so -f would only silence a warning, not make anything succeed. So this should not be what is causing the whole script to fail.
fi # first exit all subshells, then print the error if (( ! BASH_SUBSHELL )); then @@ -428,7 +428,7 @@ run_function() { $pkgfunc &>"$logpipe"
wait $teepid - rm "$logpipe" + rm -f "$logpipe"
I'm curious under what situation we could tee to a file, wait until tee exits, and then try but fail to delete the file.
IIRC the only times we use run_function() are inside run_function_safe() which sets up errexit, so if this fails then that would explain why the script would abort, but I don't get how the file is supposed to disappear in the first place.
I don't understand this either... Note that there is no evidence of this occurring when running makepkg directly. Only under the Arch devtools package. I get the feeling this is papering over an issue in devtools, and am reluctant to pull the patch unless there information to say otherwise. Allan
Hi Allan,
@@ -428,7 +428,7 @@ run_function() { $pkgfunc &>"$logpipe"
wait $teepid - rm "$logpipe" + rm -f "$logpipe"
I don't understand this either... Note that there is no evidence of this occurring when running makepkg directly. Only under the Arch devtools package.
Strictly speaking, shouldn't that be ‘wait -f’? I don't know if it's only job control that could cause a change in tee's status Also, wait's exit status is ignored; reporting an unusual status may give a clue. An rm without -f could be attempted first and if that fails then report its exit status and try again with -f, checking that second rm too. Same end result, ‘rm -f’, but perhaps more light shed on the problem. -- Cheers, Ralph.
participants (4)
-
Allan McRae
-
Eli Schwartz
-
Ralph Corderoy
-
Yardena Cohen