[pacman-dev] [PATCH v2 2/2] libmakepkg: fix regression in sending plain() output to stderr
Eli Schwartz
eschwartz at archlinux.org
Tue Jun 2 22:16:49 UTC 2020
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 <eschwartz at archlinux.org>
---
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 "$(gettext "Failure while downloading %s")" "$url"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
@@ -137,7 +137,7 @@ extract_file() {
fi
if (( ret )); then
error "$(gettext "Failed to extract %s")" "$file"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
diff --git a/scripts/libmakepkg/source/git.sh.in b/scripts/libmakepkg/source/git.sh.in
index aee944f7..b81989a4 100644
--- a/scripts/libmakepkg/source/git.sh.in
+++ b/scripts/libmakepkg/source/git.sh.in
@@ -50,7 +50,7 @@ download_git() {
msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git"
if ! git clone --mirror "$url" "$dir"; then
error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "git"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
elif (( ! HOLDVER )); then
@@ -58,7 +58,7 @@ download_git() {
# Make sure we are fetching the right repo
if [[ "$url" != "$(git config --get remote.origin.url)" ]] ; then
error "$(gettext "%s is not a clone of %s")" "$dir" "$url"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
msg2 "$(gettext "Updating %s %s repo...")" "${repo}" "git"
@@ -87,13 +87,13 @@ extract_git() {
cd_safe "${dir##*/}"
if ! git fetch; then
error "$(gettext "Failure while updating working copy of %s %s repo")" "${repo}" "git"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
cd_safe "$srcdir"
elif ! git clone -s "$dir" "${dir##*/}"; then
error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
@@ -110,7 +110,7 @@ extract_git() {
;;
*)
error "$(gettext "Unrecognized reference: %s")" "${fragment}"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
esac
fi
@@ -119,7 +119,7 @@ extract_git() {
tagname="$(git tag -l --format='%(tag)' "$ref")"
if [[ -n $tagname && $tagname != "$ref" ]]; then
error "$(gettext "Failure while checking out version %s, the git tag has been forged")" "$ref"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
fi
@@ -127,7 +127,7 @@ extract_git() {
if [[ $ref != "origin/HEAD" ]] || (( updating )) ; then
if ! git checkout --force --no-track -B makepkg $ref; then
error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
fi
diff --git a/scripts/libmakepkg/source/hg.sh.in b/scripts/libmakepkg/source/hg.sh.in
index b5af6ce0..d47458f9 100644
--- a/scripts/libmakepkg/source/hg.sh.in
+++ b/scripts/libmakepkg/source/hg.sh.in
@@ -49,7 +49,7 @@ download_hg() {
msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "hg"
if ! hg clone -U "$url" "$dir"; then
error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "hg"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
elif (( ! HOLDVER )); then
@@ -91,7 +91,7 @@ extract_hg() {
;;
*)
error "$(gettext "Unrecognized reference: %s")" "${fragment}"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
esac
fi
@@ -100,12 +100,12 @@ extract_hg() {
cd_safe "${dir##*/}"
if ! (hg pull && hg update -C -r "$ref"); then
error "$(gettext "Failure while updating working copy of %s %s repo")" "${repo}" "hg"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
elif ! hg clone -u "$ref" "$dir" "${dir##*/}"; then
error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "hg"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
diff --git a/scripts/libmakepkg/source/svn.sh.in b/scripts/libmakepkg/source/svn.sh.in
index e808b2cb..89205ec3 100644
--- a/scripts/libmakepkg/source/svn.sh.in
+++ b/scripts/libmakepkg/source/svn.sh.in
@@ -60,7 +60,7 @@ download_svn() {
;;
*)
error "$(gettext "Unrecognized reference: %s")" "${fragment}"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
esac
fi
@@ -70,7 +70,7 @@ download_svn() {
mkdir -p "$dir/.makepkg"
if ! svn checkout -r ${ref} --config-dir "$dir/.makepkg" "$url" "$dir"; then
error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "svn"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
elif (( ! HOLDVER )); then
diff --git a/scripts/libmakepkg/util/config.sh.in b/scripts/libmakepkg/util/config.sh.in
index a8975d6d..ea909f84 100644
--- a/scripts/libmakepkg/util/config.sh.in
+++ b/scripts/libmakepkg/util/config.sh.in
@@ -39,7 +39,7 @@ source_makepkg_config() {
source_safe "$MAKEPKG_CONF"
else
error "$(gettext "%s not found.")" "$MAKEPKG_CONF"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit $E_CONFIG_ERROR
fi
diff --git a/scripts/libmakepkg/util/message.sh.in b/scripts/libmakepkg/util/message.sh.in
index 061bd858..625f11b1 100644
--- a/scripts/libmakepkg/util/message.sh.in
+++ b/scripts/libmakepkg/util/message.sh.in
@@ -43,12 +43,20 @@ colorize() {
readonly ALL_OFF BOLD BLUE GREEN RED YELLOW
}
+# plainerr/plainerr are primarily used to continue a previous message on a new
+# line, depending on whether the first line is a regular message or an error
+# output
+
plain() {
(( QUIET )) && return
local mesg=$1; shift
printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@"
}
+plainerr() {
+ plain "$@" >&2
+}
+
msg() {
(( QUIET )) && return
local mesg=$1; shift
diff --git a/scripts/libmakepkg/util/source.sh.in b/scripts/libmakepkg/util/source.sh.in
index e0490661..be7c15c2 100644
--- a/scripts/libmakepkg/util/source.sh.in
+++ b/scripts/libmakepkg/util/source.sh.in
@@ -156,7 +156,7 @@ get_downloadclient() {
# if we didn't find an agent, return an error
if [[ -z $agent ]]; then
error "$(gettext "Unknown download protocol: %s")" "$proto"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1 # $E_CONFIG_ERROR
fi
@@ -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...")"
+ plainerr "$(gettext "Aborting...")"
exit 1 # $E_MISSING_PROGRAM
fi
diff --git a/scripts/libmakepkg/util/util.sh.in b/scripts/libmakepkg/util/util.sh.in
index 0d6b8410..6aeead40 100644
--- a/scripts/libmakepkg/util/util.sh.in
+++ b/scripts/libmakepkg/util/util.sh.in
@@ -78,7 +78,7 @@ dir_is_empty() {
cd_safe() {
if ! cd "$1"; then
error "$(gettext "Failed to change to directory %s")" "$1"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit 1
fi
}
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 7261fb2c..3fb0d62b 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -220,7 +220,7 @@ update_pkgver() {
# Print 'source not found' error message and exit makepkg
missing_source_file() {
error "$(gettext "Unable to find source file %s.")" "$(get_filename "$1")"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit $E_MISSING_FILE
}
@@ -362,7 +362,7 @@ error_function() {
# first exit all subshells, then print the error
if (( ! BASH_SUBSHELL )); then
error "$(gettext "A failure occurred in %s().")" "$1"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
fi
exit $E_USER_FUNCTION_FAILED
}
@@ -681,7 +681,7 @@ create_package() {
if [[ ! -d $pkgdir ]]; then
error "$(gettext "Missing %s directory.")" "\$pkgdir/"
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit $E_MISSING_PKGDIR
fi
@@ -1154,23 +1154,23 @@ lint_config || exit $E_CONFIG_ERROR
# check that all settings directories are user-writable
if ! ensure_writable_dir "BUILDDIR" "$BUILDDIR"; then
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS
fi
if (( ! (NOBUILD || GENINTEG) )) && ! ensure_writable_dir "PKGDEST" "$PKGDEST"; then
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS
fi
if ! ensure_writable_dir "SRCDEST" "$SRCDEST" ; then
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS
fi
if (( SOURCEONLY )); then
if ! ensure_writable_dir "SRCPKGDEST" "$SRCPKGDEST"; then
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS
fi
@@ -1180,7 +1180,7 @@ if (( SOURCEONLY )); then
fi
if (( LOGGING )) && ! ensure_writable_dir "LOGDEST" "$LOGDEST"; then
- plain "$(gettext "Aborting...")"
+ plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS
fi
--
2.26.2
More information about the pacman-dev
mailing list