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

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

Leif:

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Friday, July 19, 2019 1:56 AM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; 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 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.

What issue here? You mean the variable docker may have the different version 
python interpreter. The same source may have the different build result on those dockers.

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

OK. If you think that PYTHON3_ENABLE is not used, can you send RFC to remove it?

Thanks
Liming
> 
> /
>     Leif