Re: [Qemu-devel] [PATCH] edk2 build scripts: work around TianoCore#1607 without forcing Python 2

[Philippe Mathieu-Daudé]

Hi Laszlo,

On 9/18/19 7:11 PM, Laszlo Ersek wrote:
> It turns out that forcing python2 for running the edk2 "build" utility is
> neither necessary nor sufficient.
> 
> Forcing python2 is not sufficient for two reasons:
> 
> - QEMU is moving away from python2, with python2 nearing EOL,
> 
> - according to my most recent testing, the lacking dependency information
>   in the makefiles that are generated by edk2's "build" utility can cause
>   parallel build failures even when "build" is executed by python2.
> 
> And forcing python2 is not necessary because we can still return to the
> original idea of filtering out jobserver-related options from MAKEFLAGS.
> So do that.
> 
> With this patch, the guest UEFI binaries that are used as part of the BIOS
> tables test, and the OVMF and ArmVirtQemu platform firmwares, will be
> built strictly in a single job, regardless of an outermost "-jN" make
> option. Alas, there appears to be no reliable way to build edk2 in an
> (outer make, inner make) environment, with a jobserver enabled.
> 
> Cc: Eduardo Habkost <address@hidden>
> Cc: John Snow <address@hidden>
> Cc: Philippe Mathieu-Daudé <address@hidden>
> Reported-by: John Snow <address@hidden>
> Signed-off-by: Laszlo Ersek <address@hidden>
> ---
> 
> Notes:
>     - Tested on RHEL7 (where the outer "make" sets the old-style
>       "--jobserver-fds" flag) and on Fedora 29 (where the outer "make" sets
>       the new-style "--jobserver-auth" flag).
>     
>     - I've rebuilt all the edk2 binaries with this patch applied. Everything
>       works fine. However, if you test this patch, you might notice that git
>       reports all the build products as modified. That's because when using
>       the python3 code in edk2 BaseTools, the generated makefiles differ
>       greatly from the ones generated when running in python2 mode (e.g. due
>       to different random seeds in python hashes / dictionaries). As a
>       result, parts of the firmware volumes / firmware filesystems could
>       appear in a different order than before. This is harmless, and doesn't
>       necessitate checking in the rebuilt binaries.
> 
>  roms/edk2-build.sh             |  4 +---
>  roms/edk2-funcs.sh             | 17 +++++++++++++++++
>  tests/uefi-test-tools/build.sh |  6 +++---
>  3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/roms/edk2-build.sh b/roms/edk2-build.sh
> index 4f46f8a6a217..8161c55ef507 100755
> --- a/roms/edk2-build.sh
> +++ b/roms/edk2-build.sh
> @@ -27,9 +27,6 @@ shift $num_args
>  
>  cd edk2
>  
> -# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607>.
> -export PYTHON_COMMAND=python2
> -
>  # Source "edksetup.sh" carefully.
>  set +e +u +C
>  source ./edksetup.sh
> @@ -43,6 +40,7 @@ fi
>  # any), for the edk2 "build" utility.
>  source ../edk2-funcs.sh
>  edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
> +MAKEFLAGS=$(qemu_edk2_quirk_tianocore_1607 "$MAKEFLAGS")
>  edk2_thread_count=$(qemu_edk2_get_thread_count "$MAKEFLAGS")
>  qemu_edk2_set_cross_env "$emulation_target"
>  
> diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
> index a9fae7ee891b..3f4485b201f1 100644
> --- a/roms/edk2-funcs.sh
> +++ b/roms/edk2-funcs.sh
> @@ -251,3 +251,20 @@ qemu_edk2_get_thread_count()
>      printf '1\n'
>    fi
>  }
> +
> +
> +# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607> by
> +# filtering jobserver-related flags out of MAKEFLAGS. Print the result to the
> +# standard output.
> +#
> +# Parameters:
> +#   $1: the value of the MAKEFLAGS variable
> +qemu_edk2_quirk_tianocore_1607()
> +{
> +  local makeflags="$1"
> +
> +  printf %s "$makeflags" \
> +  | LC_ALL=C sed --regexp-extended \
> +      --expression='s/--jobserver-(auth|fds)=[0-9]+,[0-9]+//' \
> +      --expression='s/-j([0-9]+)?//'
> +}
> diff --git a/tests/uefi-test-tools/build.sh b/tests/uefi-test-tools/build.sh
> index 8aa7935c43bb..eba7964a163b 100755
> --- a/tests/uefi-test-tools/build.sh
> +++ b/tests/uefi-test-tools/build.sh
> @@ -29,9 +29,6 @@ export PACKAGES_PATH=$(realpath -- "$edk2_dir")
>  export WORKSPACE=$PWD
>  mkdir -p Conf
>  
> -# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607>.
> -export PYTHON_COMMAND=python2
> -
>  # Source "edksetup.sh" carefully.
>  set +e +u +C
>  source "$PACKAGES_PATH/edksetup.sh"
> @@ -46,12 +43,15 @@ fi
>  source "$edk2_dir/../edk2-funcs.sh"
>  edk2_arch=$(qemu_edk2_get_arch "$emulation_target")
>  edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
> +MAKEFLAGS=$(qemu_edk2_quirk_tianocore_1607 "$MAKEFLAGS")
> +edk2_thread_count=$(qemu_edk2_get_thread_count "$MAKEFLAGS")
>  qemu_edk2_set_cross_env "$emulation_target"
>  
>  # Build the UEFI binary
>  mkdir -p log
>  build \
>    --arch="$edk2_arch" \
> +  -n "$edk2_thread_count" \
>    --buildtarget=DEBUG \
>    --platform=UefiTestToolsPkg/UefiTestToolsPkg.dsc \
>    --tagname="$edk2_toolchain" \
> 

Very clear explanation, thanks.
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

Hmm this failed on Ubuntu Xenial which is the image we use for Travis-CI:

$ lsb_release -a
Distributor ID: Ubuntu
Description:    Ubuntu 16.04.6 LTS
Release:        16.04
Codename:       xenial

make -f Makefile.edk2
make[1]: Entering directory '/home/phil/qemu/roms'
if test -d edk2/.git; then \
        cd edk2 && git submodule update --init --force; \
fi
./edk2-build.sh \
        aarch64 \
        --arch=AARCH64 \
        --platform=ArmVirtPkg/ArmVirtQemu.dsc \
        -D NETWORK_IP6_ENABLE \
        -D NETWORK_HTTP_BOOT_ENABLE
WORKSPACE: /home/phil/qemu/roms/edk2
EDK_TOOLS_PATH: /home/phil/qemu/roms/edk2/BaseTools
CONF_PATH: /home/phil/qemu/roms/edk2/Conf
Copying $EDK_TOOLS_PATH/Conf/build_rule.template
     to /home/phil/qemu/roms/edk2/Conf/build_rule.txt
Copying $EDK_TOOLS_PATH/Conf/tools_def.template
     to /home/phil/qemu/roms/edk2/Conf/tools_def.txt
Copying $EDK_TOOLS_PATH/Conf/target.template
     to /home/phil/qemu/roms/edk2/Conf/target.txt
pyenv: python3.7: command not found
The `python3.7' command exists in these Python versions:
  3.7
  3.7.1

Makefile.edk2:62: recipe for target '../pc-bios/edk2-aarch64-code.fd' failed
make[1]: *** [../pc-bios/edk2-aarch64-code.fd] Error 127
make[1]: Leaving directory '/home/phil/qemu/roms'
Makefile:168: recipe for target 'efi' failed
make: *** [efi] Error 2
make: Leaving directory '/home/phil/qemu/roms'
The command "make -C roms efi -j2" exited with 2.

The local Python3 version is:

$ apt-cache show python3-minimal
Package: python3-minimal
Version: 3.5.1-3

Any idea which script is choosing python3.7?