Title: Support os.fwalk(dir_fd=)
Type: behavior Stage: resolved
Components: Versions: Python 3.4
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: Arfrever, georg.brandl, hynek, larry, neologix, pitrou, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

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

File name Uploaded Description Edit
larry.fwalk.dir_fd.1.diff larry, 2012-06-25 07:49 First patch adding dir_fd to os.fwalk(). review
larry.fwalk.dir_fd.4.diff larry, 2012-06-25 10:42 p a t c h _ f o u r review
larry.fwalk.and.walk.dir_fd.3.diff larry, 2012-06-25 10:43 Patch 3. May the ghost of Gerrit guide me. review
Messages (16)
msg163886 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 04:13
Consider: should os.fwalk() support dir_fd?  I think so.  In fact, in retrospect it seems like a bug that os.fwalk *doesn't* already support this.

Georg: is this a feature or a bugfix?

(Wish I'd thought of this Saturday!)

I actually did a little experimenting, and got os.fwalk(fd) to work with very little trouble.  os.walk(fd) is harder because the recursive step appends a string to the existing path, and it really needs to be relative to the fd, and there's no way to pass both of those at once with the current signature.  It's doable but it would require a separate function for the recursive step that accepted a dir_fd anyway.
msg163906 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 07:07
My first cut at a patch.  Made the logic in posix_listdir easy to follow, fixed up the docstring and docs.
msg163907 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 07:08
Whoops, wrong issue, ignore that.  (Meant for #15176, going there now.)
msg163909 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 07:49
What I did:
* Added dir_fd=None as a keyword only parameter.
* Gave top a default of ".".
* Passed through dir_fd in two spots--that was all it took!
* Made fwalk contingent on os.stat and both being
  in support_dir_fd, and os.listdir being in support_fd.
* Fixed docstring and docs.
* Added unit test for using dir_fd.
msg163918 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 09:38
Second revision of patch.
* Removed os.fwalk from os.supports_dir_fd.  As Georg points out,
  you can't test it.  (Well, you can, but only if the test would
  have succeeded.)
* Changed fstat(fd) calls to stat(fd) calls.
msg163920 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 09:45
Here's a variant of the patch adding os.walk(dir_fd=) support.  I'm not pushing for this.  I'm not sure it's a must-have; it seems like a nice-to-have, but we're past the time for nice-to-haves.
msg163922 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-06-25 10:10
Larry, please regenerate patch in Mercurial for review. I have some comments.
msg163927 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 10:30
Regenerated fwalk(dir_fd=) patch to make Rietveld happy.
msg163929 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 10:31
Regenerated walk(dir_fd=) patch to make Rietveld happy.
msg163931 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-06-25 10:33
Rietveld is not happy with git diffs.
msg163933 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 10:42
Fourth time's the charm.
msg163934 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 10:43
What's better than regenerating a bunch of diffs on the off chance that it'll make Rietveld happy?  Nothing, that's what.
msg163937 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-06-25 11:16
Let's keep it to fwalk.
msg163943 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 11:35
Sure, for 3.3 anyway.

I think walk(dir_fd=) should be okay for 3.4.  There's a better implementation anyway, where walk() just calls fwalk() and strips the last element off the yielded stuff.  But moving on!
msg163949 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-06-25 11:49
New changeset 7bebd9870c75 by Larry Hastings in branch 'default':
Issue #15177: Added dir_fd parameter to os.fwalk().
msg163950 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 11:50
While I was at it, I changed the subject to accurately reflect the changeset's final disposition.
Date User Action Args
2012-06-25 11:50:17larrysetstatus: open -> closed
title: Support os.walk(dir_fd=) and os.fwalk(dir_fd=) -> Support os.fwalk(dir_fd=)
messages: + msg163950

resolution: fixed
stage: needs patch -> resolved
2012-06-25 11:49:14python-devsetnosy: + python-dev
messages: + msg163949
2012-06-25 11:35:37larrysetmessages: + msg163943
2012-06-25 11:16:40georg.brandlsetmessages: + msg163937
2012-06-25 11:13:51larrysetfiles: - larry.fwalk.and.walk.dir_fd.2.diff
2012-06-25 11:13:44larrysetfiles: - larry.fwalk.dir_fd.3.diff
2012-06-25 11:13:39larrysetfiles: - larry.fwalk.and.walk.dir_fd.1.diff
2012-06-25 11:13:32larrysetfiles: - larry.fwalk.dir_fd.2.diff
2012-06-25 10:43:59larrysetfiles: + larry.fwalk.and.walk.dir_fd.3.diff

messages: + msg163934
2012-06-25 10:42:36larrysetfiles: + larry.fwalk.dir_fd.4.diff

messages: + msg163933
2012-06-25 10:33:03serhiy.storchakasetmessages: + msg163931
2012-06-25 10:31:28larrysetfiles: + larry.fwalk.and.walk.dir_fd.2.diff

messages: + msg163929
2012-06-25 10:30:39larrysetfiles: + larry.fwalk.dir_fd.3.diff

messages: + msg163927
2012-06-25 10:10:38serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg163922
2012-06-25 09:45:39larrysetfiles: + larry.fwalk.and.walk.dir_fd.1.diff

messages: + msg163920
2012-06-25 09:38:56larrysetfiles: + larry.fwalk.dir_fd.2.diff

messages: + msg163918
2012-06-25 09:32:25pitrousetnosy: + neologix

versions: + Python 3.4
2012-06-25 08:12:36georg.brandlsetfiles: - larry.listdir.clarification.1.diff
2012-06-25 07:49:20larrysetfiles: + larry.fwalk.dir_fd.1.diff

messages: + msg163909
2012-06-25 07:08:30larrysetstage: patch review -> needs patch
2012-06-25 07:08:21larrysetmessages: + msg163907
2012-06-25 07:07:58larrysetfiles: + larry.listdir.clarification.1.diff
keywords: + patch
messages: + msg163906

stage: needs patch -> patch review
2012-06-25 04:17:11Arfreversetnosy: + Arfrever
2012-06-25 04:13:21larrycreate