Serhiy Storchaka <storch...@gmail.com> added the comment:
> So, please make the relevant code changes I proposed on Rietveld (adding "*,"
> everywhere was it iirc) and resubmit and we can get that in. I'll give you a
> code-only review of the other patch too--I'll start on that now.
Here is a changed patches.
Personally, I doubt that followlinks-to-follow_symlinks-4.patch is
better than followlinks-to-follow_symlinks-3.patch. Also my concern is
the incompatibility of the arguments of os.walk and os.fwalk.
----------
Added file:
http://bugs.python.org/file26231/followlinks-to-follow_symlinks-4.patch
Added file: http://bugs.python.org/file26232/symlinks-to-follow_symlinks-4.patch
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue15202>
_______________________________________
diff -r a1c8302e6b27 Doc/library/os.rst
--- a/Doc/library/os.rst Sun Jul 01 12:24:20 2012 +0200
+++ b/Doc/library/os.rst Mon Jul 02 18:07:59 2012 +0300
@@ -2211,7 +2211,7 @@
os.rmdir(os.path.join(root, name))
-.. function:: fwalk(top='.', topdown=True, onerror=None, followlinks=False, *,
dir_fd=None)
+.. function:: fwalk(top='.', topdown=True, onerror=None, *,
follow_symlinks=False, dir_fd=None)
.. index::
single: directory; walking
diff -r a1c8302e6b27 Lib/os.py
--- a/Lib/os.py Sun Jul 01 12:24:20 2012 +0200
+++ b/Lib/os.py Mon Jul 02 18:07:59 2012 +0300
@@ -424,7 +424,7 @@
if {open, stat} <= supports_dir_fd and {listdir, stat} <= supports_fd:
- def fwalk(top=".", topdown=True, onerror=None, followlinks=False, *,
dir_fd=None):
+ def fwalk(top=".", topdown=True, onerror=None, *, follow_symlinks=False,
dir_fd=None):
"""Directory tree generator.
This behaves exactly like walk(), except that it yields a 4-tuple
@@ -435,7 +435,7 @@
and `dirfd` is a file descriptor referring to the directory `dirpath`.
The advantage of fwalk() over walk() is that it's safe against symlink
- races (when followlinks is False).
+ races (when follow_symlinks is False).
If dir_fd is not None, it should be a file descriptor open to a
directory,
and top should be relative; top will then be relative to that
directory.
@@ -462,13 +462,13 @@
orig_st = stat(top, follow_symlinks=False, dir_fd=dir_fd)
topfd = open(top, O_RDONLY, dir_fd=dir_fd)
try:
- if (followlinks or (st.S_ISDIR(orig_st.st_mode) and
- path.samestat(orig_st, stat(topfd)))):
- yield from _fwalk(topfd, top, topdown, onerror, followlinks)
+ if (follow_symlinks or (st.S_ISDIR(orig_st.st_mode) and
+ path.samestat(orig_st, stat(topfd)))):
+ yield from _fwalk(topfd, top, topdown, onerror,
follow_symlinks)
finally:
close(topfd)
- def _fwalk(topfd, toppath, topdown, onerror, followlinks):
+ def _fwalk(topfd, toppath, topdown, onerror, follow_symlinks):
# Note: This uses O(depth of the directory tree) file descriptors: if
# necessary, it can be adapted to only require O(1) FDs, see issue
# #13734.
@@ -499,16 +499,16 @@
for name in dirs:
try:
- orig_st = stat(name, dir_fd=topfd, follow_symlinks=followlinks)
+ orig_st = stat(name, dir_fd=topfd,
follow_symlinks=follow_symlinks)
dirfd = open(name, O_RDONLY, dir_fd=topfd)
except error as err:
if onerror is not None:
onerror(err)
return
try:
- if followlinks or path.samestat(orig_st, stat(dirfd)):
+ if follow_symlinks or path.samestat(orig_st, stat(dirfd)):
dirpath = path.join(toppath, name)
- yield from _fwalk(dirfd, dirpath, topdown, onerror,
followlinks)
+ yield from _fwalk(dirfd, dirpath, topdown, onerror,
follow_symlinks)
finally:
close(dirfd)
diff -r a1c8302e6b27 Lib/test/test_os.py
--- a/Lib/test/test_os.py Sun Jul 01 12:24:20 2012 +0200
+++ b/Lib/test/test_os.py Mon Jul 02 18:07:59 2012 +0300
@@ -745,10 +745,11 @@
"""
compare with walk() results.
"""
- for topdown, followlinks in itertools.product((True, False), repeat=2):
- d = {'topdown': topdown, 'followlinks': followlinks}
- walk_kwargs.update(d)
- fwalk_kwargs.update(d)
+ walk_kwargs = walk_kwargs.copy()
+ fwalk_kwargs = fwalk_kwargs.copy()
+ for topdown, follow_symlinks in itertools.product((True, False),
repeat=2):
+ walk_kwargs.update(topdown=topdown, followlinks=follow_symlinks)
+ fwalk_kwargs.update(topdown=topdown,
follow_symlinks=follow_symlinks)
expected = {}
for root, dirs, files in os.walk(**walk_kwargs):
@@ -774,9 +775,9 @@
def test_yields_correct_dir_fd(self):
# check returned file descriptors
- for topdown, followlinks in itertools.product((True, False), repeat=2):
- args = support.TESTFN, topdown, None, followlinks
- for root, dirs, files, rootfd in os.fwalk(*args):
+ for topdown, follow_symlinks in itertools.product((True, False),
repeat=2):
+ args = support.TESTFN, topdown, None
+ for root, dirs, files, rootfd in os.fwalk(*args,
follow_symlinks=follow_symlinks):
# check that the FD is valid
os.fstat(rootfd)
# redundant check
diff -r a1c8302e6b27 Doc/library/shutil.rst
--- a/Doc/library/shutil.rst Sun Jul 01 12:24:20 2012 +0200
+++ b/Doc/library/shutil.rst Mon Jul 02 17:57:45 2012 +0300
@@ -47,7 +47,7 @@
be copied.
-.. function:: copyfile(src, dst, symlinks=False)
+.. function:: copyfile(src, dst, *, symlinks=False)
Copy the contents (no metadata) of the file named *src* to a file named
*dst* and return *dst*. *dst* must be the complete target file name; look
at
@@ -69,7 +69,7 @@
.. versionchanged:: 3.3
Added return of the *dst*.
-.. function:: copymode(src, dst, symlinks=False)
+.. function:: copymode(src, dst, *, symlinks=False)
Copy the permission bits from *src* to *dst*. The file contents, owner, and
group are unaffected. *src* and *dst* are path names given as strings. If
@@ -80,7 +80,7 @@
.. versionchanged:: 3.3
Added *symlinks* argument.
-.. function:: copystat(src, dst, symlinks=False)
+.. function:: copystat(src, dst, *, symlinks=False)
Copy the permission bits, last access time, last modification time, and
flags
from *src* to *dst*. The file contents, owner, and group are unaffected.
*src*
@@ -91,7 +91,7 @@
.. versionchanged:: 3.3
Added *symlinks* argument.
-.. function:: copy(src, dst, symlinks=False)
+.. function:: copy(src, dst, *, symlinks=False)
Copy the file *src* to the file or directory *dst* and return the file's
destination. If *dst* is a directory, a
@@ -106,7 +106,7 @@
.. versionchanged:: 3.3
Added return of the *dst*.
-.. function:: copy2(src, dst, symlinks=False)
+.. function:: copy2(src, dst, *, symlinks=False)
Similar to :func:`shutil.copy`, including that the destination is
returned, but metadata is copied as well. This is
diff -r a1c8302e6b27 Lib/shutil.py
--- a/Lib/shutil.py Sun Jul 01 12:24:20 2012 +0200
+++ b/Lib/shutil.py Mon Jul 02 17:57:45 2012 +0300
@@ -82,10 +82,10 @@
return (os.path.normcase(os.path.abspath(src)) ==
os.path.normcase(os.path.abspath(dst)))
-def copyfile(src, dst, symlinks=False):
+def copyfile(src, dst, *, follow_symlinks=True):
"""Copy data from src to dst.
- If optional flag `symlinks` is set and `src` is a symbolic link, a new
+ If optional flag `follow_symlinks` is not set and `src` is a symbolic
link, a new
symlink will be created instead of copying the file it points to.
"""
@@ -103,7 +103,7 @@
if stat.S_ISFIFO(st.st_mode):
raise SpecialFileError("`%s` is a named pipe" % fn)
- if symlinks and os.path.islink(src):
+ if not follow_symlinks and os.path.islink(src):
os.symlink(os.readlink(src), dst)
else:
with open(src, 'rb') as fsrc:
@@ -111,15 +111,15 @@
copyfileobj(fsrc, fdst)
return dst
-def copymode(src, dst, symlinks=False):
+def copymode(src, dst, *, follow_symlinks=True):
"""Copy mode bits from src to dst.
- If the optional flag `symlinks` is set, symlinks aren't followed if and
+ If the optional flag `follow_symlinks` is not set, symlinks aren't
followed if and
only if both `src` and `dst` are symlinks. If `lchmod` isn't available (eg.
Linux), in these cases, this method does nothing.
"""
- if symlinks and os.path.islink(src) and os.path.islink(dst):
+ if not follow_symlinks and os.path.islink(src) and os.path.islink(dst):
if hasattr(os, 'lchmod'):
stat_func, chmod_func = os.lstat, os.lchmod
else:
@@ -132,10 +132,10 @@
st = stat_func(src)
chmod_func(dst, stat.S_IMODE(st.st_mode))
-def copystat(src, dst, symlinks=False):
+def copystat(src, dst, *, follow_symlinks=True):
"""Copy all stat info (mode bits, atime, mtime, flags) from src to dst.
- If the optional flag `symlinks` is set, symlinks aren't followed if and
+ If the optional flag `follow_symlinks` is not set, symlinks aren't
followed if and
only if both `src` and `dst` are symlinks.
"""
@@ -143,7 +143,7 @@
pass
# follow symlinks (aka don't not follow symlinks)
- follow = not (symlinks and os.path.islink(src) and os.path.islink(dst))
+ follow = follow_symlinks or not (os.path.islink(src) and
os.path.islink(dst))
if follow:
# use the real function if it exists
def lookup(name):
@@ -186,19 +186,19 @@
raise
if hasattr(os, 'listxattr'):
- def _copyxattr(src, dst, symlinks=False):
+ def _copyxattr(src, dst, *, follow_symlinks=True):
"""Copy extended filesystem attributes from `src` to `dst`.
Overwrite existing attributes.
- If the optional flag `symlinks` is set, symlinks won't be followed.
+ If the optional flag `follow_symlinks` is not set, symlinks won't be
followed.
"""
- for name in os.listxattr(src, follow_symlinks=symlinks):
+ for name in os.listxattr(src, follow_symlinks=follow_symlinks):
try:
- value = os.getxattr(src, name, follow_symlinks=symlinks)
- os.setxattr(dst, name, value, follow_symlinks=symlinks)
+ value = os.getxattr(src, name, follow_symlinks=follow_symlinks)
+ os.setxattr(dst, name, value, follow_symlinks=follow_symlinks)
except OSError as e:
if e.errno not in (errno.EPERM, errno.ENOTSUP, errno.ENODATA):
raise
@@ -206,36 +206,36 @@
def _copyxattr(*args, **kwargs):
pass
-def copy(src, dst, symlinks=False):
+def copy(src, dst, *, follow_symlinks=True):
"""Copy data and mode bits ("cp src dst"). Return the file's destination.
The destination may be a directory.
- If the optional flag `symlinks` is set, symlinks won't be followed. This
+ If the optional flag `follow_symlinks` is not set, symlinks won't be
followed. This
resembles GNU's "cp -P src dst".
"""
if os.path.isdir(dst):
dst = os.path.join(dst, os.path.basename(src))
- copyfile(src, dst, symlinks=symlinks)
- copymode(src, dst, symlinks=symlinks)
+ copyfile(src, dst, follow_symlinks=follow_symlinks)
+ copymode(src, dst, follow_symlinks=follow_symlinks)
return dst
-def copy2(src, dst, symlinks=False):
+def copy2(src, dst, *, follow_symlinks=True):
"""Copy data and all stat info ("cp -p src dst"). Return the file's
destination."
The destination may be a directory.
- If the optional flag `symlinks` is set, symlinks won't be followed. This
+ If the optional flag `follow_symlinks` is not set, symlinks won't be
followed. This
resembles GNU's "cp -P src dst".
"""
if os.path.isdir(dst):
dst = os.path.join(dst, os.path.basename(src))
- copyfile(src, dst, symlinks=symlinks)
- copystat(src, dst, symlinks=symlinks)
- _copyxattr(src, dst, symlinks=symlinks)
+ copyfile(src, dst, follow_symlinks=follow_symlinks)
+ copystat(src, dst, follow_symlinks=follow_symlinks)
+ _copyxattr(src, dst, follow_symlinks=follow_symlinks)
return dst
def ignore_patterns(*patterns):
@@ -307,7 +307,7 @@
# code with a custom `copy_function` may rely on copytree
# doing the right thing.
os.symlink(linkto, dstname)
- copystat(srcname, dstname, symlinks=symlinks)
+ copystat(srcname, dstname, follow_symlinks=not symlinks)
else:
# ignore dangling symlink if the flag is on
if not os.path.exists(linkto) and ignore_dangling_symlinks:
diff -r a1c8302e6b27 Lib/test/test_shutil.py
--- a/Lib/test/test_shutil.py Sun Jul 01 12:24:20 2012 +0200
+++ b/Lib/test/test_shutil.py Mon Jul 02 17:57:45 2012 +0300
@@ -277,17 +277,17 @@
os.lchmod(src_link, stat.S_IRWXO|stat.S_IRWXG)
# link to link
os.lchmod(dst_link, stat.S_IRWXO)
- shutil.copymode(src_link, dst_link, symlinks=True)
+ shutil.copymode(src_link, dst_link, follow_symlinks=False)
self.assertEqual(os.lstat(src_link).st_mode,
os.lstat(dst_link).st_mode)
self.assertNotEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
# src link - use chmod
os.lchmod(dst_link, stat.S_IRWXO)
- shutil.copymode(src_link, dst, symlinks=True)
+ shutil.copymode(src_link, dst, follow_symlinks=False)
self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
# dst link - use chmod
os.lchmod(dst_link, stat.S_IRWXO)
- shutil.copymode(src, dst_link, symlinks=True)
+ shutil.copymode(src, dst_link, follow_symlinks=False)
self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
@unittest.skipIf(hasattr(os, 'lchmod'), 'requires os.lchmod to be missing')
@@ -302,7 +302,7 @@
write_file(dst, 'foo')
os.symlink(src, src_link)
os.symlink(dst, dst_link)
- shutil.copymode(src_link, dst_link, symlinks=True) # silent fail
+ shutil.copymode(src_link, dst_link, follow_symlinks=False) # silent
fail
@support.skip_unless_symlink
def test_copystat_symlinks(self):
@@ -326,10 +326,10 @@
src_link_stat = os.lstat(src_link)
# follow
if hasattr(os, 'lchmod'):
- shutil.copystat(src_link, dst_link, symlinks=False)
+ shutil.copystat(src_link, dst_link, follow_symlinks=True)
self.assertNotEqual(src_link_stat.st_mode, os.stat(dst).st_mode)
# don't follow
- shutil.copystat(src_link, dst_link, symlinks=True)
+ shutil.copystat(src_link, dst_link, follow_symlinks=False)
dst_link_stat = os.lstat(dst_link)
if os.utime in os.supports_follow_symlinks:
for attr in 'st_atime', 'st_mtime':
@@ -341,7 +341,7 @@
if hasattr(os, 'lchflags') and hasattr(src_link_stat, 'st_flags'):
self.assertEqual(src_link_stat.st_flags, dst_link_stat.st_flags)
# tell to follow but dst is not a link
- shutil.copystat(src_link, dst, symlinks=True)
+ shutil.copystat(src_link, dst, follow_symlinks=False)
self.assertTrue(abs(os.stat(src).st_mtime - os.stat(dst).st_mtime) <
00000.1)
@@ -429,10 +429,10 @@
dst_link = os.path.join(tmp_dir, 'qux')
write_file(dst, 'bar')
os.symlink(dst, dst_link)
- shutil._copyxattr(src_link, dst_link, symlinks=True)
+ shutil._copyxattr(src_link, dst_link, follow_symlinks=False)
self.assertEqual(os.getxattr(dst_link, 'trusted.foo',
follow_symlinks=False), b'43')
self.assertRaises(OSError, os.getxattr, dst, 'trusted.foo')
- shutil._copyxattr(src_link, dst, symlinks=True)
+ shutil._copyxattr(src_link, dst, follow_symlinks=False)
self.assertEqual(os.getxattr(dst, 'trusted.foo'), b'43')
@support.skip_unless_symlink
@@ -446,12 +446,12 @@
if hasattr(os, 'lchmod'):
os.lchmod(src_link, stat.S_IRWXU | stat.S_IRWXO)
# don't follow
- shutil.copy(src_link, dst, symlinks=False)
+ shutil.copy(src_link, dst, follow_symlinks=True)
self.assertFalse(os.path.islink(dst))
self.assertEqual(read_file(src), read_file(dst))
os.remove(dst)
# follow
- shutil.copy(src_link, dst, symlinks=True)
+ shutil.copy(src_link, dst, follow_symlinks=False)
self.assertTrue(os.path.islink(dst))
self.assertEqual(os.readlink(dst), os.readlink(src_link))
if hasattr(os, 'lchmod'):
@@ -473,12 +473,12 @@
src_stat = os.stat(src)
src_link_stat = os.lstat(src_link)
# follow
- shutil.copy2(src_link, dst, symlinks=False)
+ shutil.copy2(src_link, dst, follow_symlinks=True)
self.assertFalse(os.path.islink(dst))
self.assertEqual(read_file(src), read_file(dst))
os.remove(dst)
# don't follow
- shutil.copy2(src_link, dst, symlinks=True)
+ shutil.copy2(src_link, dst, follow_symlinks=False)
self.assertTrue(os.path.islink(dst))
self.assertEqual(os.readlink(dst), os.readlink(src_link))
dst_stat = os.lstat(dst)
@@ -516,7 +516,7 @@
write_file(src, 'foo')
os.symlink(src, link)
# don't follow
- shutil.copyfile(link, dst_link, symlinks=True)
+ shutil.copyfile(link, dst_link, follow_symlinks=False)
self.assertTrue(os.path.islink(dst_link))
self.assertEqual(os.readlink(link), os.readlink(dst_link))
# follow
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
