classification
Title: Clarify the behavior of listdir(fd) in both code and documentation
Type: enhancement Stage: resolved
Components: Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: Arfrever, georg.brandl, larry, ncoghlan, python-dev
Priority: release blocker Keywords: patch

Created on 2012-06-25 03:39 by larry, last changed 2012-06-25 11:50 by larry. This issue is now closed.

Files
File name Uploaded Description Edit
larry.listdir.clarification.1.diff larry, 2012-06-25 07:09 First patch clarifying matters around our mysterious friend listdir. review
larry.listdir.clarification.4.diff larry, 2012-06-25 10:41 Patch #4, hopefully Rietveld likes this one. review
Messages (12)
msg163885 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 03:39
When you pass in a file descriptor as the first argument to listdir, its output is a list of strings, not a list of bytes.  This should be mentioned in the documentation.  It's also worth making a pass over the other functions accepting an fd.

As it happens, the code that implements this behavior is obtuse.  So I propose also modifying it to be more intelligible.
msg163887 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 04:33
Maybe the correct fix for this is to change the interface?

os.listdir(path, *, dir_fd=None)

Then we'll pick up the return type (str/bytes) from the path variable.

It would make #15177 even easier too, should we go down that route.
msg163890 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-06-25 04:57
+1 for changing the API for any cases where the filesystem path is also used to determine the return type - given that we stuffed this up for the rmtree implementation, I expect end users would be prone to making exactly the same mistake.

Retaining the ability to control the return type explicitly even when pass a descriptor would be a *lot* cleaner than other possibilities (which would all involve a bunch of manual encoding and decoding with os.fsencode() and os.fsdecode()).
msg163891 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 04:59
Nick, how do you feel specifically about listdir(path, *, dir_fd)?  I'm mentally exhausted from the past couple of days and have temporarily lost the ability to infer.
msg163895 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 05:20
Nevermind, I'm an idiot.  fdopendir doesn't support dir_fd as a separate path, so listdir can't support it either.

There's an unwritten property of the os module on a POSIX system: all the functions are essentially-atomic.  This is useful and should not be broken lightly.

I'll make a patch clarifying the behavior of listdir.
msg163896 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-06-25 05:35
Don't forget to point people to os.fsencode() if they actually wanted bytes, or os.fsdecode() if they already have bytes and want to convert them to text.
msg163908 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 07:09
My first cut at a patch.  Made the logic in posix_listdir easy to follow, fixed up the docstring and docs.  Posted in the correct issue this time.

(And Nick: I thought of that independently :D )
msg163921 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 09:49
Removed "do we have os.listdir" checks from the unit tests.  Yes, Virginia, we always have os.listdir in Python 3.3.
msg163930 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 10:32
Regenerated patch to make Reitveld happy.
msg163932 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 10:41
GRAHH HULK SMASH
Regenerating again, because I wasn't actually fresh enough last time.
msg163944 - (view) Author: Roundup Robot (python-dev) Date: 2012-06-25 11:42
New changeset 8172d8e907a4 by Larry Hastings in branch 'default':
Issue #15176: Clarified behavior, documentation, and implementation
http://hg.python.org/cpython/rev/8172d8e907a4
msg163951 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 11:50
Good news, everyone!
History
Date User Action Args
2012-06-25 11:50:45larrysetstatus: open -> closed
resolution: fixed
messages: + msg163951

stage: needs patch -> resolved
2012-06-25 11:42:33python-devsetnosy: + python-dev
messages: + msg163944
2012-06-25 11:14:07larrysetfiles: - larry.listdir.clarification.3.diff
2012-06-25 11:14:01larrysetfiles: - larry.listdir.clarification.2.diff
2012-06-25 10:41:46larrysetfiles: + larry.listdir.clarification.4.diff

messages: + msg163932
2012-06-25 10:32:48larrysetfiles: + larry.listdir.clarification.3.diff

messages: + msg163930
2012-06-25 09:49:34larrysetfiles: + larry.listdir.clarification.2.diff

messages: + msg163921
2012-06-25 07:09:52larrysetfiles: + larry.listdir.clarification.1.diff
keywords: + patch
messages: + msg163908
2012-06-25 05:35:49ncoghlansetmessages: + msg163896
2012-06-25 05:20:30larrysetmessages: + msg163895
2012-06-25 04:59:43larrysetmessages: + msg163891
2012-06-25 04:57:51ncoghlansetpriority: normal -> release blocker

messages: + msg163890
2012-06-25 04:33:04larrysetmessages: + msg163887
2012-06-25 03:57:25Arfreversetnosy: + Arfrever
2012-06-25 03:39:57larrycreate