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

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Thu, Jul 18, 2019 at 04:48:18PM +0000, Gao, Liming wrote:
> > > 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.

Yes.
But perhaps the user isn't the admin, and the admin installs a new
version of python without updating the default links, in order to let
a different user test the new version. Thinking this will not affect
users, because python, python2 and python3 all behave exactly like
before.

> 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. 

Yes, and ideally I would have noticed that and had this conversation
back then. But I didn't. Sorry.

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

Not yet.
But I can easily see this causing issues with the various docker
images we have set up for various (not just TianoCore) CI jobs.

> > 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.

Yes, but for me, that logic fails the "least amount of surprise"
litmus test. If the command invoked when I call 'python' (or 'python2'
or 'python3') is the same, then I would expect the same version to be
used by the build system. If I *wanted* applications to use the newer
version, I would change the 'python' (or 'python2' or 'python3')
symlink.

> > > > - 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. 

On Linux, the path is not required for PYTHON_COMMAND. And this is the
.sh, so I would hope the same holds true under cygwin. So
PYTHON_COMMAND=python2 provides the same functionality as
PYTHON3_ENABLE=FALSE.

*But* the latest version of my script does not behave in this way, so
that still needs to change.

> > 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?

Exactly. So I think it it not needed.

/
    Leif