On 9/20/19 10:38 AM, 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.
>
> While at it, cut short edk2's auto-detection of the python3.* minor
> version, by setting PYTHON_COMMAND to "python3" (which we expect to be
> available wherever we intend to build edk2).
Yes! This fixes it :)
> 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 <ehabk...@redhat.com>
> Cc: John Snow <js...@redhat.com>
> Cc: Philippe Mathieu-Daudé <phi...@redhat.com>
> Reported-by: John Snow <js...@redhat.com>
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> ---
>
> Notes:
> v2:
>
> - set PYTHON_COMMAND to python3; update commit message [Phil]
>
> - do not pick up any feedback tags from v1 (v2 is not a trivial update)
>
> v1:
>
> - 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/Makefile | 1 +
> tests/uefi-test-tools/Makefile | 1 +
> roms/edk2-build.sh | 4 ++--
> roms/edk2-funcs.sh | 17 +++++++++++++++++
> tests/uefi-test-tools/build.sh | 6 ++++--
> 5 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/roms/Makefile b/roms/Makefile
> index 6cf07d3b44ee..06339efa6fc4 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -141,6 +141,7 @@ build-efi-roms: build-pxe-roms
> #
> edk2-basetools:
> $(MAKE) -C edk2/BaseTools \
> + PYTHON_COMMAND=$${EDK2_PYTHON_COMMAND:-python3} \
> EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
> EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
>
> diff --git a/tests/uefi-test-tools/Makefile b/tests/uefi-test-tools/Makefile
> index 7e0177d7337e..1dcddcdbbabf 100644
> --- a/tests/uefi-test-tools/Makefile
> +++ b/tests/uefi-test-tools/Makefile
> @@ -100,6 +100,7 @@ Build/bios-tables-test.%.efi: build-edk2-tools
>
> build-edk2-tools:
> $(MAKE) -C $(edk2_dir)/BaseTools \
> + PYTHON_COMMAND=$${EDK2_PYTHON_COMMAND:-python3} \
> EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
> EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
>
> diff --git a/roms/edk2-build.sh b/roms/edk2-build.sh
> index 4f46f8a6a217..d5391c763728 100755
> --- a/roms/edk2-build.sh
> +++ b/roms/edk2-build.sh
> @@ -27,8 +27,7 @@ shift $num_args
>
> cd edk2
>
> -# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607>.
> -export PYTHON_COMMAND=python2
> +export PYTHON_COMMAND=${EDK2_PYTHON_COMMAND:-python3}
>
> # Source "edksetup.sh" carefully.
> set +e +u +C
> @@ -43,6 +42,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..3b78f3084069 100755
> --- a/tests/uefi-test-tools/build.sh
> +++ b/tests/uefi-test-tools/build.sh
> @@ -29,8 +29,7 @@ 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
> +export PYTHON_COMMAND=${EDK2_PYTHON_COMMAND:-python3}
>
> # Source "edksetup.sh" carefully.
> set +e +u +C
> @@ -46,12 +45,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" \
>
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com>
