[edk2-devel] [PATCH v2] edksetup.sh: Simplify SetupPython3 and SetupPython functions.
Leif Lindholm
leif.lindholm at linaro.org
Tue Jul 16 11:35:53 UTC 2019
+Bob, Liming, Zhijux
On Mon, Jul 15, 2019 at 08:36:06PM -0600, Rebecca Cran wrote:
> On Linux, "whereis" matches python3, python3.7, as well as
> man pages, libs etc. While on macOS it only matches the specified
> name, and so misses python3.7. Improve this by looping over
> potential version numbers and seeing if such a binary exists and
> can be executed.
So, let's start by admitting I didn't bother reviewing the existing
code very closely when it was first merged - just verifying it looked
like it may be doing what it intends to do (on Linux, sorry).
Looking more closely now, I have to say I'm not a fan.
Effectively what it does is to pick the highest-version-number python
installed in the system, regardless of what the administrator has set
as the system default - and certainly likely to.
This seems likely to lead to a really fun debugging session for
someone down the line.
So first of all, I would like to understand why we're doing this at
all? Are we likely to see installations that have a python3.7 binary,
but no python3?
If we _are_ keeping it, I would really rather rewrite this as
something that walks PATH and looks at wildcard matches rather than
something that makes assumptions of maximum version numbers.
i.e.
PYTHONS=
IFS=":"
for dir in $PATH; do
for file in $dir/python3\.*; do
if [ -f $file ]; then
PYTHONS=$file:$PYTHONS
fi
done
done
IFS=" "
(Of course, we would also need to filter out -m versions.
Or we could assume people have a functional python3 on the PATH and
would like to use that?
Best Regards,
Leif
> Signed-off-by: Rebecca Cran <rebecca at bsdio.com>
> ---
> edksetup.sh | 44 +++++++-------------------------------------
> 1 file changed, 7 insertions(+), 37 deletions(-)
>
> diff --git a/edksetup.sh b/edksetup.sh
> index 06d2f041e6..5b90e55ed8 100755
> --- a/edksetup.sh
> +++ b/edksetup.sh
> @@ -107,24 +107,10 @@ function SetupEnv()
>
> function SetupPython3()
> {
> - if [ $origin_version ];then
> - origin_version=
> - fi
> - for python in $(whereis python3)
> - do
> - python=$(echo $python | grep "[[:digit:]]$" || true)
> - python_version=${python##*python}
> - if [ -z "${python_version}" ] || (! command -v $python >/dev/null 2>&1);then
> - continue
> - fi
> - if [ -z $origin_version ];then
> - origin_version=$python_version
> - export PYTHON_COMMAND=$python
> - continue
> - fi
> - if [[ "$origin_version" < "$python_version" ]]; then
> - origin_version=$python_version
> + for ((pyver=15; pyver>=1; --pyver)); do
> + if python=$(command -v python3.$pyver); then
> export PYTHON_COMMAND=$python
> + break
> fi
> done
> return 0
> @@ -146,27 +132,11 @@ function SetupPython()
> SetupPython3
> fi
>
> - if [ $PYTHON3_ENABLE ] && [ $PYTHON3_ENABLE != TRUE ]
> - then
> - if [ $origin_version ];then
> - origin_version=
> - fi
> - for python in $(whereis python2)
> - do
> - python=$(echo $python | grep "[[:digit:]]$" || true)
> - python_version=${python##*python}
> - if [ -z "${python_version}" ] || (! command -v $python >/dev/null 2>&1);then
> - continue
> - fi
> - if [ -z $origin_version ]
> - then
> - origin_version=$python_version
> - export PYTHON_COMMAND=$python
> - continue
> - fi
> - if [[ "$origin_version" < "$python_version" ]]; then
> - origin_version=$python_version
> + if [ -n "$PYTHON3_ENABLE" ] && [ "$PYTHON3_ENABLE" != "TRUE" ]; then
> + for ((pyver=10; pyver>=1; --pyver)); do
> + if python=$(command -v python2.$pyver); then
> export PYTHON_COMMAND=$python
> + break
> fi
> done
> return 0
> --
> 2.22.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43786): https://edk2.groups.io/g/devel/message/43786
Mute This Topic: https://groups.io/mt/32485955/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list