Skip to content

Commit ec45445

Browse files
committed
cmd-osbuild: don't use output capture for generate_runvm_osbuild_config
There's a massive gotcha with bash, which is that even with `set -e`, if you're capturing output from a function, e.g. `x=$(myfunc)`, `set -e` will not be turned on in that function. That made an error in that function much harder to diagnose because the command _kept going_ even though the function failed (and then the overall operation failed later on on a much less clear error). There's a bash option to enable this (`shopt inherit_errexit`), but I think actually that in general we shouldn't be capturing output from non-trivial bash functions. It's hard in bash to ensure clean stdout output and the longer a function gets, the harder it is to keep this up (notice e.g. how that function needs to be careful to `echo` everything to stderr). In this case, it's easy to rework the flow here to avoid this, so let's do that instead.
1 parent e4429fa commit ec45445

1 file changed

Lines changed: 4 additions & 4 deletions

File tree

src/cmd-osbuild

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,8 @@ postprocess_qemu_secex() {
163163

164164
# Here we generate the input JSON we pass to runvm_osbuild for all of our image builds
165165
generate_runvm_osbuild_config() {
166-
runvm_osbuild_config_json="${workdir}/tmp/runvm-osbuild-config-${build}.json"
167-
echo "${runvm_osbuild_config_json}" # Let the caller know where the config will be
168-
if [ -f "${runvm_osbuild_config_json}" ]; then
166+
local outfile=$1; shift
167+
if [ -f "${outfile}" ]; then
169168
return # only need to generate this once per build
170169
fi
171170
rm -f "${workdir}"/tmp/runvm-osbuild-config-*.json # clean up any previous configs
@@ -399,7 +398,8 @@ main() {
399398
platforms=("${tobuild[@]}")
400399

401400
# Run OSBuild now to build the platforms that were requested.
402-
runvm_osbuild_config_json="$(generate_runvm_osbuild_config)"
401+
runvm_osbuild_config_json="${workdir}/tmp/runvm-osbuild-config-${build}.json"
402+
generate_runvm_osbuild_config "$runvm_osbuild_config_json"
403403
outdir=$(mktemp -p "${tmp_builddir}" -d)
404404
runvm_with_cache -- /usr/lib/coreos-assembler/runvm-osbuild \
405405
--config "${runvm_osbuild_config_json}" \

0 commit comments

Comments
 (0)