Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
GH-89727: Fix os.fwalk() recursion error on deep trees#119638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
barneygale commented May 28, 2024 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
Implement `os.fwalk()` using a list as a stack to avoid emitting recursion errors on deeply nested trees.
barneygale commented May 28, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
#119473 fixes the test failure. (edited 30 may: I've worked around this for now.) |
Uh oh!
There was an error while loading. Please reload this page.
barneygale commented May 30, 2024
Ready for review! I fixed the test failure mentioned above by reversing the |
carljm left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I won't claim I did a line-by-line verification of equivalence with the current code, but I walked through the new code and it looks sensible, and the existing test coverage looks reasonable.
Did you consider adding a test that sets a very low recursion limit and verifies we don't hit it? I don't feel strongly about this.
Uh oh!
There was an error while loading. Please reload this page.
barneygale commented May 30, 2024
I've enabled |
carljm commented May 30, 2024
No, that looks fine. Sorry, somehow I just totally missed that one-line removal when focused on the "added lines" side of the diff! |
barneygale commented May 30, 2024
No worries, thanks tons for the speedy review :) |
Thanks @barneygale for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
…nGH-119638) Implement `os.fwalk()` using a list as a stack to avoid emitting recursion errors on deeply nested trees. (cherry picked from commit 3c890b5) Co-authored-by: Barney Gale <barney.gale@gmail.com>
…nGH-119638) Implement `os.fwalk()` using a list as a stack to avoid emitting recursion errors on deeply nested trees. (cherry picked from commit 3c890b5) Co-authored-by: Barney Gale <barney.gale@gmail.com>
GH-119764 is a backport of this pull request to the 3.13 branch. |
GH-119765 is a backport of this pull request to the 3.12 branch. |
bedevere-bot commented May 30, 2024
|
…n#119638) Implement `os.fwalk()` using a list as a stack to avoid emitting recursion errors on deeply nested trees.
…n#119638) Implement `os.fwalk()` using a list as a stack to avoid emitting recursion errors on deeply nested trees.
Implement
os.fwalk()using a list as a stack to avoid emitting recursion errors on deeply nested trees.