| Message ID | 20170808203935.30021-2-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Aug 08, 2017 at 05:39:31PM -0300, Eduardo Habkost wrote: > It makes command-line parsing and generation of help text much > simpler. > > The optparse module is deprecated since Python 2.7, but argparse > is not available in Python 2.6 (the minimum Python version > required for building QEMU). > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Changes v1 -> v2: > * Use optparse module, as the minimum Python version for building > QEMU is 2.6 > * Reported-by: Stefan Hajnoczi <stefanha@gmail.com> > * Suggested-by: "Daniel P. Berrange" <berrange@redhat.com> > --- > scripts/qmp/qmp-shell | 63 +++++++++++++++++++-------------------------------- > 1 file changed, 23 insertions(+), 40 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Eduardo Habkost <ehabkost@redhat.com> writes: > It makes command-line parsing and generation of help text much > simpler. There's really no excuse for parsing command line arguments by hand in Python. > The optparse module is deprecated since Python 2.7, but argparse > is not available in Python 2.6 (the minimum Python version > required for building QEMU). We have a few uses of argparse in the tree. Are they okay? We also use getopt in places. Perhaps we should pick one way to parse command lines and stick to it. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > It makes command-line parsing and generation of help text much > > simpler. > > There's really no excuse for parsing command line arguments by hand in > Python. > > > The optparse module is deprecated since Python 2.7, but argparse > > is not available in Python 2.6 (the minimum Python version > > required for building QEMU). > > We have a few uses of argparse in the tree. Are they okay? > > We also use getopt in places. Perhaps we should pick one way to parse > command lines and stick to it. I just came up against this problem with keycodemapdb which I'm adding as a submodule to qemu. I used argparse there because it is the modern recommended API for python >= 2.7. I examined possibilty of using optparse instead, but it lacks key features - in particular the idea of 'subcommands' is entirely missing from optparse. The 'argparse' module is part of python core, but is *also* available as a standalone module that is implemented in terms of optparse. So, I would suggest that we just copy the 'argparse' module into the QEMU 'scripts/thirdparty' directory. Then in any files which need argparse, you can do this try: import argparse except: import os, sys sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty")) import argparse so that it tries to use the argparse provided by python, and falls back to pulling in the one in our scripts/thirdparty directory. When we finally bump our min python to 2.7, we can simply drop this compat code for the import statement. This avoids need for us to re-write code to use a deprecated API, with a worse feature set. Regards, Daniel
On Tue, Aug 08, 2017 at 05:39:31PM -0300, Eduardo Habkost wrote: > It makes command-line parsing and generation of help text much > simpler. > > The optparse module is deprecated since Python 2.7, but argparse > is not available in Python 2.6 (the minimum Python version > required for building QEMU). > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Changes v1 -> v2: > * Use optparse module, as the minimum Python version for building > QEMU is 2.6 > * Reported-by: Stefan Hajnoczi <stefanha@gmail.com> > * Suggested-by: "Daniel P. Berrange" <berrange@redhat.com> I no longer suggest this approach :-) I'd prefer if we just bundled a copy of 'argparse.py' in the local scripts/thirdparty directory, and add that to the python import path, if the system python lacks argparse. eg if we had $QEMU-GIT/scripts/thirdparty/argparse.py you can use: try: import argparse except: import os, sys sys.path.append(os.path.join(os.path.dirname(__file__), "../thirdparty")) import argparse Regards, Daniel
On Tue, Aug 15, 2017 at 10:56:07AM +0100, Daniel P. Berrange wrote: > On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote: > > Eduardo Habkost <ehabkost@redhat.com> writes: > > > > > It makes command-line parsing and generation of help text much > > > simpler. > > > > There's really no excuse for parsing command line arguments by hand in > > Python. > > > > > The optparse module is deprecated since Python 2.7, but argparse > > > is not available in Python 2.6 (the minimum Python version > > > required for building QEMU). > > > > We have a few uses of argparse in the tree. Are they okay? > > > > We also use getopt in places. Perhaps we should pick one way to parse > > command lines and stick to it. > > I just came up against this problem with keycodemapdb which I'm adding as > a submodule to qemu. I used argparse there because it is the modern > recommended API for python >= 2.7. I examined possibilty of using > optparse instead, but it lacks key features - in particular the idea > of 'subcommands' is entirely missing from optparse. > > The 'argparse' module is part of python core, but is *also* available > as a standalone module that is implemented in terms of optparse. > > So, I would suggest that we just copy the 'argparse' module into the > QEMU 'scripts/thirdparty' directory. > > Then in any files which need argparse, you can do this > > try: > import argparse > except: > import os, sys > sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty")) > import argparse What about: try: import argparse except: from thirdparty import argparse (I think we could move all our Python modules [qemu.py, qmp.py] to ./scripts/qemu/, so this would become "qemu.thirdparty"). > > so that it tries to use the argparse provided by python, and falls back > to pulling in the one in our scripts/thirdparty directory. > > When we finally bump our min python to 2.7, we can simply drop this > compat code for the import statement. This avoids need for us to > re-write code to use a deprecated API, with a worse feature set. Sounds good to me.
On Tue, Aug 15, 2017 at 05:06:46PM -0300, Eduardo Habkost wrote: > On Tue, Aug 15, 2017 at 10:56:07AM +0100, Daniel P. Berrange wrote: > > On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote: > > > Eduardo Habkost <ehabkost@redhat.com> writes: > > > > > > > It makes command-line parsing and generation of help text much > > > > simpler. > > > > > > There's really no excuse for parsing command line arguments by hand in > > > Python. > > > > > > > The optparse module is deprecated since Python 2.7, but argparse > > > > is not available in Python 2.6 (the minimum Python version > > > > required for building QEMU). > > > > > > We have a few uses of argparse in the tree. Are they okay? > > > > > > We also use getopt in places. Perhaps we should pick one way to parse > > > command lines and stick to it. > > > > I just came up against this problem with keycodemapdb which I'm adding as > > a submodule to qemu. I used argparse there because it is the modern > > recommended API for python >= 2.7. I examined possibilty of using > > optparse instead, but it lacks key features - in particular the idea > > of 'subcommands' is entirely missing from optparse. > > > > The 'argparse' module is part of python core, but is *also* available > > as a standalone module that is implemented in terms of optparse. > > > > So, I would suggest that we just copy the 'argparse' module into the > > QEMU 'scripts/thirdparty' directory. > > > > Then in any files which need argparse, you can do this > > > > try: > > import argparse > > except: > > import os, sys > > sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty")) > > import argparse > > What about: > > try: > import argparse > except: > from thirdparty import argparse > > (I think we could move all our Python modules [qemu.py, qmp.py] > to ./scripts/qemu/, so this would become "qemu.thirdparty"). > > > > > so that it tries to use the argparse provided by python, and falls back > > to pulling in the one in our scripts/thirdparty directory. > > > > When we finally bump our min python to 2.7, we can simply drop this > > compat code for the import statement. This avoids need for us to > > re-write code to use a deprecated API, with a worse feature set. > > Sounds good to me. Sounds good. Stefan
diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell index 860ffb2..ad72ef9 100755 --- a/scripts/qmp/qmp-shell +++ b/scripts/qmp/qmp-shell @@ -73,6 +73,7 @@ import sys import os import errno import atexit +import optparse class QMPCompleter(list): def complete(self, text, state): @@ -393,52 +394,34 @@ def die(msg): sys.stderr.write('ERROR: %s\n' % msg) sys.exit(1) -def fail_cmdline(option=None): - if option: - sys.stderr.write('ERROR: bad command-line option \'%s\'\n' % option) - sys.stderr.write('qmp-shell [ -v ] [ -p ] [ -H ] [ -N ] < UNIX socket path> | < TCP address:port >\n') - sys.stderr.write(' -v Verbose (echo command sent and received)\n') - sys.stderr.write(' -p Pretty-print JSON\n') - sys.stderr.write(' -H Use HMP interface\n') - sys.stderr.write(' -N Skip negotiate (for qemu-ga)\n') - sys.exit(1) - def main(): - addr = '' - qemu = None - hmp = False - pretty = False - verbose = False - negotiate = True + parser = optparse.OptionParser(description='QMP shell utility') + parser.set_usage("%prog [options] <UNIX socket path> | <TCP address:port>") + parser.add_option('-v', action='store_true', dest='verbose', + help='Verbose (echo command sent and received)') + parser.add_option('-p', action='store_true', dest='pretty', + help='Pretty-print JSON') + parser.add_option('-H', action='store_true', dest='hmp', + help='Use HMP interface') + parser.add_option('-N', action='store_false', dest='negotiate', + default=True, help='Skip negotiate (for qemu-ga)') + opts,args = parser.parse_args() + + if len(args) != 1: + parser.print_help(sys.stderr) + sys.exit(1) + addr = args[0] try: - for arg in sys.argv[1:]: - if arg == "-H": - if qemu is not None: - fail_cmdline(arg) - hmp = True - elif arg == "-p": - pretty = True - elif arg == "-N": - negotiate = False - elif arg == "-v": - verbose = True - else: - if qemu is not None: - fail_cmdline(arg) - if hmp: - qemu = HMPShell(arg) - else: - qemu = QMPShell(arg, pretty) - addr = arg - - if qemu is None: - fail_cmdline() + if opts.hmp: + qemu = HMPShell(addr) + else: + qemu = QMPShell(addr, opts.pretty) except QMPShellBadPort: die('bad port number in command-line') try: - qemu.connect(negotiate) + qemu.connect(opts.negotiate) except qmp.QMPConnectError: die('Didn\'t get QMP greeting message') except qmp.QMPCapabilitiesError: @@ -447,7 +430,7 @@ def main(): die('Could not connect to %s' % addr) qemu.show_banner() - qemu.set_verbosity(verbose) + qemu.set_verbosity(opts.verbose) while qemu.read_exec_command(qemu.get_prompt()): pass qemu.close()
It makes command-line parsing and generation of help text much simpler. The optparse module is deprecated since Python 2.7, but argparse is not available in Python 2.6 (the minimum Python version required for building QEMU). Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v1 -> v2: * Use optparse module, as the minimum Python version for building QEMU is 2.6 * Reported-by: Stefan Hajnoczi <stefanha@gmail.com> * Suggested-by: "Daniel P. Berrange" <berrange@redhat.com> --- scripts/qmp/qmp-shell | 63 +++++++++++++++++++-------------------------------- 1 file changed, 23 insertions(+), 40 deletions(-)