Re: [gentoo-portage-dev] [PATCH] xattr: centralize the various shims in one place
2013-10-21 05:00 Mike Frysinger napisał(a): On Wednesday 16 October 2013 23:42:26 Arfrever Frehtes Taifersar wrote: - cStringIO module should not be used. io module is a replacement available since Python 2.6. unfortunately, that doesn't work as well as it should. python 2.7's interface is annoyingly different when using other python 2.7 modules. i'll have the code import the old module and then fallback to using the new io module. (io.StringIO works only with unicode strings. When bytes-compatible functions are really needed, then io.BytesIO can be used, which works only with bytes. cStringIO.StringIO is designed to work with bytes, but cStringIO.StringIO().write(instance_of_unicode) implicitly encodes its argument to bytes.) In your another patch, portage.tests.bin.test_prepstrip.PrepStripFull._prepstrip() passes an instance of cStringIO.StringIO class to portage.bin.prepstrip.main(), which passes it to portage.bin.prepstrip.Prepstrip(), which passes it to print(), portage.elog.messages.eqawarn() and portage.elog.messages.ewarn(). pym/portage/bin/prepstrip.py should have: from __future__ import unicode_literals Then print('...', file=out) calls will work with an instance of io.StringIO class. (portage.elog.messages.eqawarn() and portage.elog.messages.ewarn() internally decode message, so they already work with out=io.StringIO, but not out=io.BytesIO.) -- Arfrever Frehtes Taifersar Arahesis signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH] xattr: centralize the various shims in one place
On Wednesday 16 October 2013 23:42:26 Arfrever Frehtes Taifersar wrote: 2013-10-17 04:53 Mike Frysinger napisał(a): On Wednesday 16 October 2013 22:51:17 Mike Frysinger wrote: On Wednesday 16 October 2013 20:02:50 Arfrever Frehtes wrote: 2013-10-16 23:03 Mike Frysinger napisał(a): Rather than each module implementing its own shim around the various methods for accessing extended attributes, start a dedicated module that exports a consistent API. Some things are incompatible with Python 3. See other comments below. i can run a linter on the code (probably should make this a git hook). i'm interested more in review on the bigger picture. also, none of your comments were py3 issues that i saw I said other comments, so I meant comments not related to incompatibility with Python 3. well, w/out providing specific concerns, it's hard to address feedback About incompatibility with Python 3: - subprocess.check_output(), subprocess.Popen().stdout.read(), subprocess.Popen().stderr.read() return bytes, which is incorrectly compared with str in your patches. i've changed the mock code to use subprocess directly so the tests catch this - dict.iteritems() was renamed to dict.items() (and its return type was changed from dictionary-itemiterator to dict_items, but it does not matter here). fixed - Queue module was renamed to queue. i've sent a patch upstream for this - cStringIO module should not be used. io module is a replacement available since Python 2.6. unfortunately, that doesn't work as well as it should. python 2.7's interface is annoyingly different when using other python 2.7 modules. i'll have the code import the old module and then fallback to using the new io module. - Maybe other problems... sorry, but this isn't useful. i'm going to run the unittests against python 3.2 and 3.3 and once those pass, i'm done looking at it. -mike signature.asc Description: This is a digitally signed message part.
[gentoo-portage-dev] [PATCH] xattr: centralize the various shims in one place
Rather than each module implementing its own shim around the various methods for accessing extended attributes, start a dedicated module that exports a consistent API. --- bin/xattr-helper.py | 11 +-- pym/portage/util/_xattr.py | 189 +++ pym/portage/util/movefile.py | 99 ++- 3 files changed, 213 insertions(+), 86 deletions(-) create mode 100644 pym/portage/util/_xattr.py diff --git a/bin/xattr-helper.py b/bin/xattr-helper.py index 6d99521..69b83f7 100755 --- a/bin/xattr-helper.py +++ b/bin/xattr-helper.py @@ -17,16 +17,7 @@ import re import sys from portage.util._argparse import ArgumentParser - -if hasattr(os, getxattr): - - class xattr(object): - get = os.getxattr - set = os.setxattr - list = os.listxattr - -else: - import xattr +from portage.util._xattr import xattr _UNQUOTE_RE = re.compile(br'\\[0-7]{3}') diff --git a/pym/portage/util/_xattr.py b/pym/portage/util/_xattr.py new file mode 100644 index 000..0e594f9 --- /dev/null +++ b/pym/portage/util/_xattr.py @@ -0,0 +1,189 @@ +# Copyright 2010-2013 Gentoo Foundation +# Distributed under the terms of the GNU General Public License v2 + +Portability shim for xattr support + +Exported API is the xattr object with get/get_all/set/remove/list operations. + +See the standard xattr module for more documentation. + + +import contextlib +import os +import subprocess + +from portage.exception import OperationNotSupported + + +class _XattrGetAll(object): + Implement get_all() using list()/get() if there is no easy bulk method + + @classmethod + def get_all(cls, item, nofollow=False, namespace=None): + return [(name, cls.get(item, name, nofollow=nofollow, namespace=namespace)) + for name in cls.list(item, nofollow=nofollow, namespace=namespace)] + + +class _XattrSystemCommands(_XattrGetAll): + Implement things with getfattr/setfattr + + @staticmethod + def _parse_output(output): + for line in proc.stdout.readlines(): + if line.startswith('#'): + continue + line = line.rstrip() + if not line: + continue + # The line will have the format: + # user.name0=value0 + yield line.split('=', 1) + + @classmethod + def get(item, name, nofollow=False, namespace=None): + if namespace: + name = '%s.%s' % (namespace, name) + cmd = ['getfattr', '--absolute-names', '-n', name, item] + if nofollow: + cmd += ['-h'] + proc = subprocess.call(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + proc.wait() + + value = None + for _, value in cls._parse_output(proc.stdout): + break + + proc.stdout.close() + return value + + @staticmethod + def set(item, name, value, flags=0, namespace=None): + if namespace: + name = '%s.%s' % (namespace, name) + cmd = ['setfattr', '-n', name, '-v', value, item] + subprocess.call(cmd) + + @staticmethod + def remove(item, name, nofollow=False, namespace=None): + if namespace: + name = '%s.%s' % (namespace, name) + cmd = ['setfattr', '-x', name, item] + subprocess.call(cmd) + + @classmethod + def list(cls, item, nofollow=False, namespace=None, _names_only=True): + cmd = ['getfattr', '-d', '--absolute-names', item] + if nofollow: + cmd += ['-h'] + cmd += ['-m', ('^%s[.]' % namespace) if namespace else ''] + proc = subprocess.call(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + proc.wait() + + ret = [] + if namespace: + namespace = '%s.' % namespace + for name, value in cls._parse_output(proc.stdout): + if namespace: + if name.startswith(namespace): + name = name[len(namespace):] + else: + continue + if _names_only: + ret.append(name) + else: + ret.append((name, value)) + + proc.stdout.close() + return ret + + @classmethod + def get_all(cls, item, nofollow=False, namespace=None): + cls.list(item, nofollow=nofollow, namespace=namespace, _names_only=False) + + +class _XattrStub(_XattrGetAll): + Fake
Re: [gentoo-portage-dev] [PATCH] xattr: centralize the various shims in one place
2013-10-16 23:03 Mike Frysinger napisał(a): Rather than each module implementing its own shim around the various methods for accessing extended attributes, start a dedicated module that exports a consistent API. Some things are incompatible with Python 3. See other comments below. --- bin/xattr-helper.py | 11 +-- pym/portage/util/_xattr.py | 189 +++ pym/portage/util/movefile.py | 99 ++- 3 files changed, 213 insertions(+), 86 deletions(-) create mode 100644 pym/portage/util/_xattr.py diff --git a/bin/xattr-helper.py b/bin/xattr-helper.py index 6d99521..69b83f7 100755 --- a/bin/xattr-helper.py +++ b/bin/xattr-helper.py @@ -17,16 +17,7 @@ import re import sys from portage.util._argparse import ArgumentParser - -if hasattr(os, getxattr): - - class xattr(object): - get = os.getxattr - set = os.setxattr - list = os.listxattr - -else: - import xattr +from portage.util._xattr import xattr _UNQUOTE_RE = re.compile(br'\\[0-7]{3}') diff --git a/pym/portage/util/_xattr.py b/pym/portage/util/_xattr.py new file mode 100644 index 000..0e594f9 --- /dev/null +++ b/pym/portage/util/_xattr.py @@ -0,0 +1,189 @@ +# Copyright 2010-2013 Gentoo Foundation +# Distributed under the terms of the GNU General Public License v2 + +Portability shim for xattr support + +Exported API is the xattr object with get/get_all/set/remove/list operations. + +See the standard xattr module for more documentation. + + +import contextlib +import os +import subprocess + +from portage.exception import OperationNotSupported + + +class _XattrGetAll(object): + Implement get_all() using list()/get() if there is no easy bulk method + + @classmethod + def get_all(cls, item, nofollow=False, namespace=None): + return [(name, cls.get(item, name, nofollow=nofollow, namespace=namespace)) + for name in cls.list(item, nofollow=nofollow, namespace=namespace)] + + +class _XattrSystemCommands(_XattrGetAll): + Implement things with getfattr/setfattr + + @staticmethod + def _parse_output(output): + for line in proc.stdout.readlines(): NameError: global name 'proc' is not defined + if line.startswith('#'): + continue + line = line.rstrip() + if not line: + continue + # The line will have the format: + # user.name0=value0 + yield line.split('=', 1) + + @classmethod + def get(item, name, nofollow=False, namespace=None): + if namespace: + name = '%s.%s' % (namespace, name) + cmd = ['getfattr', '--absolute-names', '-n', name, item] + if nofollow: + cmd += ['-h'] + proc = subprocess.call(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + proc.wait() AttributeError: 'int' object has no attribute 'wait' + + value = None + for _, value in cls._parse_output(proc.stdout): + break + + proc.stdout.close() + return value + + @staticmethod + def set(item, name, value, flags=0, namespace=None): + if namespace: + name = '%s.%s' % (namespace, name) + cmd = ['setfattr', '-n', name, '-v', value, item] Calling setfattr once per attribute is slower than once per file (as is now). + subprocess.call(cmd) + + @staticmethod + def remove(item, name, nofollow=False, namespace=None): + if namespace: + name = '%s.%s' % (namespace, name) + cmd = ['setfattr', '-x', name, item] + subprocess.call(cmd) + + @classmethod + def list(cls, item, nofollow=False, namespace=None, _names_only=True): + cmd = ['getfattr', '-d', '--absolute-names', item] + if nofollow: + cmd += ['-h'] + cmd += ['-m', ('^%s[.]' % namespace) if namespace else ''] + proc = subprocess.call(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + proc.wait() AttributeError: 'int' object has no attribute 'wait' + + ret = [] + if namespace: + namespace = '%s.' % namespace + for name, value in cls._parse_output(proc.stdout): + if namespace: + if name.startswith(namespace): + name = name[len(namespace):] + else: + continue + if _names_only: + ret.append(name) +
Re: [gentoo-portage-dev] [PATCH] xattr: centralize the various shims in one place
On Wednesday 16 October 2013 20:02:50 Arfrever Frehtes Taifersar Arahesis wrote: 2013-10-16 23:03 Mike Frysinger napisał(a): Rather than each module implementing its own shim around the various methods for accessing extended attributes, start a dedicated module that exports a consistent API. Some things are incompatible with Python 3. See other comments below. i can run a linter on the code (probably should make this a git hook). i'm interested more in review on the bigger picture. also, please snip context you aren't actually replying to. it makes it much harder to read replies when you don't do that. + @staticmethod + def set(item, name, value, flags=0, namespace=None): + if namespace: + name = '%s.%s' % (namespace, name) + cmd = ['setfattr', '-n', name, '-v', value, item] Calling setfattr once per attribute is slower than once per file (as is now). true, however: - setfattr is the fallback logic, not the primary code path - the # of attributes per file is low (rarely more than 2) -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH] xattr: centralize the various shims in one place
On Wednesday 16 October 2013 22:51:17 Mike Frysinger wrote: On Wednesday 16 October 2013 20:02:50 Arfrever Frehtes Taifersar Arahesis wrote: 2013-10-16 23:03 Mike Frysinger napisał(a): Rather than each module implementing its own shim around the various methods for accessing extended attributes, start a dedicated module that exports a consistent API. Some things are incompatible with Python 3. See other comments below. i can run a linter on the code (probably should make this a git hook). i'm interested more in review on the bigger picture. also, none of your comments were py3 issues that i saw -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH] xattr: centralize the various shims in one place
2013-10-17 04:53 Mike Frysinger napisał(a): On Wednesday 16 October 2013 22:51:17 Mike Frysinger wrote: On Wednesday 16 October 2013 20:02:50 Arfrever Frehtes Taifersar Arahesis wrote: 2013-10-16 23:03 Mike Frysinger napisał(a): Rather than each module implementing its own shim around the various methods for accessing extended attributes, start a dedicated module that exports a consistent API. Some things are incompatible with Python 3. See other comments below. i can run a linter on the code (probably should make this a git hook). i'm interested more in review on the bigger picture. also, none of your comments were py3 issues that i saw I said other comments, so I meant comments not related to incompatibility with Python 3. About incompatibility with Python 3: - subprocess.check_output(), subprocess.Popen().stdout.read(), subprocess.Popen().stderr.read() return bytes, which is incorrectly compared with str in your patches. - dict.iteritems() was renamed to dict.items() (and its return type was changed from dictionary-itemiterator to dict_items, but it does not matter here). - Queue module was renamed to queue. - cStringIO module should not be used. io module is a replacement available since Python 2.6. - Maybe other problems... -- Arfrever Frehtes Taifersar Arahesis signature.asc Description: This is a digitally signed message part.