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