From b40380667c98a3ab7e244e036944f731d61ffc27 Mon Sep 17 00:00:00 2001 From: Larry Hastings Date: Sun, 15 Jul 2012 10:57:38 -0700 Subject: Issue #15202: Consistently use the name "follow_symlinks" for new parameters in os and shutil functions. Patch by Serhiy Storchaka. --- Doc/library/os.rst | 6 ++++-- Doc/library/shutil.rst | 2 +- Lib/os.py | 18 ++++++++--------- Lib/shutil.py | 52 ++++++++++++++++++++++++------------------------- Lib/test/test_os.py | 15 +++++++------- Lib/test/test_shutil.py | 28 +++++++++++++------------- Misc/NEWS | 3 +++ 7 files changed, 65 insertions(+), 59 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 6d18300..1b198ce 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -2211,7 +2211,7 @@ features: 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 @@ -2224,7 +2224,9 @@ features: and *dirfd* is a file descriptor referring to the directory *dirpath*. This function always supports :ref:`paths relative to directory descriptors - `. + ` and :ref:`not following symlinks `. Note however + that, unlike other functions, the :funk:`fwalk` default value for + *follow_symlinks* is ``False``. .. note:: diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst index e8dde06..f7bfb57 100644 --- a/Doc/library/shutil.rst +++ b/Doc/library/shutil.rst @@ -47,7 +47,7 @@ Directory and files operations be copied. -.. function:: copyfile(src, dst, symlinks=False) +.. function:: copyfile(src, dst, *, follow_symlinks=True) 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 diff --git a/Lib/os.py b/Lib/os.py index 381737f..842512a 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -424,7 +424,7 @@ __all__.append("walk") 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 @@ if {open, stat} <= supports_dir_fd and {listdir, stat} <= supports_fd: 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 @@ if {open, stat} <= supports_dir_fd and {listdir, stat} <= supports_fd: 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 @@ if {open, stat} <= supports_dir_fd and {listdir, stat} <= supports_fd: 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 --git a/Lib/shutil.py b/Lib/shutil.py index 2b2a9be5..a8b9f3f 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -82,10 +82,10 @@ def _samefile(src, dst): 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 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 @@ def copyfile(src, dst, symlinks=False): 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 @@ def copyfile(src, dst, symlinks=False): 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 - only if both `src` and `dst` are symlinks. If `lchmod` isn't available (eg. - Linux), in these cases, this method does nothing. + If follow_symlinks is not set, symlinks aren't followed if and only + if both `src` and `dst` are symlinks. If `lchmod` isn't available + (e.g. Linux) 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: @@ -133,19 +133,19 @@ def copymode(src, dst, symlinks=False): chmod_func(dst, stat.S_IMODE(st.st_mode)) 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 `follow_symlinks` is false, 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 @@ -153,10 +153,10 @@ else: def _copyxattr(*args, **kwargs): pass -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. """ @@ -164,7 +164,7 @@ def copystat(src, dst, symlinks=False): 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): @@ -205,37 +205,37 @@ def copystat(src, dst, symlinks=False): break else: raise - _copyxattr(src, dst, symlinks=follow) + _copyxattr(src, dst, follow_symlinks=follow) -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 follow_symlinks is false, 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 follow_symlinks is false, 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) + copyfile(src, dst, follow_symlinks=follow_symlinks) + copystat(src, dst, follow_symlinks=follow_symlinks) return dst def ignore_patterns(*patterns): @@ -307,7 +307,7 @@ def copytree(src, dst, symlinks=False, ignore=None, copy_function=copy2, # 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 --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 7c73f1e..5c0b0c8 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -758,10 +758,11 @@ class FwalkTests(WalkTests): """ 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): @@ -787,9 +788,9 @@ class FwalkTests(WalkTests): 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 --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index cbbc36f..e5c38eb 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -277,17 +277,17 @@ class TestShutil(unittest.TestCase): 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 @@ class TestShutil(unittest.TestCase): 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 @@ class TestShutil(unittest.TestCase): 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 @@ class TestShutil(unittest.TestCase): 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) @@ -439,10 +439,10 @@ class TestShutil(unittest.TestCase): 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 @@ -456,12 +456,12 @@ class TestShutil(unittest.TestCase): 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'): @@ -483,12 +483,12 @@ class TestShutil(unittest.TestCase): 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) @@ -526,7 +526,7 @@ class TestShutil(unittest.TestCase): 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 diff --git a/Misc/NEWS b/Misc/NEWS index a01767c..9ebf65a 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,9 @@ What's New in Python 3.3.0 Beta 2? Core and Builtins ----------------- +- Issue #15202: Consistently use the name "follow_symlinks" for + new parameters in os and shutil functions. + - Issue #15314: __main__.__loader__ is now set correctly during interpreter startup -- cgit v0.12