diff mbox series

[01/10] roms: lift "edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"

Message ID 20190309004826.9027-2-lersek@redhat.com
State New
Headers show
Series bundle edk2 platform firmware with QEMU | expand

Commit Message

Laszlo Ersek March 9, 2019, 12:48 a.m. UTC
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 <lersek@redhat.com>
---
 roms/edk2-funcs.sh             | 240 ++++++++++++++++++++
 tests/uefi-test-tools/build.sh |  97 +-------
 2 files changed, 246 insertions(+), 91 deletions(-)

Comments

Philippe Mathieu-Daudé March 10, 2019, 3:17 p.m. UTC | #1
On 3/9/19 1:48 AM, 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 <lersek@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
> +# characteristics for the edk2 "build" utility.
> +#
> +# This script is meant to be sourced.
> +#
> +# Copyright (C) 2019, Red Hat, Inc.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License that accompanies this
> +# distribution. The full text of the license may be found at
> +# <http://opensource.org/licenses/bsd-license.php>.
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +
> +# Verify whether the QEMU system emulation target is supported by the UEFI spec
> +# and edk2. Print a message to the standard error, and return with nonzero
> +# status, if verification fails.
> +#
> +# Parameters:
> +#   $1: QEMU system emulation target
> +qemu_edk2_verify_arch()
> +{
> +  local emulation_target="$1"
> +  local program_name=$(basename -- "$0")
> +
> +  case "$emulation_target" in
> +    (arm|aarch64|i386|x86_64)
> +      ;;
> +    (*)
> +      printf '%s: unknown/unsupported QEMU system emulation target "%s"\n' \
> +        "$program_name" "$emulation_target" >&2
> +      return 1
> +      ;;
> +  esac
> +}
> +
> +
> +# Translate the QEMU system emulation target to the edk2 architecture
> +# identifier. Print the result to the standard output.
> +#
> +# Parameters:
> +#   $1: QEMU system emulation target
> +qemu_edk2_get_arch()
> +{
> +  local emulation_target="$1"
> +
> +  if ! qemu_edk2_verify_arch "$emulation_target"; then
> +    return 1
> +  fi
> +
> +  case "$emulation_target" in
> +    (arm)
> +      printf 'ARM\n'
> +      ;;
> +    (aarch64)
> +      printf 'AARCH64\n'
> +      ;;
> +    (i386)
> +      printf 'IA32\n'
> +      ;;
> +    (x86_64)
> +      printf 'X64\n'
> +      ;;
> +  esac
> +}
> +
> +
> +# Translate the QEMU system emulation target to the gcc cross-compilation
> +# architecture identifier. Print the result to the standard output.
> +#
> +# Parameters:
> +#   $1: QEMU system emulation target
> +qemu_edk2_get_gcc_arch()
> +{
> +  local emulation_target="$1"
> +
> +  if ! qemu_edk2_verify_arch "$emulation_target"; then
> +    return 1
> +  fi
> +
> +  case "$emulation_target" in
> +    (arm|aarch64|x86_64)
> +      printf '%s\n' "$emulation_target"
> +      ;;
> +    (i386)
> +      printf 'i686\n'
> +      ;;
> +  esac
> +}
> +
> +
> +# Determine the gcc cross-compiler prefix (if any) for use with the edk2
> +# toolchain. Print the result to the standard output.
> +#
> +# Parameters:
> +#   $1: QEMU system emulation target
> +qemu_edk2_get_cross_prefix()
> +{
> +  local emulation_target="$1"
> +  local gcc_arch
> +  local host_arch
> +
> +  if ! gcc_arch=$(qemu_edk2_get_gcc_arch "$emulation_target"); then
> +    return 1
> +  fi
> +
> +  host_arch=$(uname -m)
> +
> +  if [ "$gcc_arch" == "$host_arch" ] ||
> +     ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
> +    # no cross-compiler needed
> +    :
> +  else
> +    printf '%s-linux-gnu-\n' "$gcc_arch"
> +  fi
> +}
> +
> +
> +# Determine the edk2 toolchain tag for the QEMU system emulation target. Print
> +# the result to the standard output. Print a message to the standard error, and
> +# return with nonzero status, if the (conditional) gcc version check fails.
> +#
> +# Parameters:
> +#   $1: QEMU system emulation target
> +qemu_edk2_get_toolchain()
> +{
> +  local emulation_target="$1"
> +  local program_name=$(basename -- "$0")
> +  local cross_prefix
> +  local gcc_version
> +
> +  if ! qemu_edk2_verify_arch "$emulation_target"; then
> +    return 1
> +  fi
> +
> +  case "$emulation_target" in
> +    (arm|aarch64)
> +      printf 'GCC5\n'
> +      ;;
> +
> +    (i386|x86_64)
> +      if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
> +        return 1
> +      fi
> +
> +      gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
> +      # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
> +      # the mapping below.
> +      case "$gcc_version" in
> +        ([1-3].*|4.[0-3].*)
> +          printf '%s: unsupported gcc version "%s"\n' \
> +            "$program_name" "$gcc_version" >&2
> +          return 1
> +          ;;
> +        (4.4.*)
> +          printf 'GCC44\n'
> +          ;;
> +        (4.5.*)
> +          printf 'GCC45\n'
> +          ;;
> +        (4.6.*)
> +          printf 'GCC46\n'
> +          ;;
> +        (4.7.*)
> +          printf 'GCC47\n'
> +          ;;
> +        (4.8.*)
> +          printf 'GCC48\n'
> +          ;;
> +        (4.9.*|6.[0-2].*)
> +          printf 'GCC49\n'
> +          ;;
> +        (*)
> +          printf 'GCC5\n'
> +          ;;
> +      esac
> +      ;;
> +  esac
> +}
> +
> +
> +# Determine the name of the environment variable that exposes the
> +# cross-compiler prefix to the edk2 "build" utility. Print the result to the
> +# standard output.
> +#
> +# Parameters:
> +#   $1: QEMU system emulation target
> +qemu_edk2_get_cross_prefix_var()
> +{
> +  local emulation_target="$1"
> +  local edk2_toolchain
> +  local edk2_arch
> +
> +  if ! edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target"); then
> +    return 1
> +  fi
> +
> +  case "$emulation_target" in
> +    (arm|aarch64)
> +      if ! edk2_arch=$(qemu_edk2_get_arch "$emulation_target"); then
> +        return 1
> +      fi
> +      printf '%s_%s_PREFIX\n' "$edk2_toolchain" "$edk2_arch"
> +      ;;
> +    (i386|x86_64)
> +      printf '%s_BIN\n' "$edk2_toolchain"
> +      ;;
> +  esac
> +}
> +
> +
> +# Set and export the environment variable(s) necessary for cross-compilation,
> +# whenever needed by the edk2 "build" utility.
> +#
> +# Parameters:
> +#   $1: QEMU system emulation target
> +qemu_edk2_set_cross_env()
> +{
> +  local emulation_target="$1"
> +  local cross_prefix
> +  local cross_prefix_var
> +
> +  if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
> +    return 1
> +  fi
> +
> +  if [ -z "$cross_prefix" ]; then
> +    # Nothing to do.
> +    return 0
> +  fi
> +
> +  if ! cross_prefix_var=$(qemu_edk2_get_cross_prefix_var \
> +                            "$emulation_target"); then
> +    return 1
> +  fi
> +
> +  eval "export $cross_prefix_var=\$cross_prefix"
> +}
> diff --git a/tests/uefi-test-tools/build.sh b/tests/uefi-test-tools/build.sh
> index 155cb75c4ddb..e2b52c855c39 100755
> --- a/tests/uefi-test-tools/build.sh
> +++ b/tests/uefi-test-tools/build.sh
> @@ -38,97 +38,12 @@ if [ $ret -ne 0 ]; then
>    exit $ret
>  fi
>  
> -# Map the QEMU system emulation target to the following types of architecture
> -# identifiers:
> -# - edk2,
> -# - gcc cross-compilation.
> -# Cover only those targets that are supported by the UEFI spec and edk2.
> -case "$emulation_target" in
> -  (arm)
> -    edk2_arch=ARM
> -    gcc_arch=arm
> -    ;;
> -  (aarch64)
> -    edk2_arch=AARCH64
> -    gcc_arch=aarch64
> -    ;;
> -  (i386)
> -    edk2_arch=IA32
> -    gcc_arch=i686
> -    ;;
> -  (x86_64)
> -    edk2_arch=X64
> -    gcc_arch=x86_64
> -    ;;
> -  (*)
> -    printf '%s: unknown/unsupported QEMU system emulation target "%s"\n' \
> -      "$program_name" "$emulation_target" >&2
> -    exit 1
> -    ;;
> -esac
> -
> -# Check if cross-compilation is needed.
> -host_arch=$(uname -m)
> -if [ "$gcc_arch" == "$host_arch" ] ||
> -   ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
> -  cross_prefix=
> -else
> -  cross_prefix=${gcc_arch}-linux-gnu-
> -fi
> -
> -# Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
> -# determine the suitable edk2 toolchain as well.
> -# - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
> -#   the gcc-5+ releases.
> -# - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
> -#   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
> -#   the actual gcc releases isn't entirely trivial. Run "git-blame" on
> -#   "OvmfPkg/build.sh" in edk2 for more information.
> -# And, because the above is too simple, we have to assign cross_prefix to an
> -# edk2 build variable that is specific to both the toolchain tag and the target
> -# architecture.
> -case "$edk2_arch" in
> -  (ARM)
> -    edk2_toolchain=GCC5
> -    export GCC5_ARM_PREFIX=$cross_prefix
> -    ;;
> -  (AARCH64)
> -    edk2_toolchain=GCC5
> -    export GCC5_AARCH64_PREFIX=$cross_prefix
> -    ;;
> -  (IA32|X64)
> -    gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
> -    case "$gcc_version" in
> -      ([1-3].*|4.[0-3].*)
> -        printf '%s: unsupported gcc version "%s"\n' \
> -          "$program_name" "$gcc_version" >&2
> -        exit 1
> -        ;;
> -      (4.4.*)
> -        edk2_toolchain=GCC44
> -        ;;
> -      (4.5.*)
> -        edk2_toolchain=GCC45
> -        ;;
> -      (4.6.*)
> -        edk2_toolchain=GCC46
> -        ;;
> -      (4.7.*)
> -        edk2_toolchain=GCC47
> -        ;;
> -      (4.8.*)
> -        edk2_toolchain=GCC48
> -        ;;
> -      (4.9.*|6.[0-2].*)
> -        edk2_toolchain=GCC49
> -        ;;
> -      (*)
> -        edk2_toolchain=GCC5
> -        ;;
> -    esac
> -    eval "export ${edk2_toolchain}_BIN=\$cross_prefix"
> -    ;;
> -esac
> +# Fetch some option arguments, and set the cross-compilation environment (if
> +# any), for the edk2 "build" utility.
> +source "$edk2_dir/../edk2-funcs.sh"
> +edk2_arch=$(qemu_edk2_get_arch "$emulation_target")
> +edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
> +qemu_edk2_set_cross_env "$emulation_target"
>  
>  # Build the UEFI binary
>  mkdir -p log
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Philippe Mathieu-Daudé March 10, 2019, 5:23 p.m. UTC | #2
Hi Laszlo,

On 3/10/19 4:17 PM, Philippe Mathieu-Daudé wrote:
> On 3/9/19 1:48 AM, 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 <lersek@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
>> +# characteristics for the edk2 "build" utility.
>> +#
>> +# This script is meant to be sourced.
>> +#
>> +# Copyright (C) 2019, Red Hat, Inc.
>> +#
>> +# This program and the accompanying materials are licensed and made available
>> +# under the terms and conditions of the BSD License that accompanies this
>> +# distribution. The full text of the license may be found at
>> +# <http://opensource.org/licenses/bsd-license.php>.
>> +#
>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
>> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +
>> +# Verify whether the QEMU system emulation target is supported by the UEFI spec
>> +# and edk2. Print a message to the standard error, and return with nonzero
>> +# status, if verification fails.
>> +#
>> +# Parameters:
>> +#   $1: QEMU system emulation target
>> +qemu_edk2_verify_arch()
>> +{
>> +  local emulation_target="$1"
>> +  local program_name=$(basename -- "$0")
>> +
>> +  case "$emulation_target" in
>> +    (arm|aarch64|i386|x86_64)
>> +      ;;
>> +    (*)
>> +      printf '%s: unknown/unsupported QEMU system emulation target "%s"\n' \
>> +        "$program_name" "$emulation_target" >&2
>> +      return 1
>> +      ;;
>> +  esac
>> +}
>> +
>> +
>> +# Translate the QEMU system emulation target to the edk2 architecture
>> +# identifier. Print the result to the standard output.
>> +#
>> +# Parameters:
>> +#   $1: QEMU system emulation target
>> +qemu_edk2_get_arch()
>> +{
>> +  local emulation_target="$1"
>> +
>> +  if ! qemu_edk2_verify_arch "$emulation_target"; then
>> +    return 1
>> +  fi
>> +
>> +  case "$emulation_target" in
>> +    (arm)
>> +      printf 'ARM\n'
>> +      ;;
>> +    (aarch64)
>> +      printf 'AARCH64\n'
>> +      ;;
>> +    (i386)
>> +      printf 'IA32\n'
>> +      ;;
>> +    (x86_64)
>> +      printf 'X64\n'
>> +      ;;
>> +  esac
>> +}
>> +
>> +
>> +# Translate the QEMU system emulation target to the gcc cross-compilation
>> +# architecture identifier. Print the result to the standard output.
>> +#
>> +# Parameters:
>> +#   $1: QEMU system emulation target
>> +qemu_edk2_get_gcc_arch()
>> +{
>> +  local emulation_target="$1"
>> +
>> +  if ! qemu_edk2_verify_arch "$emulation_target"; then
>> +    return 1
>> +  fi
>> +
>> +  case "$emulation_target" in
>> +    (arm|aarch64|x86_64)
>> +      printf '%s\n' "$emulation_target"
>> +      ;;
>> +    (i386)
>> +      printf 'i686\n'
>> +      ;;
>> +  esac
>> +}
>> +
>> +
>> +# Determine the gcc cross-compiler prefix (if any) for use with the edk2
>> +# toolchain. Print the result to the standard output.
>> +#
>> +# Parameters:
>> +#   $1: QEMU system emulation target
>> +qemu_edk2_get_cross_prefix()
>> +{
>> +  local emulation_target="$1"
>> +  local gcc_arch
>> +  local host_arch
>> +
>> +  if ! gcc_arch=$(qemu_edk2_get_gcc_arch "$emulation_target"); then
>> +    return 1
>> +  fi
>> +
>> +  host_arch=$(uname -m)
>> +
>> +  if [ "$gcc_arch" == "$host_arch" ] ||
>> +     ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
>> +    # no cross-compiler needed
>> +    :
>> +  else
>> +    printf '%s-linux-gnu-\n' "$gcc_arch"

Testing on Ubuntu (bionic), the gcc-arm-linux-gnueabihf provides the
arm-linux-gnueabihf-gcc binary, therefore I use:

GCC5_ARM_PREFIX="arm-linux-gnueabihf-" make -C roms efi

But this function enforce cross prefix to be arm-linux-gnu- :(

I tried with the following Linaro toolchains:

gcc-linaro-4.9.4-2017.01-x86_64_arm-linux-gnueabihf.tar.xz
gcc-linaro-5.5.0-2017.10-x86_64_arm-linux-gnueabihf.tar.xz
gcc-linaro-7.2.1-2017.11-x86_64_arm-linux-gnueabihf.tar.xz

With the distrib toolchain or the Linaro enumerated, I get:

/bin/sh: 1: arm-linux-gnu-gcc: not found

>> +  fi
>> +}
>> +
>> +
>> +# Determine the edk2 toolchain tag for the QEMU system emulation target. Print
>> +# the result to the standard output. Print a message to the standard error, and
>> +# return with nonzero status, if the (conditional) gcc version check fails.
>> +#
>> +# Parameters:
>> +#   $1: QEMU system emulation target
>> +qemu_edk2_get_toolchain()
>> +{
>> +  local emulation_target="$1"
>> +  local program_name=$(basename -- "$0")
>> +  local cross_prefix
>> +  local gcc_version
>> +
>> +  if ! qemu_edk2_verify_arch "$emulation_target"; then
>> +    return 1
>> +  fi
>> +
>> +  case "$emulation_target" in
>> +    (arm|aarch64)
>> +      printf 'GCC5\n'
>> +      ;;
>> +
>> +    (i386|x86_64)
>> +      if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
>> +        return 1
>> +      fi
>> +
>> +      gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
>> +      # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
>> +      # the mapping below.
>> +      case "$gcc_version" in
>> +        ([1-3].*|4.[0-3].*)
>> +          printf '%s: unsupported gcc version "%s"\n' \
>> +            "$program_name" "$gcc_version" >&2
>> +          return 1
>> +          ;;
>> +        (4.4.*)
>> +          printf 'GCC44\n'
>> +          ;;
>> +        (4.5.*)
>> +          printf 'GCC45\n'
>> +          ;;
>> +        (4.6.*)
>> +          printf 'GCC46\n'
>> +          ;;
>> +        (4.7.*)
>> +          printf 'GCC47\n'
>> +          ;;
>> +        (4.8.*)
>> +          printf 'GCC48\n'
>> +          ;;
>> +        (4.9.*|6.[0-2].*)
>> +          printf 'GCC49\n'
>> +          ;;
>> +        (*)
>> +          printf 'GCC5\n'
>> +          ;;
>> +      esac
>> +      ;;
>> +  esac
>> +}
>> +
>> +
>> +# Determine the name of the environment variable that exposes the
>> +# cross-compiler prefix to the edk2 "build" utility. Print the result to the
>> +# standard output.
>> +#
>> +# Parameters:
>> +#   $1: QEMU system emulation target
>> +qemu_edk2_get_cross_prefix_var()
>> +{
>> +  local emulation_target="$1"
>> +  local edk2_toolchain
>> +  local edk2_arch
>> +
>> +  if ! edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target"); then
>> +    return 1
>> +  fi
>> +
>> +  case "$emulation_target" in
>> +    (arm|aarch64)
>> +      if ! edk2_arch=$(qemu_edk2_get_arch "$emulation_target"); then
>> +        return 1
>> +      fi
>> +      printf '%s_%s_PREFIX\n' "$edk2_toolchain" "$edk2_arch"
>> +      ;;
>> +    (i386|x86_64)
>> +      printf '%s_BIN\n' "$edk2_toolchain"
>> +      ;;
>> +  esac
>> +}
>> +
>> +
>> +# Set and export the environment variable(s) necessary for cross-compilation,
>> +# whenever needed by the edk2 "build" utility.
>> +#
>> +# Parameters:
>> +#   $1: QEMU system emulation target
>> +qemu_edk2_set_cross_env()
>> +{
>> +  local emulation_target="$1"
>> +  local cross_prefix
>> +  local cross_prefix_var
>> +
>> +  if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
>> +    return 1
>> +  fi
>> +
>> +  if [ -z "$cross_prefix" ]; then
>> +    # Nothing to do.
>> +    return 0
>> +  fi
>> +
>> +  if ! cross_prefix_var=$(qemu_edk2_get_cross_prefix_var \
>> +                            "$emulation_target"); then
>> +    return 1
>> +  fi
>> +
>> +  eval "export $cross_prefix_var=\$cross_prefix"
>> +}
>> diff --git a/tests/uefi-test-tools/build.sh b/tests/uefi-test-tools/build.sh
>> index 155cb75c4ddb..e2b52c855c39 100755
>> --- a/tests/uefi-test-tools/build.sh
>> +++ b/tests/uefi-test-tools/build.sh
>> @@ -38,97 +38,12 @@ if [ $ret -ne 0 ]; then
>>    exit $ret
>>  fi
>>  
>> -# Map the QEMU system emulation target to the following types of architecture
>> -# identifiers:
>> -# - edk2,
>> -# - gcc cross-compilation.
>> -# Cover only those targets that are supported by the UEFI spec and edk2.
>> -case "$emulation_target" in
>> -  (arm)
>> -    edk2_arch=ARM
>> -    gcc_arch=arm
>> -    ;;
>> -  (aarch64)
>> -    edk2_arch=AARCH64
>> -    gcc_arch=aarch64
>> -    ;;
>> -  (i386)
>> -    edk2_arch=IA32
>> -    gcc_arch=i686
>> -    ;;
>> -  (x86_64)
>> -    edk2_arch=X64
>> -    gcc_arch=x86_64
>> -    ;;
>> -  (*)
>> -    printf '%s: unknown/unsupported QEMU system emulation target "%s"\n' \
>> -      "$program_name" "$emulation_target" >&2
>> -    exit 1
>> -    ;;
>> -esac
>> -
>> -# Check if cross-compilation is needed.
>> -host_arch=$(uname -m)
>> -if [ "$gcc_arch" == "$host_arch" ] ||
>> -   ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
>> -  cross_prefix=
>> -else
>> -  cross_prefix=${gcc_arch}-linux-gnu-
>> -fi
>> -
>> -# Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
>> -# determine the suitable edk2 toolchain as well.
>> -# - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
>> -#   the gcc-5+ releases.
>> -# - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
>> -#   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
>> -#   the actual gcc releases isn't entirely trivial. Run "git-blame" on
>> -#   "OvmfPkg/build.sh" in edk2 for more information.
>> -# And, because the above is too simple, we have to assign cross_prefix to an
>> -# edk2 build variable that is specific to both the toolchain tag and the target
>> -# architecture.
>> -case "$edk2_arch" in
>> -  (ARM)
>> -    edk2_toolchain=GCC5
>> -    export GCC5_ARM_PREFIX=$cross_prefix
>> -    ;;
>> -  (AARCH64)
>> -    edk2_toolchain=GCC5
>> -    export GCC5_AARCH64_PREFIX=$cross_prefix
>> -    ;;
>> -  (IA32|X64)
>> -    gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
>> -    case "$gcc_version" in
>> -      ([1-3].*|4.[0-3].*)
>> -        printf '%s: unsupported gcc version "%s"\n' \
>> -          "$program_name" "$gcc_version" >&2
>> -        exit 1
>> -        ;;
>> -      (4.4.*)
>> -        edk2_toolchain=GCC44
>> -        ;;
>> -      (4.5.*)
>> -        edk2_toolchain=GCC45
>> -        ;;
>> -      (4.6.*)
>> -        edk2_toolchain=GCC46
>> -        ;;
>> -      (4.7.*)
>> -        edk2_toolchain=GCC47
>> -        ;;
>> -      (4.8.*)
>> -        edk2_toolchain=GCC48
>> -        ;;
>> -      (4.9.*|6.[0-2].*)
>> -        edk2_toolchain=GCC49
>> -        ;;
>> -      (*)
>> -        edk2_toolchain=GCC5
>> -        ;;
>> -    esac
>> -    eval "export ${edk2_toolchain}_BIN=\$cross_prefix"
>> -    ;;
>> -esac
>> +# Fetch some option arguments, and set the cross-compilation environment (if
>> +# any), for the edk2 "build" utility.
>> +source "$edk2_dir/../edk2-funcs.sh"
>> +edk2_arch=$(qemu_edk2_get_arch "$emulation_target")
>> +edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
>> +qemu_edk2_set_cross_env "$emulation_target"
>>  
>>  # Build the UEFI binary
>>  mkdir -p log
>>
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Eric Blake March 11, 2019, 12:07 p.m. UTC | #3
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 <lersek@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.
Eric Blake March 11, 2019, 12:11 p.m. UTC | #4
On 3/11/19 7:07 AM, 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.
>>


>> +++ 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.

Except that later, you have a patch that sources the file from non-bash.
See my followup on 7/10. :(
Laszlo Ersek March 12, 2019, 3:51 p.m. UTC | #5
On 03/10/19 18:23, Philippe Mathieu-Daudé wrote:
> Hi Laszlo,
> 
> On 3/10/19 4:17 PM, Philippe Mathieu-Daudé wrote:
>> On 3/9/19 1:48 AM, 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 <lersek@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
>>> +# characteristics for the edk2 "build" utility.
>>> +#
>>> +# This script is meant to be sourced.
>>> +#
>>> +# Copyright (C) 2019, Red Hat, Inc.
>>> +#
>>> +# This program and the accompanying materials are licensed and made available
>>> +# under the terms and conditions of the BSD License that accompanies this
>>> +# distribution. The full text of the license may be found at
>>> +# <http://opensource.org/licenses/bsd-license.php>.
>>> +#
>>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
>>> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>>> +
>>> +
>>> +# Verify whether the QEMU system emulation target is supported by the UEFI spec
>>> +# and edk2. Print a message to the standard error, and return with nonzero
>>> +# status, if verification fails.
>>> +#
>>> +# Parameters:
>>> +#   $1: QEMU system emulation target
>>> +qemu_edk2_verify_arch()
>>> +{
>>> +  local emulation_target="$1"
>>> +  local program_name=$(basename -- "$0")
>>> +
>>> +  case "$emulation_target" in
>>> +    (arm|aarch64|i386|x86_64)
>>> +      ;;
>>> +    (*)
>>> +      printf '%s: unknown/unsupported QEMU system emulation target "%s"\n' \
>>> +        "$program_name" "$emulation_target" >&2
>>> +      return 1
>>> +      ;;
>>> +  esac
>>> +}
>>> +
>>> +
>>> +# Translate the QEMU system emulation target to the edk2 architecture
>>> +# identifier. Print the result to the standard output.
>>> +#
>>> +# Parameters:
>>> +#   $1: QEMU system emulation target
>>> +qemu_edk2_get_arch()
>>> +{
>>> +  local emulation_target="$1"
>>> +
>>> +  if ! qemu_edk2_verify_arch "$emulation_target"; then
>>> +    return 1
>>> +  fi
>>> +
>>> +  case "$emulation_target" in
>>> +    (arm)
>>> +      printf 'ARM\n'
>>> +      ;;
>>> +    (aarch64)
>>> +      printf 'AARCH64\n'
>>> +      ;;
>>> +    (i386)
>>> +      printf 'IA32\n'
>>> +      ;;
>>> +    (x86_64)
>>> +      printf 'X64\n'
>>> +      ;;
>>> +  esac
>>> +}
>>> +
>>> +
>>> +# Translate the QEMU system emulation target to the gcc cross-compilation
>>> +# architecture identifier. Print the result to the standard output.
>>> +#
>>> +# Parameters:
>>> +#   $1: QEMU system emulation target
>>> +qemu_edk2_get_gcc_arch()
>>> +{
>>> +  local emulation_target="$1"
>>> +
>>> +  if ! qemu_edk2_verify_arch "$emulation_target"; then
>>> +    return 1
>>> +  fi
>>> +
>>> +  case "$emulation_target" in
>>> +    (arm|aarch64|x86_64)
>>> +      printf '%s\n' "$emulation_target"
>>> +      ;;
>>> +    (i386)
>>> +      printf 'i686\n'
>>> +      ;;
>>> +  esac
>>> +}
>>> +
>>> +
>>> +# Determine the gcc cross-compiler prefix (if any) for use with the edk2
>>> +# toolchain. Print the result to the standard output.
>>> +#
>>> +# Parameters:
>>> +#   $1: QEMU system emulation target
>>> +qemu_edk2_get_cross_prefix()
>>> +{
>>> +  local emulation_target="$1"
>>> +  local gcc_arch
>>> +  local host_arch
>>> +
>>> +  if ! gcc_arch=$(qemu_edk2_get_gcc_arch "$emulation_target"); then
>>> +    return 1
>>> +  fi
>>> +
>>> +  host_arch=$(uname -m)
>>> +
>>> +  if [ "$gcc_arch" == "$host_arch" ] ||
>>> +     ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
>>> +    # no cross-compiler needed
>>> +    :
>>> +  else
>>> +    printf '%s-linux-gnu-\n' "$gcc_arch"
> 
> Testing on Ubuntu (bionic), the gcc-arm-linux-gnueabihf provides the
> arm-linux-gnueabihf-gcc binary, therefore I use:
> 
> GCC5_ARM_PREFIX="arm-linux-gnueabihf-" make -C roms efi
> 
> But this function enforce cross prefix to be arm-linux-gnu- :(
> 
> I tried with the following Linaro toolchains:
> 
> gcc-linaro-4.9.4-2017.01-x86_64_arm-linux-gnueabihf.tar.xz
> gcc-linaro-5.5.0-2017.10-x86_64_arm-linux-gnueabihf.tar.xz
> gcc-linaro-7.2.1-2017.11-x86_64_arm-linux-gnueabihf.tar.xz
> 
> With the distrib toolchain or the Linaro enumerated, I get:
> 
> /bin/sh: 1: arm-linux-gnu-gcc: not found

This is a valid point, but out of scope for now. We can improve this
later, if necessary. For starters, it should work for people that
actually want to rebuild the images.

... Unfortunately, this -- i.e., abstracting away distro specifics of
cross compilation -- is a bottomless pit. For example, "roms/Makefile"
already contains

#
# cross compiler auto detection
#

for cross-building some other firmwares. But I'm quite certain you could
find Linux distros where that logic too would break.

(If I'm wrong and that logic is *guaranteed* to work, then we should
later extend the logic to arm/aarch64, and extract it to a separate
feature so it can be reused by edk2 platforms and modules as well -- not
just this series, but also "tests/uefi-test-tools/".)

Thanks
Laszlo

> 
>>> +  fi
>>> +}
>>> +
>>> +
>>> +# Determine the edk2 toolchain tag for the QEMU system emulation target. Print
>>> +# the result to the standard output. Print a message to the standard error, and
>>> +# return with nonzero status, if the (conditional) gcc version check fails.
>>> +#
>>> +# Parameters:
>>> +#   $1: QEMU system emulation target
>>> +qemu_edk2_get_toolchain()
>>> +{
>>> +  local emulation_target="$1"
>>> +  local program_name=$(basename -- "$0")
>>> +  local cross_prefix
>>> +  local gcc_version
>>> +
>>> +  if ! qemu_edk2_verify_arch "$emulation_target"; then
>>> +    return 1
>>> +  fi
>>> +
>>> +  case "$emulation_target" in
>>> +    (arm|aarch64)
>>> +      printf 'GCC5\n'
>>> +      ;;
>>> +
>>> +    (i386|x86_64)
>>> +      if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
>>> +        return 1
>>> +      fi
>>> +
>>> +      gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
>>> +      # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
>>> +      # the mapping below.
>>> +      case "$gcc_version" in
>>> +        ([1-3].*|4.[0-3].*)
>>> +          printf '%s: unsupported gcc version "%s"\n' \
>>> +            "$program_name" "$gcc_version" >&2
>>> +          return 1
>>> +          ;;
>>> +        (4.4.*)
>>> +          printf 'GCC44\n'
>>> +          ;;
>>> +        (4.5.*)
>>> +          printf 'GCC45\n'
>>> +          ;;
>>> +        (4.6.*)
>>> +          printf 'GCC46\n'
>>> +          ;;
>>> +        (4.7.*)
>>> +          printf 'GCC47\n'
>>> +          ;;
>>> +        (4.8.*)
>>> +          printf 'GCC48\n'
>>> +          ;;
>>> +        (4.9.*|6.[0-2].*)
>>> +          printf 'GCC49\n'
>>> +          ;;
>>> +        (*)
>>> +          printf 'GCC5\n'
>>> +          ;;
>>> +      esac
>>> +      ;;
>>> +  esac
>>> +}
>>> +
>>> +
>>> +# Determine the name of the environment variable that exposes the
>>> +# cross-compiler prefix to the edk2 "build" utility. Print the result to the
>>> +# standard output.
>>> +#
>>> +# Parameters:
>>> +#   $1: QEMU system emulation target
>>> +qemu_edk2_get_cross_prefix_var()
>>> +{
>>> +  local emulation_target="$1"
>>> +  local edk2_toolchain
>>> +  local edk2_arch
>>> +
>>> +  if ! edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target"); then
>>> +    return 1
>>> +  fi
>>> +
>>> +  case "$emulation_target" in
>>> +    (arm|aarch64)
>>> +      if ! edk2_arch=$(qemu_edk2_get_arch "$emulation_target"); then
>>> +        return 1
>>> +      fi
>>> +      printf '%s_%s_PREFIX\n' "$edk2_toolchain" "$edk2_arch"
>>> +      ;;
>>> +    (i386|x86_64)
>>> +      printf '%s_BIN\n' "$edk2_toolchain"
>>> +      ;;
>>> +  esac
>>> +}
>>> +
>>> +
>>> +# Set and export the environment variable(s) necessary for cross-compilation,
>>> +# whenever needed by the edk2 "build" utility.
>>> +#
>>> +# Parameters:
>>> +#   $1: QEMU system emulation target
>>> +qemu_edk2_set_cross_env()
>>> +{
>>> +  local emulation_target="$1"
>>> +  local cross_prefix
>>> +  local cross_prefix_var
>>> +
>>> +  if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
>>> +    return 1
>>> +  fi
>>> +
>>> +  if [ -z "$cross_prefix" ]; then
>>> +    # Nothing to do.
>>> +    return 0
>>> +  fi
>>> +
>>> +  if ! cross_prefix_var=$(qemu_edk2_get_cross_prefix_var \
>>> +                            "$emulation_target"); then
>>> +    return 1
>>> +  fi
>>> +
>>> +  eval "export $cross_prefix_var=\$cross_prefix"
>>> +}
>>> diff --git a/tests/uefi-test-tools/build.sh b/tests/uefi-test-tools/build.sh
>>> index 155cb75c4ddb..e2b52c855c39 100755
>>> --- a/tests/uefi-test-tools/build.sh
>>> +++ b/tests/uefi-test-tools/build.sh
>>> @@ -38,97 +38,12 @@ if [ $ret -ne 0 ]; then
>>>    exit $ret
>>>  fi
>>>  
>>> -# Map the QEMU system emulation target to the following types of architecture
>>> -# identifiers:
>>> -# - edk2,
>>> -# - gcc cross-compilation.
>>> -# Cover only those targets that are supported by the UEFI spec and edk2.
>>> -case "$emulation_target" in
>>> -  (arm)
>>> -    edk2_arch=ARM
>>> -    gcc_arch=arm
>>> -    ;;
>>> -  (aarch64)
>>> -    edk2_arch=AARCH64
>>> -    gcc_arch=aarch64
>>> -    ;;
>>> -  (i386)
>>> -    edk2_arch=IA32
>>> -    gcc_arch=i686
>>> -    ;;
>>> -  (x86_64)
>>> -    edk2_arch=X64
>>> -    gcc_arch=x86_64
>>> -    ;;
>>> -  (*)
>>> -    printf '%s: unknown/unsupported QEMU system emulation target "%s"\n' \
>>> -      "$program_name" "$emulation_target" >&2
>>> -    exit 1
>>> -    ;;
>>> -esac
>>> -
>>> -# Check if cross-compilation is needed.
>>> -host_arch=$(uname -m)
>>> -if [ "$gcc_arch" == "$host_arch" ] ||
>>> -   ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
>>> -  cross_prefix=
>>> -else
>>> -  cross_prefix=${gcc_arch}-linux-gnu-
>>> -fi
>>> -
>>> -# Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
>>> -# determine the suitable edk2 toolchain as well.
>>> -# - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
>>> -#   the gcc-5+ releases.
>>> -# - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
>>> -#   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
>>> -#   the actual gcc releases isn't entirely trivial. Run "git-blame" on
>>> -#   "OvmfPkg/build.sh" in edk2 for more information.
>>> -# And, because the above is too simple, we have to assign cross_prefix to an
>>> -# edk2 build variable that is specific to both the toolchain tag and the target
>>> -# architecture.
>>> -case "$edk2_arch" in
>>> -  (ARM)
>>> -    edk2_toolchain=GCC5
>>> -    export GCC5_ARM_PREFIX=$cross_prefix
>>> -    ;;
>>> -  (AARCH64)
>>> -    edk2_toolchain=GCC5
>>> -    export GCC5_AARCH64_PREFIX=$cross_prefix
>>> -    ;;
>>> -  (IA32|X64)
>>> -    gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
>>> -    case "$gcc_version" in
>>> -      ([1-3].*|4.[0-3].*)
>>> -        printf '%s: unsupported gcc version "%s"\n' \
>>> -          "$program_name" "$gcc_version" >&2
>>> -        exit 1
>>> -        ;;
>>> -      (4.4.*)
>>> -        edk2_toolchain=GCC44
>>> -        ;;
>>> -      (4.5.*)
>>> -        edk2_toolchain=GCC45
>>> -        ;;
>>> -      (4.6.*)
>>> -        edk2_toolchain=GCC46
>>> -        ;;
>>> -      (4.7.*)
>>> -        edk2_toolchain=GCC47
>>> -        ;;
>>> -      (4.8.*)
>>> -        edk2_toolchain=GCC48
>>> -        ;;
>>> -      (4.9.*|6.[0-2].*)
>>> -        edk2_toolchain=GCC49
>>> -        ;;
>>> -      (*)
>>> -        edk2_toolchain=GCC5
>>> -        ;;
>>> -    esac
>>> -    eval "export ${edk2_toolchain}_BIN=\$cross_prefix"
>>> -    ;;
>>> -esac
>>> +# Fetch some option arguments, and set the cross-compilation environment (if
>>> +# any), for the edk2 "build" utility.
>>> +source "$edk2_dir/../edk2-funcs.sh"
>>> +edk2_arch=$(qemu_edk2_get_arch "$emulation_target")
>>> +edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
>>> +qemu_edk2_set_cross_env "$emulation_target"
>>>  
>>>  # Build the UEFI binary
>>>  mkdir -p log
>>>
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
Laszlo Ersek March 12, 2019, 3:54 p.m. UTC | #6
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 <lersek@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
Laszlo Ersek March 12, 2019, 3:54 p.m. UTC | #7
On 03/11/19 13:11, Eric Blake wrote:
> On 3/11/19 7:07 AM, 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.
>>>
> 
> 
>>> +++ 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.
> 
> Except that later, you have a patch that sources the file from non-bash.
> See my followup on 7/10. :(
> 

Right, thanks for that; there I proposed SHELL=/bin/bash (for the fix
that I should add).

Thanks
Laszlo
diff mbox series

Patch

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
+# characteristics for the edk2 "build" utility.
+#
+# This script is meant to be sourced.
+#
+# Copyright (C) 2019, Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License that accompanies this
+# distribution. The full text of the license may be found at
+# <http://opensource.org/licenses/bsd-license.php>.
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+
+# Verify whether the QEMU system emulation target is supported by the UEFI spec
+# and edk2. Print a message to the standard error, and return with nonzero
+# status, if verification fails.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_verify_arch()
+{
+  local emulation_target="$1"
+  local program_name=$(basename -- "$0")
+
+  case "$emulation_target" in
+    (arm|aarch64|i386|x86_64)
+      ;;
+    (*)
+      printf '%s: unknown/unsupported QEMU system emulation target "%s"\n' \
+        "$program_name" "$emulation_target" >&2
+      return 1
+      ;;
+  esac
+}
+
+
+# Translate the QEMU system emulation target to the edk2 architecture
+# identifier. Print the result to the standard output.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_get_arch()
+{
+  local emulation_target="$1"
+
+  if ! qemu_edk2_verify_arch "$emulation_target"; then
+    return 1
+  fi
+
+  case "$emulation_target" in
+    (arm)
+      printf 'ARM\n'
+      ;;
+    (aarch64)
+      printf 'AARCH64\n'
+      ;;
+    (i386)
+      printf 'IA32\n'
+      ;;
+    (x86_64)
+      printf 'X64\n'
+      ;;
+  esac
+}
+
+
+# Translate the QEMU system emulation target to the gcc cross-compilation
+# architecture identifier. Print the result to the standard output.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_get_gcc_arch()
+{
+  local emulation_target="$1"
+
+  if ! qemu_edk2_verify_arch "$emulation_target"; then
+    return 1
+  fi
+
+  case "$emulation_target" in
+    (arm|aarch64|x86_64)
+      printf '%s\n' "$emulation_target"
+      ;;
+    (i386)
+      printf 'i686\n'
+      ;;
+  esac
+}
+
+
+# Determine the gcc cross-compiler prefix (if any) for use with the edk2
+# toolchain. Print the result to the standard output.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_get_cross_prefix()
+{
+  local emulation_target="$1"
+  local gcc_arch
+  local host_arch
+
+  if ! gcc_arch=$(qemu_edk2_get_gcc_arch "$emulation_target"); then
+    return 1
+  fi
+
+  host_arch=$(uname -m)
+
+  if [ "$gcc_arch" == "$host_arch" ] ||
+     ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
+    # no cross-compiler needed
+    :
+  else
+    printf '%s-linux-gnu-\n' "$gcc_arch"
+  fi
+}
+
+
+# Determine the edk2 toolchain tag for the QEMU system emulation target. Print
+# the result to the standard output. Print a message to the standard error, and
+# return with nonzero status, if the (conditional) gcc version check fails.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_get_toolchain()
+{
+  local emulation_target="$1"
+  local program_name=$(basename -- "$0")
+  local cross_prefix
+  local gcc_version
+
+  if ! qemu_edk2_verify_arch "$emulation_target"; then
+    return 1
+  fi
+
+  case "$emulation_target" in
+    (arm|aarch64)
+      printf 'GCC5\n'
+      ;;
+
+    (i386|x86_64)
+      if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
+        return 1
+      fi
+
+      gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
+      # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
+      # the mapping below.
+      case "$gcc_version" in
+        ([1-3].*|4.[0-3].*)
+          printf '%s: unsupported gcc version "%s"\n' \
+            "$program_name" "$gcc_version" >&2
+          return 1
+          ;;
+        (4.4.*)
+          printf 'GCC44\n'
+          ;;
+        (4.5.*)
+          printf 'GCC45\n'
+          ;;
+        (4.6.*)
+          printf 'GCC46\n'
+          ;;
+        (4.7.*)
+          printf 'GCC47\n'
+          ;;
+        (4.8.*)
+          printf 'GCC48\n'
+          ;;
+        (4.9.*|6.[0-2].*)
+          printf 'GCC49\n'
+          ;;
+        (*)
+          printf 'GCC5\n'
+          ;;
+      esac
+      ;;
+  esac
+}
+
+
+# Determine the name of the environment variable that exposes the
+# cross-compiler prefix to the edk2 "build" utility. Print the result to the
+# standard output.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_get_cross_prefix_var()
+{
+  local emulation_target="$1"
+  local edk2_toolchain
+  local edk2_arch
+
+  if ! edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target"); then
+    return 1
+  fi
+
+  case "$emulation_target" in
+    (arm|aarch64)
+      if ! edk2_arch=$(qemu_edk2_get_arch "$emulation_target"); then
+        return 1
+      fi
+      printf '%s_%s_PREFIX\n' "$edk2_toolchain" "$edk2_arch"
+      ;;
+    (i386|x86_64)
+      printf '%s_BIN\n' "$edk2_toolchain"
+      ;;
+  esac
+}
+
+
+# Set and export the environment variable(s) necessary for cross-compilation,
+# whenever needed by the edk2 "build" utility.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_set_cross_env()
+{
+  local emulation_target="$1"
+  local cross_prefix
+  local cross_prefix_var
+
+  if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
+    return 1
+  fi
+
+  if [ -z "$cross_prefix" ]; then
+    # Nothing to do.
+    return 0
+  fi
+
+  if ! cross_prefix_var=$(qemu_edk2_get_cross_prefix_var \
+                            "$emulation_target"); then
+    return 1
+  fi
+
+  eval "export $cross_prefix_var=\$cross_prefix"
+}
diff --git a/tests/uefi-test-tools/build.sh b/tests/uefi-test-tools/build.sh
index 155cb75c4ddb..e2b52c855c39 100755
--- a/tests/uefi-test-tools/build.sh
+++ b/tests/uefi-test-tools/build.sh
@@ -38,97 +38,12 @@  if [ $ret -ne 0 ]; then
   exit $ret
 fi
 
-# Map the QEMU system emulation target to the following types of architecture
-# identifiers:
-# - edk2,
-# - gcc cross-compilation.
-# Cover only those targets that are supported by the UEFI spec and edk2.
-case "$emulation_target" in
-  (arm)
-    edk2_arch=ARM
-    gcc_arch=arm
-    ;;
-  (aarch64)
-    edk2_arch=AARCH64
-    gcc_arch=aarch64
-    ;;
-  (i386)
-    edk2_arch=IA32
-    gcc_arch=i686
-    ;;
-  (x86_64)
-    edk2_arch=X64
-    gcc_arch=x86_64
-    ;;
-  (*)
-    printf '%s: unknown/unsupported QEMU system emulation target "%s"\n' \
-      "$program_name" "$emulation_target" >&2
-    exit 1
-    ;;
-esac
-
-# Check if cross-compilation is needed.
-host_arch=$(uname -m)
-if [ "$gcc_arch" == "$host_arch" ] ||
-   ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
-  cross_prefix=
-else
-  cross_prefix=${gcc_arch}-linux-gnu-
-fi
-
-# Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
-# determine the suitable edk2 toolchain as well.
-# - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
-#   the gcc-5+ releases.
-# - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
-#   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
-#   the actual gcc releases isn't entirely trivial. Run "git-blame" on
-#   "OvmfPkg/build.sh" in edk2 for more information.
-# And, because the above is too simple, we have to assign cross_prefix to an
-# edk2 build variable that is specific to both the toolchain tag and the target
-# architecture.
-case "$edk2_arch" in
-  (ARM)
-    edk2_toolchain=GCC5
-    export GCC5_ARM_PREFIX=$cross_prefix
-    ;;
-  (AARCH64)
-    edk2_toolchain=GCC5
-    export GCC5_AARCH64_PREFIX=$cross_prefix
-    ;;
-  (IA32|X64)
-    gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
-    case "$gcc_version" in
-      ([1-3].*|4.[0-3].*)
-        printf '%s: unsupported gcc version "%s"\n' \
-          "$program_name" "$gcc_version" >&2
-        exit 1
-        ;;
-      (4.4.*)
-        edk2_toolchain=GCC44
-        ;;
-      (4.5.*)
-        edk2_toolchain=GCC45
-        ;;
-      (4.6.*)
-        edk2_toolchain=GCC46
-        ;;
-      (4.7.*)
-        edk2_toolchain=GCC47
-        ;;
-      (4.8.*)
-        edk2_toolchain=GCC48
-        ;;
-      (4.9.*|6.[0-2].*)
-        edk2_toolchain=GCC49
-        ;;
-      (*)
-        edk2_toolchain=GCC5
-        ;;
-    esac
-    eval "export ${edk2_toolchain}_BIN=\$cross_prefix"
-    ;;
-esac
+# Fetch some option arguments, and set the cross-compilation environment (if
+# any), for the edk2 "build" utility.
+source "$edk2_dir/../edk2-funcs.sh"
+edk2_arch=$(qemu_edk2_get_arch "$emulation_target")
+edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
+qemu_edk2_set_cross_env "$emulation_target"
 
 # Build the UEFI binary
 mkdir -p log