Implement create_dir_all() to operate iteratively instead of recursively #148196
Conversation
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
Why does this need to manipulate a list of |
|
🥴 Answer to jubilee: It's not a reversible iterator!!! |
|
jubilee's counterargument to jubilee's counterargument: Okay, so we'd have to use |
|
Please feel free to question/reject my comment/suggestion if you feel it is incorrect/scope-creeping, I only am bringing it up because I had precisely this thought after wondering why code I had earlier written elsewhere was doing |
|
Followup response to jubilee: what about in the tune of "Khaaan!": RELATIVE PAAATHS! |
|
Hey jubilee, I took your suggestion and iterate through the Ancestors Iterator. One thing that I did differently however is that instead of using Does this approach look good to you? |
|
Yes, allocating once instead of many times sounds like a reasonable improvement in the spirit of my note, so if others are cool with it, then I'm cool! That way it's "just" a classic space vs. time tradeoff and getting the main improvement we're looking for. |
|
Hi @tgross35, I noticed that you were the assignee for this pull request. I wanted to ask if there is anything I should change with this PR/commits or if everything looks alright? |
|
I haven't looked at this yet so I'm going to who has. (I know it's at your limit so feel free to reroll again or throw it back if you want) |
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
r? libs |
rustbot
removed
the
S-waiting-on-review
bors
added
S-waiting-on-bors
…dir-all, r=Mark-Simulacrum Implement create_dir_all() to operate iteratively instead of recursively The current implementation of `create_dir_all(...)` in std::fs operates recursively. As mentioned in rust-lang#124309, this could run into a stack overflow with big paths. To avoid this stack overflow issue, this PR implements the method in an iterative manner, preserving the documented behavior of: ``` Recursively create a directory and all of its parent components if they are missing. This function is not atomic. If it returns an error, any parent components it was able to create will remain. If the empty path is passed to this function, it always succeeds without creating any directories. ```
Rollup of 7 pull requests Successful merges: - #146794 (std: reorganize pipe implementations) - #148196 (Implement create_dir_all() to operate iteratively instead of recursively) - #148490 (dangling pointer from temp cleanup) - #149864 (std: Don't use `linkat` on the `wasm32-wasi*` targets) - #149885 (replace addr_of_mut with &raw mut in maybeuninit docs) - #149949 (Cleanup of attribute parsing errors) - #149969 (don't use no_main and no_core to test IBT) Failed merges: - #148847 (Share Timespec between Unix and Hermit) r? `@ghost` `@rustbot` modify labels: rollup
bors
added
S-waiting-on-author
|
@Mark-Simulacrum I didn't know that I updated the code to add a check on whether the path given or path seen in ancestor is root via checking if its parent is None, so that it either returns |
|
r? libs |
|
@bors r=Mark-Simulacrum,jhpratt |
rust-bors
bot
added
S-waiting-on-bors
…=Mark-Simulacrum,jhpratt Implement create_dir_all() to operate iteratively instead of recursively The current implementation of `create_dir_all(...)` in std::fs operates recursively. As mentioned in rust-lang#124309, this could run into a stack overflow with big paths. To avoid this stack overflow issue, this PR implements the method in an iterative manner, preserving the documented behavior of: ``` Recursively create a directory and all of its parent components if they are missing. This function is not atomic. If it returns an error, any parent components it was able to create will remain. If the empty path is passed to this function, it always succeeds without creating any directories. ```
…=Mark-Simulacrum,jhpratt Implement create_dir_all() to operate iteratively instead of recursively The current implementation of `create_dir_all(...)` in std::fs operates recursively. As mentioned in rust-lang#124309, this could run into a stack overflow with big paths. To avoid this stack overflow issue, this PR implements the method in an iterative manner, preserving the documented behavior of: ``` Recursively create a directory and all of its parent components if they are missing. This function is not atomic. If it returns an error, any parent components it was able to create will remain. If the empty path is passed to this function, it always succeeds without creating any directories. ```
…=Mark-Simulacrum,jhpratt Implement create_dir_all() to operate iteratively instead of recursively The current implementation of `create_dir_all(...)` in std::fs operates recursively. As mentioned in rust-lang#124309, this could run into a stack overflow with big paths. To avoid this stack overflow issue, this PR implements the method in an iterative manner, preserving the documented behavior of: ``` Recursively create a directory and all of its parent components if they are missing. This function is not atomic. If it returns an error, any parent components it was able to create will remain. If the empty path is passed to this function, it always succeeds without creating any directories. ```
Rollup of 11 pull requests Successful merges: - #148196 (Implement create_dir_all() to operate iteratively instead of recursively) - #150494 ( Fix dso_local for external statics with linkage) - #150788 (THIR patterns: Replace `AscribeUserType` and `ExpandedConstant` wrappers with per-node data) - #150799 (Fix ICE: can't type-check body of DefId for issue #148729) - #150804 (Remove std_detect_file_io and std_detect_dlsym_getauxval features) - #150852 (std: sys: fs: uefi: Implement File::write) - #150871 (Use f64 NaN in documentation instead of sqrt(-1.0)) - #150878 (Emit an error for linking staticlibs on BPF) - #150911 (Add missing documentation for globs feature) - #150913 (compiler: Forward attributes to eii-expanded macros) - #150916 (Once again, reorganize the EII tests a bit) r? @ghost
Rollup of 12 pull requests Successful merges: - #150947 (alloctests: Don't run the longer partial-sort tests under Miri) - #148196 (Implement create_dir_all() to operate iteratively instead of recursively) - #150494 ( Fix dso_local for external statics with linkage) - #150788 (THIR patterns: Replace `AscribeUserType` and `ExpandedConstant` wrappers with per-node data) - #150799 (Fix ICE: can't type-check body of DefId for issue #148729) - #150804 (Remove std_detect_file_io and std_detect_dlsym_getauxval features) - #150852 (std: sys: fs: uefi: Implement File::write) - #150871 (Use f64 NaN in documentation instead of sqrt(-1.0)) - #150878 (Emit an error for linking staticlibs on BPF) - #150911 (Add missing documentation for globs feature) - #150913 (compiler: Forward attributes to eii-expanded macros) - #150916 (Once again, reorganize the EII tests a bit) r? @ghost
Rollup merge of #148196 - std-fs-iterative-create-dir-all, r=Mark-Simulacrum,jhpratt Implement create_dir_all() to operate iteratively instead of recursively The current implementation of `create_dir_all(...)` in std::fs operates recursively. As mentioned in #124309, this could run into a stack overflow with big paths. To avoid this stack overflow issue, this PR implements the method in an iterative manner, preserving the documented behavior of: ``` Recursively create a directory and all of its parent components if they are missing. This function is not atomic. If it returns an error, any parent components it was able to create will remain. If the empty path is passed to this function, it always succeeds without creating any directories. ```
11 participants
The current implementation of
create_dir_all(...)in std::fs operates recursively. As mentioned in #124309, this could run into a stack overflow with big paths. To avoid this stack overflow issue, this PR implements the method in an iterative manner, preserving the documented behavior of: