On 03/11/19 13:07, Eric Blake wrote: > On 3/8/19 6:48 PM, Laszlo Ersek wrote: >> Extract the dense logic for architecture and toolchain massaging from >> "tests/uefi-test-tools/build.sh", to a set of small functions. We'll reuse >> these functions for building full platform firmware images. >> >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >> --- >> roms/edk2-funcs.sh | 240 ++++++++++++++++++++ >> tests/uefi-test-tools/build.sh | 97 +------- >> 2 files changed, 246 insertions(+), 91 deletions(-) >> >> diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh >> new file mode 100644 >> index 000000000000..908c7665c6ed >> --- /dev/null >> +++ b/roms/edk2-funcs.sh >> @@ -0,0 +1,240 @@ >> +# Shell script that defines functions for determining some environmental > > No #! line, so this runs with /bin/sh, which might not be bash. However,... > >> +# characteristics for the edk2 "build" utility. >> +# >> +# This script is meant to be sourced. >> +# >> +# Copyright (C) 2019, Red Hat, Inc. > > I don't usually put a comma after the year. > > >> +# Parameters: >> +# $1: QEMU system emulation target >> +qemu_edk2_verify_arch() >> +{ >> + local emulation_target="$1" >> + local program_name=$(basename -- "$0") > > ...local is a bashism, not present in all /bin/sh implementations. Then > again... > >> +++ b/tests/uefi-test-tools/build.sh >> @@ -38,97 +38,12 @@ if [ $ret -ne 0 ]; then > > build.sh is already marked as a bash script, and... > >> +# Fetch some option arguments, and set the cross-compilation environment (if >> +# any), for the edk2 "build" utility. >> +source "$edk2_dir/../edk2-funcs.sh" > > you are only ever sourcing the file, rather than directly executing it > as a standalone script. I'd update the comments to mention that the file > is meant to be sourced. (By the way, 'source' is also a bashism, the > portable spelling is '.'). > > So nothing jumped out at me as a potential shell script trap, although I > did not closely review the logic. >
Thanks! I'll drop the comma after the year. Laszlo