Re: [Fuego] [PATCH] Convert the print statement to the print() function

Venkata,

Thanks for the detailed information.  I have done some investigation of old
Fuego docker containers, to see which ones have support for python3.

From 1.0 through 1.4, there is no support for /usr/bin/python3, let alone
the extra libraries that (current) ftc needs to fulfill its functionality.

Given this, I'd like make the invocation of python3 in common.sh in the
function run_python, conditional.  I honestly don't know if there are many
people using such old Fuego docker containers, but I think a simple auto-de=
tection
should suffice to make current tests backwards compatible with python2 (and
these older containers).

I'm still unsure what to do about the shebang lines.  They are not strictly=
 necessary.
I'm not sure under what conditions anyone would run the scripts directly (a=
s opposed
to as an argument of their desired python interpreter).

I'll discuss this a bit more in the thread with the patch you sent.
 -- Tim

> -----Original Message-----
> From: [email protected] <[email protected]>
> Sent: Friday, September 30, 2022 8:19 AM
> To: Bird, Tim <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; fuego=
@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH] Convert the print statement to the print() f=
unction
>=20
> Hi Tim,
>=20
> I am happy to talk with you again.
>=20
> Thanks for your comments and please find my answers inline below.
>=20
> >-----Original Message-----
> >From: Fuego <[email protected]> On Behalf Of Bird,=
 Tim
> >Sent: 30 September 2022 00:58
> >To: nakkala sireesha(=1B$B#T#S#I#P=1B(B TMIEC ODG Porting) <sireesha.nak=
kala@toshiba-
> >tsip.com>; [email protected]
> >Cc: hayashi kazuhiro(=1B$BNS=1B(B =1B$BOB9(=1B(B =1B$B""#S#W#C"~#A#C#T=
=1B(B)
> ><[email protected]>; [email protected]; dines=
h
> >kumar(=1B$B#T#S#I#P=1B(B TMIEC ODG Porting) <[email protected]=
om>
> >Subject: Re: [Fuego] [PATCH] Convert the print statement to the print() =
function
> >
> >Sireesha,
> >
> >I like the effort to make the scripts compatible with both the python2 a=
nd python3
> >interpreters, but I have some questions about portions of the patches.
> >
> >
> >> -----Original Message-----
> >> From: [email protected]
> >> <[email protected]>
> >>
> >> From: sireesha <[email protected]>
> >>
> >> Update parentheses for all print statements in python script.
> >> It is compatible with both python2 and python3 interpreters.
> >>
> >> Signed-off-by: sireesha <[email protected]>
> >> ---
> >>  tests/Benchmark.IOzone/parser.py   |  4 ++--
> >>  tests/Benchmark.Stream/parser.py   |  4 ++--
> >>  tests/Benchmark.bonnie/parser.py   |  8 ++++----
> >>  tests/Benchmark.lmbench2/parser.py | 13 ++++++-------
> >>  4 files changed, 14 insertions(+), 15 deletions(-)  mode change
> >> 100755 =3D> 100644 tests/Benchmark.lmbench2/parser.py
> >>
> >> diff --git a/tests/Benchmark.IOzone/parser.py
> >> b/tests/Benchmark.IOzone/parser.py
> >> index b1631ae..8a7d5b2 100755
> >> --- a/tests/Benchmark.IOzone/parser.py
> >> +++ b/tests/Benchmark.IOzone/parser.py
> >> @@ -1,11 +1,11 @@
> >> -#!/usr/bin/python
> >> +#!/usr/bin/python3
> >
> >In my setup of Fuego, this will have no effect.  All invocations of the =
parser in
> >Fuego execute parser.py as an argument of 'python'.  Specifically, see t=
he
> >run_python function in fuego-core/scripts/common.sh.  The actual invocat=
ion
> >looks like this:
> >
> >PATH=3D$ORIG_PATH TOOLCHAIN=3D$TOOLCHAIN python "$@"
> >(where the $@ arguments contain, among other things, 'parser.py')
> >
> >I believe Toshiba is using Fuego in a configuration where Fuego is run n=
atively
> >(outside the docker container).  Does /usr/bin/python in your environmen=
t default
> >to python3?
> >Or, did Toshiba change the run_python function to call python3 instead? =
or to
> >directly executed the parser.py script?
>=20
> Actually, in first we were changing our environment (/usr/bin/python -> /=
usr/bin/python3) and run all fuego python scripts with
> default shebang.
> But later we felt it is not good change, so we started updating the scrip=
ts to use python3 always,
> Also we modified fuego-core/scripts/common.sh as below
>=20
> PATH=3D$ORIG_PATH TOOLCHAIN=3D$TOOLCHAIN python3 "$@"
>=20
> We have not sent this patch to you because we are not sure how this patch=
 affects the existing environment of yours or it expects to
> support both python2 and 3?
>=20
> May be we should discuss more about this and finalize the approach consid=
ering the backward compatibility?
>=20
> I will send the patch in another mail that I am talking about, which will=
 have python -> python3 changes for those tests we were
> using, maybe we can decide the approach after seeing the patch.
>=20
> >
> >I need to understand this so that I can decide what the best solution is=
 for
> >supporting both docker-based and native-based Fuego installations, as we=
ll as
> >supporting legacy Fuego docker containers (which might not have a compat=
ible
> >python3 interpreter).
> >
> >Also...
> >If this change (to the #! header for the parser script) was needed, it w=
ould be
> >needed for all tests you are running.  Or, at least, I see unparenthesiz=
ed print
> >statements in the following tests:
> >IOzone, Stream, x11perf, gtkperf, tiobench, LTP_one_test, Interbench, ip=
erf,
> >ebizzy, bonnie, BF_power_test, aim7, blobsallad, fs_mark, nbench_byte, p=
test,
> >lmbench2
> >
> >It looks like the following test already have parenthesized print statem=
ents:
> >arch_time, LTP, autopkgtest, fuego_ftc_test, fuego_tguid_check,
> >fuego_check_tables, fio, and dd
> >
> >
> >I guess my question is what tests are you running (only those 4, or thos=
e 4 and
> >others)? and Do you plan to modify all the tests that have unparenthesiz=
ed print
> >statements (ie, with additional patches)?
> As of now we fixed print parenthesis problems in those tests which we wer=
e using
> (dhrystone, hackbench, iperf3, lmbench2, fio, bonnie, iozone, stream)
>=20
> and for the remaining tests we don=1B$B!G=1B(Bt have setup to test them a=
fter changing.
> So, we are not planning to change in other tests.
>=20
> >
> >If so, I'll hold off doing it myself.  But if you don't plan to look at =
the others, I'll do
> >a sweep through the existing tests and adjust all print statements to be
> >compatible with both python2 and python3.
> >
> >>
> >>  import os, sys
> >>  import common as plib
> >>
> >>  measurements =3D {}
> >>  cur_file =3D open(plib.TEST_LOG,'r')
> >> -print "Reading current values from " + plib.TEST_LOG +"\n"
> >> +print("Reading current values from " + plib.TEST_LOG +"\n")
> >
> >This print (which a lot of parsers have) is not really that important.  =
It's more of a
> >debug statement, and could probably be replaced with a call to dprint() =
(which is
> >defined in common.py, which is always imported as plib (!) - there are s=
o many
> >oddities in this legacy code!), or removed entirely.
> >
> >In any event, I guess I should resist the urge to change all these to dp=
rints().
> >Doing so would change the test's default output, which might be a backwa=
rd-
> >compatibility-breaking change.
> >
> >Having said all that, this change to parenthesize the print statement is=
 fine.
> >
> >>
> >>  lines =3D cur_file.readlines()
> >>  cur_file.close()
> >> diff --git a/tests/Benchmark.Stream/parser.py
> >> b/tests/Benchmark.Stream/parser.py
> >> index a564d74..69edd27 100755
> >> --- a/tests/Benchmark.Stream/parser.py
> >> +++ b/tests/Benchmark.Stream/parser.py
> >> @@ -1,4 +1,4 @@
> >> -#!/usr/bin/python
> >> +#!/usr/bin/python3
> >Same issue here as above.
> >
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> @@ -7,7 +7,7 @@ ref_section_pat =3D "^\[[\w]+.[gle]{2}\]"
> >>  # groups           1        2
> >>  cur_search_str =3D "^(\w*):\ *([\d]{1,5}.[\d]{1,4}).*"
> >>
> >> -print "Reading current values from " + plib.TEST_LOG
> >> +print("Reading current values from " + plib.TEST_LOG)
> >>  cur_file =3D open(plib.TEST_LOG,'r')
> >>
> >>  lines =3D cur_file.readlines()
> >> diff --git a/tests/Benchmark.bonnie/parser.py
> >> b/tests/Benchmark.bonnie/parser.py
> >> index b68bc3e..5f35af4 100755
> >> --- a/tests/Benchmark.bonnie/parser.py
> >> +++ b/tests/Benchmark.bonnie/parser.py
> >> @@ -5,16 +5,16 @@ import common as plib
> >>
> >>  measurements =3D {}
> >>
> >> -print "Reading current values from " + plib.TEST_LOG + "\n"
> >> +print("Reading current values from " + plib.TEST_LOG + "\n")
> >>  with open(plib.TEST_LOG,'r') as cur_file:
> >>      raw_values =3D cur_file.readlines()
> >>      results =3D raw_values[-1].rstrip("\n").split(",")
> >>
> >>  if len(results) < 26:
> >> -    print "\nFuego error reason: No results found\n"
> >> +    print("\nFuego error reason: No results found\n")
> >>      sys.exit(2)
> >>
> >> -print "Bonnie++ raw results: " + str(results)
> >> +print("Bonnie++ raw results: " + str(results))
> >>
> >>
> >>  measurements["Sequential_Output.PerChr"] =3D [] @@ -81,6 +81,6 @@ if
> >> not '+' in results[26]:
> >>
> >>  # Add a hint when the spec seems to require a bigger SIZE  if '+' in
> >> results[4]:
> >> -    print "\nWARNING: Some test result data is missing. You might nee=
d bigger
> >test data size.\nTry the test using the 'more-data' spec
> >> for better results.\n"
> >> +    print("\nWARNING: Some test result data is missing. You might
> >> + need bigger test data size.\nTry the test using the 'more-data' spec
> >> for better results.\n")
> >>
> >>  sys.exit(plib.process(measurements))
> >> diff --git a/tests/Benchmark.lmbench2/parser.py
> >> b/tests/Benchmark.lmbench2/parser.py
> >> old mode 100755
> >> new mode 100644
> >> index f06f132..5ccfe97
> >> --- a/tests/Benchmark.lmbench2/parser.py
> >> +++ b/tests/Benchmark.lmbench2/parser.py
> >> @@ -1,4 +1,4 @@
> >> -#!/usr/bin/python
> >> +#!/usr/bin/python3
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> @@ -11,7 +11,7 @@ cur_search_pat =3D
> >> re.compile("^\S+\s+Linux\s[0-9a-z.-]+\s+([\s\d.KMGT-]*)",re.MUL
> >>
> >>  cur_dict =3D {}
> >>  cur_file =3D open(plib.TEST_LOG,'r')
> >> -print "Reading current values from " + plib.TEST_LOG +"\n"
> >> +print("Reading current values from " + plib.TEST_LOG +"\n")
> >>
> >>  lines =3D cur_file.readlines()
> >>
> >> @@ -19,18 +19,17 @@ lines =3D cur_file.readlines()  t_index =3D 0  sub=
list
> >> =3D []
> >>
> >> -print lines
> >> +print(lines)
> >>
> >>  for line in lines:
> >>      result =3D cur_search_pat.findall(line)
> >>      if result and len(result[0]) > 0:
> >> -        print 'result: ', result
> >> +        print('result: ', result)
> >>
> >>          # Ugly hack to work around invalid processing of empty cells
> >>          result[0] =3D result[0].replace('       ', '     0 ')
> >>          test_res =3D result[0].rstrip('\n').split(' ')
> >> -
> >> -        print "test_res =3D %s" % (test_res)
> >> +        print("test_res =3D %s" % (test_res))
> >>
> >>          if 0 < t_index < 9:
> >>              for value in test_res:
> >> @@ -104,6 +103,6 @@ cur_dict["Memory_Latencies.Main_mem"] =3D
> >> sublist[48]  #cur_dict["Memory_Latencies.Guesses"] =3D sublist[49]
> >> cur_dict["Memory_Latencies.Rand_mem"] =3D sublist[50]
> >>
> >> -print "cur_dict =3D %s" % (cur_dict)
> >> +print("cur_dict =3D %s" % (cur_dict))
> >This is also one of those debug statements that should probably be a dpr=
int().
> >
> >I'm more inclined to change this one, because this is just showing inter=
nal data of
> >the parser, which really should not be displayed in the non-debug case.
> >
> >>
> >>  sys.exit(plib.process_data(ref_section_pat, cur_dict, 'xl', ' '))
> >> --
> >> 2.20.1
> >>
> >
> >OK.  Overall, all the print statement conversions are fine.  However, I =
don't want
> >to change the #!/usr/bin/python lines, as I don't think that has any act=
ual effect.
> >If it does need to be changed, it will need to be changed globally in al=
l parser.py
> >files.  AND I'll have to go back and test to make sure that this will wo=
rk with
> >previous docker containers (from Fuego 1.2 and above) to make sure they =
have a
> >compatible python3 interpreter installed.
> >
> Sorry for these lines (#!/usr/bin/python3), actually it is not intended t=
o send these lines for the upstream,
> Until it is decided to replace completely with python3.
>=20
> A separate patch I am talking about in the above, supposed to have these =
lines and add this change when it is decided to use.
>=20
> >Thanks for raising this issue.  Please let me know the answers to my que=
stions,
> >and we'll get this change in (and possibly other related changes).
> >
> >We may have to drag Fuego kicking and screaming into full python3
> >compatibility. :-)
> >
> > -- Tim
> >
> >
> >_______________________________________________
> >Fuego mailing list
> >[email protected]
> >https://lists.linuxfoundation.org/mailman/listinfo/fuego