On 09/19/19 15:41, Philippe Mathieu-Daudé wrote: > 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 <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: >> - 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é <phi...@redhat.com>
Thanks! > > 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 ... I don't have the slightest idea what this error message means. Edk2 contains no reference to "pyenv". > > 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? > It's the SetupPython() function in "edksetup.sh". If there is a universal pathname (or just filename) that refers to python3 on all build hosts where "make efi" is expected to run, I can assign that to PYTHON_COMMAND. Otherwise, I'm out of ideas. Thanks Laszlo
