* [gentoo-portage-dev] [PATCH] xpak-helper: rewrite to rely more on argparse
@ 2015-11-01 5:23 Mike Frysinger
2015-11-01 17:36 ` Zac Medico
0 siblings, 1 reply; 5+ messages in thread
From: Mike Frysinger @ 2015-11-01 5:23 UTC (permalink / raw
To: gentoo-portage-dev
The current code implements a lot of ad-hoc argument parsing when it
could simply let the argparse module do it all for it. This makes the
code easier to understand and extend in the process.
---
bin/xpak-helper.py | 68 ++++++++++++++++++++----------------------------------
1 file changed, 25 insertions(+), 43 deletions(-)
diff --git a/bin/xpak-helper.py b/bin/xpak-helper.py
index 8c965ec..1b2883d 100755
--- a/bin/xpak-helper.py
+++ b/bin/xpak-helper.py
@@ -2,68 +2,50 @@
# Copyright 2009-2014 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
+"""Helper to manage xpak archives"""
+
import argparse
import sys
import portage
portage._internal_caller = True
from portage import os
-def command_recompose(args):
-
- usage = "usage: recompose <binpkg_path> <metadata_dir>\n"
- if len(args) != 2:
- sys.stderr.write(usage)
- sys.stderr.write("2 arguments are required, got %s\n" % len(args))
- return 1
+def command_recompose(parser, opts):
+ """Create an xpak archive from a directory"""
+ binpkg_path = opts.binpkg_path
+ metadata_dir = opts.metadata_dir
- binpkg_path, metadata_dir = args
-
- if not os.path.isfile(binpkg_path):
- sys.stderr.write(usage)
- sys.stderr.write("Argument 1 is not a regular file: '%s'\n" % \
- binpkg_path)
- return 1
+ if os.path.exists(binpkg_path) and not os.path.isfile(binpkg_path):
+ parser.error('binpkg_path is not a regular file: %s' % binpkg_path)
if not os.path.isdir(metadata_dir):
- sys.stderr.write(usage)
- sys.stderr.write("Argument 2 is not a directory: '%s'\n" % \
- metadata_dir)
- return 1
+ parser.error('metadata_dir is not a directory: %s' % metadata_dir)
t = portage.xpak.tbz2(binpkg_path)
t.recompose(metadata_dir)
return os.EX_OK
-def main(argv):
- if argv and isinstance(argv[0], bytes):
- for i, x in enumerate(argv):
- argv[i] = portage._unicode_decode(x, errors='strict')
+def get_parser():
+ """Return the command line parser"""
+ parser = argparse.ArgumentParser(description=__doc__)
+ subparsers = parser.add_subparsers()
- valid_commands = ('recompose',)
- description = "Perform metadata operations on a binary package."
- usage = "usage: %s COMMAND [args]" % \
- os.path.basename(argv[0])
+ subparser = subparsers.add_parser('recompose',
+ help=command_recompose.__doc__)
+ subparser.set_defaults(func=command_recompose)
+ subparser.add_argument('binpkg_path', help='Path to output xpak')
+ subparser.add_argument('metadata_dir', help='Path to input dir')
- parser = argparse.ArgumentParser(description=description, usage=usage)
- options, args = parser.parse_known_args(argv[1:])
+ return parser
- if not args:
- parser.error("missing command argument")
- command = args[0]
-
- if command not in valid_commands:
- parser.error("invalid command: '%s'" % command)
-
- if command == 'recompose':
- rval = command_recompose(args[1:])
- else:
- raise AssertionError("invalid command: '%s'" % command)
+def main(argv):
+ parser = get_parser()
+ opts = parser.parse_args(argv)
+ opts.func(parser, opts)
- return rval
-if __name__ == "__main__":
- rval = main(sys.argv[:])
- sys.exit(rval)
+if __name__ == '__main__':
+ sys.exit(main(sys.argv[1:]))
--
2.5.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [gentoo-portage-dev] [PATCH] xpak-helper: rewrite to rely more on argparse
2015-11-01 5:23 [gentoo-portage-dev] [PATCH] xpak-helper: rewrite to rely more on argparse Mike Frysinger
@ 2015-11-01 17:36 ` Zac Medico
2015-11-02 16:52 ` Mike Frysinger
0 siblings, 1 reply; 5+ messages in thread
From: Zac Medico @ 2015-11-01 17:36 UTC (permalink / raw
To: gentoo-portage-dev
On 10/31/2015 10:23 PM, Mike Frysinger wrote:
> The current code implements a lot of ad-hoc argument parsing when it
> could simply let the argparse module do it all for it. This makes the
> code easier to understand and extend in the process.
> ---
> bin/xpak-helper.py | 68 ++++++++++++++++++++----------------------------------
> 1 file changed, 25 insertions(+), 43 deletions(-)
>
> diff --git a/bin/xpak-helper.py b/bin/xpak-helper.py
> index 8c965ec..1b2883d 100755
> --- a/bin/xpak-helper.py
> +++ b/bin/xpak-helper.py
[snip]
> -def main(argv):
>
> - if argv and isinstance(argv[0], bytes):
> - for i, x in enumerate(argv):
> - argv[i] = portage._unicode_decode(x, errors='strict')
You've dropped the _unicode_decode call.
In order to handle python3 with arguments containing UTF-8 characters
(in ${PKGDIR}) and a mis-matched sys.getfilesystemencoding() value, it's
safest to decode the arguments like chmod-lite.py does. We should create
a function for this code which is also duplicated in install.py:
if sys.hexversion >= 0x3000000:
# We can't trust that the filesystem encoding (locale dependent)
# correctly matches the arguments, so use surrogateescape to
# pass through the original argv bytes for Python 3.
fs_encoding = sys.getfilesystemencoding()
files = [x.encode(fs_encoding, 'surrogateescape') for x in files]
--
Thanks,
Zac
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [gentoo-portage-dev] [PATCH] xpak-helper: rewrite to rely more on argparse
2015-11-01 17:36 ` Zac Medico
@ 2015-11-02 16:52 ` Mike Frysinger
2015-11-02 17:06 ` Zac Medico
0 siblings, 1 reply; 5+ messages in thread
From: Mike Frysinger @ 2015-11-02 16:52 UTC (permalink / raw
To: gentoo-portage-dev
[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]
On 01 Nov 2015 09:36, Zac Medico wrote:
> On 10/31/2015 10:23 PM, Mike Frysinger wrote:
> > The current code implements a lot of ad-hoc argument parsing when it
> > could simply let the argparse module do it all for it. This makes the
> > code easier to understand and extend in the process.
> > ---
> > bin/xpak-helper.py | 68 ++++++++++++++++++++----------------------------------
> > 1 file changed, 25 insertions(+), 43 deletions(-)
> >
> > diff --git a/bin/xpak-helper.py b/bin/xpak-helper.py
> > index 8c965ec..1b2883d 100755
> > --- a/bin/xpak-helper.py
> > +++ b/bin/xpak-helper.py
> [snip]
> > -def main(argv):
> >
> > - if argv and isinstance(argv[0], bytes):
> > - for i, x in enumerate(argv):
> > - argv[i] = portage._unicode_decode(x, errors='strict')
>
> You've dropped the _unicode_decode call.
i did on purpose. i should have mentioned that.
> In order to handle python3 with arguments containing UTF-8 characters
> (in ${PKGDIR}) and a mis-matched sys.getfilesystemencoding() value, it's
> safest to decode the arguments like chmod-lite.py does.
it seems wrong that we have incomplete coverage here.
some tools do it and some do not.
> We should create
> a function for this code which is also duplicated in install.py:
you mean portage._decode_argv ?
what if we create a new module like "commandline" that provides an
ArgumentParser interface that takes care of this for us ?
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [gentoo-portage-dev] [PATCH] xpak-helper: rewrite to rely more on argparse
2015-11-02 16:52 ` Mike Frysinger
@ 2015-11-02 17:06 ` Zac Medico
2015-11-02 17:10 ` Zac Medico
0 siblings, 1 reply; 5+ messages in thread
From: Zac Medico @ 2015-11-02 17:06 UTC (permalink / raw
To: gentoo-portage-dev
On 11/02/2015 08:52 AM, Mike Frysinger wrote:
> On 01 Nov 2015 09:36, Zac Medico wrote:
>> In order to handle python3 with arguments containing UTF-8 characters
>> (in ${PKGDIR}) and a mis-matched sys.getfilesystemencoding() value, it's
>> safest to decode the arguments like chmod-lite.py does.
>
> it seems wrong that we have incomplete coverage here.
> some tools do it and some do not.
Yeah, complete coverage would be nice. Most if not all of the python
helpers that are called from the ebuild environment already use this
method to decode filename arguments (dohtml.py was only fixed recently,
for bug 561846).
>> We should create
>> a function for this code which is also duplicated in install.py:
>
> you mean portage._decode_argv ?
Yes, I forgot about that function.
> what if we create a new module like "commandline" that provides an
> ArgumentParser interface that takes care of this for us ?
> -mike
That sounds good. After it decodes the arguments, it should decode them
as UTF-8 with errors='strict', and exit the program immediately if it
triggers a UnicodeDecodeError.
--
Thanks,
Zac
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [gentoo-portage-dev] [PATCH] xpak-helper: rewrite to rely more on argparse
2015-11-02 17:06 ` Zac Medico
@ 2015-11-02 17:10 ` Zac Medico
0 siblings, 0 replies; 5+ messages in thread
From: Zac Medico @ 2015-11-02 17:10 UTC (permalink / raw
To: gentoo-portage-dev
On 11/02/2015 09:06 AM, Zac Medico wrote:
> On 11/02/2015 08:52 AM, Mike Frysinger wrote:
>> On 01 Nov 2015 09:36, Zac Medico wrote:
>>> In order to handle python3 with arguments containing UTF-8 characters
>>> (in ${PKGDIR}) and a mis-matched sys.getfilesystemencoding() value, it's
>>> safest to decode the arguments like chmod-lite.py does.
>>
>> it seems wrong that we have incomplete coverage here.
>> some tools do it and some do not.
>
> Yeah, complete coverage would be nice. Most if not all of the python
> helpers that are called from the ebuild environment already use this
> method to decode filename arguments (dohtml.py was only fixed recently,
> for bug 561846).
>
>>> We should create
>>> a function for this code which is also duplicated in install.py:
>>
>> you mean portage._decode_argv ?
>
> Yes, I forgot about that function.
>
>> what if we create a new module like "commandline" that provides an
>> ArgumentParser interface that takes care of this for us ?
>> -mike
>
> That sounds good. After it decodes the arguments, it should decode them
> as UTF-8 with errors='strict', and exit the program immediately if it
> triggers a UnicodeDecodeError.
>
I mean, after it _encodes_ them with surrogateescape, it should
immediately decode them as UTF-8 with errors='strict'. That way, we'll
have unicode strings to pass to argparse (we don't want to pass raw
bytes to argparse).
--
Thanks,
Zac
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-11-02 17:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-01 5:23 [gentoo-portage-dev] [PATCH] xpak-helper: rewrite to rely more on argparse Mike Frysinger
2015-11-01 17:36 ` Zac Medico
2015-11-02 16:52 ` Mike Frysinger
2015-11-02 17:06 ` Zac Medico
2015-11-02 17:10 ` Zac Medico
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox