From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id D7BCA1381F3 for ; Mon, 21 Oct 2013 03:07:10 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 2704EE0B96; Mon, 21 Oct 2013 03:07:07 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 6BD15E0AEC for ; Mon, 21 Oct 2013 03:07:06 +0000 (UTC) Received: from localhost.localdomain (localhost [127.0.0.1]) by smtp.gentoo.org (Postfix) with ESMTP id 0C46333EEAE for ; Mon, 21 Oct 2013 03:07:04 +0000 (UTC) From: Mike Frysinger To: gentoo-portage-dev@lists.gentoo.org Subject: [gentoo-portage-dev] [PATCH v2] xattr: centralize the various shims in one place Date: Sun, 20 Oct 2013 23:07:08 -0400 Message-Id: <1382324828-25755-1-git-send-email-vapier@gentoo.org> X-Mailer: git-send-email 1.8.3.2 In-Reply-To: <1381957406-21749-1-git-send-email-vapier@gentoo.org> References: <1381957406-21749-1-git-send-email-vapier@gentoo.org> Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-portage-dev@lists.gentoo.org Reply-to: gentoo-portage-dev@lists.gentoo.org X-Archives-Salt: 9bc58054-9234-4925-9c3a-dfff1a4a6412 X-Archives-Hash: 5eefb91bdafde309c6339611c975cc82 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. --- v2 - passes unittests w/python 2.6 2.7 3.2 3.3 bin/xattr-helper.py | 11 +- pym/portage/tests/util/test_xattr.py | 178 ++++++++++++++++++++++++++++++ pym/portage/util/_xattr.py | 205 +++++++++++++++++++++++++++++++++++ pym/portage/util/movefile.py | 100 ++++------------- 4 files changed, 407 insertions(+), 87 deletions(-) create mode 100644 pym/portage/tests/util/test_xattr.py 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/tests/util/test_xattr.py b/pym/portage/tests/util/test_xattr.py new file mode 100644 index 0000000..e1f6ee8 --- /dev/null +++ b/pym/portage/tests/util/test_xattr.py @@ -0,0 +1,178 @@ +# Copyright 2010-2013 Gentoo Foundation +# Distributed under the terms of the GNU General Public License v2 + +"""Tests for the portage.util._xattr module""" + +from __future__ import print_function + +try: + # Try python-3.3 module first. + from unittest import mock +except ImportError: + try: + # Try standalone module. + import mock + except ImportError: + mock = None + +import subprocess + +from portage.tests import TestCase +from portage.util._xattr import (xattr as _xattr, _XattrSystemCommands, + _XattrStub) + + +orig_popen = subprocess.Popen +def MockSubprocessPopen(stdin): + """Helper to mock (closely) a subprocess.Popen call + + The module has minor tweaks in behavior when it comes to encoding and + python versions, so use a real subprocess.Popen call to fake out the + runtime behavior. This way we don't have to also implement different + encodings as that gets ugly real fast. + """ + proc = orig_popen(['cat'], stdout=subprocess.PIPE, stdin=subprocess.PIPE) + try: + proc.stdin.write(bytes(stdin)) + except TypeError: + proc.stdin.write(bytes(stdin, 'ascii')) + return proc + + +class SystemCommandsTest(TestCase): + """Test _XattrSystemCommands""" + + OUTPUT = '\n'.join([ + '# file: /bin/ping', + 'security.capability=0sAQAAAgAgAAAAAAAAAAAAAAAAAAA=', + 'user.foo="asdf"', + '', + ]) + + def _setUp(self): + if mock is None: + self.skipTest('need mock for testing') + + return _XattrSystemCommands + + def _testGetBasic(self): + """Verify the get() behavior""" + xattr = self._setUp() + with mock.patch.object(subprocess, 'Popen') as call_mock: + # Verify basic behavior, and namespace arg works as expected. + xattr.get('/some/file', 'user.foo') + xattr.get('/some/file', 'foo', namespace='user') + self.assertEqual(call_mock.call_args_list[0], call_mock.call_args_list[1]) + + # Verify nofollow behavior. + call_mock.reset() + xattr.get('/some/file', 'user.foo', nofollow=True) + self.assertIn('-h', call_mock.call_args[0][0]) + + def testGetParsing(self): + """Verify get() parses output sanely""" + xattr = self._setUp() + with mock.patch.object(subprocess, 'Popen') as call_mock: + # Verify output parsing. + call_mock.return_value = MockSubprocessPopen('\n'.join([ + '# file: /some/file', + 'user.foo="asdf"', + '', + ])) + call_mock.reset() + self.assertEqual(xattr.get('/some/file', 'user.foo'), b'"asdf"') + + def testGetAllBasic(self): + """Verify the get_all() behavior""" + xattr = self._setUp() + with mock.patch.object(subprocess, 'Popen') as call_mock: + # Verify basic behavior. + xattr.get_all('/some/file') + + # Verify nofollow behavior. + call_mock.reset() + xattr.get_all('/some/file', nofollow=True) + self.assertIn('-h', call_mock.call_args[0][0]) + + def testGetAllParsing(self): + """Verify get_all() parses output sanely""" + xattr = self._setUp() + with mock.patch.object(subprocess, 'Popen') as call_mock: + # Verify output parsing. + call_mock.return_value = MockSubprocessPopen(self.OUTPUT) + exp = [ + (b'security.capability', b'0sAQAAAgAgAAAAAAAAAAAAAAAAAAA='), + (b'user.foo', b'"asdf"'), + ] + self.assertEqual(exp, xattr.get_all('/some/file')) + + def testSetBasic(self): + """Verify the set() behavior""" + xattr = self._setUp() + with mock.patch.object(subprocess, 'Popen') as call_mock: + # Verify basic behavior, and namespace arg works as expected. + xattr.set('/some/file', 'user.foo', 'bar') + xattr.set('/some/file', 'foo', 'bar', namespace='user') + self.assertEqual(call_mock.call_args_list[0], call_mock.call_args_list[1]) + + def testListBasic(self): + """Verify the list() behavior""" + xattr = self._setUp() + with mock.patch.object(subprocess, 'Popen') as call_mock: + # Verify basic behavior. + xattr.list('/some/file') + + # Verify nofollow behavior. + call_mock.reset() + xattr.list('/some/file', nofollow=True) + self.assertIn('-h', call_mock.call_args[0][0]) + + def testListParsing(self): + """Verify list() parses output sanely""" + xattr = self._setUp() + with mock.patch.object(subprocess, 'Popen') as call_mock: + # Verify output parsing. + call_mock.return_value = MockSubprocessPopen(self.OUTPUT) + exp = [b'security.capability', b'user.foo'] + self.assertEqual(exp, xattr.list('/some/file')) + + def testRemoveBasic(self): + """Verify the remove() behavior""" + xattr = self._setUp() + with mock.patch.object(subprocess, 'Popen') as call_mock: + # Verify basic behavior, and namespace arg works as expected. + xattr.remove('/some/file', 'user.foo') + xattr.remove('/some/file', 'foo', namespace='user') + self.assertEqual(call_mock.call_args_list[0], call_mock.call_args_list[1]) + + # Verify nofollow behavior. + call_mock.reset() + xattr.remove('/some/file', 'user.foo', nofollow=True) + self.assertIn('-h', call_mock.call_args[0][0]) + + +class StubTest(TestCase): + """Test _XattrStub""" + + def testBasic(self): + """Verify the stub is stubby""" + # Would be nice to verify raised errno is OperationNotSupported. + self.assertRaises(OSError, _XattrStub.get, '/', '') + self.assertRaises(OSError, _XattrStub.set, '/', '', '') + self.assertRaises(OSError, _XattrStub.get_all, '/') + self.assertRaises(OSError, _XattrStub.remove, '/', '') + self.assertRaises(OSError, _XattrStub.list, '/') + + +class StandardTest(TestCase): + """Test basic xattr API""" + + MODULES = (_xattr, _XattrSystemCommands, _XattrStub) + FUNCS = ('get', 'get_all', 'set', 'remove', 'list') + + def testApi(self): + """Make sure the exported API matches""" + for mod in self.MODULES: + for f in self.FUNCS: + self.assertTrue(hasattr(mod, f), + '%s func missing in %s' % (f, mod)) diff --git a/pym/portage/util/_xattr.py b/pym/portage/util/_xattr.py new file mode 100644 index 0000000..db3cf6e --- /dev/null +++ b/pym/portage/util/_xattr.py @@ -0,0 +1,205 @@ +# 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. +""" + +from __future__ import print_function + +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 output.readlines(): + if line.startswith(b'#'): + continue + line = line.rstrip() + if not line: + continue + # The lines will have the format: + # user.hex=0x12345 + # user.base64=0sAQAAAgAgAAAAAAAAAAAAAAAAAAA= + # user.string="value0" + # But since we don't do interpretation on the value (we just + # save & restore it), don't bother with decoding here. + yield line.split(b'=', 1) + + @staticmethod + def _call(*args, **kwargs): + proc = subprocess.Popen(*args, **kwargs) + proc.stdin.close() + proc.wait() + return proc + + @classmethod + def get(cls, 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 = cls._call(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + value = None + for _, value in cls._parse_output(proc.stdout): + break + + proc.stdout.close() + return value + + @classmethod + def set(cls, item, name, value, _flags=0, namespace=None): + if namespace: + name = '%s.%s' % (namespace, name) + cmd = ['setfattr', '-n', name, '-v', value, item] + cls._call(cmd) + + @classmethod + def remove(cls, item, name, nofollow=False, namespace=None): + if namespace: + name = '%s.%s' % (namespace, name) + cmd = ['setfattr', '-x', name, item] + if nofollow: + cmd += ['-h'] + cls._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 = cls._call(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + 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): + return cls.list(item, nofollow=nofollow, namespace=namespace, + _names_only=False) + + +# pylint: disable=W0613 +class _XattrStub(_XattrGetAll): + """Fake object since system doesn't support xattrs""" + + @staticmethod + def _raise(): + e = OSError('stub') + e.errno = OperationNotSupported.errno + raise e + + @classmethod + def get(cls, item, name, nofollow=False, namespace=None): + cls._raise() + + @classmethod + def set(cls, item, name, value, flags=0, namespace=None): + cls._raise() + + @classmethod + def remove(cls, item, name, nofollow=False, namespace=None): + cls._raise() + + @classmethod + def list(cls, item, nofollow=False, namespace=None): + cls._raise() + + +if hasattr(os, 'getxattr'): + # Easy as pie -- active python supports it. + class xattr(_XattrGetAll): + """Python >=3.3 and GNU/Linux""" + get = os.getxattr + set = os.setxattr + remove = os.removexattr + list = os.listxattr + +else: + try: + # Maybe we have the xattr module. + import xattr + + except ImportError: + try: + # Maybe we have the attr package. + with open(os.devnull, 'wb') as f: + subprocess.call(['getfattr', '--version'], stdout=f) + subprocess.call(['setfattr', '--version'], stdout=f) + xattr = _XattrSystemCommands + + except OSError: + # Stub it out completely. + xattr = _XattrStub + + +@contextlib.contextmanager +def preserve_xattrs(path, nofollow=False, namespace=None): + """Context manager to save/restore extended attributes on |path| + + If you want to rewrite a file (possibly replacing it with a new one), but + want to preserve the extended attributes, this will do the trick. + + # First read all the extended attributes. + with save_xattrs('/some/file'): + ... rewrite the file ... + # Now the extended attributes are restored as needed. + """ + kwargs = {'nofollow': nofollow,} + if namespace: + # Compiled xattr python module does not like it when namespace=None. + kwargs['namespace'] = namespace + + old_attrs = dict(xattr.get_all(path, **kwargs)) + try: + yield + finally: + new_attrs = dict(xattr.get_all(path, **kwargs)) + for name, value in new_attrs.iteritems(): + if name not in old_attrs: + # Clear out new ones. + xattr.remove(path, name, **kwargs) + elif new_attrs[name] != old_attrs[name]: + # Update changed ones. + xattr.set(path, name, value, **kwargs) + + for name, value in old_attrs.iteritems(): + if name not in new_attrs: + # Re-add missing ones. + xattr.set(path, name, value, **kwargs) diff --git a/pym/portage/util/movefile.py b/pym/portage/util/movefile.py index 452e77f..20859fe 100644 --- a/pym/portage/util/movefile.py +++ b/pym/portage/util/movefile.py @@ -11,7 +11,6 @@ import os as _os import shutil as _shutil import stat import sys -import subprocess import textwrap import portage @@ -23,6 +22,7 @@ from portage.exception import OperationNotSupported from portage.localization import _ from portage.process import spawn from portage.util import writemsg +from portage.util._xattr import xattr def _apply_stat(src_stat, dest): _os.chown(dest, src_stat.st_uid, src_stat.st_gid) @@ -68,86 +68,32 @@ class _xattr_excluder(object): return False -if hasattr(_os, "getxattr"): - # Python >=3.3 and GNU/Linux - def _copyxattr(src, dest, exclude=None): - - try: - attrs = _os.listxattr(src) - except OSError as e: - if e.errno != OperationNotSupported.errno: - raise - attrs = () - if attrs: - if exclude is not None and isinstance(attrs[0], bytes): - exclude = exclude.encode(_encodings['fs']) - exclude = _get_xattr_excluder(exclude) - - for attr in attrs: - if exclude(attr): - continue - try: - _os.setxattr(dest, attr, _os.getxattr(src, attr)) - raise_exception = False - except OSError: - raise_exception = True - if raise_exception: - raise OperationNotSupported(_("Filesystem containing file '%s' " - "does not support extended attribute '%s'") % - (_unicode_decode(dest), _unicode_decode(attr))) -else: +def _copyxattr(src, dest, exclude=None): + """Copy the extended attributes from |src| to |dest|""" try: - import xattr - except ImportError: - xattr = None - if xattr is not None: - def _copyxattr(src, dest, exclude=None): - - try: - attrs = xattr.list(src) - except IOError as e: - if e.errno != OperationNotSupported.errno: - raise - attrs = () + attrs = xattr.list(src) + except (OSError, IOError) as e: + if e.errno != OperationNotSupported.errno: + raise + attrs = () - if attrs: - if exclude is not None and isinstance(attrs[0], bytes): - exclude = exclude.encode(_encodings['fs']) - exclude = _get_xattr_excluder(exclude) + if attrs: + if exclude is not None and isinstance(attrs[0], bytes): + exclude = exclude.encode(_encodings['fs']) + exclude = _get_xattr_excluder(exclude) - for attr in attrs: - if exclude(attr): - continue - try: - xattr.set(dest, attr, xattr.get(src, attr)) - raise_exception = False - except IOError: - raise_exception = True - if raise_exception: - raise OperationNotSupported(_("Filesystem containing file '%s' " - "does not support extended attribute '%s'") % - (_unicode_decode(dest), _unicode_decode(attr))) - else: + for attr in attrs: + if exclude(attr): + continue try: - with open(_os.devnull, 'wb') as f: - subprocess.call(["getfattr", "--version"], stdout=f) - subprocess.call(["setfattr", "--version"], stdout=f) - except OSError: - def _copyxattr(src, dest, exclude=None): - # TODO: implement exclude - getfattr_process = subprocess.Popen(["getfattr", "-d", "--absolute-names", src], stdout=subprocess.PIPE) - getfattr_process.wait() - extended_attributes = getfattr_process.stdout.readlines() - getfattr_process.stdout.close() - if extended_attributes: - extended_attributes[0] = b"# file: " + _unicode_encode(dest) + b"\n" - setfattr_process = subprocess.Popen(["setfattr", "--restore=-"], stdin=subprocess.PIPE, stderr=subprocess.PIPE) - setfattr_process.communicate(input=b"".join(extended_attributes)) - if setfattr_process.returncode != 0: - raise OperationNotSupported("Filesystem containing file '%s' does not support extended attributes" % dest) - else: - def _copyxattr(src, dest, exclude=None): - pass + xattr.set(dest, attr, xattr.get(src, attr)) + raise_exception = False + except (OSError, IOError): + raise_exception = True + if raise_exception: + raise OperationNotSupported(_("Filesystem containing file '%s' " + "does not support extended attribute '%s'") % + (_unicode_decode(dest), _unicode_decode(attr))) def movefile(src, dest, newmtime=None, sstat=None, mysettings=None, hardlink_candidates=None, encoding=_encodings['fs']): -- 1.8.3.2