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

[Laszlo Ersek]

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