[pacman-dev] [PATCH 1/3] makepkg: restrict usage of errexit to user functions

[Dave Reisner]

It's expected that this will lead to unwanted behavior, and needs
widespread testing. It's desirable to commit this for a few reasons:

- there's no reason we can't do our own error checking for code that we
  write.
- it avoids the need for ||true hacks scattered about in the code.
- it makes us immune to upstream changes in exit codes (FS#28248)

Signed-off-by: Dave Reisner <dreisner at archlinux.org>
---
Allan, just making sure we're on the same page -- this _will_ cause breakage,
and the next patch in this series addresses one specific case. I figure getting
this patch in now gives us "ample time" to uncover and fix all these before the
next release.

 scripts/makepkg.sh.in |   47 +++++++++++++++++++++++------------------------
 1 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index f0a2252..61c95af 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1,4 +1,4 @@
-#!/bin/bash -e
+#!/bin/bash
 #
 #   makepkg - make packages compatible for use with pacman
 #   @configure_input@
@@ -430,13 +430,10 @@ run_pacman() {
 check_deps() {
 	(( $# > 0 )) || return 0
 
-	# Disable error trap in pacman subshell call as this breaks bash-3.2 compatibility
-	# Also, a non-zero return value is not unexpected and we are manually dealing them
-	set +E
 	local ret=0
 	local pmout
-	pmout=$(run_pacman -T "$@") || ret=$?
-	set -E
+	pmout=$(run_pacman -T "$@")
+	ret=$?
 
 	if (( ret == 127 )); then #unresolved deps
 		echo "$pmout"
@@ -469,13 +466,7 @@ handle_deps() {
 	fi
 
 	# we might need the new system environment
-	# avoid triggering the ERR trap and exiting
-	set +e
-	local restoretrap=$(trap -p ERR)
-	trap - ERR
 	source /etc/profile &>/dev/null
-	eval $restoretrap
-	set -e
 
 	return $R_DEPS_SATISFIED
 }
@@ -867,6 +858,23 @@ error_function() {
 	exit 2 # $E_BUILD_FAILED
 }
 
+run_function_safe() {
+	local restoretrap
+
+	set -e
+	set -E
+
+	restoretrap=$(trap -p ERR)
+	trap 'error_function $pkgfunc' ERR
+
+	run_function "$1"
+
+	eval $restoretrap
+
+	set +E
+	set +e
+}
+
 run_function() {
 	if [[ -z $1 ]]; then
 		return 1
@@ -892,7 +900,6 @@ run_function() {
 	local shellopts=$(shopt -p)
 
 	local ret=0
-	local restoretrap
 	if (( LOGGING )); then
 		local fullver=$(get_full_version)
 		local BUILDLOG="${startdir}/${pkgbase}-${fullver}-${CARCH}-$pkgfunc.log"
@@ -914,18 +921,12 @@ run_function() {
 		tee "$BUILDLOG" < "$logpipe" &
 		local teepid=$!
 
-		restoretrap=$(trap -p ERR)
-		trap 'error_function $pkgfunc' ERR
 		$pkgfunc &>"$logpipe"
-		eval $restoretrap
 
 		wait $teepid
 		rm "$logpipe"
 	else
-		restoretrap=$(trap -p ERR)
-		trap 'error_function $pkgfunc' ERR
 		$pkgfunc 2>&1
-		eval $restoretrap
 	fi
 	# reset our shell options
 	eval "$shellopts"
@@ -943,11 +944,11 @@ run_build() {
 		[[ -d /usr/lib/ccache/bin ]] && export PATH="/usr/lib/ccache/bin:$PATH"
 	fi
 
-	run_function "build"
+	run_function_safe "build"
 }
 
 run_check() {
-	run_function "check"
+	run_function_safe "check"
 }
 
 run_package() {
@@ -958,7 +959,7 @@ run_package() {
 		pkgfunc="package_$1"
 	fi
 
-	run_function "$pkgfunc"
+	run_function_safe "$pkgfunc"
 }
 
 tidy_install() {
@@ -1973,8 +1974,6 @@ for signal in TERM HUP QUIT; do
 	trap "trap_exit \"$(gettext "%s signal caught. Exiting...")\" \"$signal\"" "$signal"
 done
 trap 'trap_exit "$(gettext "Aborted by user! Exiting...")"' INT
-trap 'trap_exit "$(gettext "An unknown error has occurred. Exiting...")"' ERR
-set -E
 
 # preserve environment variables and canonicalize path
 [[ -n ${PKGDEST} ]] && _PKGDEST=$(canonicalize_path ${PKGDEST})
-- 
1.7.9