followlinks/follow_symlinks/symlinks flags unification.

[larry]

Just reviewed the code for now--not a full review.  (Maybe later--right now I'm
in a hurry.)

http://bugs.python.org/review/15202/diff/5262/Lib/os.py
File Lib/os.py (right):

http://bugs.python.org/review/15202/diff/5262/Lib/os.py#newcode336
Lib/os.py:336: def walk(top, topdown=True, onerror=None,
follow_symlinks=_sentry, *, followlinks=_sentry):
I appreciate the rigor of using the _sentry here.  However, users examining the
Signature of this function would be baffled.  How about 'follow_symlinks=False'?

http://bugs.python.org/review/15202/diff/5262/Lib/os.py#newcode374
Lib/os.py:374: optional argument 'follow_symlinks' to true.
True is capitalized in Python; if you mean the value, use the proper name, if
you mean "a true value" then say that.  (I know, not your error, please fix
anyway.)

http://bugs.python.org/review/15202/diff/5262/Lib/os.py#newcode399
Lib/os.py:399: "'follow_symlinks' and 'followlinks' arguments are incompatible")
Bug: the default for follow_symlinks / followlinks should be False.  If the user
specifies neither follow_symlinks nor followlinks, follow_symlinks will be
interpreted as True.

http://bugs.python.org/review/15202/diff/5262/Lib/os.py#newcode440
Lib/os.py:440: def fwalk(top=".", topdown=True, onerror=None,
follow_symlinks=False, *, dir_fd=None):
Please get Georg's confirmation that simply renaming os.fwalk(followlinks) to
os.fwalk(follow_symlinks) is permissible at this late stage.

[storchaka]

http://bugs.python.org/review/15202/diff/5262/Lib/os.py
File Lib/os.py (right):

http://bugs.python.org/review/15202/diff/5262/Lib/os.py#newcode336
Lib/os.py:336: def walk(top, topdown=True, onerror=None,
follow_symlinks=_sentry, *, followlinks=_sentry):
On 2012/06/27 22:39:51, larry wrote:
> I appreciate the rigor of using the _sentry here.  However, users examining
the
> Signature of this function would be baffled.  How about
'follow_symlinks=False'?

How then to distinguish between `walk(top, follow_symlinks=True,
followlinks=False)` or `walk(top, follow_symlinks=False, followlinks=True)` from
legal use of the function?

http://bugs.python.org/review/15202/diff/5262/Lib/os.py#newcode399
Lib/os.py:399: "'follow_symlinks' and 'followlinks' arguments are incompatible")
On 2012/06/27 22:39:51, larry wrote:
> Bug: the default for follow_symlinks / followlinks should be False.  If the
user
> specifies neither follow_symlinks nor followlinks, follow_symlinks will be
> interpreted as True.

No. If both follow_symlinks and followlinks are _sentry, then follow_symlinks =
False (as default).

[larry]

I did a lot of editing on the documentation.  I'd feel better if someone else
would take the next review pass, so we could
get a fresh pair of eyes reviewing the docs.

http://bugs.python.org/review/15202/diff/5292/Doc/library/shutil.rst
File Doc/library/shutil.rst (right):

http://bugs.python.org/review/15202/diff/5292/Doc/library/shutil.rst#newcode50
Doc/library/shutil.rst:50: .. function:: copyfile(src, dst,
follow_symlinks=True)
follow_symlinks should always be keyword-only.  Please add a "*," between dst
and follow_symlinks for all functions here.

http://bugs.python.org/review/15202/diff/5292/Doc/library/shutil.rst#newcode76
Doc/library/shutil.rst:76: *follow_symlinks* is false, *src* a symbolic link and
the operating system supports
Please remove the last sentence of this paragraph, and add the following new
paragraph:

"""
If *follow_symlinks* is false, and both *src* and *dst* are
symbolic links, :func:`copymode` will attempt to modify
the mode of *dst* itself (rather than the file it points to).  This
functionality is not available on every platform; please see :func:`copystat`
for more information.  If :func:`copymode` cannot modify symbolic links on the
local platform, if asked 
to do so it will do nothing and return.
"""

http://bugs.python.org/review/15202/diff/5292/Doc/library/shutil.rst#newcode88
Doc/library/shutil.rst:88: symbolic links and *follow_symlinks* false, the stats
of the link will be copied as
Please remove the last sentence of this paragraph, and add the following
kajillion new paragraphs:

"""
If *follow_symlinks* is false, and *src* and *dst* both
refer to symbolic links, :func:`copystat` will operate on
the symbolic links themselves rather than the files the
symbolic links refer to--reading the information from the
*src* symbolic link, and writing the information to the
*dst* symbolic link.

.. note:

   Not all platforms provide the ability to examine and
   modify symbolic links.  Python can tell you whether or
   not this functionality is available.

   If ``os.chmod in os.supports_follow_symlinks`` is
   ``True``, :func:`copystat` can modify the permission
   bits of a symbolic link.

   If ``os.utime in os.supports_follow_symlinks`` is
   ``True``, :func:`copystat` can modify the last access
   and modification times of a symbolic link.

   If ``os.chflags in os.supports_follow_symlinks`` is
   ``True``, :func:`copystat` can modify the flags of
   a symbolic link.  (``os.chflags`` is not available on
   all platforms.)

   On platforms where some or all of this functionality
   is unavailable, when asked to modify a symbolic link,
   :func:`copystat` will copy everything it can.
   :func:`copystat` never returns failure.

   Please see :data:`os.supports_follow_symlinks`
   for more information.
"""

You may notice I didn't mention checking ``os.stat in
os.supports_symbolic_links``.  That's because AFAICT lstat() functionality is
available on all platforms Python supports.  Yes, there is a HAVE_LSTAT
define--but you'll notice posix_lstat() is unconditionally defined!

http://bugs.python.org/review/15202/diff/5292/Doc/library/shutil.rst#newcode99
Doc/library/shutil.rst:99: directory specified.  Permission bits are copied. 
*src* and *dst* are path
Okay, I've rewritten this whole paragraph.  Please replace it with the following
(cleaned up to 80 cols):

"""
Copies the file *src* to the file or directory *dst*.  *src* and *dst* should be
strings.  If *dst* specifies a
directory, the file will be copied into *dst* using the base filename from
*src*.  Returns the path to the newly created file.

If *follow_symlinks* is false, and *src* is a symbolic link, *dst* will be
created as a symbolic link.  If *follow_symlinks* is
true and *src* is a symbolic link, *dst* will be a copy of
the file *src* refers to.

:func:`copy` copies the file data and the file's permission
mode (see :func:`os.chmod`).  Other metadata, like the
file's creation and modification times, is not preserved.
To preserve all file metadata from the original, use
:func:`~shutil.copy2` instead.
"""

http://bugs.python.org/review/15202/diff/5292/Doc/library/shutil.rst#newcode111
Doc/library/shutil.rst:111: Similar to :func:`shutil.copy`, including that the
destination is
Please replace this entire paragraph with:

"""
Identical to :func:`~shutil.copy` except that :func:`copy2` also attempts to
preserve all file metadata.

When *follow_symlinks* is false, and *src* is a symbolic
link, :func:`copy2` attempts to copy all metadata from the
*src* symbolic link to the newly-created *dst* symbolic link.
However, this functionality is not available on all platforms.
On platforms where some or all of this functionality is
unavailable, :func:`copy2` will preserve all the metadata
it can; :func:`copy2` never returns failure.

:func:`copy2` uses :func:`copystat` to copy the file metadata.  Please see
:func:`copystat` for more information
about platform support for modifying symbolic links.
"""

http://bugs.python.org/review/15202/diff/5292/Lib/shutil.py
File Lib/shutil.py (right):

http://bugs.python.org/review/15202/diff/5292/Lib/shutil.py#newcode85
Lib/shutil.py:85: def copyfile(src, dst, follow_symlinks=True):
follow_symlinks should always be keyword-only.  Please add a "*," between dst
and follow_symlinks, here and in all other methods in this file.

http://bugs.python.org/review/15202/diff/5292/Lib/shutil.py#newcode88
Lib/shutil.py:88: If optional flag `follow_symlinks` is not set and `src` is a
symbolic link, a new
Please change this paragraph to:

"""
If follow_symlinks is False and src is a symbolic link,
dst will be created as a copy of that symbolic link
(rather than the file it points to).
"""

http://bugs.python.org/review/15202/diff/5292/Lib/shutil.py#newcode117
Lib/shutil.py:117: If the optional flag `follow_symlinks` is not set, symlinks
aren't followed if and
Please change this paragraph to:

"""
If follow_symlinks is False, and src and dst are both
symbolic links, copymode will change the mode of the dst
symbolic link rather than the file it points to.  (This
functionality is not available on all platforms.)
"""

Let's move the specific details about lchmod into the full docs.

http://bugs.python.org/review/15202/diff/5292/Lib/shutil.py#newcode138
Lib/shutil.py:138: If the optional flag `follow_symlinks` is not set, symlinks
aren't followed if and
Please change to:

"""
If follow_symlinks is False, and src and dst are both
symbolic links, copystat will read the information from
the src symbolic link and write it to the dst symbolic
link, rather than reading from and writing to the files
they point to.  (This functionality is not available on
all platforms.)
"""

[storchaka]

Larry, as now the patch will mainly consist of your edits may be you take the
maintaining? You can quickly respond to comments and questions to your
revisions.

http://bugs.python.org/review/15202/diff/5292/Doc/library/shutil.rst
File Doc/library/shutil.rst (right):

http://bugs.python.org/review/15202/diff/5292/Doc/library/shutil.rst#newcode50
Doc/library/shutil.rst:50: .. function:: copyfile(src, dst,
follow_symlinks=True)
On 2012/07/01 17:44:36, larry wrote:
> follow_symlinks should always be keyword-only.  Please add a "*," between dst
> and follow_symlinks for all functions here.

I fully agree with you. I don't suggested this himself only because I forgot.

[larry]

Okay, I'll produce the next patch.  Thanks, Serhiy!

[larry]

Yup, just one change, adding "*,".

http://bugs.python.org/review/15202/diff/5291/Lib/os.py
File Lib/os.py (right):

http://bugs.python.org/review/15202/diff/5291/Lib/os.py#newcode427
Lib/os.py:427: def fwalk(top=".", topdown=True, onerror=None,
follow_symlinks=False, *, dir_fd=None):
As with the other patch set: please add "*," in front of follow_symlinks.

[storchaka]

http://bugs.python.org/review/15202/diff/5291/Lib/os.py
File Lib/os.py (right):

http://bugs.python.org/review/15202/diff/5291/Lib/os.py#newcode427
Lib/os.py:427: def fwalk(top=".", topdown=True, onerror=None,
follow_symlinks=False, *, dir_fd=None):
On 2012/07/02 16:12:12, larry wrote:
> As with the other patch set: please add "*," in front of follow_symlinks.

Why not in front of any other?

[larry]

http://bugs.python.org/review/15202/diff/5291/Lib/os.py
File Lib/os.py (right):

http://bugs.python.org/review/15202/diff/5291/Lib/os.py#newcode427
Lib/os.py:427: def fwalk(top=".", topdown=True, onerror=None,
follow_symlinks=False, *, dir_fd=None):
On 2012/07/02 17:02:19, storchaka wrote:
> On 2012/07/02 16:12:12, larry wrote:
> > As with the other patch set: please add "*," in front of follow_symlinks.
> Why not in front of any other?

fwalk is *almost* the same as walk, and walk has a long history of taking
unlabeled boolean arguments.  I see your point but I'm not comfortable making
that big a change at this point.  I'd want to see a lot of support for that
change before making it.