feat: add atomic_write_file, atomic_write_file_with_retries, and get_atomic_path #6
Conversation
dsherret
changed the title
src/fs.rs
Outdated
| mode: u32, | ||
| ) -> std::io::Result<()> { | ||
| let mut file = sys.fs_open(temp_file_path, &OpenOptions::write())?; | ||
| file.fs_file_set_permissions(mode)?; |
There was a problem hiding this comment.
I copied this from the CLI, but I kind of feel like we should only be setting the file permissions on open (the OpenOptions allow for that). Thoughts?
There was a problem hiding this comment.
I'm not sure I understand what you mean by "setting the file permissions on open". It seems desirable that the file is written with proper permissions.
There was a problem hiding this comment.
@bartlomieju I did a comimt to show it.
| + FsRemoveFile | ||
| + FsRename | ||
| + ThreadSleep | ||
| + SystemRandom, |
There was a problem hiding this comment.
I know this seems slightly out of control, but it makes using this crate much easier. A big problem atm is Rust doesn't have a way to create aliases of traits, so there's no way to just say "deno_path_util uses these traits under the name DenoPathUtilSys, so now accept TSys: DenoPathUtilSys". Yes, we could do pub trait DenoPathUtilSys: FsCreateDirAll + ... {}, but a huge negative to that is then people need to implement the DenoPathUtilSys crate for file system implementation and that might not always be possible if the file system implementation lives in a different crate (they'd have to create an adapter struct, which is a PITA)... so we add some verbosity here for a better experience for consumers.
| use thiserror::Error; | ||
| use url::Url; | ||
|
|
||
| pub mod fs; |
There was a problem hiding this comment.
This should arguably be in a separate crate, but I don't think it's worth the additional maintenance burden.
Moved from the CLI and update to use https://github.com/dsherret/sys_traits