libmakepkg: fix regression in sending plain() output to stderr

In commit 882e707e40 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.

(cherry picked from commit bf458cced7)
This commit is contained in:
Eli Schwartz 2020-06-02 17:50:24 -04:00 committed by Andrew Gregory
parent 22e6daa794
commit f4da297de2
12 changed files with 41 additions and 33 deletions

View file

@ -43,7 +43,7 @@ get_vcsclient() {
# if we didn't find an client, return an error # if we didn't find an client, return an error
if [[ -z $client ]]; then if [[ -z $client ]]; then
error "$(gettext "Unknown download protocol: %s")" "$proto" error "$(gettext "Unknown download protocol: %s")" "$proto"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit $E_CONFIG_ERROR exit $E_CONFIG_ERROR
fi fi

View file

@ -112,7 +112,7 @@ check_pgpsigs() {
if (( warnings )); then if (( warnings )); then
warning "$(gettext "Warnings have occurred while verifying the signatures.")" 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 fi
} }

View file

@ -52,7 +52,7 @@ download_bzr() {
msg2 "$(gettext "Branching %s...")" "${displaylocation}" msg2 "$(gettext "Branching %s...")" "${displaylocation}"
if ! bzr branch "$url" "$dir" --no-tree --use-existing-dir; then if ! bzr branch "$url" "$dir" --no-tree --use-existing-dir; then
error "$(gettext "Failure while branching %s")" "${displaylocation}" error "$(gettext "Failure while branching %s")" "${displaylocation}"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
elif (( ! HOLDVER )); then elif (( ! HOLDVER )); then
@ -83,7 +83,7 @@ extract_bzr() {
;; ;;
*) *)
error "$(gettext "Unrecognized reference: %s")" "${fragment}" error "$(gettext "Unrecognized reference: %s")" "${fragment}"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
esac esac
fi fi
@ -98,12 +98,12 @@ extract_bzr() {
cd_safe "${dir##*/}" cd_safe "${dir##*/}"
if ! (bzr pull "$dir" -q --overwrite -r "$rev" && bzr clean-tree -q --detritus --force); then 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" error "$(gettext "Failure while updating working copy of %s %s repo")" "${repo}" "bzr"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
elif ! bzr checkout "$dir" -r "$rev"; then elif ! bzr checkout "$dir" -r "$rev"; then
error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "bzr" error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "bzr"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi

View file

@ -72,7 +72,7 @@ download_file() {
if ! command -- "${cmdline[@]}" >&2; then if ! command -- "${cmdline[@]}" >&2; then
[[ ! -s $dlfile ]] && rm -f -- "$dlfile" [[ ! -s $dlfile ]] && rm -f -- "$dlfile"
error "$(gettext "Failure while downloading %s")" "$url" error "$(gettext "Failure while downloading %s")" "$url"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
@ -137,7 +137,7 @@ extract_file() {
fi fi
if (( ret )); then if (( ret )); then
error "$(gettext "Failed to extract %s")" "$file" error "$(gettext "Failed to extract %s")" "$file"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi

View file

@ -50,7 +50,7 @@ download_git() {
msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git"
if ! git clone --mirror "$url" "$dir"; then if ! git clone --mirror "$url" "$dir"; then
error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "git" error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "git"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
elif (( ! HOLDVER )); then elif (( ! HOLDVER )); then
@ -58,7 +58,7 @@ download_git() {
# Make sure we are fetching the right repo # Make sure we are fetching the right repo
if [[ "$url" != "$(git config --get remote.origin.url)" ]] ; then if [[ "$url" != "$(git config --get remote.origin.url)" ]] ; then
error "$(gettext "%s is not a clone of %s")" "$dir" "$url" error "$(gettext "%s is not a clone of %s")" "$dir" "$url"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
msg2 "$(gettext "Updating %s %s repo...")" "${repo}" "git" msg2 "$(gettext "Updating %s %s repo...")" "${repo}" "git"
@ -87,13 +87,13 @@ extract_git() {
cd_safe "${dir##*/}" cd_safe "${dir##*/}"
if ! git fetch; then if ! git fetch; then
error "$(gettext "Failure while updating working copy of %s %s repo")" "${repo}" "git" error "$(gettext "Failure while updating working copy of %s %s repo")" "${repo}" "git"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
cd_safe "$srcdir" cd_safe "$srcdir"
elif ! git clone -s "$dir" "${dir##*/}"; then elif ! git clone -s "$dir" "${dir##*/}"; then
error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git" error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
@ -110,7 +110,7 @@ extract_git() {
;; ;;
*) *)
error "$(gettext "Unrecognized reference: %s")" "${fragment}" error "$(gettext "Unrecognized reference: %s")" "${fragment}"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
esac esac
fi fi
@ -119,7 +119,7 @@ extract_git() {
tagname="$(git tag -l --format='%(tag)' "$ref")" tagname="$(git tag -l --format='%(tag)' "$ref")"
if [[ -n $tagname && $tagname != "$ref" ]]; then if [[ -n $tagname && $tagname != "$ref" ]]; then
error "$(gettext "Failure while checking out version %s, the git tag has been forged")" "$ref" error "$(gettext "Failure while checking out version %s, the git tag has been forged")" "$ref"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
fi fi
@ -127,7 +127,7 @@ extract_git() {
if [[ $ref != "origin/HEAD" ]] || (( updating )) ; then if [[ $ref != "origin/HEAD" ]] || (( updating )) ; then
if ! git checkout --force --no-track -B makepkg "$ref" --; then if ! git checkout --force --no-track -B makepkg "$ref" --; then
error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git" error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
fi fi

View file

@ -49,7 +49,7 @@ download_hg() {
msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "hg" msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "hg"
if ! hg clone -U "$url" "$dir"; then if ! hg clone -U "$url" "$dir"; then
error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "hg" error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "hg"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
elif (( ! HOLDVER )); then elif (( ! HOLDVER )); then
@ -91,7 +91,7 @@ extract_hg() {
;; ;;
*) *)
error "$(gettext "Unrecognized reference: %s")" "${fragment}" error "$(gettext "Unrecognized reference: %s")" "${fragment}"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
esac esac
fi fi
@ -100,12 +100,12 @@ extract_hg() {
cd_safe "${dir##*/}" cd_safe "${dir##*/}"
if ! (hg pull && hg update -C -r "$ref"); then if ! (hg pull && hg update -C -r "$ref"); then
error "$(gettext "Failure while updating working copy of %s %s repo")" "${repo}" "hg" error "$(gettext "Failure while updating working copy of %s %s repo")" "${repo}" "hg"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
elif ! hg clone -u "$ref" "$dir" "${dir##*/}"; then elif ! hg clone -u "$ref" "$dir" "${dir##*/}"; then
error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "hg" error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "hg"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi

View file

@ -60,7 +60,7 @@ download_svn() {
;; ;;
*) *)
error "$(gettext "Unrecognized reference: %s")" "${fragment}" error "$(gettext "Unrecognized reference: %s")" "${fragment}"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
esac esac
fi fi
@ -70,7 +70,7 @@ download_svn() {
mkdir -p "$dir/.makepkg" mkdir -p "$dir/.makepkg"
if ! svn checkout -r ${ref} --config-dir "$dir/.makepkg" "$url" "$dir"; then if ! svn checkout -r ${ref} --config-dir "$dir/.makepkg" "$url" "$dir"; then
error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "svn" error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "svn"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
elif (( ! HOLDVER )); then elif (( ! HOLDVER )); then

View file

@ -39,7 +39,7 @@ source_makepkg_config() {
source_safe "$MAKEPKG_CONF" source_safe "$MAKEPKG_CONF"
else else
error "$(gettext "%s not found.")" "$MAKEPKG_CONF" error "$(gettext "%s not found.")" "$MAKEPKG_CONF"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit $E_CONFIG_ERROR exit $E_CONFIG_ERROR
fi fi

View file

@ -43,12 +43,20 @@ colorize() {
readonly ALL_OFF BOLD BLUE GREEN RED YELLOW 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() { plain() {
(( QUIET )) && return (( QUIET )) && return
local mesg=$1; shift local mesg=$1; shift
printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@" printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@"
} }
plainerr() {
plain "$@" >&2
}
msg() { msg() {
(( QUIET )) && return (( QUIET )) && return
local mesg=$1; shift local mesg=$1; shift

View file

@ -156,7 +156,7 @@ get_downloadclient() {
# if we didn't find an agent, return an error # if we didn't find an agent, return an error
if [[ -z $agent ]]; then if [[ -z $agent ]]; then
error "$(gettext "Unknown download protocol: %s")" "$proto" error "$(gettext "Unknown download protocol: %s")" "$proto"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 # $E_CONFIG_ERROR exit 1 # $E_CONFIG_ERROR
fi fi
@ -165,7 +165,7 @@ get_downloadclient() {
if [[ ! -x $program ]]; then if [[ ! -x $program ]]; then
local baseprog="${program##*/}" local baseprog="${program##*/}"
error "$(gettext "The download program %s is not installed.")" "$baseprog" error "$(gettext "The download program %s is not installed.")" "$baseprog"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 # $E_MISSING_PROGRAM exit 1 # $E_MISSING_PROGRAM
fi fi

View file

@ -78,7 +78,7 @@ dir_is_empty() {
cd_safe() { cd_safe() {
if ! cd "$1"; then if ! cd "$1"; then
error "$(gettext "Failed to change to directory %s")" "$1" error "$(gettext "Failed to change to directory %s")" "$1"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit 1 exit 1
fi fi
} }

View file

@ -219,7 +219,7 @@ update_pkgver() {
# Print 'source not found' error message and exit makepkg # Print 'source not found' error message and exit makepkg
missing_source_file() { missing_source_file() {
error "$(gettext "Unable to find source file %s.")" "$(get_filename "$1")" error "$(gettext "Unable to find source file %s.")" "$(get_filename "$1")"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit $E_MISSING_FILE exit $E_MISSING_FILE
} }
@ -354,7 +354,7 @@ error_function() {
# first exit all subshells, then print the error # first exit all subshells, then print the error
if (( ! BASH_SUBSHELL )); then if (( ! BASH_SUBSHELL )); then
error "$(gettext "A failure occurred in %s().")" "$1" error "$(gettext "A failure occurred in %s().")" "$1"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
fi fi
exit $E_USER_FUNCTION_FAILED exit $E_USER_FUNCTION_FAILED
} }
@ -672,7 +672,7 @@ create_package() {
if [[ ! -d $pkgdir ]]; then if [[ ! -d $pkgdir ]]; then
error "$(gettext "Missing %s directory.")" "\$pkgdir/" error "$(gettext "Missing %s directory.")" "\$pkgdir/"
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit $E_MISSING_PKGDIR exit $E_MISSING_PKGDIR
fi fi
@ -1145,23 +1145,23 @@ lint_config || exit $E_CONFIG_ERROR
# check that all settings directories are user-writable # check that all settings directories are user-writable
if ! ensure_writable_dir "BUILDDIR" "$BUILDDIR"; then if ! ensure_writable_dir "BUILDDIR" "$BUILDDIR"; then
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS exit $E_FS_PERMISSIONS
fi fi
if (( ! (NOBUILD || GENINTEG) )) && ! ensure_writable_dir "PKGDEST" "$PKGDEST"; then if (( ! (NOBUILD || GENINTEG) )) && ! ensure_writable_dir "PKGDEST" "$PKGDEST"; then
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS exit $E_FS_PERMISSIONS
fi fi
if ! ensure_writable_dir "SRCDEST" "$SRCDEST" ; then if ! ensure_writable_dir "SRCDEST" "$SRCDEST" ; then
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS exit $E_FS_PERMISSIONS
fi fi
if (( SOURCEONLY )); then if (( SOURCEONLY )); then
if ! ensure_writable_dir "SRCPKGDEST" "$SRCPKGDEST"; then if ! ensure_writable_dir "SRCPKGDEST" "$SRCPKGDEST"; then
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS exit $E_FS_PERMISSIONS
fi fi
@ -1171,7 +1171,7 @@ if (( SOURCEONLY )); then
fi fi
if (( LOGGING )) && ! ensure_writable_dir "LOGDEST" "$LOGDEST"; then if (( LOGGING )) && ! ensure_writable_dir "LOGDEST" "$LOGDEST"; then
plain "$(gettext "Aborting...")" plainerr "$(gettext "Aborting...")"
exit $E_FS_PERMISSIONS exit $E_FS_PERMISSIONS
fi fi