Attempt to improve the std::fs::create_dir_all docs related to atomicity
#141472
Conversation
|
rustbot has assigned @workingjubilee. Use |
rustbot
added
S-waiting-on-review
| /// If this function returns an error, some of the parent components might have | ||
| /// been created already. |
There was a problem hiding this comment.
The old and the new paragraph are about completely different things. The old one is still correct, if an error is encountered it may still have created some of the intermediate directories, i.e. it's not atomic.
E.g. some intermediate path component contains an invalid character or the filesystem becomes full just in the middle of the operation or something like that.
There was a problem hiding this comment.
Ok, I thought it says that an error can be returned if some of the parent directories already exist. Perhaps it can be reworded better? For example,
This function is not atomic. In the event of an error, the intermediate parent components that were created will persist.
There was a problem hiding this comment.
I don't think that's an improvement either.
|
"fix xyz" is often not a useful PR subject as it does not actually specify what you are fixing. there are 40 lines in the docs in question. you are only touching two. |
fluiderson
changed the title
std::fs::create_dir_all docsstd::fs::create_dir_all docs related to atomicity
|
I've updated the title, it should reflect the changes more precisely now. As for my proposal to make the paragraph more comprehensive, I think I'm not alone on misinterpreting it. The current text is a bit ambiguous and might be taken as "if this function returns an error, already created directories might be a cause of that error". |
|
Then specify that the function may create some directories even if it also errors. |
|
a bit better. squash commits please? |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Attempt to improve the `std::fs::create_dir_all` docs related to atomicity The original paragraph was added in rust-lang#124520. It doesn't match the actual code logic. It says "function returns an error" if "the parent components" _(which also implies directories)_ "have been created already". The code is as follows: https://github.com/rust-lang/rust/blob/e88e85463468ce5d5ce468414eb69e3b15fa8d42/library/std/src/fs.rs#L3146 https://github.com/rust-lang/rust/blob/e88e85463468ce5d5ce468414eb69e3b15fa8d42/library/std/src/fs.rs#L3160 These lines suppress all errors if any path component is a directory. I've updated the paragraph to mirror this.
Rollup of 9 pull requests Successful merges: - #134696 (Implement `normalize_lexically`) - #138744 (Add methods to TCP and UDP sockets to modify hop limit (refresh of #94678)) - #140539 (Simplify `attribute_groups`) - #140863 ([rustdoc] Unify type aliases rendering with other ADT) - #140936 (Clarify WTF-8 safety docs) - #140952 (Specify that split_ascii_whitespace uses the same definition as is_ascii_whitespace) - #141472 (Attempt to improve the `std::fs::create_dir_all` docs related to atomicity) - #141502 (ci: move PR job x86_64-gnu-tools to codebuild) - #141559 (const-check: stop recommending the use of rustc_allow_const_fn_unstable) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 8 pull requests Successful merges: - #134696 (Implement `normalize_lexically`) - #140539 (Simplify `attribute_groups`) - #140863 ([rustdoc] Unify type aliases rendering with other ADT) - #140936 (Clarify WTF-8 safety docs) - #140952 (Specify that split_ascii_whitespace uses the same definition as is_ascii_whitespace) - #141472 (Attempt to improve the `std::fs::create_dir_all` docs related to atomicity) - #141502 (ci: move PR job x86_64-gnu-tools to codebuild) - #141559 (const-check: stop recommending the use of rustc_allow_const_fn_unstable) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #141472 - fluiderson:dev, r=workingjubilee Attempt to improve the `std::fs::create_dir_all` docs related to atomicity The original paragraph was added in #124520. It doesn't match the actual code logic. It says "function returns an error" if "the parent components" _(which also implies directories)_ "have been created already". The code is as follows: https://github.com/rust-lang/rust/blob/e88e85463468ce5d5ce468414eb69e3b15fa8d42/library/std/src/fs.rs#L3146 https://github.com/rust-lang/rust/blob/e88e85463468ce5d5ce468414eb69e3b15fa8d42/library/std/src/fs.rs#L3160 These lines suppress all errors if any path component is a directory. I've updated the paragraph to mirror this.
Attempt to improve the `std::fs::create_dir_all` docs related to atomicity The original paragraph was added in rust-lang#124520. It doesn't match the actual code logic. It says "function returns an error" if "the parent components" _(which also implies directories)_ "have been created already". The code is as follows: https://github.com/rust-lang/rust/blob/e88e85463468ce5d5ce468414eb69e3b15fa8d42/library/std/src/fs.rs#L3146 https://github.com/rust-lang/rust/blob/e88e85463468ce5d5ce468414eb69e3b15fa8d42/library/std/src/fs.rs#L3160 These lines suppress all errors if any path component is a directory. I've updated the paragraph to mirror this.
The original paragraph was added in #124520. It doesn't match the actual code logic. It says "function returns an error" if "the parent components" (which also implies directories) "have been created already". The code is as follows:
rust/library/std/src/fs.rs
Line 3146 in e88e854
rust/library/std/src/fs.rs
Line 3160 in e88e854
These lines suppress all errors if any path component is a directory. I've updated the paragraph to mirror this.