Re: [gentoo-portage-dev] [PATCH v4] xattr: centralize the various shims in one place
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 11/06/15 09:06, Brian Dolbec wrote: Personally, I'd prefer you stay with tabs, even if it does not line up exactly with the ( ignoring personal preferences for tab settings ( 1 tab = 4 spaces,...) Mixing tabs spaces is a poor idea, so I agree with Brian. (However, if I were king of the universe or somesuch, there would be no tabs in our Python code -- at all.) - -- Alexander berna...@gentoo.org https://secure.plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQIcBAEBCgAGBQJVetSlAAoJENQqWdRUGk8BbHsQANJusb5/FRKL+wKRNzVtcF7b 06AwCEcYDiMHWOjzeWOwVNnKFRmuDCadUpAj1Zwt78wxB2TZd4Dk2UoO8y6mrKmX AvN6tTCExoisEkP3xUCUkXMX2dorNnttrOThQg4YeczMG1UfKce8PkXHjyQEFHCe yNY13UPvJsuZbjfQF4TvlRcC9CetvyjpKpheMbuxcGhtJiRU1Wm+K9KTVzRWT1lI h5u5nTYXlmAPTNHUFbyr2tSEIotIBye8YtiBkpocjduqomYEF+bWQyROGEys0MQO xTmyQT9+hkTUE5tyjDKRtg3cViG4YTK4cDau8FQ0sRj5NZOVCsxnYJvCXtxU10bE TCZl0KrXoNGkZ2iUWcjuFXxc7BbCpV/+pMWypAFxoa5SZMoy6/8G1gImdVH86lT9 k5JmTcD3jrjookaBPd6FqVg2BcED6UHDjm4c3m5f8iUpCgvRcWB0/o3o++UvFMKe mHboHvlIS2WOC2r+cN9xNzT5Og18H7rs350b0A1KopItLKVF1QCfizSi3UuNHAtM /WmuJ6y55lgS/uqeUys7V+K5AJ6rWQpOCkcfVumVRnAYs8Arr2Bkjkg0f3Z1h9Ui 7P5lBM9QENZtnQxn+gYpARdkMcLxKjDUDCskpsZw5UYCVb1wImM9GqhtKK5LV2Lk 1c1kIz90PECAoMH9xe4f =jgxr -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH v4] xattr: centralize the various shims in one place
On Wed, 10 Jun 2015 22:43:38 -0700 Zac Medico zmed...@gentoo.org wrote: On 06/10/2015 10:39 PM, Mike Frysinger wrote: On 10 Jun 2015 11:54, Zac Medico wrote: On 05/30/2015 01:59 PM, Mike Frysinger wrote: LGTM, except this one line is indented with spaces instead of tabs in vartree.py: def tar_contents(contents, root, tar, protect=None, onProgress=None, - xattr=False): + xattrs=False): i don't know if we have a standard here. sometimes it's a single tab, sometimes it's spaces to line up. i prefer the latter as that's generally what PEP8 does. -mike Ah, ok. The test pass, so guess it's fine that way. But I still don't like it being mixed. It may not be a problem due to it being contained inside the def statement. But I've had some strange results when code is run with mixed spaces/tabs indents. Portage code is not nearly pep8 and we have not set pep8 as a standard for us to adhere to. Although, I personally try to keep my stuff close to pep8. Aside from the fact I prefer a few things different than that standard. In this case, it is minor, but 1 tab indent (which is normal) also does not show the continuation of the def readily. I have at times just given it an additional indent tab to differentiate it from the following code. I personally don't adhere to the line up all params with the foo(param1, param2) It's fine when it is close to the left side, but when the start ( is near the right side, it can be down right impossible to stay within the 80 col. limit for long parameters. Besides the often times wasted space of having all params on separate lines. IMHO it can take away from readability at times while increasing readability for complex function calls. I've come across code in portage with both a mix of tabs and spaces on the same line(s)... In such cases, I have my editor convert all leading spaces to tabs. I don't look for odd cases like the above. But I will usually take note of them in the commit diff while making the commit. Personally, I'd prefer you stay with tabs, even if it does not line up exactly with the ( ignoring personal preferences for tab settings ( 1 tab = 4 spaces,...) Yes, I very much like the code being centralized in one place. :) We can look at any more convention changes after the next lead election. -- Brian Dolbec dolsen
Re: [gentoo-portage-dev] [PATCH v4] xattr: centralize the various shims in one place
On 30 May 2015 16:59, Mike Frysinger wrote: 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. ping ... are people ok with this change in API ? -mike signature.asc Description: Digital signature
Re: [gentoo-portage-dev] [PATCH v4] xattr: centralize the various shims in one place
On 05/30/2015 01:59 PM, Mike Frysinger wrote: 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. --- v4 - merge in recent quickpkg changes - add a XATTRS_WORKS symbol for easy testing - use - with -m when matching all bin/quickpkg | 12 +- bin/xattr-helper.py | 11 +- pym/portage/dbapi/vartree.py | 10 +- pym/portage/tests/util/test_xattr.py | 178 +++ pym/portage/util/_xattr.py | 228 +++ pym/portage/util/movefile.py | 100 --- pym/portage/util/xattr.py| 20 --- 7 files changed, 441 insertions(+), 118 deletions(-) create mode 100644 pym/portage/tests/util/test_xattr.py create mode 100644 pym/portage/util/_xattr.py delete mode 100644 pym/portage/util/xattr.py LGTM, except this one line is indented with spaces instead of tabs in vartree.py: def tar_contents(contents, root, tar, protect=None, onProgress=None, - xattr=False): + xattrs=False): -- Thanks, Zac
Re: [gentoo-portage-dev] [PATCH v4] xattr: centralize the various shims in one place
On 10 Jun 2015 11:54, Zac Medico wrote: On 05/30/2015 01:59 PM, Mike Frysinger wrote: LGTM, except this one line is indented with spaces instead of tabs in vartree.py: def tar_contents(contents, root, tar, protect=None, onProgress=None, - xattr=False): + xattrs=False): i don't know if we have a standard here. sometimes it's a single tab, sometimes it's spaces to line up. i prefer the latter as that's generally what PEP8 does. -mike signature.asc Description: Digital signature
Re: [gentoo-portage-dev] [PATCH v4] xattr: centralize the various shims in one place
On 06/10/2015 10:39 PM, Mike Frysinger wrote: On 10 Jun 2015 11:54, Zac Medico wrote: On 05/30/2015 01:59 PM, Mike Frysinger wrote: LGTM, except this one line is indented with spaces instead of tabs in vartree.py: def tar_contents(contents, root, tar, protect=None, onProgress=None, - xattr=False): + xattrs=False): i don't know if we have a standard here. sometimes it's a single tab, sometimes it's spaces to line up. i prefer the latter as that's generally what PEP8 does. -mike Ah, ok. The test pass, so guess it's fine that way. -- Thanks, Zac
[gentoo-portage-dev] [PATCH v4] 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.
---
v4
- merge in recent quickpkg changes
- add a XATTRS_WORKS symbol for easy testing
- use - with -m when matching all
bin/quickpkg | 12 +-
bin/xattr-helper.py | 11 +-
pym/portage/dbapi/vartree.py | 10 +-
pym/portage/tests/util/test_xattr.py | 178 +++
pym/portage/util/_xattr.py | 228 +++
pym/portage/util/movefile.py | 100 ---
pym/portage/util/xattr.py| 20 ---
7 files changed, 441 insertions(+), 118 deletions(-)
create mode 100644 pym/portage/tests/util/test_xattr.py
create mode 100644 pym/portage/util/_xattr.py
delete mode 100644 pym/portage/util/xattr.py
diff --git a/bin/quickpkg b/bin/quickpkg
index 726abff..262fda4 100755
--- a/bin/quickpkg
+++ b/bin/quickpkg
@@ -21,8 +21,8 @@ from portage.dbapi.dep_expand import dep_expand
from portage.dep import Atom, use_reduce
from portage.exception import (AmbiguousPackageName, InvalidAtom, InvalidData,
InvalidDependString, PackageSetNotFound, PermissionDenied)
-from portage.util import ConfigProtect, ensure_dirs, shlex_split
-import portage.util.xattr as _xattr
+from portage.util import ConfigProtect, ensure_dirs, shlex_split, _xattr
+xattr = _xattr.xattr
from portage.dbapi.vartree import dblink, tar_contents
from portage.checksum import perform_md5
from portage._sets import load_default_config, SETPREFIX
@@ -36,7 +36,7 @@ def quickpkg_atom(options, infos, arg, eout):
vartree = trees[vartree]
vardb = vartree.dbapi
bintree = trees[bintree]
- xattr = 'xattr' in settings.features
+ xattrs = 'xattr' in settings.features
include_config = options.include_config == y
include_unmodified_config = options.include_unmodified_config == y
@@ -137,8 +137,8 @@ def quickpkg_atom(options, infos, arg, eout):
# The tarfile module will write pax headers holding the
# xattrs only if PAX_FORMAT is specified here.
tar = tarfile.open(binpkg_tmpfile, w:bz2,
- format=tarfile.PAX_FORMAT if xattr else
tarfile.DEFAULT_FORMAT)
- tar_contents(contents, root, tar, protect=protect,
xattr=xattr)
+ format=tarfile.PAX_FORMAT if xattrs else
tarfile.DEFAULT_FORMAT)
+ tar_contents(contents, root, tar, protect=protect,
xattrs=xattrs)
tar.close()
xpak.tbz2(binpkg_tmpfile).recompose_mem(xpdata)
finally:
@@ -238,7 +238,7 @@ def quickpkg_main(options, args, eout):
eout.eerror(No write access to '%s' % bintree.pkgdir)
return errno.EACCES
- if 'xattr' in portage.settings.features and not hasattr(_xattr,
'getxattr'):
+ if 'xattr' in portage.settings.features and not _xattr.XATTRS_WORKS:
eout.eerror(No xattr support library was found,
so xattrs will not be preserved!)
portage.settings.unlock()
diff --git a/bin/xattr-helper.py b/bin/xattr-helper.py
index 3e9b81e..19f25f9 100755
--- a/bin/xattr-helper.py
+++ b/bin/xattr-helper.py
@@ -19,16 +19,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/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index 62d880e..e755fde 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -35,7 +35,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
'portage.util.movefile:movefile',
'portage.util.path:first_existing,iter_parents',
'portage.util.writeable_check:get_ro_checker',
- 'portage.util:xattr@_xattr',
+ 'portage.util._xattr:xattr',
'portage.util._dyn_libs.PreservedLibsRegistry:PreservedLibsRegistry',
'portage.util._dyn_libs.LinkageMapELF:LinkageMapELF@LinkageMap',
'portage.util._async.SchedulerInterface:SchedulerInterface',
@@ -5268,7 +5268,7 @@ def write_contents(contents, root, f):
f.write(line)
def tar_contents(contents, root, tar, protect=None, onProgress=None,
- xattr=False):
+ xattrs=False):
os = _os_merge
encoding = _encodings['merge']
@@ -5390,13 +5390,13 @@ def tar_contents(contents, root, tar, protect=None,
onProgress=None,
encoding=encoding,
errors='strict')
-
