From: Thierry Reding treding@nvidia.com
This script is incompatible with Python 3, so make sure it is run using Python 2 explicitly. This establishes compatibility on systems that default to Python 3 and which make the python symlink point to the Python 3 binary.
For further background, see:
https://www.python.org/dev/peps/pep-0394/
Signed-off-by: Thierry Reding treding@nvidia.com --- parse-platforms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/parse-platforms.py b/parse-platforms.py index 748374953370..1b7b856773de 100755 --- a/parse-platforms.py +++ b/parse-platforms.py @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/python2
import sys, os, argparse, ConfigParser
Hi Thierry,
On Wed, Mar 27, 2019 at 02:04:39PM +0100, Thierry Reding wrote:
This script is incompatible with Python 3, so make sure it is run using Python 2 explicitly. This establishes compatibility on systems that default to Python 3 and which make the python symlink point to the Python 3 binary.
Thank you for your report and patch.
However, EDK2 has recently enabled support for using both Python 2 and Python 3, so my preference would be not to lock down hard on 2.
The below is my attempt at making the script compatible with both, and it appears to work at my end.
It *does* require additionally installing the configparser module(s), which on Debian means $ sudo apt-get install python-metaconfig python3-metaconfig or if packaging is too old-school $ pip install configparser # sudo? I dunno, I'm old-school.
Would this alternative solution also work for you?
Regards,
Leif
diff --git a/parse-platforms.py b/parse-platforms.py index 7483749..8674b5d 100755 --- a/parse-platforms.py +++ b/parse-platforms.py @@ -1,36 +1,37 @@ #!/usr/bin/python
-import sys, os, argparse, ConfigParser +from __future__ import print_function +import sys, os, argparse, configparser
default_filename='platforms.config'
def list_platforms(): - for p in platforms: print p + for p in platforms: print(p)
def shortlist_platforms(): - for p in platforms: print p, + for p in platforms: print(p, end=' ')
def get_images(): if args.platform: try: value = config.get(args.platform, "EXTRA_FILES") - print value, + print(value, end=' ') except: pass try: value = config.get(args.platform, "BUILD_ATF") if value == "yes": - print "bl1.bin fip.bin" + print("bl1.bin fip.bin") return True except: try: value = config.get(args.platform, "UEFI_BIN") - print value + print(value) return True except: - print "No images found!" + print("No images found!") else: - print "No platform specified!" + print("No platform specified!")
return False
@@ -43,7 +44,7 @@ def get_executables(): except: return False
- print "%s/%s" % (arch, value) + print("%s/%s" % (arch, value)) return True except: pass @@ -56,14 +57,14 @@ def get_option(): try: value = config.get(args.platform, args.option) if value: - print value + print(value) return True except: return True # Option not found, return True, and no output else: - print "No option specified!" + print("No option specified!") else: - print "No platform specified!" + print("No platform specified!") return False
parser = argparse.ArgumentParser(description='Parses platform configuration for Linaro UEFI build scripts.') @@ -78,7 +79,7 @@ if args.config_file: else: config_filename = os.path.dirname(os.path.realpath(sys.argv[0])) + '/' + default_filename
-config = ConfigParser.ConfigParser() +config = configparser.ConfigParser() config.read(config_filename)
platforms = config.sections() @@ -92,7 +93,5 @@ commands = {"shortlist": shortlist_platforms, try: retval = commands[args.command]() except: - print ("Unrecognized command '%s'" % args.command) - -if retval != True: + print("Unrecognized command '%s'" % args.command) sys.exit(1)
On Thu, Mar 28, 2019 at 10:29:31AM +0000, Leif Lindholm wrote:
Hi Thierry,
On Wed, Mar 27, 2019 at 02:04:39PM +0100, Thierry Reding wrote:
This script is incompatible with Python 3, so make sure it is run using Python 2 explicitly. This establishes compatibility on systems that default to Python 3 and which make the python symlink point to the Python 3 binary.
Thank you for your report and patch.
However, EDK2 has recently enabled support for using both Python 2 and Python 3, so my preference would be not to lock down hard on 2.
The below is my attempt at making the script compatible with both, and it appears to work at my end.
It *does* require additionally installing the configparser module(s), which on Debian means $ sudo apt-get install python-metaconfig python3-metaconfig or if packaging is too old-school $ pip install configparser # sudo? I dunno, I'm old-school.
Would this alternative solution also work for you?
My distribution doesn't seem to provide the python-metaconfig package. But how about we try to dynamically detect which one is available? The patch below seems to work fine for me.
You made most of those changes, so feel free to roll in mine and make this a proper commit if you think this is good. Also feel free to add my:
Reviewed-by: Thierry Reding treding@nvidia.com --- >8 --- diff --git a/parse-platforms.py b/parse-platforms.py index 748374953370..4697ba234940 100755 --- a/parse-platforms.py +++ b/parse-platforms.py @@ -1,36 +1,45 @@ #!/usr/bin/python
-import sys, os, argparse, ConfigParser +from __future__ import print_function + +import sys, os, argparse + +try: + # try Python 3 configparser module first + from configparser import ConfigParser +except: + # fallback to Python 2 ConfigParser + from ConfigParser import ConfigParser
default_filename='platforms.config'
def list_platforms(): - for p in platforms: print p + for p in platforms: print(p)
def shortlist_platforms(): - for p in platforms: print p, + for p in platforms: print(p, end = ' ')
def get_images(): if args.platform: try: value = config.get(args.platform, "EXTRA_FILES") - print value, + print(value, end = ' ') except: pass try: value = config.get(args.platform, "BUILD_ATF") if value == "yes": - print "bl1.bin fip.bin" + print("bl1.bin fip.bin") return True except: try: value = config.get(args.platform, "UEFI_BIN") - print value + print(value) return True except: - print "No images found!" + print("No images found!") else: - print "No platform specified!" + print("No platform specified!")
return False
@@ -43,7 +52,7 @@ def get_executables(): except: return False
- print "%s/%s" % (arch, value) + print("%s/%s" % (arch, value)) return True except: pass @@ -56,14 +65,14 @@ def get_option(): try: value = config.get(args.platform, args.option) if value: - print value + print(value) return True except: return True # Option not found, return True, and no output else: - print "No option specified!" + print("No option specified!") else: - print "No platform specified!" + print("No platform specified!") return False
parser = argparse.ArgumentParser(description='Parses platform configuration for Linaro UEFI build scripts.') @@ -78,7 +87,7 @@ if args.config_file: else: config_filename = os.path.dirname(os.path.realpath(sys.argv[0])) + '/' + default_filename
-config = ConfigParser.ConfigParser() +config = ConfigParser() config.read(config_filename)
platforms = config.sections() @@ -92,7 +101,5 @@ commands = {"shortlist": shortlist_platforms, try: retval = commands[args.command]() except: - print ("Unrecognized command '%s'" % args.command) - -if retval != True: + print("Unrecognized command '%s'" % args.command) sys.exit(1)
On Thu, Mar 28, 2019 at 01:15:32PM +0100, Thierry Reding wrote:
On Thu, Mar 28, 2019 at 10:29:31AM +0000, Leif Lindholm wrote:
Hi Thierry,
On Wed, Mar 27, 2019 at 02:04:39PM +0100, Thierry Reding wrote:
This script is incompatible with Python 3, so make sure it is run using Python 2 explicitly. This establishes compatibility on systems that default to Python 3 and which make the python symlink point to the Python 3 binary.
Thank you for your report and patch.
However, EDK2 has recently enabled support for using both Python 2 and Python 3, so my preference would be not to lock down hard on 2.
The below is my attempt at making the script compatible with both, and it appears to work at my end.
It *does* require additionally installing the configparser module(s), which on Debian means $ sudo apt-get install python-metaconfig python3-metaconfig or if packaging is too old-school $ pip install configparser # sudo? I dunno, I'm old-school.
Would this alternative solution also work for you?
My distribution doesn't seem to provide the python-metaconfig package.
I think the .rpm distros just call it python-configparser or something similar.
But how about we try to dynamically detect which one is available? The patch below seems to work fine for me.
You made most of those changes, so feel free to roll in mine and make
Well, in all honesty, 2to3 made most of those changes ;)
this a proper commit if you think this is good.
Yes, this is a big improvement. It means the change doesn't risk breaking existing users.
Also feel free to add my:
Reviewed-by: Thierry Reding treding@nvidia.com
Thanks. I've added one final tweak...
--- >8 --- diff --git a/parse-platforms.py b/parse-platforms.py index 748374953370..4697ba234940 100755 --- a/parse-platforms.py +++ b/parse-platforms.py @@ -1,36 +1,45 @@ #!/usr/bin/python -import sys, os, argparse, ConfigParser +from __future__ import print_function
+import sys, os, argparse
+try:
- # try Python 3 configparser module first
- from configparser import ConfigParser
+except:
- # fallback to Python 2 ConfigParser
- from ConfigParser import ConfigParser
...which is to 'try' the fallback as well, and print a message and exit if it fails. This should be more friendly to Python 3 users who don't have the configparser module installed.
Pushed as 4507234. Thanks!
default_filename='platforms.config' def list_platforms():
- for p in platforms: print p
- for p in platforms: print(p)
def shortlist_platforms():
- for p in platforms: print p,
- for p in platforms: print(p, end = ' ')
def get_images(): if args.platform: try: value = config.get(args.platform, "EXTRA_FILES")
print value,
print(value, end = ' ') except: pass try: value = config.get(args.platform, "BUILD_ATF") if value == "yes":
print "bl1.bin fip.bin"
print("bl1.bin fip.bin") return True except: try: value = config.get(args.platform, "UEFI_BIN")
print value
print(value) return True except:
print "No images found!"
print("No images found!")
print "No platform specified!"
print("No platform specified!")return False @@ -43,7 +52,7 @@ def get_executables(): except: return False
print "%s/%s" % (arch, value)
print("%s/%s" % (arch, value)) return True except: pass@@ -56,14 +65,14 @@ def get_option(): try: value = config.get(args.platform, args.option) if value:
print value
print(value) return True except: return True # Option not found, return True, and no output else:
print "No option specified!"
print("No option specified!")
print "No platform specified!"
print("No platform specified!")parser = argparse.ArgumentParser(description='Parses platform configuration for Linaro UEFI build scripts.') @@ -78,7 +87,7 @@ if args.config_file: else: config_filename = os.path.dirname(os.path.realpath(sys.argv[0])) + '/' + default_filename -config = ConfigParser.ConfigParser() +config = ConfigParser() config.read(config_filename) platforms = config.sections() @@ -92,7 +101,5 @@ commands = {"shortlist": shortlist_platforms, try: retval = commands[args.command]() except:
- print ("Unrecognized command '%s'" % args.command)
-if retval != True:
- print("Unrecognized command '%s'" % args.command) sys.exit(1)
On Thu, Mar 28, 2019 at 03:57:22PM +0000, Leif Lindholm wrote:
On Thu, Mar 28, 2019 at 01:15:32PM +0100, Thierry Reding wrote:
On Thu, Mar 28, 2019 at 10:29:31AM +0000, Leif Lindholm wrote:
Hi Thierry,
On Wed, Mar 27, 2019 at 02:04:39PM +0100, Thierry Reding wrote:
This script is incompatible with Python 3, so make sure it is run using Python 2 explicitly. This establishes compatibility on systems that default to Python 3 and which make the python symlink point to the Python 3 binary.
Thank you for your report and patch.
However, EDK2 has recently enabled support for using both Python 2 and Python 3, so my preference would be not to lock down hard on 2.
The below is my attempt at making the script compatible with both, and it appears to work at my end.
It *does* require additionally installing the configparser module(s), which on Debian means $ sudo apt-get install python-metaconfig python3-metaconfig or if packaging is too old-school $ pip install configparser # sudo? I dunno, I'm old-school.
Would this alternative solution also work for you?
My distribution doesn't seem to provide the python-metaconfig package.
I think the .rpm distros just call it python-configparser or something similar.
I'm not using an RPM distro either, though Arch Linux does seem to have a python2-configparser that supposedly would do the same thing.
But how about we try to dynamically detect which one is available? The patch below seems to work fine for me.
You made most of those changes, so feel free to roll in mine and make
Well, in all honesty, 2to3 made most of those changes ;)
this a proper commit if you think this is good.
Yes, this is a big improvement. It means the change doesn't risk breaking existing users.
Also feel free to add my:
Reviewed-by: Thierry Reding treding@nvidia.com
Thanks. I've added one final tweak...
--- >8 --- diff --git a/parse-platforms.py b/parse-platforms.py index 748374953370..4697ba234940 100755 --- a/parse-platforms.py +++ b/parse-platforms.py @@ -1,36 +1,45 @@ #!/usr/bin/python -import sys, os, argparse, ConfigParser +from __future__ import print_function
+import sys, os, argparse
+try:
- # try Python 3 configparser module first
- from configparser import ConfigParser
+except:
- # fallback to Python 2 ConfigParser
- from ConfigParser import ConfigParser
...which is to 'try' the fallback as well, and print a message and exit if it fails. This should be more friendly to Python 3 users who don't have the configparser module installed.
Pushed as 4507234. Thanks!
Strictly that shouldn't be necessary. configparser is part of the standard library in Python 3, so I don't think it would ever be missing. Still, having a simple error message is probably better than a stack trace in the unlikely case that this would still happen.
Thanks, Thierry
-
Leif Lindholm -
Thierry Reding