chmln / handlr

A better xdg-utils
MIT License
608 stars 24 forks source link

Add atomic file saving #44

Open nyanpasu64 opened 3 years ago

nyanpasu64 commented 3 years ago

Fixes #43.

I wrote my own atomic file saving module (common::atomic_save). It was adapted from the atomicwrites crate, but with an added flag to disable directory fsync (atomicwrites always performs directory fsync). I chose to not fsync the directory after writing the file, because it's unnecessary to prevent corruption and hurts performance (to some degree I don't know). Note that my code depends on nix and does not compile on Windows (though I assume handlr is only meant for Unix platforms?).

Both atomicwrites and my fork create a temporary directory within the folder (~/.config/) to write the temporary file to. I'm not sure why this is done instead of creating a temporary file directly in ~/.config/, but I didn't change it.

Note that atomicwrites doesn't use renameat2. There's an open bug report at https://github.com/untitaker/rust-atomicwrites/issues/28, where some people have suggested that renameat2 is better suited for atomic saving. This is suboptimal and I haven't tried fixing it, but I don't think it's an issue for atomically saving mimeapps.list.

Unfortunately this branch has "unused enum case" warnings because I moved atomicwrites from a crate to a module, and didn't use all enum cases in this program. The warnings would not be present if the code was located in a library crate. I'm not clear on the exact details of what causes the warning to happen or not, but there's some discussion on Reddit /r/rust.

Maybe this code should be upstreamed to atomicwrites. However it's an API-breaking change (requiring a new semver version) which they may or may not accept, so even if I did, you'd have to wait for atomicwrites to be updated.