From 6ebab10caa5be6fc1cfd244e745851687d4bd70d Mon Sep 17 00:00:00 2001
From: Leah Rowe <leah@libreboot.org>
Date: Wed, 27 Mar 2024 01:19:39 +0000
Subject: safer, simpler error handling in lbmk

in shell scripts, a function named the same as a program included in
the $PATH will override that program. for example, you could make a
function called ls() and this would override the standand "ls".

in lbmk, a part of it was first trying to run the "fail" command,
deferring to "err", because some scripts call fail() which does
some minor cleanup before calling err.

in most cases, fail() is not defined, and it's possible that the user
could have a program called "fail" in their $PATH, the behaviour of
which we could not determine, and it could have disastrous effects.

lbmk error handling has been re-engineered in such a way that the
err function is defined in a variable, which defaults to err_ which
calls err_, so defined under include/err.sh.

in functions that require cleanup prior to error handling, a fail()
function is still defined, and err is overridden, thus:

err="fail"

this change has made xx_() obsolete, so now only x_ is used. the x_
function is a wrapper that can be used to run a command and exit with
non-zero status (from lbmk) if the command fails. the xx_ command
did the same thing, but called fail() which would have called err();
now everything is $err

example:

	rm -f "$filename" || err "could not delete file"

this would now be:

	rm -f "$filename" || $err "could not delete file"

overriding of err= must be done *after* including err.sh. for
example:

err="fail"
. "include/err.sh"

^ this is wrong. instead, one must do:

. "include/err.sh"
err="fail"

this is because err is set as a global variable under err.sh

the new error handling is much cleaner, and safer. it also reduces
the chance of mistakes such as: calling err when you meant to
call fail. this is because the standard way is now to call $err,
so you set err="fail" at the top of the script and all is well.

Signed-off-by: Leah Rowe <leah@libreboot.org>
---
 script/update/release | 92 +++++++++++++++++++++++++--------------------------
 script/update/trees   | 30 ++++++++---------
 2 files changed, 61 insertions(+), 61 deletions(-)

(limited to 'script/update')

diff --git a/script/update/release b/script/update/release
index 357840c2..c4101b5a 100755
--- a/script/update/release
+++ b/script/update/release
@@ -13,11 +13,11 @@ main()
 {
 	vdir="release"
 	while getopts d:m: option; do
-		[ -z "${OPTARG}" ] && err "Empty argument not allowed"
+		[ -z "${OPTARG}" ] && $err "Empty argument not allowed"
 		case "${option}" in
 		d) vdir="${OPTARG}" ;;
 		m) mode="${OPTARG}" ;;
-		*) err "Invalid option" ;;
+		*) $err "Invalid option" ;;
 		esac
 	done
 
@@ -29,7 +29,7 @@ main()
 	src_dirname="${relname}_src"
 	srcdir="${vdir}/${src_dirname}"
 
-	[ -e "${vdir}" ] && err "already exists: \"${vdir}\""
+	[ -e "${vdir}" ] && $err "already exists: \"${vdir}\""
 
 	mkvdir
 	build_release
@@ -39,45 +39,45 @@ main()
 
 mkvdir()
 {
-	mkdir -p "${vdir}" || err "mkvdir: !mkdir -p \"${vdir}\""
-	git clone . "${srcdir}" || err "mkdir: !gitclone \"${srcdir}\""
-	insert_version_files "$srcdir" || err "mkvdir $srcdir: versionfile"
+	mkdir -p "${vdir}" || $err "mkvdir: !mkdir -p \"${vdir}\""
+	git clone . "${srcdir}" || $err "mkdir: !gitclone \"${srcdir}\""
+	insert_version_files "$srcdir" || $err "mkvdir $srcdir: versionfile"
 }
 
 build_release()
 {
 	_xm="build_release ${vdir}"
 	(
-	cd "${srcdir}" || err "${_xm}: !cd \"${srcdir}\""
+	cd "${srcdir}" || $err "${_xm}: !cd \"${srcdir}\""
 	fetch_trees
 	[ "${mode}" = "u-boot" ] || x_ mv src/docs docs
-	) || err "can't create release files"
+	) || $err "can't create release files"
 
 	git log --graph --pretty=format:'%Cred%h%Creset %s %Creset' \
 	    --abbrev-commit > "${srcdir}/CHANGELOG" || \
-	    err "build_release $srcdir: couldn't generate changelog"
+	    $err "build_release $srcdir: couldn't generate changelog"
 
 	(
 	if [ "${mode}" = "u-boot" ]; then
-		cd "${srcdir}/src/" || err "${_xm}: mktarball \"${srcdir}\""
+		cd "${srcdir}/src/" || $err "${_xm}: mktarball \"${srcdir}\""
 		mktarball u-boot "../../${srcdir##*/}.tar.xz" || \
-		    err "$_xm: mksrc"
+		    $err "$_xm: mksrc"
 		# make a src archive containing only u-boot
 	else
-		cd "${srcdir%/*}" || err "${_xm}: mktarball \"${srcdir}\""
+		cd "${srcdir%/*}" || $err "${_xm}: mktarball \"${srcdir}\""
 		mktarball "${srcdir##*/}" "${srcdir##*/}.tar.xz" || \
-		    err "$_xm: mksrc"
+		    $err "$_xm: mksrc"
 	fi
-	) || err "can't create src tarball"
+	) || $err "can't create src tarball"
 	[ "${mode}" = "src" ] && return 0
 	[ "${mode}" = "u-boot" ] && return 0
 
 	(
-	cd "${srcdir}" || err "${_xm}: 2 !cd \"${srcdir}\""
+	cd "${srcdir}" || $err "${_xm}: 2 !cd \"${srcdir}\""
 	mkrom_images
-	) || err "can't build rom images"
+	) || $err "can't build rom images"
 
-	rm -Rf "${srcdir}" || err "!rm -Rf ${srcdir}"
+	rm -Rf "${srcdir}" || $err "!rm -Rf ${srcdir}"
 }
 
 fetch_trees()
@@ -85,7 +85,7 @@ fetch_trees()
 	for x in config/git/*; do
 		[ "${mode}" = "u-boot" ] && break
 		[ ! -f "${x}" ] || ./update trees -f "${x#config/git/}" || \
-		    err "${_xm}: fetch ${x#config/git/}"
+		    $err "${_xm}: fetch ${x#config/git/}"
 	done
 	[ "${mode}" = "u-boot" ] && x_ ./update trees -f u-boot
 
@@ -93,56 +93,56 @@ fetch_trees()
 		[ -f "${x}" ] || continue
 		xp="${x#*/}"; xp="${xp%/*}"
 		[ -L "${xp}" ] || rm -Rf "src/${xp}/${xp}" || \
-		    err "!rm -Rf \"src/${xp}/${xp}\""
+		    $err "!rm -Rf \"src/${xp}/${xp}\""
 	done
 
-	find . -name ".git" -exec rm -Rf {} + || err "$_xm: rm .git"
-	find . -name ".gitmodules" -exec rm -Rf {} + || err "$_xm: rm .gitmod"
+	find . -name ".git" -exec rm -Rf {} + || $err "$_xm: rm .git"
+	find . -name ".gitmodules" -exec rm -Rf {} + || $err "$_xm: rm .gitmod"
 	x_ rm -Rf tmp .git
 }
 
 mkrom_images()
 {
-	./build roms all || err "${_xm}: roms-all"
-	./build serprog rp2040 || err "${_xm}: rp2040"
-	./build serprog stm32 || err "${_xm}: stm32"
+	./build roms all || $err "${_xm}: roms-all"
+	./build serprog rp2040 || $err "${_xm}: rp2040"
+	./build serprog stm32 || $err "${_xm}: stm32"
 
 	for rombuild in bin/*; do
 		[ -d "${rombuild}" ] || continue
 		handle_rom_archive "${rombuild}"
 	done
 
-	mv "release/${version}/roms/" ../roms || err "${_xm}: copy roms/"
+	mv "release/${version}/roms/" ../roms || $err "${_xm}: copy roms/"
 }
 
 handle_rom_archive()
 {
 	builddir="${1}"
 	romdir="tmp/romdir"
-	rm -Rf "${romdir}" || err "!rm romdir, handle_rom_archive"
+	rm -Rf "${romdir}" || $err "!rm romdir, handle_rom_archive"
 	target="${builddir##*/}"
 
 	if [ ! -f "config/coreboot/${target}/target.cfg" ]; then
 		# No config, just make a tarball
 		tarball="release/${version}/roms/${relname}_${target}.tar.xz"
 		insert_copying_files "${builddir}" || \
-		    err "!insert copy, handle, ${builddir}"
+		    $err "!insert copy, handle, ${builddir}"
 		mktarball "${builddir}" "${tarball}"
 		return 0
 	fi
 
 	romdir="${romdir}/bin/${target}"
-	mkdir -p "${romdir}" || err "!mkdir -p romdir, handle_rom_archive"
-	cp "$builddir/"* "$romdir" || err "!cp romdir, handle_rom_archive"
+	mkdir -p "${romdir}" || $err "!mkdir -p romdir, handle_rom_archive"
+	cp "$builddir/"* "$romdir" || $err "!cp romdir, handle_rom_archive"
 
 	nukerom
 
 	printf "Generating release/%s/roms/%s-%s_%s.tar.xz\n" \
 	    "${version}" "${projectname}" "${version}" "${target##*/}"
 	insert_version_files "${romdir}" || \
-	    err "mkrom_tarball ${romdir}: versionfile"
+	    $err "mkrom_tarball ${romdir}: versionfile"
 
-	insert_copying_files "$romdir" || err "!insert copy, handle 2, $romdir"
+	insert_copying_files "$romdir" || $err "!insert copy, handle 2, $romdir"
 	mkrom_tarball
 }
 
@@ -152,18 +152,18 @@ nukerom()
 
 	# Hash the images before removing vendor files
 	# which "./vendor inject" uses for verification
-	rm -f "${romdir}/vendorhashes" || err "!rm ${romdir}/vendorhashes"
-	touch "${romdir}/vendorhashes" || err "!touch ${romdir}/vendorhashes"
+	rm -f "${romdir}/vendorhashes" || $err "!rm ${romdir}/vendorhashes"
+	touch "${romdir}/vendorhashes" || $err "!touch ${romdir}/vendorhashes"
 	(
-	cd "${romdir}" || err "!cd romdir ${romdir}, nukerom"
+	cd "${romdir}" || $err "!cd romdir ${romdir}, nukerom"
 	sha512sum ./*.rom >> vendorhashes || \
-	    err "!create vendorhashes, nukerom"
-	) || err "can't create vendor hashes"
+	    $err "!create vendorhashes, nukerom"
+	) || $err "can't create vendor hashes"
 
 	for romfile in "${romdir}"/*.rom; do
 		[ -f "${romfile}" ] || continue
 		./vendor inject -r "$romfile" -b "$target" -n nuke || \
-		    err "!vendor inject (nuke) ${romfile}, nukerom"
+		    $err "!vendor inject (nuke) ${romfile}, nukerom"
 	done
 }
 
@@ -188,13 +188,13 @@ mkrom_tarball()
 {
 	archivename="${relname}_${target##*/}"
 	f="release/${version}/roms/${archivename}"
-	mkdir -p "${f%/*}" || err "mkrom_tarball: !mkdir -p ${f%/*}"
+	mkdir -p "${f%/*}" || $err "mkrom_tarball: !mkdir -p ${f%/*}"
 	(
-	cd "${romdir%"/bin/$target"}" || err "!cd ${romdir%"/bin/$target"}"
+	cd "${romdir%"/bin/$target"}" || $err "!cd ${romdir%"/bin/$target"}"
 	mktarball "bin/${target}" "${archivename}.tar.xz"
-	) || err "can't create rom tarball"
+	) || $err "can't create rom tarball"
 	mv "${romdir%"/bin/${target}"}/${archivename}.tar.xz"* "${f%/*}" || \
-	    err "mktar ${f%/*}/${romdir%"/bin/$target"}/$archivename.tar.xz"
+	    $err "mktar ${f%/*}/${romdir%"/bin/$target"}/$archivename.tar.xz"
 
 	printf "Created ROM archive: %s" "${f%/*}/${archivename}.tar.xz"
 }
@@ -212,20 +212,20 @@ mktarball()
 	tar_implementation=$(tar --version | head -n1) || :
 
 	[ "${2%/*}" = "${2}" ] || \
-		mkdir -p "${2%/*}" || err "mk, !mkdir -p \"${2%/*}\""
+		mkdir -p "${2%/*}" || $err "mk, !mkdir -p \"${2%/*}\""
 	if [ "${tar_implementation% *}" = "tar (GNU tar)" ]; then
 		tar --sort=name --owner=root:0 --group=root:0 \
 		    --mtime="UTC 2024-02-25" -c "$1" | xz -T0 -9e > "$2" || \
-		    err "mktarball 1, ${1}"
+		    $err "mktarball 1, ${1}"
 	else
 		# TODO: reproducible tarballs on non-GNU systems
-		tar -c "$1" | xz -T0 -9e > "$2" || err "mktarball 2, $1"
+		tar -c "$1" | xz -T0 -9e > "$2" || $err "mktarball 2, $1"
 	fi
 	(
 	[ "${2%/*}" != "${2}" ] && x_ cd "${2%/*}"
 	sha512sum "${2##*/}" > "${2##*/}.sha512" || \
-	    err "!sha512sum \"${2##*/}\" > \"${2##*/}.sha512\""
-	) || err "failed to create tarball checksum"
+	    $err "!sha512sum \"${2##*/}\" > \"${2##*/}.sha512\""
+	) || $err "failed to create tarball checksum"
 }
 
 main $@
diff --git a/script/update/trees b/script/update/trees
index 51ea821c..40ff1878 100755
--- a/script/update/trees
+++ b/script/update/trees
@@ -28,12 +28,12 @@ main()
 		-s) mode="savedefconfig" ;;
 		-l) mode="olddefconfig" ;;
 		-n) mode="nconfig" ;;
-		*) err "Invalid option" ;;
+		*) $err "Invalid option" ;;
 		esac
 		shift; project="${OPTARG#src/}"; shift
 	done
-	[ -z "$_f" ] && err "missing flag (-m/-u/-b/-c/-x/-f/-s/-l/-n)"
-	[ -z "$project" ] && err "project name not specified"
+	[ -z "$_f" ] && $err "missing flag (-m/-u/-b/-c/-x/-f/-s/-l/-n)"
+	[ -z "$project" ] && $err "project name not specified"
 	elfdir="elf/${project}"
 	cfgsdir="config/${project}"
 
@@ -66,17 +66,17 @@ build_targets()
 	[ "$elfdir" = "elf/coreboot" ] && \
 		elfdir="elf/coreboot_nopayload_DO_NOT_FLASH"
 
-	[ -d "$cfgsdir" ] || err "directory, $cfgsdir, does not exist"
+	[ -d "$cfgsdir" ] || $err "directory, $cfgsdir, does not exist"
 
 	listfile="${cfgsdir}/build.list"
-	[ -f "$listfile" ] || err "list file, $listfile, does not exist"
+	[ -f "$listfile" ] || $err "list file, $listfile, does not exist"
 
 	# Build for all targets if no argument is given
 	[ $# -gt 0 ] && target1="$1"
 	[ "$target1" = "utils" ] && [ "$project" = "coreboot" ] && \
 		shift 1
 	targets=$(items "$cfgsdir") || \
-	    err "Cannot get options for $cfgsdir"
+	    $err "Cannot get options for $cfgsdir"
 	[ $# -gt 0 ] && targets=$@
 
 	[ -z "$mode" ] && x_ mkdir -p "$elfdir"
@@ -130,7 +130,7 @@ handle_src_tree()
 
 	x_ mkdir -p "${elfdir}/${target}"
 
-	[ -z "$tree" ] && err "handle_src_tree $project/$tree: tree unset"
+	[ -z "$tree" ] && $err "handle_src_tree $project/$tree: tree unset"
 
 	codedir="src/${project}/${tree}"
 
@@ -155,7 +155,7 @@ load_project_config()
 
 	[ -f "${1}/target.cfg" ] || return 0
 	. "${1}/target.cfg" || \
-	    err "load_project_config ${1}: cannot load target.cfg"; return 0
+	    $err "load_project_config ${1}: cannot load target.cfg"; return 0
 }
 
 check_cross_compiler()
@@ -178,7 +178,7 @@ check_cross_compiler()
 
 check_config()
 {
-	[ -f "$config" ] || err "check_config: ${project}/${target}: no config"
+	[ -f "$config" ] || $err "check_config: ${project}/${target}: no config"
 
 	dest_dir="${elfdir}/${target}/${config_name}"
 	# TODO: very hacky check. do it properly (based on build.list)
@@ -197,7 +197,7 @@ handle_makefile()
 	[ -n "$mode" ] || make -C "$codedir" silentoldconfig || \
 	    make -C "$codedir" oldconfig || :
 
-	run_make_command || err "handle_makefile $codedir: no makefile!"
+	run_make_command || $err "handle_makefile $codedir: no makefile!"
 
 	if [ -e "${codedir}/.git" ] && [ "$project" = "u-boot" ] && \
 	    [ "$mode" = "distclean" ]; then
@@ -220,7 +220,7 @@ run_make_command()
 		printf "%s\n" "${version%%-*}" > "$codedir/.coreboot-version"
 
 	make $mode -j$(nproc) $makeargs -C "$codedir" || \
-	    err "run_make $codedir: !make $mode"
+	    $err "run_make $codedir: !make $mode"
 
 	[ "$mode" != "clean" ] && return 0
 	make -C "$codedir" distclean 2>/dev/null || :
@@ -232,9 +232,9 @@ check_cmake()
 		check_makefile "${1}" || \
 		    cmake -B "${1}" "${1}/${cmakedir}" || \
 		    check_makefile "${1}" || \
-		    err "check_cmake ${1}: can't cmake ${cmakedir}"
+		    $err "check_cmake ${1}: can't cmake ${cmakedir}"
 	[ -z "${cmakedir}" ] || check_makefile "${1}" || \
-		err "check_cmake ${1}: could not generate Makefile"
+		$err "check_cmake ${1}: could not generate Makefile"
 	return 0
 }
 
@@ -242,11 +242,11 @@ check_autoconf()
 {
 	(
 	_cfgopt=""
-	cd "${1}" || err "!cd $1"
+	cd "${1}" || $err "!cd $1"
 	[ -f "bootstrap" ] && x_ ./bootstrap $bootstrapargs
 	[ -f "autogen.sh" ] && x_ ./autogen.sh ${autogenargs}
 	[ -f "configure" ] && x_ ./configure $autoconfargs; return 0
-	) || err "can't bootstrap project: $1"
+	) || $err "can't bootstrap project: $1"
 }
 
 check_makefile()
-- 
cgit v1.2.1