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

[Philippe Mathieu-Daudé]

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.