Re: [pacman-dev] [PATCH v2 2/2] libmakepkg: fix regression in sending plain() output to stderr

2020-06-10 Thread Allan McRae
On 3/6/20 8:16 am, Eli Schwartz wrote:
> In commit 882e707e40bbade0111cf3bdedbdac4d4b70453b we changed message
> output to go to stdout by default, unless it was an error. The plain()
> function doesn't *look* like an error function, but in practice it was
> -- it's used to continue multiline messages, and all in-tree uses were
> for warning/error.
> 
> This is a problem both because we're sending output to the wrong place,
> and because in some cases, we were performing error logging from a
> function which would otherwise return a value to be captured in a
> variable using command substution.
> 
> Fix this and straighten out the API by providing two functions: one for
> continuing msg output, and one which wraps this by sending output to
> stderr, for continuing error output.
> 
> Change all callers to use the second function.
> 
> Signed-off-by: Eli Schwartz 
> ---

I did not see this patch before reviewing v1.  This approach also looks
fine to me.  Pulling this version.

Allan


[pacman-dev] [PATCH v2 2/2] libmakepkg: fix regression in sending plain() output to stderr

2020-06-02 Thread Eli Schwartz
In commit 882e707e40bbade0111cf3bdedbdac4d4b70453b we changed message
output to go to stdout by default, unless it was an error. The plain()
function doesn't *look* like an error function, but in practice it was
-- it's used to continue multiline messages, and all in-tree uses were
for warning/error.

This is a problem both because we're sending output to the wrong place,
and because in some cases, we were performing error logging from a
function which would otherwise return a value to be captured in a
variable using command substution.

Fix this and straighten out the API by providing two functions: one for
continuing msg output, and one which wraps this by sending output to
stderr, for continuing error output.

Change all callers to use the second function.

Signed-off-by: Eli Schwartz 
---
 scripts/libmakepkg/executable/vcs.sh.in  |  2 +-
 .../libmakepkg/integrity/verify_signature.sh.in  |  2 +-
 scripts/libmakepkg/source/bzr.sh.in  |  8 
 scripts/libmakepkg/source/file.sh.in |  4 ++--
 scripts/libmakepkg/source/git.sh.in  | 14 +++---
 scripts/libmakepkg/source/hg.sh.in   |  8 
 scripts/libmakepkg/source/svn.sh.in  |  4 ++--
 scripts/libmakepkg/util/config.sh.in |  2 +-
 scripts/libmakepkg/util/message.sh.in|  8 
 scripts/libmakepkg/util/source.sh.in |  4 ++--
 scripts/libmakepkg/util/util.sh.in   |  2 +-
 scripts/makepkg.sh.in| 16 
 12 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/scripts/libmakepkg/executable/vcs.sh.in 
b/scripts/libmakepkg/executable/vcs.sh.in
index 6eb93fae..436b82db 100644
--- a/scripts/libmakepkg/executable/vcs.sh.in
+++ b/scripts/libmakepkg/executable/vcs.sh.in
@@ -43,7 +43,7 @@ get_vcsclient() {
# if we didn't find an client, return an error
if [[ -z $client ]]; then
error "$(gettext "Unknown download protocol: %s")" "$proto"
-   plain "$(gettext "Aborting...")"
+   plainerr "$(gettext "Aborting...")"
exit $E_CONFIG_ERROR
fi
 
diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in 
b/scripts/libmakepkg/integrity/verify_signature.sh.in
index a1bf7f1c..2be5c493 100644
--- a/scripts/libmakepkg/integrity/verify_signature.sh.in
+++ b/scripts/libmakepkg/integrity/verify_signature.sh.in
@@ -112,7 +112,7 @@ check_pgpsigs() {
 
if (( warnings )); then
warning "$(gettext "Warnings have occurred while verifying the 
signatures.")"
-   plain "$(gettext "Please make sure you really trust them.")"
+   plainerr "$(gettext "Please make sure you really trust them.")"
fi
 }
 
diff --git a/scripts/libmakepkg/source/bzr.sh.in 
b/scripts/libmakepkg/source/bzr.sh.in
index 1227c739..d8e47185 100644
--- a/scripts/libmakepkg/source/bzr.sh.in
+++ b/scripts/libmakepkg/source/bzr.sh.in
@@ -52,7 +52,7 @@ download_bzr() {
msg2 "$(gettext "Branching %s...")" "${displaylocation}"
if ! bzr branch "$url" "$dir" --no-tree --use-existing-dir; then
error "$(gettext "Failure while branching %s")" 
"${displaylocation}"
-   plain "$(gettext "Aborting...")"
+   plainerr "$(gettext "Aborting...")"
exit 1
fi
elif (( ! HOLDVER )); then
@@ -83,7 +83,7 @@ extract_bzr() {
;;
*)
error "$(gettext "Unrecognized reference: %s")" 
"${fragment}"
-   plain "$(gettext "Aborting...")"
+   plainerr "$(gettext "Aborting...")"
exit 1
esac
fi
@@ -98,12 +98,12 @@ extract_bzr() {
cd_safe "${dir##*/}"
if ! (bzr pull "$dir" -q --overwrite -r "$rev" && bzr 
clean-tree -q --detritus --force); then
error "$(gettext "Failure while updating working copy 
of %s %s repo")" "${repo}" "bzr"
-   plain "$(gettext "Aborting...")"
+   plainerr "$(gettext "Aborting...")"
exit 1
fi
elif ! bzr checkout "$dir" -r "$rev"; then
error "$(gettext "Failure while creating working copy of %s %s 
repo")" "${repo}" "bzr"
-   plain "$(gettext "Aborting...")"
+   plainerr "$(gettext "Aborting...")"
exit 1
fi
 
diff --git a/scripts/libmakepkg/source/file.sh.in 
b/scripts/libmakepkg/source/file.sh.in
index 2b804564..b29aeb04 100644
--- a/scripts/libmakepkg/source/file.sh.in
+++ b/scripts/libmakepkg/source/file.sh.in
@@ -72,7 +72,7 @@ download_file() {
if ! command -- "${cmdline[@]}" >&2; then
[[ ! -s $dlfile ]] && rm -f -- "$dlfile"
error