Fix #5837

Since the introduction of pkg_with_pb(), direct calls to this function
were added using the following format:

if ! pkg_with_pb ... | tee -a $logfile; then

What happened in this case is if was always getting a successfull
return code from tee call, and pkg_with_pb return was always being
ignored leading pfSense-upgrade to keep running when a fatal error
happened.

Since we need to print pkg output and also send it to log, _exec() call
is the best option here.

Added necessary flags to make sure necessary actions (unlock additional
packages or cleaning up kernel package annotation) are executed on
_exit call in fatal cases and replaced all direct calls by _exec()

1 files changed, 23 insertions, 23 deletions

diff --git a/src/usr/local/sbin/pfSense-upgrade b/src/usr/local/sbin/pfSense-upgrade
index 11f970d..1d1c534 100755
--- a/src/usr/local/sbin/pfSense-upgrade
+++ b/src/usr/local/sbin/pfSense-upgrade
@@ -144,6 +144,14 @@ _exit() {
 		kill ${nc_pid}
 	fi
 
+	if [ -n "${delete_annotation}" ]; then
+		pkg ${pkg_chroot} annotate -q -D ${kernel_pkg} next_stage
+	fi
+
+	if [ -n "${unlock_additional_pkgs}" ]; then
+		pkg_unlock "${pkg_prefix}*"
+	fi
+
 	local _rc=${1:-"0"}
 
 	# If EVENT_PIPE is defined, GUI is calling
@@ -205,18 +213,11 @@ fetch_upgrade_packages() {
 
 	fi
 
-	_echo ">>> Downloading upgrade packages..."
-	if ! pkg_with_pb ${pkg_chroot} upgrade -F 2>&1 | tee -a ${logfile}; then
-		_echo "ERROR: It was not possible to download packages"
-		_exit 1
-	fi
+	_exec "pkg_with_pb ${pkg_chroot} upgrade -F" "Downloading upgrade packages"
 
 	if [ -n "${_pkgs_to_fetch}" ]; then
-		_echo ">>> Fetching packages not present on upgrade partition..."
-		if ! pkg_with_pb ${pkg_chroot} fetch -d ${_pkgs_to_fetch} 2>&1 | tee -a ${logfile}; then
-			_echo "ERROR: It was not possible to fetch packages"
-			_exit 1
-		fi
+		_exec "pkg_with_pb ${pkg_chroot} fetch -d ${_pkgs_to_fetch}" \
+			"Fetching packages not present on upgrade partition"
 	fi
 }
 
@@ -363,6 +364,7 @@ pkg_upgrade() {
 
 	if [ "${next_stage}" = "2" ]; then
 		pkg_lock "${pkg_prefix}*"
+		unlock_additional_pkgs=1
 
 		# XXX: Workaround to upgrade strongswan
 		# If those symlinks are present, pkg exit because it expects them
@@ -383,13 +385,9 @@ pkg_upgrade() {
 		fi
 
 		if [ $(pkg ${pkg_chroot} upgrade -nq | wc -l) -gt 1 ]; then
-			_echo "Upgrading necessary packages..."
-			if ! pkg ${pkg_chroot} upgrade 2>&1 | tee -a ${logfile}; then
-				pkg ${pkg_chroot} annotate -q -D ${kernel_pkg} next_stage
-				pkg_unlock "${pkg_prefix}*"
-				_echo "ERROR: An error occurred when upgrade was running..."
-				_exit 1
-			fi
+			delete_annotation=1
+			_exec "pkg ${pkg_chroot} upgrade" "Upgrading necessary packages"
+			delete_annotation=""
 		fi
 
 		# XXX: workaround for #5300
@@ -400,6 +398,7 @@ pkg_upgrade() {
 		next_stage=3
 
 		pkg_unlock "${pkg_prefix}*"
+		unlock_additional_pkgs=""
 
 		if [ -n "${need_reboot}" -a "${platform}" = "nanobsd" ]; then
 			switch_active_nanobsd_partition
@@ -414,12 +413,9 @@ pkg_upgrade() {
 
 	if [ "${next_stage}" = "3" ]; then
 		if [ $(pkg upgrade -nq | wc -l) -gt 1 ]; then
-			_echo "Upgrading necessary packages..."
-			if ! pkg ${pkg_chroot} upgrade 2>&1 | tee -a ${logfile}; then
-				pkg ${pkg_chroot} annotate -q -D ${kernel_pkg} next_stage
-				_echo "ERROR: An error occurred when upgrade was running..."
-				_exit 1
-			fi
+			delete_annotation=1
+			_exec "pkg ${pkg_chroot} upgrade" "Upgrading necessary packages"
+			delete_annotation=""
 		fi
 
 		pkg ${pkg_chroot} annotate -q -D ${kernel_pkg} next_stage
@@ -703,6 +699,10 @@ else
 	export HTTP_USER_AGENT="${product}/${product_version}"
 fi
 
+# Flags used in _exit
+export delete_annotation=""
+export unlock_additional_pkgs=""
+
 # Upgrade process on nanobsd will happen in chroot
 export pkg_chroot=""
 export chroot_dir=""