Re: [Fuego] [PATCH] python3: Use python3 in the scripts to fully migrate

OK -  I have some more comments on this patch.

See inline below.

> -----Original Message-----
> From: [email protected] <[email protected]>
>=20
> The python scripts are failing to run in an environment where `python`
> is not defined, though the scripts are migrated to work on python3
> the shebang is still pointing to python which may be undefined
>=20
> Signed-off-by: venkata pyla <[email protected]>
> ---
>  scripts/check-dependencies              | 2 +-

My first comment is that invocation using just the interpreter name of 'pyt=
hon' is
in a lot more places than just these 4 files, and the parser.py scripts.

I see it also in deorphan-runs.py, gen-page.py, jdiff, test_parser.sh , gen=
eric_parser.py,
test_filelock.py (in the scripts directory).  Many of these you are likely =
not using.

However, some of these are intended for use by Fuego users, for special cir=
cumstances
(like deorphan-runs.py and test_parser.sh).

>  scripts/common.sh                       | 8 ++++----
>  scripts/ftc                             | 2 +-
>  scripts/functions.sh                    | 2 +-
>  tests/Benchmark.Dhrystone/parser.py     | 2 +-
>  tests/Benchmark.IOzone/parser.py        | 2 +-
>  tests/Benchmark.Stream/parser.py        | 2 +-
>  tests/Benchmark.bonnie/parser.py        | 2 +-
>  tests/Benchmark.cyclictest/parser.py    | 2 +-
>  tests/Benchmark.fio/parser.py           | 2 +-
>  tests/Benchmark.hackbench/parser.py     | 2 +-
>  tests/Benchmark.lmbench2/parser.py      | 2 +-
>  tests/Benchmark.migratetest/parser.py   | 2 +-
>  tests/Benchmark.pmqtest/parser.py       | 2 +-
>  tests/Benchmark.ptsematest/parser.py    | 2 +-
>  tests/Benchmark.signaltest/parser.py    | 2 +-
>  tests/Benchmark.sigwaittest/parser.py   | 2 +-
>  tests/Benchmark.svsematest/parser.py    | 2 +-
>  tests/Functional.LTP/fuego_test.sh      | 4 ++--
>  tests/Functional.LTP/ltp_process.py     | 2 +-
>  tests/Functional.LTP/parser.py          | 2 +-
>  tests/Functional.LTP_one_test/parser.py | 2 +-
>  tests/Functional.autopkgtest/parser.py  | 2 +-
>  tests/Functional.linaro/parser.py       | 2 +-
>  24 files changed, 28 insertions(+), 28 deletions(-)
>=20
> diff --git a/scripts/check-dependencies b/scripts/check-dependencies
> index 583012e..7369bd4 100755
> --- a/scripts/check-dependencies
> +++ b/scripts/check-dependencies
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3

check-dependencies is used by Functional.LTP, so I'm surprised you haven't =
run into this
one.  Maybe you have and this program works fine with python3.

check-dependencies is invoked directly (not using run_python), or by callin=
g the interpreter,
but it is only called in one place (Functional.LTP/fuego_test.sh), so maybe=
 that one place could
call run_python, to make it so that this would work with a conditional inte=
rpreter.
(see below for how I might make common.sh auto-detect the python interprete=
r)

>  # vim: set ts=3D4 sw=3D4 et :
>  #
>  # check_dependencies - check config dependencies listed in a file
> diff --git a/scripts/common.sh b/scripts/common.sh
> index f916d84..cf93ddb 100644
> --- a/scripts/common.sh
> +++ b/scripts/common.sh
> @@ -108,20 +108,20 @@ function run_python() {
>      if [ ! -z $ORIG_PATH ] ; then
>          dprint "run_python with PATH=3D$ORIG_PATH, TOOLCHAIN=3D$TOOLCHAI=
N"
>          export TOOLCHAIN
> -        PATH=3D$ORIG_PATH TOOLCHAIN=3D$TOOLCHAIN python "$@"
> +        PATH=3D$ORIG_PATH TOOLCHAIN=3D$TOOLCHAIN python3 "$@"
For this one and the next, I'm thinking of using a variable to indicate the=
 interpreter:
PYTHON_EXE, that I would detect outside of the run_python function, with so=
mething like this:

INTERPRETER_LIST=3D"python3 python python2"
for interpreter in $INTERPRETER_LIST ; do
    if [ -x /usr/bin/${interpreter} ] ; then
        PYTHON_EXE=3D"/usr/bin/${interpreter}"
        break
    fi
done
if [ -z "$PYTHON_EXE" ] ; then
    abort_job "No python interpreter found!"
fi

this line would then become:
PATH=3D$ORIG_PATH TOOLCHAIN=3D$TOOLCHAIN $PYTHON_EXE "$@"

Note the use of $ORIG_PATH.  Some toolchain setup scrips modify the PATH so=
 that
the 'python' that is executed is one that the SDK for the toolchain set up,=
 with=20
libs from the target sysroot.  This is not desired for Fuego's use of pytho=
n, which
should always use the host's python interpreter.  I'll have to see if this =
proposed
change (which would use the fullpath to the interpreter, instead of just th=
e interpreter
found in the PATH), would affect this use of ORIG_PATH with some toolchains=
.

Let me know if you have any thoughts about this.
Specifically, does your toolchain setup script set ORIG_PATH?

>      else
>          dprint "run_python with TOOLCHAIN=3D$TOOLCHAIN"
>          export TOOLCHAIN
> -        TOOLCHAIN=3D$TOOLCHAIN python "$@"
> +        TOOLCHAIN=3D$TOOLCHAIN python3 "$@"
and this would become:
TOOLCHAIN=3D$TOOLCHAIN $PYTHON_EXE "$@"

>      fi
>  }
>=20
>  function run_python_quiet() {
>      if [ ! -z $ORIG_PATH ]
>      then
> -        PATH=3D$ORIG_PATH python "$@"
> +        PATH=3D$ORIG_PATH python3 "$@"
As near as I can tell, run_python_quiet is only ever used by overlays/base/=
base-params.fuegoclass
(and it shows up in expected values in Functional.fuego_ftc_test).
This is used to invoke sercp, serlogin, and sersh for serial port connectio=
ns to the target.
(that is, when the TRANSPORT=3Dserial)

I have NOT tested sercp, serlogin and sersh for python3 compatibility (but =
I already know
they are NOT python3 compatible right now).  Given that they
are managing serial port data flow on the  serial port at a byte-at-a-time =
level, they will need
extensive analysis to make sure that the string handling does not break whe=
n they are run
with python3 (where strings default to Unicode instead of byte strings).
I have another project on github that also handles data flow on a serial po=
rt, and it was
very difficult to make it work correctly on both python2 and python3.
This one I'll have to defer.

If I understand correctly, the way that you use Fuego is by installing it n=
atively onto
the device under test, and using TRANSPORT=3Dlocal.  If you don't use the s=
erial TRANSPORT
or the serial port tools (serio suite of tools), then this shouldn't affect=
 you.

In any event, this can not be changed to python3 yet.
>      else
> -        python "$@"
> +        python3 "$@"
Nor this one. (see above)

>      fi
>  }
>=20
> diff --git a/scripts/ftc b/scripts/ftc
> index adce17b..4c5ff7e 100755
> --- a/scripts/ftc
> +++ b/scripts/ftc
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3

Have you been running ftc with this change already?

ftc has a large number of dependencies on python modules, especially in the=
 area of
report generation.  The docker container never installs python3 versions of=
 some of
these esoteric modules, such as reportlab and openpyxl.

Just out of curiosity, do you import these modules in your environment, or =
just not
use the ftc functionality that depends on them?

Also, ftc has a few remaining python3 incompatibilities:
  -  raw_input vs input
  - some urllib-related fixes, for the python3 refactoring of ulrparse and =
urllib

Is ftc functioning correctly with python3 now, for all your usage scenarios=
?

>  #
>  # vim: set ts=3D4 sw=3D4 et :
>  #
> diff --git a/scripts/functions.sh b/scripts/functions.sh
> index 7afe423..5a6e0e5 100755
> --- a/scripts/functions.sh
> +++ b/scripts/functions.sh
> @@ -506,7 +506,7 @@ function build {
>          call_if_present test_build
>          ret=3D$?
>          build_end_time=3D$(date +"%s.%N")
> -        build_duration=3D$(python -c "print($build_end_time - $build_sta=
rt_time)")
> +        build_duration=3D$(python3 -c "print($build_end_time - $build_st=
art_time)")
If I do the PYTHON_EXE thing in common.sh, I should be able to use that her=
e as well.

>=20
>          # test_build may change the current dir
>          # get back to root of build dir, before 'touch'
> diff --git a/tests/Benchmark.Dhrystone/parser.py b/tests/Benchmark.Dhryst=
one/parser.py
> index fd07cff..868f9de 100755
> --- a/tests/Benchmark.Dhrystone/parser.py
> +++ b/tests/Benchmark.Dhrystone/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
Finally, on all these parser shebangs, I'm not sure what to do.

I experimented with creating a wrapper script, called run_python.sh, that d=
etermines
the correct interpreter to use, and changing these to:
#!/fuego-core/scripts/run_python.sh

It works, but I don't know how robust it is.  These parser are never invoke=
d directly,
as far as I can tell.  It might be better to actually remove the shebang li=
ne to make
it explicitly impossible to treat the parser.py scripts as standalone progr=
ams.

I'm not sure if I'd be breaking anyone's workflow with this or not, so I he=
sitate
to do that.

What do you think?

>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.IOzone/parser.py b/tests/Benchmark.IOzone/pa=
rser.py
> index b1631ae..a56aa0e 100755
> --- a/tests/Benchmark.IOzone/parser.py
> +++ b/tests/Benchmark.IOzone/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, sys
>  import common as plib
> diff --git a/tests/Benchmark.Stream/parser.py b/tests/Benchmark.Stream/pa=
rser.py
> index a564d74..926e611 100755
> --- a/tests/Benchmark.Stream/parser.py
> +++ b/tests/Benchmark.Stream/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.bonnie/parser.py b/tests/Benchmark.bonnie/pa=
rser.py
> index b68bc3e..962a7cd 100755
> --- a/tests/Benchmark.bonnie/parser.py
> +++ b/tests/Benchmark.bonnie/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.cyclictest/parser.py b/tests/Benchmark.cycli=
ctest/parser.py
> index c29393e..4252654 100755
> --- a/tests/Benchmark.cyclictest/parser.py
> +++ b/tests/Benchmark.cyclictest/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  import os, re, sys
>  import common as plib
>=20
> diff --git a/tests/Benchmark.fio/parser.py b/tests/Benchmark.fio/parser.p=
y
> index ab3ea34..fedb734 100644
> --- a/tests/Benchmark.fio/parser.py
> +++ b/tests/Benchmark.fio/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  # See common.py for description of command-line arguments
>=20
>  import os, sys
> diff --git a/tests/Benchmark.hackbench/parser.py b/tests/Benchmark.hackbe=
nch/parser.py
> index a22c480..3ac9ba5 100755
> --- a/tests/Benchmark.hackbench/parser.py
> +++ b/tests/Benchmark.hackbench/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.lmbench2/parser.py b/tests/Benchmark.lmbench=
2/parser.py
> index f06f132..410b46e 100755
> --- a/tests/Benchmark.lmbench2/parser.py
> +++ b/tests/Benchmark.lmbench2/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.migratetest/parser.py b/tests/Benchmark.migr=
atetest/parser.py
> index 627ec2d..21b7c56 100755
> --- a/tests/Benchmark.migratetest/parser.py
> +++ b/tests/Benchmark.migratetest/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  import os, re, sys
>  import common as plib
>=20
> diff --git a/tests/Benchmark.pmqtest/parser.py b/tests/Benchmark.pmqtest/=
parser.py
> index 05ee57b..a5c31ba 100755
> --- a/tests/Benchmark.pmqtest/parser.py
> +++ b/tests/Benchmark.pmqtest/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.ptsematest/parser.py b/tests/Benchmark.ptsem=
atest/parser.py
> index 05ee57b..a5c31ba 100755
> --- a/tests/Benchmark.ptsematest/parser.py
> +++ b/tests/Benchmark.ptsematest/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.signaltest/parser.py b/tests/Benchmark.signa=
ltest/parser.py
> index 1992b21..95d4340 100755
> --- a/tests/Benchmark.signaltest/parser.py
> +++ b/tests/Benchmark.signaltest/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.sigwaittest/parser.py b/tests/Benchmark.sigw=
aittest/parser.py
> index 25a0262..120b7b5 100755
> --- a/tests/Benchmark.sigwaittest/parser.py
> +++ b/tests/Benchmark.sigwaittest/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  import os, re, sys
>  import common as plib
>=20
> diff --git a/tests/Benchmark.svsematest/parser.py b/tests/Benchmark.svsem=
atest/parser.py
> index 05ee57b..a5c31ba 100755
> --- a/tests/Benchmark.svsematest/parser.py
> +++ b/tests/Benchmark.svsematest/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Functional.LTP/fuego_test.sh b/tests/Functional.LTP/fu=
ego_test.sh
> index ef58bcc..95dea2c 100755
> --- a/tests/Functional.LTP/fuego_test.sh
> +++ b/tests/Functional.LTP/fuego_test.sh
> @@ -475,9 +475,9 @@ function test_processing {
>          #  ImportError: No module named style
>          if [ -n "$ORIG_PATH" ] ; then
>              # Use ORIG_PATH, if defined, so that python works properly
> -            PATH=3D$ORIG_PATH python ltp_process.py
> +            PATH=3D$ORIG_PATH python3 ltp_process.py
>          else
> -            python ltp_process.py
> +            python3 ltp_process.py
>          fi
>=20
>          [ -e results.xlsx ] && cp results.xlsx ${LOGDIR}/results.xlsx
> diff --git a/tests/Functional.LTP/ltp_process.py b/tests/Functional.LTP/l=
tp_process.py
> index 8e2b637..27bc856 100644
> --- a/tests/Functional.LTP/ltp_process.py
> +++ b/tests/Functional.LTP/ltp_process.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  # -*- coding: UTF-8 -*-
>  from openpyxl import Workbook
>  from openpyxl.styles import Border, Side, PatternFill, Color, Alignment
> diff --git a/tests/Functional.LTP/parser.py b/tests/Functional.LTP/parser=
.py
> index 9ad7659..3467328 100755
> --- a/tests/Functional.LTP/parser.py
> +++ b/tests/Functional.LTP/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  # -*- coding: UTF-8 -*-
>  import os, os.path, re, sys
>  import common as plib
> diff --git a/tests/Functional.LTP_one_test/parser.py b/tests/Functional.L=
TP_one_test/parser.py
> index 312bd5b..7002e35 100755
> --- a/tests/Functional.LTP_one_test/parser.py
> +++ b/tests/Functional.LTP_one_test/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  # See common.py for description of command-line arguments
>=20
>  import os
> diff --git a/tests/Functional.autopkgtest/parser.py b/tests/Functional.au=
topkgtest/parser.py
> index ba3dea1..ba8d2a0 100755
> --- a/tests/Functional.autopkgtest/parser.py
> +++ b/tests/Functional.autopkgtest/parser.py
> @@ -1,4 +1,4 @@
> -#!/bin/python
> +#!/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Functional.linaro/parser.py b/tests/Functional.linaro/=
parser.py
> index 48b502b..5bbb5de 100755
> --- a/tests/Functional.linaro/parser.py
> +++ b/tests/Functional.linaro/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, sys, collections
>  import common as plib
> --
> 2.20.1
>=20

Finally, before changing the shebang in all the parser.py scripts, I'd like=
 to implement
the compatibility changes first.  That is, change all the print statements =
to be parenthesized,
BEFORE changing the shebang lines.  This way the code is never left in a fa=
ulty state
(such that a git bisect would fail).

Let me know your thoughts on the issues above.

It looks like at least some of you are on vacation the next few days, but I=
'll wait to hear
back before I start attacking this problem.

By the way - what distribution of Linux are you testing?  Is this for CIP? =
or for some
Toshiba-internal distro of Linux?  (just curious).

 -- Tim