All of lore.kernel.org
 help / color / mirror / Atom feed
* [Fuego] [PATCH] Convert the print statement to the print() function
@ 2022-09-29 13:20 sireesha.nakkala
  2022-09-29 19:27 ` Bird, Tim
  0 siblings, 1 reply; 11+ messages in thread
From: sireesha.nakkala @ 2022-09-29 13:20 UTC (permalink / raw)
  To: tim.bird, fuego
  Cc: kazuhiro3.hayashi, daniel.sangorrin, dinesh.kumar, sireesha

From: sireesha <sireesha.nakkala@toshiba-tsip.com>

Update parentheses for all print statements in python script.
It is compatible with both python2 and python3 interpreters.

Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
---
 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 => 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
 
 import os, sys
 import common as plib
 
 measurements = {}
 cur_file = open(plib.TEST_LOG,'r')
-print "Reading current values from " + plib.TEST_LOG +"\n"
+print("Reading current values from " + plib.TEST_LOG +"\n")
 
 lines = 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
 
 import os, re, sys
 import common as plib
@@ -7,7 +7,7 @@ ref_section_pat = "^\[[\w]+.[gle]{2}\]"
 # groups           1        2
 cur_search_str = "^(\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 = open(plib.TEST_LOG,'r')
 
 lines = 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 = {}
 
-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 = cur_file.readlines()
     results = 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"] = []
@@ -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 need 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 = re.compile("^\S+\s+Linux\s[0-9a-z.-]+\s+([\s\d.KMGT-]*)",re.MUL
 
 cur_dict = {}
 cur_file = open(plib.TEST_LOG,'r')
-print "Reading current values from " + plib.TEST_LOG +"\n"
+print("Reading current values from " + plib.TEST_LOG +"\n")
 
 lines = cur_file.readlines()
 
@@ -19,18 +19,17 @@ lines = cur_file.readlines()
 t_index = 0
 sublist = []
 
-print lines
+print(lines)
 
 for line in lines:
     result = 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] = result[0].replace('       ', '     0 ')
         test_res = result[0].rstrip('\n').split(' ')
-
-        print "test_res = %s" % (test_res)
+        print("test_res = %s" % (test_res))
 
         if 0 < t_index < 9:
             for value in test_res:
@@ -104,6 +103,6 @@ cur_dict["Memory_Latencies.Main_mem"] = sublist[48]
 #cur_dict["Memory_Latencies.Guesses"] = sublist[49]
 cur_dict["Memory_Latencies.Rand_mem"] = sublist[50]
 
-print "cur_dict = %s" % (cur_dict)
+print("cur_dict = %s" % (cur_dict))
 
 sys.exit(plib.process_data(ref_section_pat, cur_dict, 'xl', ' '))
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Fuego] [PATCH] Convert the print statement to the print() function
  2022-09-29 13:20 [Fuego] [PATCH] Convert the print statement to the print() function sireesha.nakkala
@ 2022-09-29 19:27 ` Bird, Tim
  2022-09-30 14:18   ` Venkata.Pyla
  2022-09-30 14:26   ` [Fuego] [PATCH] python3: Use python3 in the scripts to fully migrate venkata.pyla
  0 siblings, 2 replies; 11+ messages in thread
From: Bird, Tim @ 2022-09-29 19:27 UTC (permalink / raw)
  To: sireesha.nakkala, fuego; +Cc: kazuhiro3.hayashi, daniel.sangorrin, dinesh.kumar

Sireesha,

I like the effort to make the scripts compatible with both the python2
and python3 interpreters, but I have some questions about portions of the patches.


> -----Original Message-----
> From: sireesha.nakkala@toshiba-tsip.com <sireesha.nakkala@toshiba-tsip.com>
> 
> From: sireesha <sireesha.nakkala@toshiba-tsip.com>
> 
> Update parentheses for all print statements in python script.
> It is compatible with both python2 and python3 interpreters.
> 
> Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
> ---
>  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 => 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 the run_python
function in fuego-core/scripts/common.sh.  The actual invocation looks like this:

PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python "$@"
(where the $@ arguments contain, among other things, 'parser.py')

I believe Toshiba is using Fuego in a configuration where Fuego is run natively
(outside the docker container).  Does /usr/bin/python in your environment default
to python3?

Or, did Toshiba change the run_python function to call python3 instead? or to directly
executed the parser.py script?

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 well as
supporting legacy Fuego docker containers (which might not have a compatible
python3 interpreter).

Also...
If this change (to the #! header for the parser script) was needed, it would be needed
for all tests you are running.  Or, at least, I see unparenthesized print statements
in the following tests:
IOzone, Stream, x11perf, gtkperf, tiobench, LTP_one_test, Interbench, iperf,
ebizzy, bonnie, BF_power_test, aim7, blobsallad, fs_mark, nbench_byte, ptest, lmbench2

It looks like the following test already have parenthesized print statements:
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 those 4 and others)? and Do
you plan to modify all the tests that have unparenthesized print statements (ie, with additional patches)?

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 = {}
>  cur_file = 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
so many oddities in this legacy code!), or removed entirely.

In any event, I guess I should resist the urge to change all these to dprints().
Doing so would change the test's default output, which might be a
backward-compatibility-breaking change.

Having said all that, this change to parenthesize the print statement is fine.

> 
>  lines = 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 = "^\[[\w]+.[gle]{2}\]"
>  # groups           1        2
>  cur_search_str = "^(\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 = open(plib.TEST_LOG,'r')
> 
>  lines = 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 = {}
> 
> -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 = cur_file.readlines()
>      results = 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"] = []
> @@ -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 need 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 = re.compile("^\S+\s+Linux\s[0-9a-z.-]+\s+([\s\d.KMGT-]*)",re.MUL
> 
>  cur_dict = {}
>  cur_file = open(plib.TEST_LOG,'r')
> -print "Reading current values from " + plib.TEST_LOG +"\n"
> +print("Reading current values from " + plib.TEST_LOG +"\n")
> 
>  lines = cur_file.readlines()
> 
> @@ -19,18 +19,17 @@ lines = cur_file.readlines()
>  t_index = 0
>  sublist = []
> 
> -print lines
> +print(lines)
> 
>  for line in lines:
>      result = 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] = result[0].replace('       ', '     0 ')
>          test_res = result[0].rstrip('\n').split(' ')
> -
> -        print "test_res = %s" % (test_res)
> +        print("test_res = %s" % (test_res))
> 
>          if 0 < t_index < 9:
>              for value in test_res:
> @@ -104,6 +103,6 @@ cur_dict["Memory_Latencies.Main_mem"] = sublist[48]
>  #cur_dict["Memory_Latencies.Guesses"] = sublist[49]
>  cur_dict["Memory_Latencies.Rand_mem"] = sublist[50]
> 
> -print "cur_dict = %s" % (cur_dict)
> +print("cur_dict = %s" % (cur_dict))
This is also one of those debug statements that should probably be a dprint().

I'm more inclined to change this one, because this is just showing internal
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 actual effect.  If it does need to be
changed, it will need to be changed globally in all parser.py files.  AND I'll have to go back and
test to make sure that this will work with previous docker containers (from Fuego 1.2 and above) to make
sure they have a compatible python3 interpreter installed.

Thanks for raising this issue.  Please let me know the answers to my questions, 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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fuego] [PATCH] Convert the print statement to the print() function
  2022-09-29 19:27 ` Bird, Tim
@ 2022-09-30 14:18   ` Venkata.Pyla
  2022-10-04 17:51     ` Bird, Tim
  2022-09-30 14:26   ` [Fuego] [PATCH] python3: Use python3 in the scripts to fully migrate venkata.pyla
  1 sibling, 1 reply; 11+ messages in thread
From: Venkata.Pyla @ 2022-09-30 14:18 UTC (permalink / raw)
  To: Tim.Bird, sireesha.nakkala; +Cc: kazuhiro3.hayashi, dinesh.kumar, fuego

Hi Tim,

I am happy to talk with you again.

Thanks for your comments and please find my answers inline below.

>-----Original Message-----
>From: Fuego <fuego-bounces@lists.linuxfoundation.org> On Behalf Of Bird, Tim
>Sent: 30 September 2022 00:58
>To: nakkala sireesha(TSIP TMIEC ODG Porting) <sireesha.nakkala@toshiba-
>tsip.com>; fuego@lists.linuxfoundation.org
>Cc: hayashi kazuhiro(林 和宏 □SWC◯ACT)
><kazuhiro3.hayashi@toshiba.co.jp>; daniel.sangorrin@toshiba.co.jp; dinesh
>kumar(TSIP TMIEC ODG Porting) <dinesh.kumar@toshiba-tsip.com>
>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 and python3
>interpreters, but I have some questions about portions of the patches.
>
>
>> -----Original Message-----
>> From: sireesha.nakkala@toshiba-tsip.com
>> <sireesha.nakkala@toshiba-tsip.com>
>>
>> From: sireesha <sireesha.nakkala@toshiba-tsip.com>
>>
>> Update parentheses for all print statements in python script.
>> It is compatible with both python2 and python3 interpreters.
>>
>> Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
>> ---
>>  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 => 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 the
>run_python function in fuego-core/scripts/common.sh.  The actual invocation
>looks like this:
>
>PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python "$@"
>(where the $@ arguments contain, among other things, 'parser.py')
>
>I believe Toshiba is using Fuego in a configuration where Fuego is run natively
>(outside the docker container).  Does /usr/bin/python in your environment default
>to python3?
>Or, did Toshiba change the run_python function to call python3 instead? or to
>directly executed the parser.py script?

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 scripts to use python3 always,
Also we modified fuego-core/scripts/common.sh as below

PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python3 "$@"

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?

May be we should discuss more about this and finalize the approach considering the backward compatibility?

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.

>
>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 well as
>supporting legacy Fuego docker containers (which might not have a compatible
>python3 interpreter).
>
>Also...
>If this change (to the #! header for the parser script) was needed, it would be
>needed for all tests you are running.  Or, at least, I see unparenthesized print
>statements in the following tests:
>IOzone, Stream, x11perf, gtkperf, tiobench, LTP_one_test, Interbench, iperf,
>ebizzy, bonnie, BF_power_test, aim7, blobsallad, fs_mark, nbench_byte, ptest,
>lmbench2
>
>It looks like the following test already have parenthesized print statements:
>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 those 4 and
>others)? and Do you plan to modify all the tests that have unparenthesized print
>statements (ie, with additional patches)?
As of now we fixed print parenthesis problems in those tests which we were using
(dhrystone, hackbench, iperf3, lmbench2, fio, bonnie, iozone, stream)

and for the remaining tests we don’t have setup to test them after changing.
So, we are not planning to change in other tests.

>
>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 = {}
>>  cur_file = 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 so many
>oddities in this legacy code!), or removed entirely.
>
>In any event, I guess I should resist the urge to change all these to dprints().
>Doing so would change the test's default output, which might be a backward-
>compatibility-breaking change.
>
>Having said all that, this change to parenthesize the print statement is fine.
>
>>
>>  lines = 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 = "^\[[\w]+.[gle]{2}\]"
>>  # groups           1        2
>>  cur_search_str = "^(\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 = open(plib.TEST_LOG,'r')
>>
>>  lines = 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 = {}
>>
>> -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 = cur_file.readlines()
>>      results = 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"] = [] @@ -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 need 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 =
>> re.compile("^\S+\s+Linux\s[0-9a-z.-]+\s+([\s\d.KMGT-]*)",re.MUL
>>
>>  cur_dict = {}
>>  cur_file = open(plib.TEST_LOG,'r')
>> -print "Reading current values from " + plib.TEST_LOG +"\n"
>> +print("Reading current values from " + plib.TEST_LOG +"\n")
>>
>>  lines = cur_file.readlines()
>>
>> @@ -19,18 +19,17 @@ lines = cur_file.readlines()  t_index = 0  sublist
>> = []
>>
>> -print lines
>> +print(lines)
>>
>>  for line in lines:
>>      result = 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] = result[0].replace('       ', '     0 ')
>>          test_res = result[0].rstrip('\n').split(' ')
>> -
>> -        print "test_res = %s" % (test_res)
>> +        print("test_res = %s" % (test_res))
>>
>>          if 0 < t_index < 9:
>>              for value in test_res:
>> @@ -104,6 +103,6 @@ cur_dict["Memory_Latencies.Main_mem"] =
>> sublist[48]  #cur_dict["Memory_Latencies.Guesses"] = sublist[49]
>> cur_dict["Memory_Latencies.Rand_mem"] = sublist[50]
>>
>> -print "cur_dict = %s" % (cur_dict)
>> +print("cur_dict = %s" % (cur_dict))
>This is also one of those debug statements that should probably be a dprint().
>
>I'm more inclined to change this one, because this is just showing internal 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 actual effect.
>If it does need to be changed, it will need to be changed globally in all parser.py
>files.  AND I'll have to go back and test to make sure that this will work 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 to send these lines for the upstream, 
Until it is decided to replace completely with python3.

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.

>Thanks for raising this issue.  Please let me know the answers to my questions,
>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
>Fuego@lists.linuxfoundation.org
>https://lists.linuxfoundation.org/mailman/listinfo/fuego


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Fuego] [PATCH] python3: Use python3 in the scripts to fully migrate
  2022-09-29 19:27 ` Bird, Tim
  2022-09-30 14:18   ` Venkata.Pyla
@ 2022-09-30 14:26   ` venkata.pyla
  2022-10-05  0:22     ` Bird, Tim
  1 sibling, 1 reply; 11+ messages in thread
From: venkata.pyla @ 2022-09-30 14:26 UTC (permalink / raw)
  To: tim.bird, fuego; +Cc: kazuhiro3.hayashi, dinesh.kumar, sireesha.nakkala

From: venkata pyla <venkata.pyla@toshiba-tsip.com>

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

Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
---
 scripts/check-dependencies              | 2 +-
 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(-)

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
 # vim: set ts=4 sw=4 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=$ORIG_PATH, TOOLCHAIN=$TOOLCHAIN"
         export TOOLCHAIN
-        PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python "$@"
+        PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python3 "$@"
     else
         dprint "run_python with TOOLCHAIN=$TOOLCHAIN"
         export TOOLCHAIN
-        TOOLCHAIN=$TOOLCHAIN python "$@"
+        TOOLCHAIN=$TOOLCHAIN python3 "$@"
     fi
 }
 
 function run_python_quiet() {
     if [ ! -z $ORIG_PATH ]
     then
-        PATH=$ORIG_PATH python "$@"
+        PATH=$ORIG_PATH python3 "$@"
     else
-        python "$@"
+        python3 "$@"
     fi
 }
 
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
 #
 # vim: set ts=4 sw=4 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=$?
         build_end_time=$(date +"%s.%N")
-        build_duration=$(python -c "print($build_end_time - $build_start_time)")
+        build_duration=$(python3 -c "print($build_end_time - $build_start_time)")
 
         # 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.Dhrystone/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
 
 import os, re, sys
 import common as plib
diff --git a/tests/Benchmark.IOzone/parser.py b/tests/Benchmark.IOzone/parser.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
 
 import os, sys
 import common as plib
diff --git a/tests/Benchmark.Stream/parser.py b/tests/Benchmark.Stream/parser.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
 
 import os, re, sys
 import common as plib
diff --git a/tests/Benchmark.bonnie/parser.py b/tests/Benchmark.bonnie/parser.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
 
 import os, re, sys
 import common as plib
diff --git a/tests/Benchmark.cyclictest/parser.py b/tests/Benchmark.cyclictest/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
 
diff --git a/tests/Benchmark.fio/parser.py b/tests/Benchmark.fio/parser.py
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
 
 import os, sys
diff --git a/tests/Benchmark.hackbench/parser.py b/tests/Benchmark.hackbench/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
 
 import os, re, sys
 import common as plib
diff --git a/tests/Benchmark.lmbench2/parser.py b/tests/Benchmark.lmbench2/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
 
 import os, re, sys
 import common as plib
diff --git a/tests/Benchmark.migratetest/parser.py b/tests/Benchmark.migratetest/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
 
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
 
 import os, re, sys
 import common as plib
diff --git a/tests/Benchmark.ptsematest/parser.py b/tests/Benchmark.ptsematest/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
 
 import os, re, sys
 import common as plib
diff --git a/tests/Benchmark.signaltest/parser.py b/tests/Benchmark.signaltest/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
 
 import os, re, sys
 import common as plib
diff --git a/tests/Benchmark.sigwaittest/parser.py b/tests/Benchmark.sigwaittest/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
 
diff --git a/tests/Benchmark.svsematest/parser.py b/tests/Benchmark.svsematest/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
 
 import os, re, sys
 import common as plib
diff --git a/tests/Functional.LTP/fuego_test.sh b/tests/Functional.LTP/fuego_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=$ORIG_PATH python ltp_process.py
+            PATH=$ORIG_PATH python3 ltp_process.py
         else
-            python ltp_process.py
+            python3 ltp_process.py
         fi
 
         [ -e results.xlsx ] && cp results.xlsx ${LOGDIR}/results.xlsx
diff --git a/tests/Functional.LTP/ltp_process.py b/tests/Functional.LTP/ltp_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.LTP_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
 
 import os
diff --git a/tests/Functional.autopkgtest/parser.py b/tests/Functional.autopkgtest/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
 
 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
 
 import os, sys, collections
 import common as plib
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Fuego] [PATCH] Convert the print statement to the print() function
  2022-09-30 14:18   ` Venkata.Pyla
@ 2022-10-04 17:51     ` Bird, Tim
  0 siblings, 0 replies; 11+ messages in thread
From: Bird, Tim @ 2022-10-04 17:51 UTC (permalink / raw)
  To: Venkata.Pyla, sireesha.nakkala; +Cc: kazuhiro3.hayashi, dinesh.kumar, fuego

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-detection
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 (as 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: Venkata.Pyla@toshiba-tsip.com <Venkata.Pyla@toshiba-tsip.com>
> Sent: Friday, September 30, 2022 8:19 AM
> To: Bird, Tim <Tim.Bird@sony.com>; sireesha.nakkala@toshiba-tsip.com
> Cc: kazuhiro3.hayashi@toshiba.co.jp; dinesh.kumar@toshiba-tsip.com; fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH] Convert the print statement to the print() function
> 
> Hi Tim,
> 
> I am happy to talk with you again.
> 
> Thanks for your comments and please find my answers inline below.
> 
> >-----Original Message-----
> >From: Fuego <fuego-bounces@lists.linuxfoundation.org> On Behalf Of Bird, Tim
> >Sent: 30 September 2022 00:58
> >To: nakkala sireesha(TSIP TMIEC ODG Porting) <sireesha.nakkala@toshiba-
> >tsip.com>; fuego@lists.linuxfoundation.org
> >Cc: hayashi kazuhiro(林 和宏 □SWC◯ACT)
> ><kazuhiro3.hayashi@toshiba.co.jp>; daniel.sangorrin@toshiba.co.jp; dinesh
> >kumar(TSIP TMIEC ODG Porting) <dinesh.kumar@toshiba-tsip.com>
> >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 and python3
> >interpreters, but I have some questions about portions of the patches.
> >
> >
> >> -----Original Message-----
> >> From: sireesha.nakkala@toshiba-tsip.com
> >> <sireesha.nakkala@toshiba-tsip.com>
> >>
> >> From: sireesha <sireesha.nakkala@toshiba-tsip.com>
> >>
> >> Update parentheses for all print statements in python script.
> >> It is compatible with both python2 and python3 interpreters.
> >>
> >> Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
> >> ---
> >>  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 => 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 the
> >run_python function in fuego-core/scripts/common.sh.  The actual invocation
> >looks like this:
> >
> >PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python "$@"
> >(where the $@ arguments contain, among other things, 'parser.py')
> >
> >I believe Toshiba is using Fuego in a configuration where Fuego is run natively
> >(outside the docker container).  Does /usr/bin/python in your environment default
> >to python3?
> >Or, did Toshiba change the run_python function to call python3 instead? or to
> >directly executed the parser.py script?
> 
> 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 scripts to use python3 always,
> Also we modified fuego-core/scripts/common.sh as below
> 
> PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python3 "$@"
> 
> 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?
> 
> May be we should discuss more about this and finalize the approach considering the backward compatibility?
> 
> 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.
> 
> >
> >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 well as
> >supporting legacy Fuego docker containers (which might not have a compatible
> >python3 interpreter).
> >
> >Also...
> >If this change (to the #! header for the parser script) was needed, it would be
> >needed for all tests you are running.  Or, at least, I see unparenthesized print
> >statements in the following tests:
> >IOzone, Stream, x11perf, gtkperf, tiobench, LTP_one_test, Interbench, iperf,
> >ebizzy, bonnie, BF_power_test, aim7, blobsallad, fs_mark, nbench_byte, ptest,
> >lmbench2
> >
> >It looks like the following test already have parenthesized print statements:
> >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 those 4 and
> >others)? and Do you plan to modify all the tests that have unparenthesized print
> >statements (ie, with additional patches)?
> As of now we fixed print parenthesis problems in those tests which we were using
> (dhrystone, hackbench, iperf3, lmbench2, fio, bonnie, iozone, stream)
> 
> and for the remaining tests we don’t have setup to test them after changing.
> So, we are not planning to change in other tests.
> 
> >
> >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 = {}
> >>  cur_file = 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 so many
> >oddities in this legacy code!), or removed entirely.
> >
> >In any event, I guess I should resist the urge to change all these to dprints().
> >Doing so would change the test's default output, which might be a backward-
> >compatibility-breaking change.
> >
> >Having said all that, this change to parenthesize the print statement is fine.
> >
> >>
> >>  lines = 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 = "^\[[\w]+.[gle]{2}\]"
> >>  # groups           1        2
> >>  cur_search_str = "^(\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 = open(plib.TEST_LOG,'r')
> >>
> >>  lines = 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 = {}
> >>
> >> -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 = cur_file.readlines()
> >>      results = 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"] = [] @@ -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 need 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 =
> >> re.compile("^\S+\s+Linux\s[0-9a-z.-]+\s+([\s\d.KMGT-]*)",re.MUL
> >>
> >>  cur_dict = {}
> >>  cur_file = open(plib.TEST_LOG,'r')
> >> -print "Reading current values from " + plib.TEST_LOG +"\n"
> >> +print("Reading current values from " + plib.TEST_LOG +"\n")
> >>
> >>  lines = cur_file.readlines()
> >>
> >> @@ -19,18 +19,17 @@ lines = cur_file.readlines()  t_index = 0  sublist
> >> = []
> >>
> >> -print lines
> >> +print(lines)
> >>
> >>  for line in lines:
> >>      result = 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] = result[0].replace('       ', '     0 ')
> >>          test_res = result[0].rstrip('\n').split(' ')
> >> -
> >> -        print "test_res = %s" % (test_res)
> >> +        print("test_res = %s" % (test_res))
> >>
> >>          if 0 < t_index < 9:
> >>              for value in test_res:
> >> @@ -104,6 +103,6 @@ cur_dict["Memory_Latencies.Main_mem"] =
> >> sublist[48]  #cur_dict["Memory_Latencies.Guesses"] = sublist[49]
> >> cur_dict["Memory_Latencies.Rand_mem"] = sublist[50]
> >>
> >> -print "cur_dict = %s" % (cur_dict)
> >> +print("cur_dict = %s" % (cur_dict))
> >This is also one of those debug statements that should probably be a dprint().
> >
> >I'm more inclined to change this one, because this is just showing internal 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 actual effect.
> >If it does need to be changed, it will need to be changed globally in all parser.py
> >files.  AND I'll have to go back and test to make sure that this will work 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 to send these lines for the upstream,
> Until it is decided to replace completely with python3.
> 
> 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.
> 
> >Thanks for raising this issue.  Please let me know the answers to my questions,
> >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
> >Fuego@lists.linuxfoundation.org
> >https://lists.linuxfoundation.org/mailman/listinfo/fuego


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fuego] [PATCH] python3: Use python3 in the scripts to fully migrate
  2022-09-30 14:26   ` [Fuego] [PATCH] python3: Use python3 in the scripts to fully migrate venkata.pyla
@ 2022-10-05  0:22     ` Bird, Tim
  2022-10-06 23:59       ` Bird, Tim
  2022-10-10 13:00       ` Venkata.Pyla
  0 siblings, 2 replies; 11+ messages in thread
From: Bird, Tim @ 2022-10-05  0:22 UTC (permalink / raw)
  To: venkata.pyla, fuego; +Cc: kazuhiro3.hayashi, dinesh.kumar, sireesha.nakkala

OK -  I have some more comments on this patch.

See inline below.

> -----Original Message-----
> From: venkata.pyla@toshiba-tsip.com <venkata.pyla@toshiba-tsip.com>
> 
> 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
> 
> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> ---
>  scripts/check-dependencies              | 2 +-

My first comment is that invocation using just the interpreter name of 'python' 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 , generic_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 circumstances
(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(-)
> 
> 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 calling 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 interpreter.
(see below for how I might make common.sh auto-detect the python interpreter)

>  # vim: set ts=4 sw=4 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=$ORIG_PATH, TOOLCHAIN=$TOOLCHAIN"
>          export TOOLCHAIN
> -        PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python "$@"
> +        PATH=$ORIG_PATH TOOLCHAIN=$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 something like this:

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

this line would then become:
PATH=$ORIG_PATH TOOLCHAIN=$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 
libs from the target sysroot.  This is not desired for Fuego's use of python, 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 the 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=$TOOLCHAIN"
>          export TOOLCHAIN
> -        TOOLCHAIN=$TOOLCHAIN python "$@"
> +        TOOLCHAIN=$TOOLCHAIN python3 "$@"
and this would become:
TOOLCHAIN=$TOOLCHAIN $PYTHON_EXE "$@"

>      fi
>  }
> 
>  function run_python_quiet() {
>      if [ ! -z $ORIG_PATH ]
>      then
> -        PATH=$ORIG_PATH python "$@"
> +        PATH=$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 connections to the target.
(that is, when the TRANSPORT=serial)

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 when 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 port, 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 natively onto
the device under test, and using TRANSPORT=local.  If you don't use the serial 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
>  }
> 
> 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=4 sw=4 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=$?
>          build_end_time=$(date +"%s.%N")
> -        build_duration=$(python -c "print($build_end_time - $build_start_time)")
> +        build_duration=$(python3 -c "print($build_end_time - $build_start_time)")
If I do the PYTHON_EXE thing in common.sh, I should be able to use that here as well.

> 
>          # 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.Dhrystone/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 determines
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 invoked directly,
as far as I can tell.  It might be better to actually remove the shebang line to make
it explicitly impossible to treat the parser.py scripts as standalone programs.

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

What do you think?

> 
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.IOzone/parser.py b/tests/Benchmark.IOzone/parser.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
> 
>  import os, sys
>  import common as plib
> diff --git a/tests/Benchmark.Stream/parser.py b/tests/Benchmark.Stream/parser.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
> 
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.bonnie/parser.py b/tests/Benchmark.bonnie/parser.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
> 
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.cyclictest/parser.py b/tests/Benchmark.cyclictest/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
> 
> diff --git a/tests/Benchmark.fio/parser.py b/tests/Benchmark.fio/parser.py
> 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
> 
>  import os, sys
> diff --git a/tests/Benchmark.hackbench/parser.py b/tests/Benchmark.hackbench/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
> 
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.lmbench2/parser.py b/tests/Benchmark.lmbench2/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
> 
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.migratetest/parser.py b/tests/Benchmark.migratetest/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
> 
> 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
> 
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.ptsematest/parser.py b/tests/Benchmark.ptsematest/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
> 
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.signaltest/parser.py b/tests/Benchmark.signaltest/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
> 
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Benchmark.sigwaittest/parser.py b/tests/Benchmark.sigwaittest/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
> 
> diff --git a/tests/Benchmark.svsematest/parser.py b/tests/Benchmark.svsematest/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
> 
>  import os, re, sys
>  import common as plib
> diff --git a/tests/Functional.LTP/fuego_test.sh b/tests/Functional.LTP/fuego_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=$ORIG_PATH python ltp_process.py
> +            PATH=$ORIG_PATH python3 ltp_process.py
>          else
> -            python ltp_process.py
> +            python3 ltp_process.py
>          fi
> 
>          [ -e results.xlsx ] && cp results.xlsx ${LOGDIR}/results.xlsx
> diff --git a/tests/Functional.LTP/ltp_process.py b/tests/Functional.LTP/ltp_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.LTP_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
> 
>  import os
> diff --git a/tests/Functional.autopkgtest/parser.py b/tests/Functional.autopkgtest/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
> 
>  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
> 
>  import os, sys, collections
>  import common as plib
> --
> 2.20.1
> 

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fuego] [PATCH] python3: Use python3 in the scripts to fully migrate
  2022-10-05  0:22     ` Bird, Tim
@ 2022-10-06 23:59       ` Bird, Tim
  2022-10-10 13:00       ` Venkata.Pyla
  1 sibling, 0 replies; 11+ messages in thread
From: Bird, Tim @ 2022-10-06 23:59 UTC (permalink / raw)
  To: venkata.pyla, fuego; +Cc: kazuhiro3.hayashi, dinesh.kumar, sireesha.nakkala

Just to follow up on this...

> -----Original Message-----
> From: Bird, Tim
> 
...
> 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 faulty state
> (such that a git bisect would fail).

I have done this for python print statements I parser.py and create_xml*.py and fuego_test.sh
scripts, under the fuego-core/tests directory.

These have been pushed to the master branch of fuego-core on bitbucket.

I found a surprising number of other oddball cases where the python interpreter is used,
can called directly.  There's one I especially worry about for your case in Functional.LTP.
But maybe you've already deal with this.  It looks like ltp_process.py doesn't have any
unparenthesized print statements, but it does have a '/usr/bin/python' shebang line.

Did you already convert this shebang at your site?

I'm working on my PYTHON_EXE idea next, and I'll let you know how it goes.
 -- Tim


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fuego] [PATCH] python3: Use python3 in the scripts to fully migrate
  2022-10-05  0:22     ` Bird, Tim
  2022-10-06 23:59       ` Bird, Tim
@ 2022-10-10 13:00       ` Venkata.Pyla
  2022-10-19 17:44         ` [Fuego] Report on python3 compatibility in Fuego (was something else) Bird, Tim
  1 sibling, 1 reply; 11+ messages in thread
From: Venkata.Pyla @ 2022-10-10 13:00 UTC (permalink / raw)
  To: Tim.Bird, fuego; +Cc: kazuhiro3.hayashi, dinesh.kumar, sireesha.nakkala

Hi Tim,

Sorry for the late, I am on vacation last week.

Please find my answers below.

>-----Original Message-----
>From: Bird, Tim <Tim.Bird@sony.com>
>Sent: 05 October 2022 05:52
>To: pyla venkata(TSIP TMIEC ODG Porting) <Venkata.Pyla@toshiba-
>tsip.com>; fuego@lists.linuxfoundation.org
>Cc: nakkala sireesha(TSIP TMIEC ODG Porting) <sireesha.nakkala@toshiba-
>tsip.com>; dinesh kumar(TSIP TMIEC ODG Porting) <dinesh.kumar@toshiba-
>tsip.com>; hayashi kazuhiro(林 和宏 □SWC◯ACT)
><kazuhiro3.hayashi@toshiba.co.jp>
>Subject: RE: [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: venkata.pyla@toshiba-tsip.com <venkata.pyla@toshiba-tsip.com>
>>
>> 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
>>
>> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
>> ---
>>  scripts/check-dependencies              | 2 +-
>
>My first comment is that invocation using just the interpreter name of 'python' 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 ,
>generic_parser.py, test_filelock.py (in the scripts directory).  Many of these you
>are likely not using.
Yes, many of the files were not used by Toshiba, so we fixed only in the tests where it is breaking.

>
>However, some of these are intended for use by Fuego users, for special
>circumstances (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(-)
>>
>> 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.
We use LTP test and so we changed the shebang here, and as you suggested below it is good to use `run_python` which will internally find the right interpreter as you wrote the code, then this shebang is not required or we can remove.

>
>check-dependencies is invoked directly (not using run_python), or by calling 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 interpreter.
>(see below for how I might make common.sh auto-detect the python interpreter)
This will be a good solution for the backward compatibility.

>
>>  # vim: set ts=4 sw=4 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=$ORIG_PATH, TOOLCHAIN=$TOOLCHAIN"
>>          export TOOLCHAIN
>> -        PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python "$@"
>> +        PATH=$ORIG_PATH TOOLCHAIN=$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
>something like this:
>
>INTERPRETER_LIST="python3 python python2"
>for interpreter in $INTERPRETER_LIST ; do
>    if [ -x /usr/bin/${interpreter} ] ; then
>        PYTHON_EXE="/usr/bin/${interpreter}"
>        break
>    fi
>done
>if [ -z "$PYTHON_EXE" ] ; then
>    abort_job "No python interpreter found!"
>fi
>
>this line would then become:
>PATH=$ORIG_PATH TOOLCHAIN=$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
>libs from the target sysroot.  This is not desired for Fuego's use of python, 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 the
>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?
We are not using the ORIG_PATH variable, in our case the host environment interpreter is sufficient to run the scripts.

But in our patch we have modified in both the places when ORIG_PATH is defined and not defined because we wanted at least change python version problem in the same file.

>
>>      else
>>          dprint "run_python with TOOLCHAIN=$TOOLCHAIN"
>>          export TOOLCHAIN
>> -        TOOLCHAIN=$TOOLCHAIN python "$@"
>> +        TOOLCHAIN=$TOOLCHAIN python3 "$@"
>and this would become:
>TOOLCHAIN=$TOOLCHAIN $PYTHON_EXE "$@"
>
>>      fi
>>  }
>>
>>  function run_python_quiet() {
>>      if [ ! -z $ORIG_PATH ]
>>      then
>> -        PATH=$ORIG_PATH python "$@"
>> +        PATH=$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 connections to the
>target.
>(that is, when the TRANSPORT=serial)
>
>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
>when 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 port, 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 natively
>onto the device under test, and using TRANSPORT=local.  If you don't use the
>serial TRANSPORT or the serial port tools (serio suite of tools), then this shouldn't
>affect you.
Yes, this change is not required for us, as mentioned above we modified all python versions in the same file.
You can ignore this change and thanks for letting me know about the function 'run_python_quiet'

>
>In any event, this can not be changed to python3 yet.
>>      else
>> -        python "$@"
>> +        python3 "$@"
>Nor this one. (see above)
>
>>      fi
>>  }
>>
>> 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?
We installed python3 dependencies in our target machine.
as you may know we are not using the docker-container and installing the fuego-core directly on the target machine, and the python3 dependencies are already installed in the target machine.

>
>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?
It is working fine with our current usage scenarios, because in our environment we were not using the Jenkins or the features related to those functions
I think we should fix them because they are obviously required when completely migrate to pyhon3.

>
>>  #
>>  # vim: set ts=4 sw=4 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=$?
>>          build_end_time=$(date +"%s.%N")
>> -        build_duration=$(python -c "print($build_end_time - $build_start_time)")
>> +        build_duration=$(python3 -c "print($build_end_time -
>> + $build_start_time)")
>If I do the PYTHON_EXE thing in common.sh, I should be able to use that here as
>well.
>
>>
>>          # 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.Dhrystone/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
>determines 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 invoked
>directly, as far as I can tell.  It might be better to actually remove the shebang
>line to make it explicitly impossible to treat the parser.py scripts as standalone
>programs.
>
>I'm not sure if I'd be breaking anyone's workflow with this or not, so I hesitate to
>do that.
>
>What do you think?
In our use cases also we were not directly running them, so we can remove the shebang lines if no one else also not using it directly.

>
>>
>>  import os, re, sys
>>  import common as plib
>> diff --git a/tests/Benchmark.IOzone/parser.py
>> b/tests/Benchmark.IOzone/parser.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
>>
>>  import os, sys
>>  import common as plib
>> diff --git a/tests/Benchmark.Stream/parser.py
>> b/tests/Benchmark.Stream/parser.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
>>
>>  import os, re, sys
>>  import common as plib
>> diff --git a/tests/Benchmark.bonnie/parser.py
>> b/tests/Benchmark.bonnie/parser.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
>>
>>  import os, re, sys
>>  import common as plib
>> diff --git a/tests/Benchmark.cyclictest/parser.py
>> b/tests/Benchmark.cyclictest/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
>>
>> diff --git a/tests/Benchmark.fio/parser.py
>> b/tests/Benchmark.fio/parser.py 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
>>
>>  import os, sys
>> diff --git a/tests/Benchmark.hackbench/parser.py
>> b/tests/Benchmark.hackbench/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
>>
>>  import os, re, sys
>>  import common as plib
>> diff --git a/tests/Benchmark.lmbench2/parser.py
>> b/tests/Benchmark.lmbench2/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
>>
>>  import os, re, sys
>>  import common as plib
>> diff --git a/tests/Benchmark.migratetest/parser.py
>> b/tests/Benchmark.migratetest/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
>>
>> 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
>>
>>  import os, re, sys
>>  import common as plib
>> diff --git a/tests/Benchmark.ptsematest/parser.py
>> b/tests/Benchmark.ptsematest/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
>>
>>  import os, re, sys
>>  import common as plib
>> diff --git a/tests/Benchmark.signaltest/parser.py
>> b/tests/Benchmark.signaltest/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
>>
>>  import os, re, sys
>>  import common as plib
>> diff --git a/tests/Benchmark.sigwaittest/parser.py
>> b/tests/Benchmark.sigwaittest/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
>>
>> diff --git a/tests/Benchmark.svsematest/parser.py
>> b/tests/Benchmark.svsematest/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
>>
>>  import os, re, sys
>>  import common as plib
>> diff --git a/tests/Functional.LTP/fuego_test.sh
>> b/tests/Functional.LTP/fuego_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=$ORIG_PATH python ltp_process.py
>> +            PATH=$ORIG_PATH python3 ltp_process.py
>>          else
>> -            python ltp_process.py
>> +            python3 ltp_process.py
>>          fi
>>
>>          [ -e results.xlsx ] && cp results.xlsx ${LOGDIR}/results.xlsx
>> diff --git a/tests/Functional.LTP/ltp_process.py
>> b/tests/Functional.LTP/ltp_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.LTP_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
>>
>>  import os
>> diff --git a/tests/Functional.autopkgtest/parser.py
>> b/tests/Functional.autopkgtest/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
>>
>>  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
>>
>>  import os, sys, collections
>>  import common as plib
>> --
>> 2.20.1
>>
>
>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 faulty 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).
We are using on Debian based distribution with Bullseye version, this is one of the reason why we are changing the scripts to support python3, because bullseye in not supporting python2.

At the first we have changed in few tests and some common function scripts which we are using, because in our environment the fuego-core runs on test/target machine directly this patch works for us, and I am not confident these changes will work with docker-container, so the reason we have not shared this patch initially and only the shared some print statement fixes and other python2to3 compatible fixes.

After studying your suggestion in the above, there are quite few other conversions required to fully convert to python3.
Let me know if I can do some of the changes.

>
> -- Tim



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Fuego] Report on python3 compatibility in Fuego (was something else)
  2022-10-10 13:00       ` Venkata.Pyla
@ 2022-10-19 17:44         ` Bird, Tim
  2022-10-27  6:10           ` Venkata.Pyla
  0 siblings, 1 reply; 11+ messages in thread
From: Bird, Tim @ 2022-10-19 17:44 UTC (permalink / raw)
  To: Venkata.Pyla, fuego; +Cc: kazuhiro3.hayashi, dinesh.kumar, sireesha.nakkala

OK - I'm hijacking this thread to so a general report on python3 compatibility in Fuego.

I recently completed a long stretch of work on this, and here are some notes about
the work.

I modified common.sh, functions.sh, and test-parser.sh to set and use PY_EXE
I experimented with doing an autodetect to use python3 or python (2).
But ultimately I decided that doing an runtime autodetect is not a good policy, as it means
that factors unrelated to Fuego (such as installing a version of python) can affect
Fuego in unexpected ways.

So, I instead opted to create a script which can switch Fuego from python to python3
usage, or vice-versa.  That is, if a user switches Fuego to python3, but then discovers
problems, they can switch back to python2.  The new utility is called set-python, and
is in fuego-core/scripts.  It also checks that the required libraries for the specified
python version are present on the system.  In order to switch to a version of python,
that version of python must be present on your system.

The tool is "idempotent", meaning that if you run it twice with the same arguments,
it should not change anything the second time.  Specifically, it should not have
any deleterious effects.  So, if you're already done a conversion to python3, you
can run it again and it should not cause any problems.  Note that you can also
run the tool with '-c' to just check the status, without making any changes.

The tool must be run in fuego-core/scripts, outside of any docker container.  Use '-h'
to get usage help.  The script changes all shebang python lines in fuego scripts, with
the exception of parser.py shebang lines, which are always ignored.

I removed all the shebang lines from the parser core (fuego-core/scripts/parser/*.py), but
decided to leave the shebang lines in all the test/*/parser.py files.  They could be removed
(and probably should be), but I did a lot of changes this release and wanted to push these changes
out before doing another large set of changes.  The shebang lines in parser.py files are never
used, so they are superfluous, and don't reflect how the scripts will actually be called.

I also cleaned up remaining python3 incompatibilities in ftc and a few other scripts.
Specifically, I handled the input vs. raw_input issue, and the urllib.parse vs. urlparse issue.

Please try out the latest master branch and let me know if you have any problems, especially
with python3 compatibility.

If you could run set-python in your lab, and let me know what it shows you, that would be helpful.
That is please do:
  $ cd fuego-core/scripts ; set-python python3 (or 'set-python -c python3')
and let me know the results.

I haven't decided to switch Fuego to install using python3 by default, but I may do so after I
experiment with making serio python3 compatible, and when I do the next major release.
I have modified the install scripts to load all the needed python3 libraries, but haven't done
a docker test yet with these.

Regards,
 -- Tim

> -----Original Message-----
> From: Venkata.Pyla@toshiba-tsip.com <Venkata.Pyla@toshiba-tsip.com>
> Sent: Monday, October 10, 2022 7:00 AM
> To: Bird, Tim <Tim.Bird@sony.com>; fuego@lists.linuxfoundation.org
> Cc: sireesha.nakkala@toshiba-tsip.com; dinesh.kumar@toshiba-tsip.com; kazuhiro3.hayashi@toshiba.co.jp
> Subject: RE: [PATCH] python3: Use python3 in the scripts to fully migrate
> 
> Hi Tim,
> 
> Sorry for the late, I am on vacation last week.
> 
> Please find my answers below.
> 
> >-----Original Message-----
> >From: Bird, Tim <Tim.Bird@sony.com>
> >Sent: 05 October 2022 05:52
> >To: pyla venkata(TSIP TMIEC ODG Porting) <Venkata.Pyla@toshiba-
> >tsip.com>; fuego@lists.linuxfoundation.org
> >Cc: nakkala sireesha(TSIP TMIEC ODG Porting) <sireesha.nakkala@toshiba-
> >tsip.com>; dinesh kumar(TSIP TMIEC ODG Porting) <dinesh.kumar@toshiba-
> >tsip.com>; hayashi kazuhiro(林 和宏 □SWC◯ACT)
> ><kazuhiro3.hayashi@toshiba.co.jp>
> >Subject: RE: [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: venkata.pyla@toshiba-tsip.com <venkata.pyla@toshiba-tsip.com>
> >>
> >> 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
> >>
> >> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> >> ---
> >>  scripts/check-dependencies              | 2 +-
> >
> >My first comment is that invocation using just the interpreter name of 'python' 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 ,
> >generic_parser.py, test_filelock.py (in the scripts directory).  Many of these you
> >are likely not using.
> Yes, many of the files were not used by Toshiba, so we fixed only in the tests where it is breaking.
> 
> >
> >However, some of these are intended for use by Fuego users, for special
> >circumstances (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(-)
> >>
> >> 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.
> We use LTP test and so we changed the shebang here, and as you suggested below it is good to use `run_python` which will internally
> find the right interpreter as you wrote the code, then this shebang is not required or we can remove.
> 
> >
> >check-dependencies is invoked directly (not using run_python), or by calling 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 interpreter.
> >(see below for how I might make common.sh auto-detect the python interpreter)
> This will be a good solution for the backward compatibility.
> 
> >
> >>  # vim: set ts=4 sw=4 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=$ORIG_PATH, TOOLCHAIN=$TOOLCHAIN"
> >>          export TOOLCHAIN
> >> -        PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python "$@"
> >> +        PATH=$ORIG_PATH TOOLCHAIN=$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
> >something like this:
> >
> >INTERPRETER_LIST="python3 python python2"
> >for interpreter in $INTERPRETER_LIST ; do
> >    if [ -x /usr/bin/${interpreter} ] ; then
> >        PYTHON_EXE="/usr/bin/${interpreter}"
> >        break
> >    fi
> >done
> >if [ -z "$PYTHON_EXE" ] ; then
> >    abort_job "No python interpreter found!"
> >fi
> >
> >this line would then become:
> >PATH=$ORIG_PATH TOOLCHAIN=$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
> >libs from the target sysroot.  This is not desired for Fuego's use of python, 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 the
> >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?
> We are not using the ORIG_PATH variable, in our case the host environment interpreter is sufficient to run the scripts.
> 
> But in our patch we have modified in both the places when ORIG_PATH is defined and not defined because we wanted at least
> change python version problem in the same file.
> 
> >
> >>      else
> >>          dprint "run_python with TOOLCHAIN=$TOOLCHAIN"
> >>          export TOOLCHAIN
> >> -        TOOLCHAIN=$TOOLCHAIN python "$@"
> >> +        TOOLCHAIN=$TOOLCHAIN python3 "$@"
> >and this would become:
> >TOOLCHAIN=$TOOLCHAIN $PYTHON_EXE "$@"
> >
> >>      fi
> >>  }
> >>
> >>  function run_python_quiet() {
> >>      if [ ! -z $ORIG_PATH ]
> >>      then
> >> -        PATH=$ORIG_PATH python "$@"
> >> +        PATH=$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 connections to the
> >target.
> >(that is, when the TRANSPORT=serial)
> >
> >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
> >when 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 port, 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 natively
> >onto the device under test, and using TRANSPORT=local.  If you don't use the
> >serial TRANSPORT or the serial port tools (serio suite of tools), then this shouldn't
> >affect you.
> Yes, this change is not required for us, as mentioned above we modified all python versions in the same file.
> You can ignore this change and thanks for letting me know about the function 'run_python_quiet'
> 
> >
> >In any event, this can not be changed to python3 yet.
> >>      else
> >> -        python "$@"
> >> +        python3 "$@"
> >Nor this one. (see above)
> >
> >>      fi
> >>  }
> >>
> >> 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?
> We installed python3 dependencies in our target machine.
> as you may know we are not using the docker-container and installing the fuego-core directly on the target machine, and the python3
> dependencies are already installed in the target machine.
> 
> >
> >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?
> It is working fine with our current usage scenarios, because in our environment we were not using the Jenkins or the features related
> to those functions
> I think we should fix them because they are obviously required when completely migrate to pyhon3.
> 
> >
> >>  #
> >>  # vim: set ts=4 sw=4 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=$?
> >>          build_end_time=$(date +"%s.%N")
> >> -        build_duration=$(python -c "print($build_end_time - $build_start_time)")
> >> +        build_duration=$(python3 -c "print($build_end_time -
> >> + $build_start_time)")
> >If I do the PYTHON_EXE thing in common.sh, I should be able to use that here as
> >well.
> >
> >>
> >>          # 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.Dhrystone/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
> >determines 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 invoked
> >directly, as far as I can tell.  It might be better to actually remove the shebang
> >line to make it explicitly impossible to treat the parser.py scripts as standalone
> >programs.
> >
> >I'm not sure if I'd be breaking anyone's workflow with this or not, so I hesitate to
> >do that.
> >
> >What do you think?
> In our use cases also we were not directly running them, so we can remove the shebang lines if no one else also not using it directly.
> 
> >
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> diff --git a/tests/Benchmark.IOzone/parser.py
> >> b/tests/Benchmark.IOzone/parser.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
> >>
> >>  import os, sys
> >>  import common as plib
> >> diff --git a/tests/Benchmark.Stream/parser.py
> >> b/tests/Benchmark.Stream/parser.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
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> diff --git a/tests/Benchmark.bonnie/parser.py
> >> b/tests/Benchmark.bonnie/parser.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
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> diff --git a/tests/Benchmark.cyclictest/parser.py
> >> b/tests/Benchmark.cyclictest/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
> >>
> >> diff --git a/tests/Benchmark.fio/parser.py
> >> b/tests/Benchmark.fio/parser.py 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
> >>
> >>  import os, sys
> >> diff --git a/tests/Benchmark.hackbench/parser.py
> >> b/tests/Benchmark.hackbench/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
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> diff --git a/tests/Benchmark.lmbench2/parser.py
> >> b/tests/Benchmark.lmbench2/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
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> diff --git a/tests/Benchmark.migratetest/parser.py
> >> b/tests/Benchmark.migratetest/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
> >>
> >> 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
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> diff --git a/tests/Benchmark.ptsematest/parser.py
> >> b/tests/Benchmark.ptsematest/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
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> diff --git a/tests/Benchmark.signaltest/parser.py
> >> b/tests/Benchmark.signaltest/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
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> diff --git a/tests/Benchmark.sigwaittest/parser.py
> >> b/tests/Benchmark.sigwaittest/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
> >>
> >> diff --git a/tests/Benchmark.svsematest/parser.py
> >> b/tests/Benchmark.svsematest/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
> >>
> >>  import os, re, sys
> >>  import common as plib
> >> diff --git a/tests/Functional.LTP/fuego_test.sh
> >> b/tests/Functional.LTP/fuego_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=$ORIG_PATH python ltp_process.py
> >> +            PATH=$ORIG_PATH python3 ltp_process.py
> >>          else
> >> -            python ltp_process.py
> >> +            python3 ltp_process.py
> >>          fi
> >>
> >>          [ -e results.xlsx ] && cp results.xlsx ${LOGDIR}/results.xlsx
> >> diff --git a/tests/Functional.LTP/ltp_process.py
> >> b/tests/Functional.LTP/ltp_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.LTP_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
> >>
> >>  import os
> >> diff --git a/tests/Functional.autopkgtest/parser.py
> >> b/tests/Functional.autopkgtest/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
> >>
> >>  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
> >>
> >>  import os, sys, collections
> >>  import common as plib
> >> --
> >> 2.20.1
> >>
> >
> >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 faulty 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).
> We are using on Debian based distribution with Bullseye version, this is one of the reason why we are changing the scripts to support
> python3, because bullseye in not supporting python2.
> 
> At the first we have changed in few tests and some common function scripts which we are using, because in our environment the
> fuego-core runs on test/target machine directly this patch works for us, and I am not confident these changes will work with docker-
> container, so the reason we have not shared this patch initially and only the shared some print statement fixes and other
> python2to3 compatible fixes.
> 
> After studying your suggestion in the above, there are quite few other conversions required to fully convert to python3.
> Let me know if I can do some of the changes.
> 
> >
> > -- Tim
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fuego] Report on python3 compatibility in Fuego (was something else)
  2022-10-19 17:44         ` [Fuego] Report on python3 compatibility in Fuego (was something else) Bird, Tim
@ 2022-10-27  6:10           ` Venkata.Pyla
  2022-10-27 23:53             ` Bird, Tim
  0 siblings, 1 reply; 11+ messages in thread
From: Venkata.Pyla @ 2022-10-27  6:10 UTC (permalink / raw)
  To: Tim.Bird, fuego; +Cc: kazuhiro3.hayashi, dinesh.kumar, sireesha.nakkala

Hi Tim,

Thanks for improving the python3 compatibility problems, please see my inline comments.

Thanks,
Venkata.
 
>-----Original Message-----
>From: Bird, Tim <Tim.Bird@sony.com>
>Sent: 19 October 2022 23:15
>To: pyla venkata(TSIP TMIEC ODG Porting) <Venkata.Pyla@toshiba-
>tsip.com>; fuego@lists.linuxfoundation.org
>Cc: nakkala sireesha(TSIP TMIEC ODG Porting) <sireesha.nakkala@toshiba-
>tsip.com>; dinesh kumar(TSIP TMIEC ODG Porting)
><dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 □SWC◯ACT)
><kazuhiro3.hayashi@toshiba.co.jp>
>Subject: Report on python3 compatibility in Fuego (was something else)
>
>OK - I'm hijacking this thread to so a general report on python3 compatibility in
>Fuego.
>
>I recently completed a long stretch of work on this, and here are some notes
>about the work.
>
>I modified common.sh, functions.sh, and test-parser.sh to set and use PY_EXE I
>experimented with doing an autodetect to use python3 or python (2).
>But ultimately I decided that doing an runtime autodetect is not a good policy,
>as it means that factors unrelated to Fuego (such as installing a version of
>python) can affect Fuego in unexpected ways.
>
>So, I instead opted to create a script which can switch Fuego from python to
>python3 usage, or vice-versa.  That is, if a user switches Fuego to python3, but
>then discovers problems, they can switch back to python2.  The new utility is
>called set-python, and is in fuego-core/scripts.  It also checks that the required
>libraries for the specified python version are present on the system.  In order to
>switch to a version of python, that version of python must be present on your
>system.
>
>The tool is "idempotent", meaning that if you run it twice with the same
>arguments, it should not change anything the second time.  Specifically, it
>should not have any deleterious effects.  So, if you're already done a conversion
>to python3, you can run it again and it should not cause any problems.  Note
>that you can also run the tool with '-c' to just check the status, without making
>any changes.

'set-python' tool is a nice tool that you prepared, it is checking and modifying the shebang lines according to the system python version, this perfectly suited for us as well.
I applied your changes in our project and I randomly checked and it is working.

We will further verify while executing some of the tests and if there are any problems we will send fixes for them or report to you.

>
>The tool must be run in fuego-core/scripts, outside of any docker container.
>Use '-h'
>to get usage help.  The script changes all shebang python lines in fuego scripts,
>with the exception of parser.py shebang lines, which are always ignored.
>
>I removed all the shebang lines from the parser core (fuego-
>core/scripts/parser/*.py), but decided to leave the shebang lines in all the
>test/*/parser.py files.  They could be removed (and probably should be), but I
>did a lot of changes this release and wanted to push these changes out before
>doing another large set of changes.  The shebang lines in parser.py files are
>never used, so they are superfluous, and don't reflect how the scripts will
>actually be called.
>
>I also cleaned up remaining python3 incompatibilities in ftc and a few other
>scripts.
>Specifically, I handled the input vs. raw_input issue, and the urllib.parse vs.
>urlparse issue.
>
>Please try out the latest master branch and let me know if you have any
>problems, especially with python3 compatibility.
>
>If you could run set-python in your lab, and let me know what it shows you,
>that would be helpful.
>That is please do:
>  $ cd fuego-core/scripts ; set-python python3 (or 'set-python -c python3') and
>let me know the results.

I executed the 'set-python' in our environment with your patches and below are the results

1)
root@cloud:/fuego-core/scripts# ./set-python -c python3
Python version I am executing='python3' (3, 9, 2)
Python version requested='python3'
Report:
  Missing module jenkins

Here are the lines that need to change!
== check-dependencies ==
== deorphan-runs.py ==
== ftc ==
== gen-page.py ==
== jdiff ==
== ../tests/Functional.LTP/ltp_process.py ==
== ../tests/Functional.apertis_sanity_check/make-script-from-yaml.py ==
== common.sh ==
== test-parser.sh ==

2)
root@cloud:/fuego-core/scripts# ./set-python python3
Python version I am executing='python3' (3, 9, 2)
Python version requested='python3'
Report:
  Missing module jenkins

Here are some line changes!
== check-dependencies ==
#!/usr/bin/python
 -- is now --
#!/usr/bin/python3
== deorphan-runs.py ==
#!/usr/bin/python
 -- is now --
#!/usr/bin/python3
== ftc ==
 -- is now --
#!/usr/bin/python3
== gen-page.py ==
#!/usr/bin/python
 -- is now --
#!/usr/bin/python3
== jdiff ==
#!/usr/bin/python
 -- is now --
#!/usr/bin/python3
== ../tests/Functional.LTP/ltp_process.py ==
#!/usr/bin/python
 -- is now --
#!/usr/bin/python3
== ../tests/Functional.apertis_sanity_check/make-script-from-yaml.py ==
#!/usr/bin/python
 -- is now --
#!/usr/bin/python3
== common.sh ==
PY_EXE=/usr/bin/python
 -- is now --
PY_EXE=/usr/bin/python3
== test-parser.sh ==
PY_EXE=/usr/bin/python
 -- is now --
PY_EXE=/usr/bin/python3

I saw the shebang lines are changed to python3 and the tests are working.

>
>I haven't decided to switch Fuego to install using python3 by default, but I may
>do so after I experiment with making serio python3 compatible, and when I do
>the next major release.
>I have modified the install scripts to load all the needed python3 libraries, but
>haven't done a docker test yet with these.
>
>Regards,
> -- Tim
>
>> -----Original Message-----
>> From: Venkata.Pyla@toshiba-tsip.com <Venkata.Pyla@toshiba-tsip.com>
>> Sent: Monday, October 10, 2022 7:00 AM
>> To: Bird, Tim <Tim.Bird@sony.com>; fuego@lists.linuxfoundation.org
>> Cc: sireesha.nakkala@toshiba-tsip.com; dinesh.kumar@toshiba-tsip.com;
>> kazuhiro3.hayashi@toshiba.co.jp
>> Subject: RE: [PATCH] python3: Use python3 in the scripts to fully
>> migrate
>>
>> Hi Tim,
>>
>> Sorry for the late, I am on vacation last week.
>>
>> Please find my answers below.
>>
>> >-----Original Message-----
>> >From: Bird, Tim <Tim.Bird@sony.com>
>> >Sent: 05 October 2022 05:52
>> >To: pyla venkata(TSIP TMIEC ODG Porting) <Venkata.Pyla@toshiba-
>> >tsip.com>; fuego@lists.linuxfoundation.org
>> >Cc: nakkala sireesha(TSIP TMIEC ODG Porting)
>> ><sireesha.nakkala@toshiba- tsip.com>; dinesh kumar(TSIP TMIEC ODG
>> >Porting) <dinesh.kumar@toshiba- tsip.com>; hayashi kazuhiro(林 和宏
>> >□SWC◯ACT)
>> ><kazuhiro3.hayashi@toshiba.co.jp>
>> >Subject: RE: [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: venkata.pyla@toshiba-tsip.com <venkata.pyla@toshiba-tsip.com>
>> >>
>> >> 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
>> >>
>> >> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
>> >> ---
>> >>  scripts/check-dependencies              | 2 +-
>> >
>> >My first comment is that invocation using just the interpreter name
>> >of 'python' 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
>> >, generic_parser.py, test_filelock.py (in the scripts directory).
>> >Many of these you are likely not using.
>> Yes, many of the files were not used by Toshiba, so we fixed only in the tests
>where it is breaking.
>>
>> >
>> >However, some of these are intended for use by Fuego users, for
>> >special circumstances (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(-)
>> >>
>> >> 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.
>> We use LTP test and so we changed the shebang here, and as you
>> suggested below it is good to use `run_python` which will internally find the
>right interpreter as you wrote the code, then this shebang is not required or we
>can remove.
>>
>> >
>> >check-dependencies is invoked directly (not using run_python), or by
>> >calling 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
>interpreter.
>> >(see below for how I might make common.sh auto-detect the python
>> >interpreter)
>> This will be a good solution for the backward compatibility.
>>
>> >
>> >>  # vim: set ts=4 sw=4 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=$ORIG_PATH,
>TOOLCHAIN=$TOOLCHAIN"
>> >>          export TOOLCHAIN
>> >> -        PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python "$@"
>> >> +        PATH=$ORIG_PATH TOOLCHAIN=$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 something like this:
>> >
>> >INTERPRETER_LIST="python3 python python2"
>> >for interpreter in $INTERPRETER_LIST ; do
>> >    if [ -x /usr/bin/${interpreter} ] ; then
>> >        PYTHON_EXE="/usr/bin/${interpreter}"
>> >        break
>> >    fi
>> >done
>> >if [ -z "$PYTHON_EXE" ] ; then
>> >    abort_job "No python interpreter found!"
>> >fi
>> >
>> >this line would then become:
>> >PATH=$ORIG_PATH TOOLCHAIN=$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 libs from the target sysroot.  This is not
>> >desired for Fuego's use of python, 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 the
>> >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?
>> We are not using the ORIG_PATH variable, in our case the host environment
>interpreter is sufficient to run the scripts.
>>
>> But in our patch we have modified in both the places when ORIG_PATH is
>> defined and not defined because we wanted at least change python version
>problem in the same file.
>>
>> >
>> >>      else
>> >>          dprint "run_python with TOOLCHAIN=$TOOLCHAIN"
>> >>          export TOOLCHAIN
>> >> -        TOOLCHAIN=$TOOLCHAIN python "$@"
>> >> +        TOOLCHAIN=$TOOLCHAIN python3 "$@"
>> >and this would become:
>> >TOOLCHAIN=$TOOLCHAIN $PYTHON_EXE "$@"
>> >
>> >>      fi
>> >>  }
>> >>
>> >>  function run_python_quiet() {
>> >>      if [ ! -z $ORIG_PATH ]
>> >>      then
>> >> -        PATH=$ORIG_PATH python "$@"
>> >> +        PATH=$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
>> >connections to the target.
>> >(that is, when the TRANSPORT=serial)
>> >
>> >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 when 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 port, 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 natively onto the device under test, and using
>> >TRANSPORT=local.  If you don't use the serial TRANSPORT or the serial
>> >port tools (serio suite of tools), then this shouldn't affect you.
>> Yes, this change is not required for us, as mentioned above we modified all
>python versions in the same file.
>> You can ignore this change and thanks for letting me know about the function
>'run_python_quiet'
>>
>> >
>> >In any event, this can not be changed to python3 yet.
>> >>      else
>> >> -        python "$@"
>> >> +        python3 "$@"
>> >Nor this one. (see above)
>> >
>> >>      fi
>> >>  }
>> >>
>> >> 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?
>> We installed python3 dependencies in our target machine.
>> as you may know we are not using the docker-container and installing
>> the fuego-core directly on the target machine, and the python3 dependencies
>are already installed in the target machine.
>>
>> >
>> >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?
>> It is working fine with our current usage scenarios, because in our
>> environment we were not using the Jenkins or the features related to
>> those functions I think we should fix them because they are obviously
>required when completely migrate to pyhon3.
>>
>> >
>> >>  #
>> >>  # vim: set ts=4 sw=4 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=$?
>> >>          build_end_time=$(date +"%s.%N")
>> >> -        build_duration=$(python -c "print($build_end_time -
>$build_start_time)")
>> >> +        build_duration=$(python3 -c "print($build_end_time -
>> >> + $build_start_time)")
>> >If I do the PYTHON_EXE thing in common.sh, I should be able to use
>> >that here as well.
>> >
>> >>
>> >>          # 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.Dhrystone/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 determines 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
>> >invoked directly, as far as I can tell.  It might be better to
>> >actually remove the shebang line to make it explicitly impossible to
>> >treat the parser.py scripts as standalone programs.
>> >
>> >I'm not sure if I'd be breaking anyone's workflow with this or not,
>> >so I hesitate to do that.
>> >
>> >What do you think?
>> In our use cases also we were not directly running them, so we can remove
>the shebang lines if no one else also not using it directly.
>>
>> >
>> >>
>> >>  import os, re, sys
>> >>  import common as plib
>> >> diff --git a/tests/Benchmark.IOzone/parser.py
>> >> b/tests/Benchmark.IOzone/parser.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
>> >>
>> >>  import os, sys
>> >>  import common as plib
>> >> diff --git a/tests/Benchmark.Stream/parser.py
>> >> b/tests/Benchmark.Stream/parser.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
>> >>
>> >>  import os, re, sys
>> >>  import common as plib
>> >> diff --git a/tests/Benchmark.bonnie/parser.py
>> >> b/tests/Benchmark.bonnie/parser.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
>> >>
>> >>  import os, re, sys
>> >>  import common as plib
>> >> diff --git a/tests/Benchmark.cyclictest/parser.py
>> >> b/tests/Benchmark.cyclictest/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
>> >>
>> >> diff --git a/tests/Benchmark.fio/parser.py
>> >> b/tests/Benchmark.fio/parser.py 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
>> >>
>> >>  import os, sys
>> >> diff --git a/tests/Benchmark.hackbench/parser.py
>> >> b/tests/Benchmark.hackbench/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
>> >>
>> >>  import os, re, sys
>> >>  import common as plib
>> >> diff --git a/tests/Benchmark.lmbench2/parser.py
>> >> b/tests/Benchmark.lmbench2/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
>> >>
>> >>  import os, re, sys
>> >>  import common as plib
>> >> diff --git a/tests/Benchmark.migratetest/parser.py
>> >> b/tests/Benchmark.migratetest/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
>> >>
>> >> 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
>> >>
>> >>  import os, re, sys
>> >>  import common as plib
>> >> diff --git a/tests/Benchmark.ptsematest/parser.py
>> >> b/tests/Benchmark.ptsematest/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
>> >>
>> >>  import os, re, sys
>> >>  import common as plib
>> >> diff --git a/tests/Benchmark.signaltest/parser.py
>> >> b/tests/Benchmark.signaltest/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
>> >>
>> >>  import os, re, sys
>> >>  import common as plib
>> >> diff --git a/tests/Benchmark.sigwaittest/parser.py
>> >> b/tests/Benchmark.sigwaittest/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
>> >>
>> >> diff --git a/tests/Benchmark.svsematest/parser.py
>> >> b/tests/Benchmark.svsematest/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
>> >>
>> >>  import os, re, sys
>> >>  import common as plib
>> >> diff --git a/tests/Functional.LTP/fuego_test.sh
>> >> b/tests/Functional.LTP/fuego_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=$ORIG_PATH python ltp_process.py
>> >> +            PATH=$ORIG_PATH python3 ltp_process.py
>> >>          else
>> >> -            python ltp_process.py
>> >> +            python3 ltp_process.py
>> >>          fi
>> >>
>> >>          [ -e results.xlsx ] && cp results.xlsx
>> >> ${LOGDIR}/results.xlsx diff --git
>> >> a/tests/Functional.LTP/ltp_process.py
>> >> b/tests/Functional.LTP/ltp_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.LTP_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
>> >>
>> >>  import os
>> >> diff --git a/tests/Functional.autopkgtest/parser.py
>> >> b/tests/Functional.autopkgtest/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
>> >>
>> >>  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
>> >>
>> >>  import os, sys, collections
>> >>  import common as plib
>> >> --
>> >> 2.20.1
>> >>
>> >
>> >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 faulty 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).
>> We are using on Debian based distribution with Bullseye version, this
>> is one of the reason why we are changing the scripts to support python3,
>because bullseye in not supporting python2.
>>
>> At the first we have changed in few tests and some common function
>> scripts which we are using, because in our environment the fuego-core
>> runs on test/target machine directly this patch works for us, and I am
>> not confident these changes will work with docker- container, so the
>> reason we have not shared this patch initially and only the shared
>> some print statement fixes and other
>> python2to3 compatible fixes.
>>
>> After studying your suggestion in the above, there are quite few other
>conversions required to fully convert to python3.
>> Let me know if I can do some of the changes.
>>
>> >
>> > -- Tim
>>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fuego] Report on python3 compatibility in Fuego (was something else)
  2022-10-27  6:10           ` Venkata.Pyla
@ 2022-10-27 23:53             ` Bird, Tim
  0 siblings, 0 replies; 11+ messages in thread
From: Bird, Tim @ 2022-10-27 23:53 UTC (permalink / raw)
  To: Venkata.Pyla, fuego; +Cc: kazuhiro3.hayashi, dinesh.kumar, sireesha.nakkala

Venkata,

Thanks very much for testing this out and reporting back.
 -- Tim


> -----Original Message-----
> From: Venkata.Pyla@toshiba-tsip.com <Venkata.Pyla@toshiba-tsip.com>
> 
> Hi Tim,
> 
> Thanks for improving the python3 compatibility problems, please see my inline comments.
> 
> Thanks,
> Venkata.
> 
> >-----Original Message-----
> >From: Bird, Tim <Tim.Bird@sony.com>
> >Sent: 19 October 2022 23:15
> >To: pyla venkata(TSIP TMIEC ODG Porting) <Venkata.Pyla@toshiba-
> >tsip.com>; fuego@lists.linuxfoundation.org
> >Cc: nakkala sireesha(TSIP TMIEC ODG Porting) <sireesha.nakkala@toshiba-
> >tsip.com>; dinesh kumar(TSIP TMIEC ODG Porting)
> ><dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 □SWC◯ACT)
> ><kazuhiro3.hayashi@toshiba.co.jp>
> >Subject: Report on python3 compatibility in Fuego (was something else)
> >
> >OK - I'm hijacking this thread to so a general report on python3 compatibility in
> >Fuego.
> >
> >I recently completed a long stretch of work on this, and here are some notes
> >about the work.
> >
> >I modified common.sh, functions.sh, and test-parser.sh to set and use PY_EXE I
> >experimented with doing an autodetect to use python3 or python (2).
> >But ultimately I decided that doing an runtime autodetect is not a good policy,
> >as it means that factors unrelated to Fuego (such as installing a version of
> >python) can affect Fuego in unexpected ways.
> >
> >So, I instead opted to create a script which can switch Fuego from python to
> >python3 usage, or vice-versa.  That is, if a user switches Fuego to python3, but
> >then discovers problems, they can switch back to python2.  The new utility is
> >called set-python, and is in fuego-core/scripts.  It also checks that the required
> >libraries for the specified python version are present on the system.  In order to
> >switch to a version of python, that version of python must be present on your
> >system.
> >
> >The tool is "idempotent", meaning that if you run it twice with the same
> >arguments, it should not change anything the second time.  Specifically, it
> >should not have any deleterious effects.  So, if you're already done a conversion
> >to python3, you can run it again and it should not cause any problems.  Note
> >that you can also run the tool with '-c' to just check the status, without making
> >any changes.
> 
> 'set-python' tool is a nice tool that you prepared, it is checking and modifying the shebang lines according to the system python
> version, this perfectly suited for us as well.
> I applied your changes in our project and I randomly checked and it is working.
> 
> We will further verify while executing some of the tests and if there are any problems we will send fixes for them or report to you.
> 
> >
> >The tool must be run in fuego-core/scripts, outside of any docker container.
> >Use '-h'
> >to get usage help.  The script changes all shebang python lines in fuego scripts,
> >with the exception of parser.py shebang lines, which are always ignored.
> >
> >I removed all the shebang lines from the parser core (fuego-
> >core/scripts/parser/*.py), but decided to leave the shebang lines in all the
> >test/*/parser.py files.  They could be removed (and probably should be), but I
> >did a lot of changes this release and wanted to push these changes out before
> >doing another large set of changes.  The shebang lines in parser.py files are
> >never used, so they are superfluous, and don't reflect how the scripts will
> >actually be called.
> >
> >I also cleaned up remaining python3 incompatibilities in ftc and a few other
> >scripts.
> >Specifically, I handled the input vs. raw_input issue, and the urllib.parse vs.
> >urlparse issue.
> >
> >Please try out the latest master branch and let me know if you have any
> >problems, especially with python3 compatibility.
> >
> >If you could run set-python in your lab, and let me know what it shows you,
> >that would be helpful.
> >That is please do:
> >  $ cd fuego-core/scripts ; set-python python3 (or 'set-python -c python3') and
> >let me know the results.
> 
> I executed the 'set-python' in our environment with your patches and below are the results
> 
> 1)
> root@cloud:/fuego-core/scripts# ./set-python -c python3
> Python version I am executing='python3' (3, 9, 2)
> Python version requested='python3'
> Report:
>   Missing module jenkins
> 
> Here are the lines that need to change!
> == check-dependencies ==
> == deorphan-runs.py ==
> == ftc ==
> == gen-page.py ==
> == jdiff ==
> == ../tests/Functional.LTP/ltp_process.py ==
> == ../tests/Functional.apertis_sanity_check/make-script-from-yaml.py ==
> == common.sh ==
> == test-parser.sh ==
> 
> 2)
> root@cloud:/fuego-core/scripts# ./set-python python3
> Python version I am executing='python3' (3, 9, 2)
> Python version requested='python3'
> Report:
>   Missing module jenkins
> 
> Here are some line changes!
> == check-dependencies ==
> #!/usr/bin/python
>  -- is now --
> #!/usr/bin/python3
> == deorphan-runs.py ==
> #!/usr/bin/python
>  -- is now --
> #!/usr/bin/python3
> == ftc ==
>  -- is now --
> #!/usr/bin/python3
> == gen-page.py ==
> #!/usr/bin/python
>  -- is now --
> #!/usr/bin/python3
> == jdiff ==
> #!/usr/bin/python
>  -- is now --
> #!/usr/bin/python3
> == ../tests/Functional.LTP/ltp_process.py ==
> #!/usr/bin/python
>  -- is now --
> #!/usr/bin/python3
> == ../tests/Functional.apertis_sanity_check/make-script-from-yaml.py ==
> #!/usr/bin/python
>  -- is now --
> #!/usr/bin/python3
> == common.sh ==
> PY_EXE=/usr/bin/python
>  -- is now --
> PY_EXE=/usr/bin/python3
> == test-parser.sh ==
> PY_EXE=/usr/bin/python
>  -- is now --
> PY_EXE=/usr/bin/python3
> 
> I saw the shebang lines are changed to python3 and the tests are working.
> 
> >
> >I haven't decided to switch Fuego to install using python3 by default, but I may
> >do so after I experiment with making serio python3 compatible, and when I do
> >the next major release.
> >I have modified the install scripts to load all the needed python3 libraries, but
> >haven't done a docker test yet with these.
> >
> >Regards,
> > -- Tim
> >
> >> -----Original Message-----
> >> From: Venkata.Pyla@toshiba-tsip.com <Venkata.Pyla@toshiba-tsip.com>
> >> Sent: Monday, October 10, 2022 7:00 AM
> >> To: Bird, Tim <Tim.Bird@sony.com>; fuego@lists.linuxfoundation.org
> >> Cc: sireesha.nakkala@toshiba-tsip.com; dinesh.kumar@toshiba-tsip.com;
> >> kazuhiro3.hayashi@toshiba.co.jp
> >> Subject: RE: [PATCH] python3: Use python3 in the scripts to fully
> >> migrate
> >>
> >> Hi Tim,
> >>
> >> Sorry for the late, I am on vacation last week.
> >>
> >> Please find my answers below.
> >>
> >> >-----Original Message-----
> >> >From: Bird, Tim <Tim.Bird@sony.com>
> >> >Sent: 05 October 2022 05:52
> >> >To: pyla venkata(TSIP TMIEC ODG Porting) <Venkata.Pyla@toshiba-
> >> >tsip.com>; fuego@lists.linuxfoundation.org
> >> >Cc: nakkala sireesha(TSIP TMIEC ODG Porting)
> >> ><sireesha.nakkala@toshiba- tsip.com>; dinesh kumar(TSIP TMIEC ODG
> >> >Porting) <dinesh.kumar@toshiba- tsip.com>; hayashi kazuhiro(林 和宏
> >> >□SWC◯ACT)
> >> ><kazuhiro3.hayashi@toshiba.co.jp>
> >> >Subject: RE: [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: venkata.pyla@toshiba-tsip.com <venkata.pyla@toshiba-tsip.com>
> >> >>
> >> >> 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
> >> >>
> >> >> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> >> >> ---
> >> >>  scripts/check-dependencies              | 2 +-
> >> >
> >> >My first comment is that invocation using just the interpreter name
> >> >of 'python' 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
> >> >, generic_parser.py, test_filelock.py (in the scripts directory).
> >> >Many of these you are likely not using.
> >> Yes, many of the files were not used by Toshiba, so we fixed only in the tests
> >where it is breaking.
> >>
> >> >
> >> >However, some of these are intended for use by Fuego users, for
> >> >special circumstances (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(-)
> >> >>
> >> >> 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.
> >> We use LTP test and so we changed the shebang here, and as you
> >> suggested below it is good to use `run_python` which will internally find the
> >right interpreter as you wrote the code, then this shebang is not required or we
> >can remove.
> >>
> >> >
> >> >check-dependencies is invoked directly (not using run_python), or by
> >> >calling 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
> >interpreter.
> >> >(see below for how I might make common.sh auto-detect the python
> >> >interpreter)
> >> This will be a good solution for the backward compatibility.
> >>
> >> >
> >> >>  # vim: set ts=4 sw=4 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=$ORIG_PATH,
> >TOOLCHAIN=$TOOLCHAIN"
> >> >>          export TOOLCHAIN
> >> >> -        PATH=$ORIG_PATH TOOLCHAIN=$TOOLCHAIN python "$@"
> >> >> +        PATH=$ORIG_PATH TOOLCHAIN=$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 something like this:
> >> >
> >> >INTERPRETER_LIST="python3 python python2"
> >> >for interpreter in $INTERPRETER_LIST ; do
> >> >    if [ -x /usr/bin/${interpreter} ] ; then
> >> >        PYTHON_EXE="/usr/bin/${interpreter}"
> >> >        break
> >> >    fi
> >> >done
> >> >if [ -z "$PYTHON_EXE" ] ; then
> >> >    abort_job "No python interpreter found!"
> >> >fi
> >> >
> >> >this line would then become:
> >> >PATH=$ORIG_PATH TOOLCHAIN=$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 libs from the target sysroot.  This is not
> >> >desired for Fuego's use of python, 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 the
> >> >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?
> >> We are not using the ORIG_PATH variable, in our case the host environment
> >interpreter is sufficient to run the scripts.
> >>
> >> But in our patch we have modified in both the places when ORIG_PATH is
> >> defined and not defined because we wanted at least change python version
> >problem in the same file.
> >>
> >> >
> >> >>      else
> >> >>          dprint "run_python with TOOLCHAIN=$TOOLCHAIN"
> >> >>          export TOOLCHAIN
> >> >> -        TOOLCHAIN=$TOOLCHAIN python "$@"
> >> >> +        TOOLCHAIN=$TOOLCHAIN python3 "$@"
> >> >and this would become:
> >> >TOOLCHAIN=$TOOLCHAIN $PYTHON_EXE "$@"
> >> >
> >> >>      fi
> >> >>  }
> >> >>
> >> >>  function run_python_quiet() {
> >> >>      if [ ! -z $ORIG_PATH ]
> >> >>      then
> >> >> -        PATH=$ORIG_PATH python "$@"
> >> >> +        PATH=$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
> >> >connections to the target.
> >> >(that is, when the TRANSPORT=serial)
> >> >
> >> >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 when 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 port, 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 natively onto the device under test, and using
> >> >TRANSPORT=local.  If you don't use the serial TRANSPORT or the serial
> >> >port tools (serio suite of tools), then this shouldn't affect you.
> >> Yes, this change is not required for us, as mentioned above we modified all
> >python versions in the same file.
> >> You can ignore this change and thanks for letting me know about the function
> >'run_python_quiet'
> >>
> >> >
> >> >In any event, this can not be changed to python3 yet.
> >> >>      else
> >> >> -        python "$@"
> >> >> +        python3 "$@"
> >> >Nor this one. (see above)
> >> >
> >> >>      fi
> >> >>  }
> >> >>
> >> >> 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?
> >> We installed python3 dependencies in our target machine.
> >> as you may know we are not using the docker-container and installing
> >> the fuego-core directly on the target machine, and the python3 dependencies
> >are already installed in the target machine.
> >>
> >> >
> >> >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?
> >> It is working fine with our current usage scenarios, because in our
> >> environment we were not using the Jenkins or the features related to
> >> those functions I think we should fix them because they are obviously
> >required when completely migrate to pyhon3.
> >>
> >> >
> >> >>  #
> >> >>  # vim: set ts=4 sw=4 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=$?
> >> >>          build_end_time=$(date +"%s.%N")
> >> >> -        build_duration=$(python -c "print($build_end_time -
> >$build_start_time)")
> >> >> +        build_duration=$(python3 -c "print($build_end_time -
> >> >> + $build_start_time)")
> >> >If I do the PYTHON_EXE thing in common.sh, I should be able to use
> >> >that here as well.
> >> >
> >> >>
> >> >>          # 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.Dhrystone/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 determines 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
> >> >invoked directly, as far as I can tell.  It might be better to
> >> >actually remove the shebang line to make it explicitly impossible to
> >> >treat the parser.py scripts as standalone programs.
> >> >
> >> >I'm not sure if I'd be breaking anyone's workflow with this or not,
> >> >so I hesitate to do that.
> >> >
> >> >What do you think?
> >> In our use cases also we were not directly running them, so we can remove
> >the shebang lines if no one else also not using it directly.
> >>
> >> >
> >> >>
> >> >>  import os, re, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Benchmark.IOzone/parser.py
> >> >> b/tests/Benchmark.IOzone/parser.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
> >> >>
> >> >>  import os, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Benchmark.Stream/parser.py
> >> >> b/tests/Benchmark.Stream/parser.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
> >> >>
> >> >>  import os, re, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Benchmark.bonnie/parser.py
> >> >> b/tests/Benchmark.bonnie/parser.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
> >> >>
> >> >>  import os, re, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Benchmark.cyclictest/parser.py
> >> >> b/tests/Benchmark.cyclictest/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
> >> >>
> >> >> diff --git a/tests/Benchmark.fio/parser.py
> >> >> b/tests/Benchmark.fio/parser.py 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
> >> >>
> >> >>  import os, sys
> >> >> diff --git a/tests/Benchmark.hackbench/parser.py
> >> >> b/tests/Benchmark.hackbench/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
> >> >>
> >> >>  import os, re, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Benchmark.lmbench2/parser.py
> >> >> b/tests/Benchmark.lmbench2/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
> >> >>
> >> >>  import os, re, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Benchmark.migratetest/parser.py
> >> >> b/tests/Benchmark.migratetest/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
> >> >>
> >> >> 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
> >> >>
> >> >>  import os, re, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Benchmark.ptsematest/parser.py
> >> >> b/tests/Benchmark.ptsematest/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
> >> >>
> >> >>  import os, re, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Benchmark.signaltest/parser.py
> >> >> b/tests/Benchmark.signaltest/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
> >> >>
> >> >>  import os, re, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Benchmark.sigwaittest/parser.py
> >> >> b/tests/Benchmark.sigwaittest/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
> >> >>
> >> >> diff --git a/tests/Benchmark.svsematest/parser.py
> >> >> b/tests/Benchmark.svsematest/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
> >> >>
> >> >>  import os, re, sys
> >> >>  import common as plib
> >> >> diff --git a/tests/Functional.LTP/fuego_test.sh
> >> >> b/tests/Functional.LTP/fuego_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=$ORIG_PATH python ltp_process.py
> >> >> +            PATH=$ORIG_PATH python3 ltp_process.py
> >> >>          else
> >> >> -            python ltp_process.py
> >> >> +            python3 ltp_process.py
> >> >>          fi
> >> >>
> >> >>          [ -e results.xlsx ] && cp results.xlsx
> >> >> ${LOGDIR}/results.xlsx diff --git
> >> >> a/tests/Functional.LTP/ltp_process.py
> >> >> b/tests/Functional.LTP/ltp_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.LTP_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
> >> >>
> >> >>  import os
> >> >> diff --git a/tests/Functional.autopkgtest/parser.py
> >> >> b/tests/Functional.autopkgtest/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
> >> >>
> >> >>  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
> >> >>
> >> >>  import os, sys, collections
> >> >>  import common as plib
> >> >> --
> >> >> 2.20.1
> >> >>
> >> >
> >> >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 faulty 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).
> >> We are using on Debian based distribution with Bullseye version, this
> >> is one of the reason why we are changing the scripts to support python3,
> >because bullseye in not supporting python2.
> >>
> >> At the first we have changed in few tests and some common function
> >> scripts which we are using, because in our environment the fuego-core
> >> runs on test/target machine directly this patch works for us, and I am
> >> not confident these changes will work with docker- container, so the
> >> reason we have not shared this patch initially and only the shared
> >> some print statement fixes and other
> >> python2to3 compatible fixes.
> >>
> >> After studying your suggestion in the above, there are quite few other
> >conversions required to fully convert to python3.
> >> Let me know if I can do some of the changes.
> >>
> >> >
> >> > -- Tim
> >>



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-10-27 23:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29 13:20 [Fuego] [PATCH] Convert the print statement to the print() function sireesha.nakkala
2022-09-29 19:27 ` Bird, Tim
2022-09-30 14:18   ` Venkata.Pyla
2022-10-04 17:51     ` Bird, Tim
2022-09-30 14:26   ` [Fuego] [PATCH] python3: Use python3 in the scripts to fully migrate venkata.pyla
2022-10-05  0:22     ` Bird, Tim
2022-10-06 23:59       ` Bird, Tim
2022-10-10 13:00       ` Venkata.Pyla
2022-10-19 17:44         ` [Fuego] Report on python3 compatibility in Fuego (was something else) Bird, Tim
2022-10-27  6:10           ` Venkata.Pyla
2022-10-27 23:53             ` Bird, Tim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.