variant: improve error handling in shell scripts
* Add exit handlers to the shell scripts so that if something fails,
we don't leave the repo in a half-committed state.
* Add `set -e` once instead of `|| exit 1` everywhere.
* Bump version numbers in scripts.
BUG=b:149701259
TEST=See detailed test steps in testdata/error_recovery_test.md
Change-Id: Ic038c2cf9e0cd9398bc89edfad8aee96e6669c36
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/dev-util/+/2422447
Tested-by: Paul Fagerburg <pfagerburg@chromium.org>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Commit-Queue: Paul Fagerburg <pfagerburg@chromium.org>
diff --git a/contrib/variant/add_variant_to_yaml.sh b/contrib/variant/add_variant_to_yaml.sh
index 285332a..15a535f 100755
--- a/contrib/variant/add_variant_to_yaml.sh
+++ b/contrib/variant/add_variant_to_yaml.sh
@@ -3,8 +3,9 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-VERSION="1.2.4"
+VERSION="1.3.0"
SCRIPT=$(basename -- "${0}")
+set -e
export LC_ALL=C
@@ -44,7 +45,7 @@
YAML=model.yaml
-cd "${HOME}/trunk/src/overlays/overlay-${BASE}/chromeos-base/chromeos-config-bsp-${BASE}/files" || exit 1
+cd "${HOME}/trunk/src/overlays/overlay-${BASE}/chromeos-base/chromeos-config-bsp-${BASE}/files"
if [[ ! -e "${YAML}" ]]; then
echo "${YAML} does not exist."
@@ -61,12 +62,28 @@
# Start a branch. Use YMD timestamp to avoid collisions.
DATE=$(date +%Y%m%d)
BRANCH="create_${VARIANT}_${DATE}"
-repo start "${BRANCH}" . || exit 1
+repo start "${BRANCH}" .
+
+cleanup() {
+ # If there is an error after the `repo start`, then restore modified files
+ # to clean up and `repo abandon` the new branch.
+ cd "${HOME}/trunk/src/overlays/overlay-${BASE}/chromeos-base/chromeos-config-bsp-${BASE}"
+ git restore --staged "*.ebuild"
+ git restore "*.ebuild"
+ if [[ ! -z "${NEWEBUILD}" ]] ; then
+ rm -f "${NEWEBUILD}"
+ fi
+ cd files
+ git restore --staged "${YAML}"
+ git restore "${YAML}"
+ repo abandon "${BRANCH}" .
+}
+trap 'cleanup' ERR
# ebuild is located 1 directory up.
-pushd .. || exit 1
+pushd ..
revbump_ebuild
-popd || exit 1
+popd
# Append a new device-name to the end of the model.yaml file.
cat <<EOF >>"${YAML}"
@@ -79,7 +96,6 @@
config: *base_config
EOF
git add "${YAML}"
-
# Building the config.json and other files from the yaml requires that the
# changes have been made to both the public and private yaml files, which
# we can't guarantee here, so we will not automate the steps in the TEST=
diff --git a/contrib/variant/copy_cras_config.sh b/contrib/variant/copy_cras_config.sh
index f381d8d..755af06 100755
--- a/contrib/variant/copy_cras_config.sh
+++ b/contrib/variant/copy_cras_config.sh
@@ -3,8 +3,9 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-VERSION="1.0.2"
+VERSION="1.1.0"
SCRIPT=$(basename -- "${0}")
+set -e
export LC_ALL=C
@@ -35,30 +36,47 @@
# This is the name of the reference board that we're using to make the variant.
# ${var,,} converts to all lowercase.
REFERENCE="${2,,}"
-# ${var^} capitalizes first letter only.
-REFERENCE_CAPITALIZED="${REFERENCE^}"
# This is the name of the variant that is being cloned.
VARIANT="${3,,}"
-VARIANT_CAPITALIZED="${VARIANT^}"
# Assign BUG= text, or "None" if that parameter wasn't specified.
BUG=${4:-None}
-cd "${HOME}/trunk/src/overlays/overlay-${BASE}/chromeos-base/chromeos-bsp-${BASE}/files/cras-config" || exit 1
+cd "${HOME}/trunk/src/overlays/overlay-${BASE}/chromeos-base/chromeos-bsp-${BASE}/files/cras-config"
# Start a branch. Use YMD timestamp to avoid collisions.
DATE=$(date +%Y%m%d)
BRANCH="create_${VARIANT}_${DATE}"
-repo start "${BRANCH}" . || exit 1
+repo start "${BRANCH}" .
+
+cleanup() {
+ # If there is an error after the `repo start`, then restore modified files
+ # to clean up and `repo abandon` the new branch.
+ cd "${HOME}/trunk/src/overlays/overlay-${BASE}/chromeos-base/chromeos-bsp-${BASE}"
+ git restore --staged "*.ebuild"
+ git restore "*.ebuild"
+ if [[ ! -z "${NEWEBUILD}" ]] ; then
+ rm -f "${NEWEBUILD}"
+ fi
+ cd "files/cras-config"
+ if [[ -e "${VARIANT}" ]] ; then
+ rm -Rf "${VARIANT}"
+ # Use || true so that if the new files haven't been added yet, the error
+ # won't terminate the script before we can finish cleaning up.
+ git restore --staged "${VARIANT}" || true
+ fi
+ repo abandon "${BRANCH}" .
+}
+trap 'cleanup' ERR
# ebuild will be located 2 directories up.
-pushd ../.. || exit 1
+pushd ../..
revbump_ebuild
-popd || exit 1
+popd
-mkdir "${VARIANT_CAPITALIZED}"
-cp "${REFERENCE_CAPITALIZED}"/* "${VARIANT_CAPITALIZED}"
-git add "${VARIANT_CAPITALIZED}"
+mkdir "${VARIANT}"
+cp "${REFERENCE}"/* "${VARIANT}"
+git add "${VARIANT}"
git commit -m "${BASE}: Add ${VARIANT} cras config
Create a new cras config for the ${VARIANT} variant as a
diff --git a/contrib/variant/create_coreboot_config.sh b/contrib/variant/create_coreboot_config.sh
index ea15ab4..b0056db 100755
--- a/contrib/variant/create_coreboot_config.sh
+++ b/contrib/variant/create_coreboot_config.sh
@@ -3,8 +3,9 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-VERSION="2.1.2"
+VERSION="2.2.0"
SCRIPT=$(basename -- "${0}")
+set -e
export LC_ALL=C
@@ -44,7 +45,8 @@
# Work in third_party/chromiumos-overlay/sys-boot/coreboot/files/configs
# unless CB_CONFIG_DIR is set, in which case work in that dir
DEFAULT_CB_CONFIG_DIR="third_party/chromiumos-overlay/sys-boot/coreboot/files/configs"
-cd "${HOME}/trunk/src/${CB_CONFIG_DIR:-${DEFAULT_CB_CONFIG_DIR}}" || exit 1
+CB="${HOME}/trunk/src/${CB_CONFIG_DIR:-${DEFAULT_CB_CONFIG_DIR}}"
+cd "${CB}"
# Make sure the variant doesn't already exist.
if [[ -e "config.${VARIANT}" ]]; then
@@ -56,7 +58,21 @@
# Start a branch. Use YMD timestamp to avoid collisions.
DATE=$(date +%Y%m%d)
BRANCH="create_${VARIANT}_${DATE}"
-repo start "${BRANCH}" . || exit 1
+repo start "${BRANCH}" .
+
+cleanup() {
+ # If there is an error after the `repo start`, then remove the added files
+ # and `repo abandon` the new branch.
+ cd "${CB}"
+ if [[ -e "config.${VARIANT}" ]] ; then
+ rm "config.${VARIANT}"
+ # Use || true so that if the new files haven't been added yet, the error
+ # won't terminate the script before we can finish cleaning up.
+ git restore --staged "config.${VARIANT}" || true
+ fi
+ repo abandon "${BRANCH}" .
+}
+trap 'cleanup' ERR
# There are multiple usages of the reference board name that we want to change,
# using the Hatch reference board and the Kohaku variant in this example.
diff --git a/contrib/variant/create_coreboot_variant.sh b/contrib/variant/create_coreboot_variant.sh
index b0dbb2d..419a509 100755
--- a/contrib/variant/create_coreboot_variant.sh
+++ b/contrib/variant/create_coreboot_variant.sh
@@ -3,11 +3,13 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
+VERSION="4.1.3"
+SCRIPT="$(basename -- "$0")"
set -e
-VERSION="4.1.2"
-SCRIPT="$(basename -- "$0")"
-
+# Disable a warning about CB_SRC_DIR not being defined; we look for it in
+# the environment, and if it isn't found, then we'll exit.
+# shellcheck disable=SC2154
if [[ -z "${CB_SRC_DIR}" ]]; then
echo "CB_SRC_DIR must be set in the environment"
exit 1
@@ -82,10 +84,11 @@
# If there is an error after the `repo start` and before we start adding
# changes to git, then delete the new variant directory and `repo abandon`
# the new branch.
- rm -Rf "variants/$1"
- repo abandon "$2" .
+ cd "${CB_SRC_DIR}/src/mainboard/google/${BASE}"
+ rm -Rf "variants/${VARIANT}"
+ repo abandon "${BRANCH}" .
}
-trap 'abandon "${VARIANT}" "${BRANCH}"' ERR
+trap 'abandon' ERR
# Copy the template tree to the target.
mkdir -p "variants/${VARIANT}/"
@@ -101,15 +104,15 @@
# After adding changes to git, now to recover from an error we need to
# remove the variant from the git commit, restore Kconfig and Kconfig.name,
# and delete the .new files if they exist.
- rm -Rf "variants/$1"
- git restore --staged "variants/$1"
+ cd "${CB_SRC_DIR}/src/mainboard/google/${BASE}"
+ git restore --staged "variants/${VARIANT}"
git restore --staged Kconfig Kconfig.name
git restore Kconfig Kconfig.name
rm -f Kconfig.new Kconfig.name.new
# And call the previous trap function.
- abandon "$1" "$2"
+ abandon
}
-trap 'restore_git "${VARIANT}" "${BRANCH}"' ERR
+trap 'restore_git' ERR
# Now add the new variant to Kconfig and Kconfig.name
# These files are in the current directory, e.g. src/mainboard/google/hatch
diff --git a/contrib/variant/create_initial_ec_image.sh b/contrib/variant/create_initial_ec_image.sh
index 1ea99db..07c4616 100755
--- a/contrib/variant/create_initial_ec_image.sh
+++ b/contrib/variant/create_initial_ec_image.sh
@@ -3,8 +3,9 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-VERSION="1.0.4"
+VERSION="1.1.0"
SCRIPT=$(basename -- "${0}")
+set -e
export LC_ALL=C
@@ -34,7 +35,7 @@
BUG=${3:-None}
# All of the necessary files are in platform/ec/board
-cd "${HOME}/trunk/src/platform/ec/board" || exit 1
+cd "${HOME}/trunk/src/platform/ec/board"
# Make sure that the reference board exists.
if [[ ! -e "${REF}" ]]; then
@@ -51,7 +52,21 @@
# Start a branch. Use YMD timestamp to avoid collisions.
DATE=$(date +%Y%m%d)
BRANCH="create_${VARIANT}_${DATE}"
-repo start "${BRANCH}" . || exit 1
+repo start "${BRANCH}" .
+
+cleanup() {
+ # If there is an error after the `repo start`, then remove the added files
+ # and `repo abandon` the new branch.
+ cd "${HOME}/trunk/src/platform/ec/board"
+ if [[ -e "${VARIANT}" ]] ; then
+ rm -Rf "${VARIANT}"
+ # Use || true so that if the new files haven't been added yet, the error
+ # won't terminate the script before we can finish cleaning up.
+ git restore --staged "${VARIANT}" || true
+ fi
+ repo abandon "${BRANCH}" .
+}
+trap 'cleanup' ERR
mkdir "${VARIANT}"
cp "${REF}"/* "${VARIANT}"
@@ -62,9 +77,9 @@
sed -i -e "s/Copyright.*20[0-9][0-9]/Copyright ${YEAR}/" {} +
# Build the code; exit if it fails.
-pushd .. || exit 1
-make -j BOARD="${VARIANT}" || exit 1
-popd || exit 1
+pushd ..
+make -j BOARD="${VARIANT}"
+popd
git add "${VARIANT}"/*
diff --git a/contrib/variant/fw_build_config.sh b/contrib/variant/fw_build_config.sh
index 8925528..11aa978 100755
--- a/contrib/variant/fw_build_config.sh
+++ b/contrib/variant/fw_build_config.sh
@@ -3,7 +3,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-VERSION="1.0.2"
+VERSION="1.0.3"
SCRIPT=$(basename -- "${0}")
set -e
@@ -44,7 +44,7 @@
BRANCH="create_${VARIANT}_${DATE}"
repo start "${BRANCH}" .
-function catch() {
+cleanup() {
# If there is an error after the `repo start`, then restore modified files
# to clean up. `git restore --staged` will remove files added to a commit,
# and `git restore` will restore the file to its unmodified state. Then
@@ -57,7 +57,7 @@
git restore sw_build_config/platform/chromeos-config/generated/project-config.json
repo abandon "$1" .
}
-trap 'catch "${BRANCH}"' ERR
+trap 'cleanup "${BRANCH}"' ERR
# Change the _FW_BUILD_CONFIG from None to _${VARIANT_UPPER}.
sed -i -e "s/_FW_BUILD_CONFIG = None/_FW_BUILD_CONFIG = program.firmware_build_config(_${VARIANT_UPPER})/" config.star
diff --git a/contrib/variant/testdata/error_recovery_test.md b/contrib/variant/testdata/error_recovery_test.md
new file mode 100644
index 0000000..7fecca4
--- /dev/null
+++ b/contrib/variant/testdata/error_recovery_test.md
@@ -0,0 +1,87 @@
+# Test error recovery in shell scripts
+
+To test the error recovery functions in the various shell scripts, add
+`false` in various places to induce failure and observe that everything
+is cleaned up.
+
+## add_variant_to_yaml.sh
+
+For each test case, run `add_variant_to_yaml.sh hatch tiamat`
+
+Add `false` at the following places in the script:
+* before `revbump_ebuild`
+* before `git add "${YAML}"`
+* before `git commit`
+* after `git commit`
+
+Check `/mnt/host/source/src/overlays` to ensure the branch has been deleted
+and no changes or new files are left.
+
+## copy_cras_config.sh (zork only)
+
+For each test case, run `copy_cras_config.sh zork trembyle grue`
+
+Add `false` at the following places in the script:
+* before `revbump_ebuild`
+* before `mkdir`
+* before `cp`
+* before `git add`
+* before `git commit`
+* after `git commit`
+
+Check `/mnt/host/source/src/overlays` to ensure the branch has been deleted
+and no changes or new files are left.
+
+## create_coreboot_config.sh
+
+For each test case, run `create_coreboot_config.sh hatch hatch tiamat`
+
+Add `false` at the following places in the script:
+* before `sed`
+* before `git add`
+* before `git commit`
+* after `git commit`
+
+Check `/mnt/host/source/src/third_party/chromiumos-overlay` to ensure the
+branch has been deleted and no changes or new files are left.
+
+## create_coreboot_variant.sh
+
+For each test case, run `CB_SRC_DIR=/mnt/host/source/src/third_party/coreboot
+create_coreboot_variant.sh hatch hatch tiamat`
+
+Add `false` at the following places in the script:
+* before `kconfig.py`
+* before `mv`
+* before `git add`
+* before `git commit`
+* after `git commit`
+
+Check `/mnt/host/source/src/third_party/coreboot` to ensure the branch has been
+deleted and no changes or new files are left.
+
+## create_initial_ec_image.sh
+
+For each test case, run `create_initial_ec_image.sh hatch tiamat`
+
+Add `false` at the following places in the script:
+* before `mkdir`
+* before `cp`
+* before `find`
+* before `make`
+* before `git add`
+* before `git commit`
+* after `git commit`
+
+Check `/mnt/host/source/src/platform/ec/board` to ensure the branch has been
+deleted and no changes or new files are left.
+
+## Others
+
+`fw_build_config.sh` is not tested here. Because of the scripts use `repo`
+and `git` commands, it is not sufficient to create a directory with the
+required files.
+
+The private scripts (mainly `add_variant.sh` in
+`private-overlays/baseboard-hatch-private`) will have separate test
+instructions.
