Re: [gentoo-portage-dev] [PATCH] xattr: centralize the various shims in one place

2013-10-22 Thread Arfrever Frehtes Taifersar Arahesis
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

2013-10-20 Thread Mike Frysinger
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

2013-10-16 Thread Mike Frysinger
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 Thread Arfrever Frehtes Taifersar Arahesis
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

2013-10-16 Thread Mike Frysinger
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

2013-10-16 Thread Mike Frysinger
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-16 Thread Arfrever Frehtes Taifersar Arahesis
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.