Re: [PATCH 1/1] edksetup.sh: rework python executable scanning

["Laszlo Ersek" <lersek@redhat.com>]

Hi Leif,

On 07/16/19 21:07, Leif Lindholm wrote:
> If PYTHON_COMMAND is set, use that.
> If PYTHON_COMMAND is not set, use first working of "python", "python3", "python2".
> If none of those work, search the path for python*[0-9], using the highest version
> number across x.y.z format.
> 
> Finally, set PYTHON3_ENABLE if selected python is python 3.
> 
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> 
> This is my somewhat overkill proposal as an alternative to Rebecca's 5/6.
> It is certainly more complex than that patch, and arguably as complex as
> the current upstream implementation, but the semantics and flow is more
> clear (to me, at least).
> 
> An alternative version to *this* patch would be one that drops the
> FindHighestVersionedExecutable() function, and the if-statement that
> calls it. This would still leave us with a solution that would use (in order):
> * PYTHON_COMMAND
> * python
> * python3
> * python2
> and fail with an error if $PYTHON_COMMAND (if set externally) or none of the
> others could execute $PYTHON_COMMAND --version.

I think I'd be fine, personally, with this change. It's still a
behavioral change in two aspects, and so I'd like the BaseTools
maintainers to comment on those:


(1) If I understand correctly, the proposed patch would favor "python"
(no version number) over "python3". That diverges from current practice.

I think this is a relevant question because the BaseTools maintainers
prefer python3 over python2, if a system offers both, even if the system
default "python" points to "python2".

I deduce this claim from
<https://lists.01.org/pipermail/edk2-devel/2018-December/034459.html>:

"we have no enough resource to fully verify Python2 and Python3 both. We
will focus on Python3 validation. If anyone can help verify Python2, it
will be great. And, if you meet with the issue on Python2, please file
BZ. We still fix them."

So the primary target seems to be python3; considered "more thoroughly
validated" at all times (my words).


(2) The original proposal (see it included e.g. in
<https://lists.01.org/pipermail/edk2-devel/2019-January/035815.html>)
gave some significance to the PYTHON3_ENABLE variable (coming from the
user's environment). Doesn't this patch erase that?

(

Looking back at that specification -- I basically don't understand what
PYTHON3_ENABLE=TRUE was supposed to stand for, coming from the user's env.

I do understand the other two cases: PYTHON3_ENABLE unset, plus
PYTHON_COMMAND either set or unset. This last variation is actually what
my question (1) concerns.

)


Furthermore,

(3) after looking at FindHighestVersionedExecutable(), I think I would
prefer the variant of this patch -- assuming the behavioral change is OK
in the first place -- that does not have
FindHighestVersionedExecutable(). It's not that
FindHighestVersionedExecutable() is too complex -- the problem is that
the function is complex *and* a good chunk of it will never be used in
practice. I doubt we have to prepare for python4 or python5 at this
point. Similarly for patch levels.

I believe one thing I like about Rebecca's 5/6 is that, while it does
check for some surprisingly high (and arbitrary) minor releases -- such
as python3.15, python2.10 --, the loops are really small and simple. No
extra complexity is needed for covering all practically relevant
releases. In case we ever exceeded "python3.15", it would take a
one-liner to bump the limit (and it would take a simple patch to factor
out the loop to a function, and check for python4.[0..15] even).

Thanks,
Laszlo