[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Antoine Pitrou pit...@free.fr added the comment: The .4 patches both LGTM, please commit! You're the one with commit rights here ;) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 758a9023d836 by Larry Hastings in branch 'default': Issue #15202: Consistently use the name follow_symlinks for http://hg.python.org/cpython/rev/758a9023d836 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Éric Araujo mer...@netwok.org added the comment: There are versionadded notes in the doc that still use the old names (e.g. symlinks for shutil.copyfile). -- nosy: +eric.araujo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: Sorry; the patch didn't apply cleanly, and it looks like I bungled doing it manually. Fixing now. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset e26113f17309 by Larry Hastings in branch 'default': Issue #15202: Additional documentation fixes inadvertently omitted http://hg.python.org/cpython/rev/e26113f17309 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: The .4 patches both LGTM, please commit! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: Serihy: for the followlinks patch, how about we plan ahead: I give you feedback for just the code (if there is any), and then I take over the patch and do the doc rewrite that I will find irresistable. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: Storchaka: change of plan. Since doc changes are much lower risk than code changes, how about we go with your existing patch and I'll fix up the docs separately. 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
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.rstSun Jul 01 12:24:20 2012 +0200
+++ b/Doc/library/os.rstMon 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
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Antoine Pitrou pit...@free.fr added the comment: What's the urge to make parameters keyword-only? Also, copyfile() shouldn't change signature. -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: Removed file: http://bugs.python.org/file26232/symlinks-to-follow_symlinks-4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: Added file: http://bugs.python.org/file26233/symlinks-to-follow_symlinks-4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Serhiy Storchaka storch...@gmail.com added the comment: What's the urge to make parameters keyword-only? If anyone uses these functions with a new symlinks parameter in 3.3b1, he will get loud error (follow_symlinks == not symlinks). Also, copyfile() shouldn't change signature. Why not? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Antoine Pitrou pit...@free.fr added the comment: Also, copyfile() shouldn't change signature. Why not? Seems like I have misread the docs. Apparently the symlinks parameter was added in 3.3. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: What's the urge to make parameters keyword-only? I suggest that boolean parameters are best made keyword-only. Otherwise you have mystery meat like shutil.copyfile(src, dst, True) Also, all the extant uses of follow_symlinks in os are keyword-only, and as long as it's a new parameter it'd be nice to keep it consistent. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: storchaka: I can (finally!) spend some time reviewing patches today. Which ones do I look at? I'm guessing just the last two, followlinks-to-follow_symlinks-2.patch and symlinks-to-follow_symlinks-2.patch. But I wanted to confirm before I got knee-deep in it. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: Bad news: Fearless Leader (Georg) just told me on #python-dev that he's had a change of heart and doesn't want this in 3.3. I've marked the bug 3.4, and there's no rush on doing this work--we can't check it in until the 3.4 branch exists. Sorry, Serhiy :( -- versions: +Python 3.4 -Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: Georg just clarified: we can just change the parameter names for new APIs. It's the deprecation / new parameter stuff we can't do for 3.3. So cut a (much) simpler patch and let's get it in right quick. -- versions: +Python 3.3 -Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Serhiy Storchaka storch...@gmail.com added the comment:
Here is an updated patches (with index 3). No aliased parameters, only
new parameters renamed. Some minor errors have fixed.
--
Added file:
http://bugs.python.org/file26225/followlinks-to-follow_symlinks-3.patch
Added file: http://bugs.python.org/file26226/symlinks-to-follow_symlinks-3.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15202
___diff -r a1c8302e6b27 Doc/library/os.rst
--- a/Doc/library/os.rstSun Jul 01 12:24:20 2012 +0200
+++ b/Doc/library/os.rstSun Jul 01 15:54:02 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 Sun Jul 01 15:54:02 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 Sun Jul 01 15:54:02 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,8 +775,8 @@
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 topdown, follow_symlinks in itertools.product((True,
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: Removed file: http://bugs.python.org/file26194/followlinks-to-follow_symlinks-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: Removed file: http://bugs.python.org/file26195/symlinks-to-follow_symlinks-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Serhiy Storchaka storch...@gmail.com added the comment:
Here is updated patches. The type of exception thrown when specifying
mutually exclusive arguments changed from ValueError to TypeError (in
accordance with the raised in similar conflicts exceptions). Slightly
modified documentation. Taken into account some of Larry's comments.
Only os.walk had followlinks and shutil.copytree had symlinks before
3.3. So these are the only functions preserved the old arguments.
os.fwalk and another five public functions from shutil got a new
argument only in 3.3, for them it just renamed (may be to make them
keyword-only?). Larry worrying, isn't it too late? But it would be
strange to have in 3.3 new and immediately obsolete arguments. Moreover
the support for the two arguments complicate and slow down the code. If
you postpone it to 3.4, there will not be another solution, but now we
can do less mess.
--
Added file:
http://bugs.python.org/file26194/followlinks-to-follow_symlinks-2.patch
Added file: http://bugs.python.org/file26195/symlinks-to-follow_symlinks-2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15202
___diff -r d0f775432705 Doc/library/os.rst
--- a/Doc/library/os.rstThu Jun 28 01:45:48 2012 +0200
+++ b/Doc/library/os.rstThu Jun 28 11:02:52 2012 +0300
@@ -2128,7 +2128,8 @@
and the *dir_fd*, *follow_symlinks*, and *ns* parameters.
-.. function:: walk(top, topdown=True, onerror=None, followlinks=False)
+.. function:: walk(top, topdown=True, onerror=None, followlinks=False, *,
+ follow_symlinks=False)
.. index::
single: directory; walking
@@ -2168,12 +2169,12 @@
is available as the ``filename`` attribute of the exception object.
By default, :func:`walk` will not walk down into symbolic links that
resolve to
- directories. Set *followlinks* to ``True`` to visit directories pointed to
by
+ directories. Set *follow_symlinks* to ``True`` to visit directories pointed
to by
symlinks, on systems that support them.
.. note::
- Be aware that setting *followlinks* to ``True`` can lead to infinite
+ Be aware that setting *follow_symlinks* to ``True`` can lead to infinite
recursion if a link points to a parent directory of itself. :func:`walk`
does not keep track of the directories it visited already.
@@ -2210,8 +2211,13 @@
for name in dirs:
os.rmdir(os.path.join(root, name))
-
-.. function:: fwalk(top='.', topdown=True, onerror=None, followlinks=False, *,
dir_fd=None)
+ .. versionchanged:: 3.3
+ Added *follow_symlinks* as alias to *followlinks*. *followlinks* should
+ not be used in new code. A :exc:`TypeError` is raised if both
+ *followlinks* and *follow_symlinks* are specified.
+
+
+.. function:: fwalk(top='.', topdown=True, onerror=None,
follow_symlinks=False, *, dir_fd=None)
.. index::
single: directory; walking
diff -r d0f775432705 Lib/os.py
--- a/Lib/os.py Thu Jun 28 01:45:48 2012 +0200
+++ b/Lib/os.py Thu Jun 28 11:02:52 2012 +0300
@@ -331,7 +331,9 @@
__all__.extend([makedirs, removedirs, renames])
-def walk(top, topdown=True, onerror=None, followlinks=False):
+_sentry = object()
+
+def walk(top, topdown=True, onerror=None, followlinks=_sentry, *,
follow_symlinks=_sentry):
Directory tree generator.
For each directory in the directory tree rooted at top (including top
@@ -369,7 +371,7 @@
By default, os.walk does not follow symbolic links to subdirectories on
systems that support them. In order to get this functionality, set the
-optional argument 'followlinks' to true.
+optional argument 'follow_symlinks' to True.
Caution: if you pass a relative pathname for top, don't change the
current working directory between resumptions of walk. walk never
@@ -387,7 +389,15 @@
if 'CVS' in dirs:
dirs.remove('CVS') # don't visit CVS directories
+if follow_symlinks is _sentry:
+follow_symlinks = followlinks is not _sentry and followlinks
+elif followlinks is not _sentry:
+raise TypeError(walk() got values for both 'follow_symlinks' and
+ 'followlinks' arguments)
+yield from _walk(top, topdown, onerror, follow_symlinks)
+
+def _walk(top, topdown, onerror, follow_symlinks):
islink, join, isdir = path.islink, path.join, path.isdir
# We may not have read permission for top, in which case we can't
@@ -415,8 +425,8 @@
yield top, dirs, nondirs
for name in dirs:
new_path = join(top, name)
-if followlinks or not islink(new_path):
-yield from walk(new_path, topdown, onerror, followlinks)
+if follow_symlinks or not islink(new_path):
+yield from walk(new_path, topdown, onerror, follow_symlinks)
if not topdown:
yield top, dirs, nondirs
[issue15202] followlinks/follow_symlinks/symlinks flags unification.
New submission from Serhiy Storchaka storch...@gmail.com: Now three different flag are used to denote the same behavior. followlinks is used in os.(f)walk, symlinks is used in shutil functions (with opposite meaning), and follow_symlinks is just added to many os functions. Unfortunately, symlinks, like followlinks, is used prior to 3.3. But follow_symlinks appeared only in 3.3 (besides, it's too long a name for this frequently used parameter) and it can be unified with one of the earlier spelling. Replacing follow_symlinks to followlinks is simpler than to symlinks. -- components: Library (Lib) files: followlinks.patch keywords: patch messages: 164130 nosy: storchaka priority: normal severity: normal status: open title: followlinks/follow_symlinks/symlinks flags unification. Added file: http://bugs.python.org/file26176/followlinks.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: -- title: followlinks/follow_symlinks/symlinks flags unification. - followlinks/follow_symlinks/symlinks flags unification ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: -- nosy: +larry ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Hynek Schlawack h...@ox.cx added the comment: Yeah, would be nice if that was consistent. -- nosy: +hynek ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: -- type: - behavior versions: +Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: I assert that followlinks and symlinks are both bad names. I dislike followlinks because links is ambiguous; both hard links and soft links are links, but it's only modifying behavior regarding one of them. Also, it's not PEP-8-compliant (which we can forgive because I'm pretty sure it predates PEP 8). symlinks is far worse, because it's so ambiguous--quick, what does symlinks=False mean? Examine symlinks, or follow them? I agree that we can't rename followlinks and symlinks in 3.x. All we can do for now is move forward. At the same time I refused to be shackled by misguided old nomenclature. So, certainly, I don't want to see follow_symlinks changed. True story: the reason I started writing the patch for #14626 was so I could make sure it used the name follow_symlinks. I was dead certain Serhiy would use one of the existing names ;-) If you really really want this to happen, you'll have to get Georg's permission--and the sooner the better. Already I suspect it is too late. If it ships in 3.3 it will absolutely be too late. I suggest another approach: add a redundant follow_symlinks argument to os.walk, os.fwalk, and the shutil functions. Prefer the new name in documentation but document the presence of the old one. Throw an exception if both are specified in an invocation. We could do that in 3.4. -- nosy: +georg.brandl ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Serhiy Storchaka storch...@gmail.com added the comment: Also, it's not PEP-8-compliant (which we can forgive because I'm pretty sure it predates PEP 8). I don't see how this is contrary to PEP 8. PEP 8 says nothing about the names of function arguments. Already I suspect it is too late. If it ships in 3.3 it will absolutely be too late. Unfortunately, too little time has passed between #14626 accepting and shipping of 3.3b1. In any case, with accepted #14626 better than without it. I suggest another approach: add a redundant follow_symlinks argument to os.walk, os.fwalk, and the shutil functions. This is a bad solution, and I do not think that the benefits would outweigh the disadvantages. I am -0. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: I don't see how this is contrary to PEP 8. PEP 8 says nothing about the names of function arguments. Certainly it says *something*: http://www.python.org/dev/peps/pep-0008/#function-and-method-arguments But I grant you, it only specifically addresses self, cls, and function argument names that clash with keywords. I therefore suggest that function arguments are most similar to method names and instance variables--after all, they *are* instance variables. And method names and instance variables say Use the function naming rules. And function names should be lowercase, with words separated by underscores as necessary to improve readability. followlinks is comprised of two words but they are not separated by underscores. (We can also reason by process of elimination: function arguments are wholly dissimilar to constants, package/module names, classes, and exception. All the remaining types of identifiers in Python follow the above rule.) It might actually be nice to clarify PEP 8 on this. Unfortunately, too little time has passed between #14626 accepting and shipping of 3.3b1. I am genuinely sorry about that--but #14626 just wasn't ready earlier. I'm glad you think it's an improvement--we can certainly agree on that! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
R. David Murray rdmur...@bitdance.com added the comment: Although I do dislike how long it is, I think I agree with larry on this one that follow_symlinks is the cleanest choice. And we are post feature-freeze, so I think changing it should be done only if there is a strong reason. -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Georg Brandl ge...@python.org added the comment: IMO adding follow_symlinks to the functions currently supporting symlinks/followlinks is a bugfix. Such a patch would be ok to go into 3.3. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Serhiy Storchaka storch...@gmail.com added the comment: I dislike followlinks because links is ambiguous; both hard links and soft links are links, but it's only modifying behavior regarding one of them. Technically, in Unix world any file is a hard link. It is impossible to distinguish a hard link from the original, they are completely equal. So I don't think that there will be a ambiguity, what links are implied, especially, if the documentation is clearly says symbolic links. I therefore suggest that function arguments are most similar to method names and instance variables--after all, they *are* instance variables. Technically, they are local variables. followlinks is comprised of two words but they are not separated by underscores. As getgrouplist or sendfile. What about such argument names as filename and newline? If being a consistent in that, then it must be follow_symbolic_links. And source_directory_file_descriptor instead src_dir_fd. It might actually be nice to clarify PEP 8 on this. I agree. I'm glad you think it's an improvement--we can certainly agree on that! Of course. This is the issue, for the solution of which I registered here. All other issues and patches were just distractions and practice. ;-) I am grateful to you. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Serhiy Storchaka storch...@gmail.com added the comment:
IMO adding follow_symlinks to the functions currently supporting
symlinks/followlinks is a bugfix. Such a patch would be ok to go into 3.3.
Here is an other patch, that implements Larry's suggestion about
renaming followlinks in (f)walk to follow_symlinks (with keeping old
name as alias). I hope native speakers corrected me in docs. However, I
don't think that this is the best solution (but it better than nothing).
--
Added file:
http://bugs.python.org/file26182/followlinks-to-follow_symlinks.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15202
___diff -r b66e82c9f852 Doc/library/os.rst
--- a/Doc/library/os.rstTue Jun 26 23:05:27 2012 +0200
+++ b/Doc/library/os.rstWed Jun 27 20:49:59 2012 +0300
@@ -2128,7 +2128,7 @@
and the *dir_fd*, *follow_symlinks*, and *ns* parameters.
-.. function:: walk(top, topdown=True, onerror=None, followlinks=False)
+.. function:: walk(top, topdown=True, onerror=None, follow_symlinks=False)
.. index::
single: directory; walking
@@ -2168,12 +2168,12 @@
is available as the ``filename`` attribute of the exception object.
By default, :func:`walk` will not walk down into symbolic links that
resolve to
- directories. Set *followlinks* to ``True`` to visit directories pointed to
by
+ directories. Set *follow_symlinks* to ``True`` to visit directories pointed
to by
symlinks, on systems that support them.
.. note::
- Be aware that setting *followlinks* to ``True`` can lead to infinite
+ Be aware that setting *follow_symlinks* to ``True`` can lead to infinite
recursion if a link points to a parent directory of itself. :func:`walk`
does not keep track of the directories it visited already.
@@ -2210,8 +2210,12 @@
for name in dirs:
os.rmdir(os.path.join(root, name))
-
-.. function:: fwalk(top='.', topdown=True, onerror=None, followlinks=False, *,
dir_fd=None)
+ .. versionchanged:: 3.3
+ *followlinks* renamed to *follow_symlinks*. For backward compatibility
+ the old name accepted as alias to new one.
+
+
+.. function:: fwalk(top='.', topdown=True, onerror=None,
follow_symlinks=False, *, dir_fd=None)
.. index::
single: directory; walking
diff -r b66e82c9f852 Lib/os.py
--- a/Lib/os.py Tue Jun 26 23:05:27 2012 +0200
+++ b/Lib/os.py Wed Jun 27 20:49:59 2012 +0300
@@ -331,7 +331,9 @@
__all__.extend([makedirs, removedirs, renames])
-def walk(top, topdown=True, onerror=None, followlinks=False):
+_sentry = object()
+
+def walk(top, topdown=True, onerror=None, follow_symlinks=_sentry, *,
followlinks=_sentry):
Directory tree generator.
For each directory in the directory tree rooted at top (including top
@@ -369,7 +371,7 @@
By default, os.walk does not follow symbolic links to subdirectories on
systems that support them. In order to get this functionality, set the
-optional argument 'followlinks' to true.
+optional argument 'follow_symlinks' to true.
Caution: if you pass a relative pathname for top, don't change the
current working directory between resumptions of walk. walk never
@@ -387,7 +389,18 @@
if 'CVS' in dirs:
dirs.remove('CVS') # don't visit CVS directories
+if follow_symlinks is _sentry:
+if followlinks is _sentry:
+follow_symlinks = False
+else:
+follow_symlinks = followlinks
+elif followlinks is not _sentry:
+raise ValueError(
+'follow_symlinks' and 'followlinks' arguments are
incompatible)
+yield from _walk(top, topdown, onerror, follow_symlinks)
+
+def _walk(top, topdown, onerror, follow_symlinks):
islink, join, isdir = path.islink, path.join, path.isdir
# We may not have read permission for top, in which case we can't
@@ -415,8 +428,8 @@
yield top, dirs, nondirs
for name in dirs:
new_path = join(top, name)
-if followlinks or not islink(new_path):
-yield from walk(new_path, topdown, onerror, followlinks)
+if follow_symlinks or not islink(new_path):
+yield from walk(new_path, topdown, onerror, follow_symlinks)
if not topdown:
yield top, dirs, nondirs
@@ -424,7 +437,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 +448,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
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: Removed file: http://bugs.python.org/file26182/followlinks-to-follow_symlinks.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Changes by Serhiy Storchaka storch...@gmail.com: Added file: http://bugs.python.org/file26184/followlinks-to-follow_symlinks.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15202] followlinks/follow_symlinks/symlinks flags unification
Larry Hastings la...@hastings.org added the comment: after all, they *are* instance variables. Technically, they are local variables. Yeah, tbh I was thinking instance of an invocation here. But PEP 8 probably means instance of a class here. Still, there are three classes of naming things in Python: all uppercase (constants), CapCase (class and exception names), and everything else (lowercase with underscores). I suggest that arguments and local variables belong in the latter camp. At no point does PEP 8 suggest naming anything in lowercase without underscores. As getgrouplist or sendfile. What about such argument names as filename and newline? If being a consistent in that, then it must be follow_symbolic_links. And source_directory_file_descriptor instead src_dir_fd. You are muddling the issue--PEP 8 makes no ruling on abbreviations. Personally, I prefer not to use abbreviations where possible. I've found over the years that they are painfully ambiguous--I can never remember what abbreviation I used. (control becomes... ctl? cntl?) MvL also makes a good point that abbreviations are a hinderance to people who speak little or no English. That said, naming things after their POSIX counterparts has to be okay, and abbreviations are occasionally called for when they are widely understood / have a precedent in the library. So with every example you cite I prefer the extant name over any counterproposal you explicitly suggested. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15202 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
