Issue #15202: Consistently use the name "follow_symlinks" for

new parameters in os and shutil functions.  Patch by Serhiy Storchaka.

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
-   <dir_fd>`.
+   <dir_fd>` and :ref:`not following symlinks <follow_symlinks>`.  Note however
+   that, unlike other functions, the :funk:`fwalk` default value for
+   *follow_symlinks* is ``False``.
 
    .. note::
 

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

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)
 

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:

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

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

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