From bf458cced7c0845f7b6fabb887d3878ae4cd51b2 Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Tue, 2 Jun 2020 17:50:24 -0400 Subject: libmakepkg: fix regression in sending plain() output to stderr 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. --- scripts/libmakepkg/executable/vcs.sh.in | 2 +- scripts/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 a29be3c5..95e653ca 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 8f6d6d3a..d1d11eb3 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 f2859740..caf809f3 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 } @@ -355,7 +355,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 } @@ -674,7 +674,7 @@ create_package() { if [[ ! -d $pkgdir ]]; then error "$(gettext "Missing %s directory.")" "\$pkgdir/" - plain "$(gettext "Aborting...")" + plainerr "$(gettext "Aborting...")" exit $E_MISSING_PKGDIR fi @@ -1147,23 +1147,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 @@ -1173,7 +1173,7 @@ if (( SOURCEONLY )); then fi if (( LOGGING )) && ! ensure_writable_dir "LOGDEST" "$LOGDEST"; then - plain "$(gettext "Aborting...")" + plainerr "$(gettext "Aborting...")" exit $E_FS_PERMISSIONS fi -- cgit v1.2.3-24-g4f1b