Billy McCulloch added the comment:
I stand by the patch file I previously submitted on 2016-05-04. A more detailed
analysis / description of my reasoning follows.
Change 1 in _get_default_tempdir:
A PermissionError is thrown on Windows if you attempt to create a file whose
filename matches an existing directory. As the code currently stands, the `if`
statement checks whether the proposed file's *parent* directory is a directory
(which it is, or a FileNotFoundError would have been thrown), instead of
whether the proposed filename conflicts with an existing directory. The edited
expression is really a typo that, in the context of the code block, always
evaluates `True`.
Here’s what we’re now saying in the `if` block: when a PermissionError is
raised, if we’re on Windows (the only currently supported platform to throw
nonsense errors at us) AND the filename we chose simply conflicts with an
existing directory AND we supposedly have write access to the parent directory,
then we were just unlucky with the chosen name and should try again in the same
parent directory. (I say supposedly because Windows seems to erroneously report
True on this check, even when we don’t have write access. I wouldn’t be
surprised if this last check does something useful in certain contexts, I just
don’t know what they are.)
Change 2 in _mkstemp_inner:
Same as above for Change 1. While _get_default_tempdir uses this code block to
make sure the system tempdir is really writable, and _mkstemp_inner does it so
that a file descriptor can be returned, the result and arguments are the same.
Change 3 in mkdtemp:
For _get_default_tempdir and _mkstemp_inner, the blocks of code in question are
creating temporary files. As such, they need to handle the oddball case for
Windows where attempts to create a file with a filename which conflicts with an
existing directory name result in a PermissionError. The same block of error
handling code is copied in mkdtemp, even though this function never tries to
create a file – it only tries to create a directory. As such, in the case that
we try to create a directory with a name that already exists (whether as a file
or a directory), we wouldn't be dealing with a PermissionError, we'd have a
FileExistsError, which is already handled in mkdtemp by the preceding lines.
The only way I’ve seen a PermissionError crop up for a call to mkdir on Windows
is if the user doesn’t have permission to create filesystem objects in the
parent directory. This is the intended usage of a PermissionError, so no
special handling needed is required. Remember, a PermissionError shouldn’t
happen if mkdtemp is called without a `dir` kwarg, because the _sanitize_params
will invoke _get_default_tempdir, which will check to ensure that the parent
directory is writable. As such, this block of code was superfluous, and the
patch should not raise PermissionError in user code where it previously was
caught.
--
___
Python tracker
<https://bugs.python.org/issue22107>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
