[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[STINNER Victor]

STINNER Victor added the comment:

"""
Today we're doing something like this:

tmpdir = tempfile.mkdtemp('', 'hgtests.', d and 
d.decode('utf-8')).encode('utf-8')
"""

Don't do that. UTF-8 is not the right encoding.

Use os.fsencode() and os.fsdecode(). Internally, Python uses 
sys.getfilesystemencoding() with 'surrogateescape' error handler (but 'strict' 
error handler on Windows).

If you use UTF-8, you can get indirectly mojibake.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[STINNER Victor]

STINNER Victor added the comment:

I don't understand the rationale to add new functions using bytes filenames.

IMHO it's a bad practice to pass bytes filenames to write portable code. On 
Windows, bytes lead to strange bugs: os.listdir(bytes) gives you filenames 
which don't exist when a filename cannot be encoded to the ANSI code page.

On UNIX, os.fsencode() and os.fsdecode() allows you to convert filenames 
between unicode and bytes. It provides nice properties like 
os.fsencode(os.fsdecode(value))==value where value is bytes. No need to add new 
APIs, use existing APIs an use os.fsencode() and os.fsdecode() on top of them.

Bytes filenames are deprecated on Windows since Python 3.2. The new 
os.scandir() function doesn't support bytes for example, it's a deliberate 
choice.

Maybe I missed an important technical issue?

--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Gregory P. Smith]

Gregory P. Smith added the comment:

code review comments addressed.

--
Added file: http://bugs.python.org/file39464/issue24230-gps02.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Martin Panter]

Martin Panter added the comment:

Looks good in general. I think the signatures in Doc/library/tempfile.rst need 
updating for the following functions (possibly also version changed notices 
added somewhere):

* NamedTemporaryFile
* TemporaryFile
* SpooledTemporaryFile
* TemporaryDirectory

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Roundup Robot]

Roundup Robot added the comment:

New changeset 870899ce71f4 by Gregory P. Smith in branch 'default':
Issue 24230: The tempfile module now accepts bytes for prefix, suffix and dir
https://hg.python.org/cpython/rev/870899ce71f4

--
nosy: +python-dev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Gregory P. Smith]

Gregory P. Smith added the comment:

that should take care of it.  if you see any other issues with this post 
commit, either raise them here or fix them directly.  the reviews were helpful.

--
resolution:  - fixed
stage: patch review - commit review
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Augie Fackler]

Augie Fackler added the comment:

I just removed my hack that gave us unicode to hand to mkdtemp() and everything 
still passes with your new commit. Thanks much for the quick response!

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Gregory P. Smith]

Gregory P. Smith added the comment:

updated, thanks for the great reviews.

--
Added file: http://bugs.python.org/file39469/issue24230-gps03.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Added comments on Rietveld. But I'm not sure that this change is worth to do. 
Only low-level functions in os.path and os modules (and few other) support 
bytes names (and only bytes, not bytearray!). High level interfaces, such as in 
tarfile or zipfile, work only with str paths. The question is what level the 
tempfile module?

In any way Mercurial people need their own wrapper if they want to support 
Python 3.5.

--
nosy: +serhiy.storchaka

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Martin Panter]

Martin Panter added the comment:

Doing a quick review, expect a few comments on Rietveld

--
nosy: +vadmium

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[R. David Murray]

R. David Murray added the comment:

We have a number of other places in the stdlib where bytes-in-bytes-out is 
observed, at various levels of abstraction.  I think this is reasonable.  To 
answer your direct question, I think tempfile is a convenience-and-do-it-right 
wrapper around what is really a pretty low level operation (creating a 
temporary file/directory securely).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I was vague about this feature, but if other core developer found it useful, I 
have no objections. Only few nitpicks to the implementation.

--
nosy: +georg.brandl, ncoghlan, pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Augie Fackler]

Augie Fackler added the comment:

I'll build a patched Python3.5 tomorrow (I'm on the road today) and let you 
know if this does everything we need. I'd be shocked if it wasn't plenty.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Augie Fackler]

Augie Fackler added the comment:

 In any way Mercurial people need their own wrapper if they want to support 
 Python 3.5.

We need format operations on bytes objects, so we have no plans to support 
Python3 for any version before 3.5.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Gregory P. Smith]

Gregory P. Smith added the comment:

Attached is a patch with implementation, tests and docs.  I didn't have to 
duplicate too much thankfully.  Just figure out where to put the type 
conversions.  Review would be nice, but I'll err on the side of getting this in 
before beta 1 (May 24 per https://www.python.org/dev/peps/pep-0478/) so any 
corrections can be dealt with during the beta cycle.

Augie  Matt, can you confirm that this patch does what y'all need for 
Mercurial?

--
keywords: +patch
stage: needs patch - patch review
Added file: http://bugs.python.org/file39436/issue24230-gps01.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Augie Fackler]

New submission from Augie Fackler:

Many things in the OS module work with bytestrings for paths in Python 3, but 
tempfile isn't so happy. It'd be very useful to be able to have the same 
bytes-in-bytes-out behavior in tempfile as for the os.path.* methods.

This is something we're working around in hg, but I figured I'd file a bug 
anyway in case we can improve the situation.

--
components: Library (Lib)
messages: 243498
nosy: durin42
priority: normal
severity: normal
status: open
title: tempfile.mkdtemp() doesn't work with bytes paths
versions: Python 3.5

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Matt Mackall]

Matt Mackall added the comment:

Another way of putting it is:

os.listdir(bytes) - [bytes,...]
os.listdir(unicode) - [unicode,...]

is the usual pattern, and tempfile isn't following it.

--
nosy: +Matt.Mackall

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[R. David Murray]

R. David Murray added the comment:

I understand now.  The problem is that this would be a backward incompatible 
change.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[R. David Murray]

R. David Murray added the comment:

Would you care to supply a patch?  The beta deadline is this coming weekend.

--
stage:  - needs patch
type:  - enhancement

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[R. David Murray]

R. David Murray added the comment:

I'm sorry, hit send before I finished thinking.  You are saying bytes input are 
rejected, so yes this could be fixed.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[R. David Murray]

R. David Murray added the comment:

Can you explain what you are looking for in more detail?  It isn't obvious to 
me what bytes in bytes out means in the context of the tempfile API.

--
nosy: +r.david.murray

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Augie Fackler]

Augie Fackler added the comment:

Today we're doing something like this:

tmpdir = tempfile.mkdtemp('', 'hgtests.', d and 
d.decode('utf-8')).encode('utf-8')

but would rather have

tmpdir = tempfile.mkdtemp(b'', b'hgtests.', d)
# and now tmpdir is a bytestring containing the path to the tmpdir

basically, we want to maintain our pedantry around paths being sequences of 
bytes.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Gregory P. Smith]

Changes by Gregory P. Smith g...@krypto.org:


--
assignee:  - gregory.p.smith
nosy: +gregory.p.smith

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24230] tempfile.mkdtemp() doesn't work with bytes paths

[Gregory P. Smith]

Gregory P. Smith added the comment:

it seems a little messy to code this up the way Lib/tempfile.py is written but 
i'll take a stab at it tonight.  Probably via parallel implementations of the 
methods internally rather than conditional logic throughout given how they work.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24230
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com