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

["Liming Gao" <liming.gao@intel.com>]

Leif:

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Leif Lindholm
> Sent: Thursday, July 18, 2019 6:37 AM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Rebecca Cran <rebecca@bsdio.com>; Feng, Bob C <bob.c.feng@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
> Subject: Re: [edk2-devel] [PATCH 1/1] edksetup.sh: rework python executable scanning
> 
> On Wed, Jul 17, 2019 at 03:23:26AM +0000, Gao, Liming wrote:
> > Leif:
> >   I agree to discuss the behavior first, then review the code logic in detail. I add my comments below.
> >
> > > -----Original Message-----
> > > From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> > > So, first of all - I am entirely happy with dropping the function.
> > > But I wanted to have it written anyway in case someone came up with a
> > > really good explanation for why we needed to completely cover the same
> > > pattern as the code currently in tree: and the situation does not
> > > relate to python4 or python5 - it relates to picking the latest of
> > > 2.7, 2.7.6, 3.5, 3.5.2, 3.5,4, 3.7 (which is my interpretation of the
> > > behaviour of current HEAD).
> > >
> > > So basically - I think we need to reach an agreement (with BaseTools
> > > maintainers, and existing users) about what the behaviour should be.
> > >
> > > - What does PYTHON3_ENABLE mean? Is it for probing only, or are we
> > >   setting it for later use by BaseTools?
> >
> > PYTHON3_EANBLE is to decide python3 enable or not. It has high priority.
> > Once it is set, PYTHON_COMMAND will be ignored.
> >   If it is set to TRUE, edksetup.sh will find Python3 in the system, set PYTHON_COMMAND env.
> >   If it is set to other value, edksetup.sh will find Python2 in the system, set PYTHON_COMMAND env.
> > If PYTHON3_EANBLE is not set, PYTHON_COMMAND will be used if PYTHON_COMMAND is set.
> > If PYTHON3_EANBLE is not set, and PYTHON_COMMAND is not set, the
> > default behavior will set PYTHON3_EANBLE to TRUE.
> 
> (typo as discussed in other email corrected above (python3->python2))
> 
> > So, the default behavior is to use highest version Python3. User can
> > set PYTHON_COMMAND for their python version.
> >
> > > - What should the priority order be when looking for python
> > >   executables?
> >
> > Find the high version python. When we enable Python3, we find
> > Python37 does great performance optimization.
> > So, we think high version python can bring more benefit.
> 
> This is where I disagree.
> As a user/admin, I am no more interested in my build tools deciding I
> would prefer another python than the default one than I am in they
> deciding I would prefer another toolchain.
> 
> If I build a specific commit of edk2 (including BaseTools) from a
> specific command line, then I expect any subsequent builds to behave
> identically unless I have reconfigured the system. Installing another
> version of python without changing the system default shoulds not
> change that.

I agree this is a good point to keep the same build behavior even if user 
environment is changed. But, I think user installs new version python, he 
may want to use it. Current edksetup.sh can easily apply the new version python. 
Now, the difference is the default policy to choose python version. 
Your suggestion is to use default version python interpreter or base on PATH to find the python interpreter. 
Current logic is to find the high version in the available python interpreter. 
It is added @d8238aaf862a55eec77040844c71a02c71294e86 commit. 

Do you meet with the real problem with the high version python interpreter? 

> 
> If I _want_ to use the newly installed python, I can change the
> python/python2/python3 links. And if I don't want to do that, I can
> set PYTHON_COMMAND.
> "We have seen better performance with 3.7" is an excellent argment for
> suggesting people install, and use, python 3.7. I do not see it as a
> good argument for always preferring the highest version available.
> 

User can set PYTHON_COMMAND to keep the same python interpreter.

> > > - Can we use simply 'python' as the default?
> >
> > Based on previous discussion, we recommend to use Python3 as default.
> 
> If python3 is to be the default, then I see no use for PYTHON3_£NABLE.

Now, BaseTools has not drop Python2 support. If user wants to use Python2, 
he can simply set PYTHON3_ENABLE=FALSE, then he doesn't need to find python path 
to set PYTHON_COMMAND env. 

> If PYTHON_COMMAND is set, it should always be respected. If it's not
> set, python3 is picked in preference anyway.

So, PYTHON_COMMAND is higher priority than PYTHON3_ENABLE.
That means PYTHON3_ENABLE value will be ignored. Right?

Thanks
Liming
> 
> > > - Do we need functionality for more than selecting between
> > >   python2/python3?
> >
> > Yes. Find the high version python installed in the system.
> 
> Best Regards,
> 
> Leif
> 
>