Message ID | 20190918171141.15957-1-lersek@redhat.com |
---|---|
State | New |
Headers | show |
Series | edk2 build scripts: work around TianoCore#1607 without forcing Python 2 | expand |
On 9/18/19 1: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, > Thank you :) > - 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 <ehabkost@redhat.com> > Cc: John Snow <jsnow@redhat.com> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com> > Reported-by: John Snow <jsnow@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> Looks good to me, given your explanation of the situation so far. Reviewed-by: John Snow <jsnow@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" \ >
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 <ehabkost@redhat.com> > Cc: John Snow <jsnow@redhat.com> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com> > Reported-by: John Snow <jsnow@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@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é <philmd@redhat.com> 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?
Hi Bruce, 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 <ehabkost@redhat.com> > Cc: John Snow <jsnow@redhat.com> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com> > Reported-by: John Snow <jsnow@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@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). Since I plan to queue this patch, do you mind testing this patch on your distribution? I don't want to break your workflow. Beware it creates ~3.5GiB of temporary data in roms/edk2/Build. Thanks! Meanwhile: Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> > - 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" \ >
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. FYI I tried uninstalling python2 on Fedora 29, $ make -C roms efi -j8 make: Entering directory '/home/phil/source/qemu/roms' make -C edk2/BaseTools \ EXTRA_OPTFLAGS='' \ EXTRA_LDFLAGS='' make[1]: Entering directory '/home/phil/source/qemu/roms/edk2/BaseTools' [...] make -C Tests make[2]: Entering directory '/home/phil/source/qemu/roms/edk2/BaseTools/Tests' /bin/sh: python: command not found make[2]: *** [GNUmakefile:11: test] Error 127 'python' seems to be provided by python-unversioned-command which is wired to Python2: $ dnf info python-unversioned-command Last metadata expiration check: 0:03:08 ago on Thu 19 Sep 2019 04:21:21 PM UTC. Available Packages Name : python-unversioned-command Version : 2.7.16 Release : 2.fc29 Arch : noarch Size : 13 k Source : python2-2.7.16-2.fc29.src.rpm Repo : updates Summary : The "python" command that runs Python 2 URL : https://www.python.org/ License : Python Description : This package contains /usr/bin/python - the "python" command that runs Python 2. I had to manually run update-alternatives to continue: $ sudo update-alternatives --install /usr/bin/python python /usr/bin/python3 69 Not sure this is the expected behavior, it is confusing.
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 <ehabkost@redhat.com> >> Cc: John Snow <jsnow@redhat.com> >> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> >> Reported-by: John Snow <jsnow@redhat.com> >> Signed-off-by: Laszlo Ersek <lersek@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é <philmd@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
On 09/19/19 18:39, Philippe Mathieu-Daudé wrote: > 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. > > FYI I tried uninstalling python2 on Fedora 29, > > $ make -C roms efi -j8 > make: Entering directory '/home/phil/source/qemu/roms' > make -C edk2/BaseTools \ > EXTRA_OPTFLAGS='' \ > EXTRA_LDFLAGS='' > make[1]: Entering directory '/home/phil/source/qemu/roms/edk2/BaseTools' > [...] > make -C Tests > make[2]: Entering directory > '/home/phil/source/qemu/roms/edk2/BaseTools/Tests' > /bin/sh: python: command not found > make[2]: *** [GNUmakefile:11: test] Error 127 > > 'python' seems to be provided by python-unversioned-command which is > wired to Python2: > > $ dnf info python-unversioned-command > Last metadata expiration check: 0:03:08 ago on Thu 19 Sep 2019 04:21:21 > PM UTC. > Available Packages > Name : python-unversioned-command > Version : 2.7.16 > Release : 2.fc29 > Arch : noarch > Size : 13 k > Source : python2-2.7.16-2.fc29.src.rpm > Repo : updates > Summary : The "python" command that runs Python 2 > URL : https://www.python.org/ > License : Python > Description : This package contains /usr/bin/python - the "python" > command that runs Python 2. > > I had to manually run update-alternatives to continue: > > $ sudo update-alternatives --install /usr/bin/python python > /usr/bin/python3 69 > > Not sure this is the expected behavior, it is confusing. > The python detection is not fool-proof in edk2. A description is given at: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools-Support-Python2-Python3 To summarize that, it works like this, on Linux: - if you set PYTHON_COMMAND, then the binary pointed to by PYTHON_COMMAND will be used. The edk2 build infrastructure will determine whether the pointed-to binary is python 2 or python 3, and branch to the corresponding implementation of the build tools. - Otherwise, *minor* version auto-detection is attempted. With PYTHON3_ENABLE unset, or set to "TRUE", this minor version autodetection will aim at minor versions of python3. - Otherwise (= PYTHON3_ENABLE set to a string different from "TRUE"), the minor version auto-detection will focus on python2. With this patch applied, the middle case is active. Apparently it fails, because the edk2 build tools developers could not foresee the situations that you've exposed the auto-detection to, on Ubuntu and Fedora. Back when I tested the python3 enablement in edk2, I checked the patches in the following environments: - on RHEL-7 with the system python 2, - on RHEL-7 with python3.4 from EPEL-7, - on RHEL-8 with python3.6, - on RHEL-8 with platform-python. Everything worked fine for me. I have no clue what's going on in Ubuntu and in Fedora. Can we require all build host installations -- where we expect to run "make efi" -- to provide a Python 3 binary on $PATH that is plainly called "python3"? Then I think this patch should work. (If necessary, I could set "PYTHON_COMMAND=python3", too.) Thanks! Laszlo
On 9/19/19 9:08 PM, Laszlo Ersek wrote: > On 09/19/19 18:39, Philippe Mathieu-Daudé wrote: >> 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. >> >> FYI I tried uninstalling python2 on Fedora 29, >> >> $ make -C roms efi -j8 >> make: Entering directory '/home/phil/source/qemu/roms' >> make -C edk2/BaseTools \ >> EXTRA_OPTFLAGS='' \ >> EXTRA_LDFLAGS='' ^ this is the 'edk2-basetools' target from roms/Makefile. >> make[1]: Entering directory '/home/phil/source/qemu/roms/edk2/BaseTools' >> [...] >> make -C Tests >> make[2]: Entering directory >> '/home/phil/source/qemu/roms/edk2/BaseTools/Tests' >> /bin/sh: python: command not found >> make[2]: *** [GNUmakefile:11: test] Error 127 >> >> 'python' seems to be provided by python-unversioned-command which is >> wired to Python2: >> >> $ dnf info python-unversioned-command >> Last metadata expiration check: 0:03:08 ago on Thu 19 Sep 2019 04:21:21 >> PM UTC. >> Available Packages >> Name : python-unversioned-command >> Version : 2.7.16 >> Release : 2.fc29 >> Arch : noarch >> Size : 13 k >> Source : python2-2.7.16-2.fc29.src.rpm >> Repo : updates >> Summary : The "python" command that runs Python 2 >> URL : https://www.python.org/ >> License : Python >> Description : This package contains /usr/bin/python - the "python" >> command that runs Python 2. >> >> I had to manually run update-alternatives to continue: >> >> $ sudo update-alternatives --install /usr/bin/python python >> /usr/bin/python3 69 >> >> Not sure this is the expected behavior, it is confusing. >> > > The python detection is not fool-proof in edk2. A description is given at: > > https://github.com/tianocore/tianocore.github.io/wiki/BaseTools-Support-Python2-Python3 > > To summarize that, it works like this, on Linux: > > - if you set PYTHON_COMMAND, then the binary pointed to by > PYTHON_COMMAND will be used. The edk2 build infrastructure will > determine whether the pointed-to binary is python 2 or python 3, and > branch to the corresponding implementation of the build tools. > > - Otherwise, *minor* version auto-detection is attempted. With > PYTHON3_ENABLE unset, or set to "TRUE", this minor version autodetection > will aim at minor versions of python3. > > - Otherwise (= PYTHON3_ENABLE set to a string different from "TRUE"), > the minor version auto-detection will focus on python2. What you document regarding PYTHON3_ENABLE is valid once we sourced edksetup.sh which is done in Makefile.edk2, one step after the previous call: efi: edk2-basetools # call 1 (python failing) $(MAKE) -f Makefile.edk2 # call 2 sourcing edksetup.sh > With this patch applied, the middle case is active. Apparently it fails, > because the edk2 build tools developers could not foresee the situations > that you've exposed the auto-detection to, on Ubuntu and Fedora. Back > when I tested the python3 enablement in edk2, I checked the patches in > the following environments: > > - on RHEL-7 with the system python 2, > - on RHEL-7 with python3.4 from EPEL-7, > - on RHEL-8 with python3.6, > - on RHEL-8 with platform-python. > > Everything worked fine for me. I have no clue what's going on in Ubuntu > and in Fedora. > > Can we require all build host installations -- where we expect to run > "make efi" -- to provide a Python 3 binary on $PATH that is plainly > called "python3"? Kevin recently suggested a similar patch (in another area): https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg04377.html > Then I think this patch should work. (If necessary, I could set > "PYTHON_COMMAND=python3", too.) Yes, I confirm forcing "PYTHON_COMMAND=python3 make -C roms efi" works. Not sure what is the cleaner way to fix this although... Regards, Phil.
On 09/19/19 21:56, Philippe Mathieu-Daudé wrote: > On 9/19/19 9:08 PM, Laszlo Ersek wrote: >> On 09/19/19 18:39, Philippe Mathieu-Daudé wrote: >>> 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. >>> >>> FYI I tried uninstalling python2 on Fedora 29, >>> >>> $ make -C roms efi -j8 >>> make: Entering directory '/home/phil/source/qemu/roms' >>> make -C edk2/BaseTools \ >>> EXTRA_OPTFLAGS='' \ >>> EXTRA_LDFLAGS='' > > ^ this is the 'edk2-basetools' target from roms/Makefile. > >>> make[1]: Entering directory '/home/phil/source/qemu/roms/edk2/BaseTools' >>> [...] >>> make -C Tests >>> make[2]: Entering directory >>> '/home/phil/source/qemu/roms/edk2/BaseTools/Tests' >>> /bin/sh: python: command not found >>> make[2]: *** [GNUmakefile:11: test] Error 127 >>> >>> 'python' seems to be provided by python-unversioned-command which is >>> wired to Python2: >>> >>> $ dnf info python-unversioned-command >>> Last metadata expiration check: 0:03:08 ago on Thu 19 Sep 2019 04:21:21 >>> PM UTC. >>> Available Packages >>> Name : python-unversioned-command >>> Version : 2.7.16 >>> Release : 2.fc29 >>> Arch : noarch >>> Size : 13 k >>> Source : python2-2.7.16-2.fc29.src.rpm >>> Repo : updates >>> Summary : The "python" command that runs Python 2 >>> URL : https://www.python.org/ >>> License : Python >>> Description : This package contains /usr/bin/python - the "python" >>> command that runs Python 2. >>> >>> I had to manually run update-alternatives to continue: >>> >>> $ sudo update-alternatives --install /usr/bin/python python >>> /usr/bin/python3 69 >>> >>> Not sure this is the expected behavior, it is confusing. >>> >> >> The python detection is not fool-proof in edk2. A description is given at: >> >> https://github.com/tianocore/tianocore.github.io/wiki/BaseTools-Support-Python2-Python3 >> >> To summarize that, it works like this, on Linux: >> >> - if you set PYTHON_COMMAND, then the binary pointed to by >> PYTHON_COMMAND will be used. The edk2 build infrastructure will >> determine whether the pointed-to binary is python 2 or python 3, and >> branch to the corresponding implementation of the build tools. >> >> - Otherwise, *minor* version auto-detection is attempted. With >> PYTHON3_ENABLE unset, or set to "TRUE", this minor version autodetection >> will aim at minor versions of python3. >> >> - Otherwise (= PYTHON3_ENABLE set to a string different from "TRUE"), >> the minor version auto-detection will focus on python2. > > What you document regarding PYTHON3_ENABLE is valid once we sourced > edksetup.sh which is done in Makefile.edk2, one step after the previous > call: > > efi: edk2-basetools # call 1 (python failing) > $(MAKE) -f Makefile.edk2 # call 2 sourcing edksetup.sh > >> With this patch applied, the middle case is active. Apparently it fails, >> because the edk2 build tools developers could not foresee the situations >> that you've exposed the auto-detection to, on Ubuntu and Fedora. Back >> when I tested the python3 enablement in edk2, I checked the patches in >> the following environments: >> >> - on RHEL-7 with the system python 2, >> - on RHEL-7 with python3.4 from EPEL-7, >> - on RHEL-8 with python3.6, >> - on RHEL-8 with platform-python. >> >> Everything worked fine for me. I have no clue what's going on in Ubuntu >> and in Fedora. >> >> Can we require all build host installations -- where we expect to run >> "make efi" -- to provide a Python 3 binary on $PATH that is plainly >> called "python3"? > > Kevin recently suggested a similar patch (in another area): > https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg04377.html > >> Then I think this patch should work. (If necessary, I could set >> "PYTHON_COMMAND=python3", too.) > > Yes, I confirm forcing "PYTHON_COMMAND=python3 make -C roms efi" works. > > Not sure what is the cleaner way to fix this although... Thanks for the analysis! I understand the issue now. - "edk2/BaseTools/GNUmakefile" runs $(MAKE) in three subdirs: - Source/C, - Source/Python, - Tests (which depends on the former two) - "edk2/BaseTools/Source/C/GNUmakefile" builds fine - "edk2/BaseTools/Source/Python/GNUmakefile" does nothing - "edk2/BaseTools/Tests/GNUmakefile" depends on PYTHON_COMMAND -- which should either come from the user, or from sourcing "edksetup.sh" Therefore the issue is: - the "edk2-basetools" target in "roms/Makefile" does not run (more precisely, does not "source") edksetup.sh - the "build-edk2-tools" target in "tests/uefi-test-tools/Makefile" does not run (more precisely, does not "source") edksetup.sh I don't think I will reorganize the dependencies in those makefiles. Nor will I source edksetup.sh in the makefile recipes. Instead, I'll directly set PYTHON_COMMAND=python3 in the "tools" recipes, and in the build wrapper shell scripts. I'll try to post v2 soon. Thanks! Laszlo
On 9/19/19 11:40 PM, Laszlo Ersek wrote: > On 09/19/19 21:56, Philippe Mathieu-Daudé wrote: >> On 9/19/19 9:08 PM, Laszlo Ersek wrote: >>> On 09/19/19 18:39, Philippe Mathieu-Daudé wrote: >>>> 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. >>>> >>>> FYI I tried uninstalling python2 on Fedora 29, >>>> >>>> $ make -C roms efi -j8 >>>> make: Entering directory '/home/phil/source/qemu/roms' >>>> make -C edk2/BaseTools \ >>>> EXTRA_OPTFLAGS='' \ >>>> EXTRA_LDFLAGS='' >> >> ^ this is the 'edk2-basetools' target from roms/Makefile. >> >>>> make[1]: Entering directory '/home/phil/source/qemu/roms/edk2/BaseTools' >>>> [...] >>>> make -C Tests >>>> make[2]: Entering directory >>>> '/home/phil/source/qemu/roms/edk2/BaseTools/Tests' >>>> /bin/sh: python: command not found >>>> make[2]: *** [GNUmakefile:11: test] Error 127 >>>> >>>> 'python' seems to be provided by python-unversioned-command which is >>>> wired to Python2: >>>> >>>> $ dnf info python-unversioned-command >>>> Last metadata expiration check: 0:03:08 ago on Thu 19 Sep 2019 04:21:21 >>>> PM UTC. >>>> Available Packages >>>> Name : python-unversioned-command >>>> Version : 2.7.16 >>>> Release : 2.fc29 >>>> Arch : noarch >>>> Size : 13 k >>>> Source : python2-2.7.16-2.fc29.src.rpm >>>> Repo : updates >>>> Summary : The "python" command that runs Python 2 >>>> URL : https://www.python.org/ >>>> License : Python >>>> Description : This package contains /usr/bin/python - the "python" >>>> command that runs Python 2. >>>> >>>> I had to manually run update-alternatives to continue: >>>> >>>> $ sudo update-alternatives --install /usr/bin/python python >>>> /usr/bin/python3 69 >>>> >>>> Not sure this is the expected behavior, it is confusing. >>>> >>> >>> The python detection is not fool-proof in edk2. A description is given at: >>> >>> https://github.com/tianocore/tianocore.github.io/wiki/BaseTools-Support-Python2-Python3 >>> >>> To summarize that, it works like this, on Linux: >>> >>> - if you set PYTHON_COMMAND, then the binary pointed to by >>> PYTHON_COMMAND will be used. The edk2 build infrastructure will >>> determine whether the pointed-to binary is python 2 or python 3, and >>> branch to the corresponding implementation of the build tools. >>> >>> - Otherwise, *minor* version auto-detection is attempted. With >>> PYTHON3_ENABLE unset, or set to "TRUE", this minor version autodetection >>> will aim at minor versions of python3. >>> >>> - Otherwise (= PYTHON3_ENABLE set to a string different from "TRUE"), >>> the minor version auto-detection will focus on python2. >> >> What you document regarding PYTHON3_ENABLE is valid once we sourced >> edksetup.sh which is done in Makefile.edk2, one step after the previous >> call: >> >> efi: edk2-basetools # call 1 (python failing) >> $(MAKE) -f Makefile.edk2 # call 2 sourcing edksetup.sh >> >>> With this patch applied, the middle case is active. Apparently it fails, >>> because the edk2 build tools developers could not foresee the situations >>> that you've exposed the auto-detection to, on Ubuntu and Fedora. Back >>> when I tested the python3 enablement in edk2, I checked the patches in >>> the following environments: >>> >>> - on RHEL-7 with the system python 2, >>> - on RHEL-7 with python3.4 from EPEL-7, >>> - on RHEL-8 with python3.6, >>> - on RHEL-8 with platform-python. >>> >>> Everything worked fine for me. I have no clue what's going on in Ubuntu >>> and in Fedora. >>> >>> Can we require all build host installations -- where we expect to run >>> "make efi" -- to provide a Python 3 binary on $PATH that is plainly >>> called "python3"? >> >> Kevin recently suggested a similar patch (in another area): >> https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg04377.html >> >>> Then I think this patch should work. (If necessary, I could set >>> "PYTHON_COMMAND=python3", too.) >> >> Yes, I confirm forcing "PYTHON_COMMAND=python3 make -C roms efi" works. >> >> Not sure what is the cleaner way to fix this although... > > Thanks for the analysis! > > I understand the issue now. > > - "edk2/BaseTools/GNUmakefile" runs $(MAKE) in three subdirs: > - Source/C, > - Source/Python, > - Tests (which depends on the former two) > > - "edk2/BaseTools/Source/C/GNUmakefile" builds fine > - "edk2/BaseTools/Source/Python/GNUmakefile" does nothing > - "edk2/BaseTools/Tests/GNUmakefile" depends on PYTHON_COMMAND -- which > should either come from the user, or from sourcing "edksetup.sh" > > Therefore the issue is: > > - the "edk2-basetools" target in "roms/Makefile" does not > run (more precisely, does not "source") edksetup.sh > > - the "build-edk2-tools" target in "tests/uefi-test-tools/Makefile" > does not run (more precisely, does not "source") edksetup.sh > > I don't think I will reorganize the dependencies in those makefiles. Nor Exactly, nor do I. > will I source edksetup.sh in the makefile recipes. Instead, I'll > directly set PYTHON_COMMAND=python3 in the "tools" recipes, and in the > build wrapper shell scripts. I notice this: $ git grep -i python configure ... configure:1832:python="$python -B" ... configure:7287:echo "PYTHON=$python" >> $config_host_mak ... configure:7863:echo "export PYTHON='$python'" >> "$iotests_common_env" While config-host.mak is too QEMU specific, the iotest common.env is very simple: $ cat tests/qemu-iotests/common.env # Automatically generated by configure - do not modify export PYTHON='python3 -B' I'm not sure we need to run ./configure to run any make target in the roms/ directory (since we use 'make -C roms ...'), this script doesn't even clone the required submodules, we need to call 'make git-submodule-update' first. Note that when there is no 'python3' in $PATH, the ./configure script checks for python2, but does not use generic 'python': configure:901:for binary in "${PYTHON-python3}" python python2 configure:905: python="$binary" > I'll try to post v2 soon. While forcing PYTHON_COMMAND in roms/Makefile would help, I think part of the proper way to fix this is generic to QEMU. Not sure what is the cleaner way yet :/ Regards, Phil.
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" \
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 <ehabkost@redhat.com> Cc: John Snow <jsnow@redhat.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Reported-by: John Snow <jsnow@redhat.com> Signed-off-by: Laszlo Ersek <lersek@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(-)