On 18/5/20 8:34 am, Eli Schwartz wrote:
> 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
>
> Solve this on two levels:
> - redirect this error() continuation to stderr so the user sees it.
> - catch erroring returns in get_downloadclient and propagate them
>
> bash 4.4 can use wait $! to retrieve the return value of an asynchronous
> subshell such as <(...), which means, now we target that as our minimum,
> we can sanely handle errors in such functions.
>
> Signed-off-by: Eli Schwartz
> ---
>
> Actually, maybe every use of plain() needs to do this. Or else make
> plain() do this by default. But it's technically used to "continue the
> previous message", and there's no guarantee it merits going to stderr,
> even though every time we use plain(), it does.
> > What to do...
>
I can take this patch as is, but as you point out, this is a more
systemic issue with plain() always being used to follow error() or
warning() so all current usages should be on stderr. So I'd like two
patches - one dealing with global stderr output issue, and one with the
$! usage.
Options for plain() are, we manually add >&2 when needed, or we provide
a wrapper function that does that. My suggestion is:
plain() {
(( QUIET )) && return
local mesg=$1; shift
printf "${BOLD}${mesg}${ALL_OFF}\n" "$@"
}
bet rid of ${BOLD} for plain() and add a bold() function that prints to
stderr.
> scripts/libmakepkg/source/file.sh.in | 1 +
> scripts/libmakepkg/util/source.sh.in | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/libmakepkg/source/file.sh.in
> b/scripts/libmakepkg/source/file.sh.in
> index 819320c2..28f373fb 100644
> --- a/scripts/libmakepkg/source/file.sh.in
> +++ b/scripts/libmakepkg/source/file.sh.in
> @@ -42,6 +42,7 @@ download_file() {
> # find the client we should use for this URL
> local -a cmdline
> IFS=' ' read -a cmdline < <(get_downloadclient "$proto")
> + wait $! || exit
> (( ${#cmdline[@]} )) || exit
>
> local filename=$(get_filename "$netfile")
> diff --git a/scripts/libmakepkg/util/source.sh.in
> b/scripts/libmakepkg/util/source.sh.in
> index e0490661..1bf5b814 100644
> --- a/scripts/libmakepkg/util/source.sh.in
> +++ b/scripts/libmakepkg/util/source.sh.in
> @@ -165,7 +165,7 @@ get_downloadclient() {
> if [[ ! -x $program ]]; then
> local baseprog="${program##*/}"
> error "$(gettext "The download program %s is not installed.")"
> "$baseprog"
> - plain "$(gettext "Aborting...")"
> + plain "$(gettext "Aborting...")" >&2
> exit 1 # $E_MISSING_PROGRAM
> fi
>
>
