[pacman-dev,v2,1/2] makepkg: correctly handle missing download clients

Message ID 20200602221649.1387436-1-eschwartz@archlinux.org
State Accepted, archived
Headers show
Series [pacman-dev,v2,1/2] makepkg: correctly handle missing download clients | expand

Commit Message

Eli Schwartz June 2, 2020, 10:16 p.m. UTC
This was broken in commit 882e707e40bbade0111cf3bdedbdac4d4b70453b,
which changed 'plain()' messages to go to stdout, which was then
captured as the download client in question: cmdline=("Aborting...").

The result was a very confusing error message e.g.

/usr/share/makepkg/source/file.sh: line 72: $'\E[1m': command not found

or with makepkg --nocolor:

/usr/share/makepkg/source/file.sh: line 72: Aborting...: command not found

The problem here is that we checked to see if an asynchronous subshell,
in our case <(...), failed, by checking if its captured stdout is
non-empty. Which is terrible, and also a limitation of old bash. But
bash 4.4 can use wait $! to retrieve the return value of an asynchronous
subshell. Now we target that as our minimum, we can sanely handle errors
in such functions.

Losing error messages on stdout by capturing them in a variable instead
of printing them, continues to be a problem, but this will be fixed
systematically in a later commit.

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>

v2: split out the plain() function redirection into its own patch, which
will be handled more consistently.

 scripts/libmakepkg/source/file.sh.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/scripts/libmakepkg/source/file.sh.in b/scripts/libmakepkg/source/file.sh.in
index 819320c2..2b804564 100644
--- a/scripts/libmakepkg/source/file.sh.in
+++ b/scripts/libmakepkg/source/file.sh.in
@@ -42,7 +42,7 @@  download_file() {
 	# find the client we should use for this URL
 	local -a cmdline
 	IFS=' ' read -a cmdline < <(get_downloadclient "$proto")
-	(( ${#cmdline[@]} )) || exit
+	wait $! || exit
 	local filename=$(get_filename "$netfile")
 	local url=$(get_url "$netfile")