classification
Title: remove "rmdir" argument from os.unlink, add "dir_fd" to os.rmdir
Type: behavior Stage: resolved
Components: Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: Arfrever, georg.brandl, hynek, larry, pitrou, python-dev
Priority: critical Keywords: patch

Created on 2012-06-23 17:15 by georg.brandl, last changed 2012-06-23 23:58 by larry. This issue is now closed.

Files
File name Uploaded Description Edit
move-dirfd-to-rmdir.patch georg.brandl, 2012-06-23 17:15 review
Messages (6)
msg163647 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-06-23 17:15
As Antoine noted in #14626, the "rmdir" argument to os.unlink doesn't really make sense since we already deviate from just mapping posix functionality one-on-one.

Attached is a patch removing "rmdir" from os.unlink, and instead adding "dir_fd" to os.rmdir.
msg163650 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-06-23 17:18
Looks fine to me.
msg163662 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-23 20:19
I think it's a good idea, and I didn't spot anything on my first pass at reviewing the patch.  But I'm on my way out the door and won't be back for a few hours.

Tell you what: once I'm back, if I don't spot anything out of place, I'll just check it in.  If I have feedback I'll file a second patch.  Work for you, Georg?

(p.s. Antoine, Georg, anybody, if you have any more bright ideas like this, I'd love to hear 'em.  But... now's the time.)
msg163668 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-06-23 20:37
Works for me.
msg163694 - (view) Author: Roundup Robot (python-dev) Date: 2012-06-23 23:55
New changeset 3b7230997425 by Larry Hastings in branch 'default':
Issue #15154: Add "dir_fd" parameter to os.rmdir, remove "rmdir"
http://hg.python.org/cpython/rev/3b7230997425
msg163695 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-23 23:56
I made two documentation changes to the patch before committing it:
* I restored the text under os.remove about how it doesn't handle
  directories.
* I added a Misc/NEWS entry.

But I didn't have to touch the code--that looked dead-on to me.  Proper job, Georg!
History
Date User Action Args
2012-06-23 23:58:06larrysetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-06-23 23:56:56larrysetmessages: + msg163695
2012-06-23 23:55:46python-devsetnosy: + python-dev
messages: + msg163694
2012-06-23 20:37:05georg.brandlsetmessages: + msg163668
2012-06-23 20:19:15larrysetmessages: + msg163662
2012-06-23 18:04:32hyneksetnosy: + hynek
2012-06-23 17:18:57pitrousetmessages: + msg163650
2012-06-23 17:16:57Arfreversetnosy: + Arfrever
2012-06-23 17:15:10georg.brandlcreate