Skip to content
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

To disk, or not to disk #48

Closed
hinto-janai opened this issue Dec 19, 2023 · 4 comments
Closed

To disk, or not to disk #48

hinto-janai opened this issue Dec 19, 2023 · 4 comments
Labels
C-discussion General discussion or questions.

Comments

@hinto-janai
Copy link
Contributor

What

How will Cuprate associate (de)serializable data with locations on disk?

If #46 is the "where", then this is the "how".

Concrete example: cuprate wants to read/write cuprate.toml to/from disk, how do we obtain that PATH?

Options, from least-invasive to most-invasive:

  • Ad-hoc PATH creation where needed, Monero-style
  • dirs combined with const PATH end nodes (dirs::config_dir() += "cuprate.toml"), maybe saved into a OnceLock for easy global referencing
  • disk

To disk...

The library disk allows for associations of data types (struct, enum) with locations on disk (~/.config/cuprate/cuprate.toml).

It has (de)serialization as well, so a struct can be directly created/saved from disk with:

// create a `Config` instance from disk.
let config: Config = Config::from_file()?;

// write it to disk.
config.save()?;

This works because it "knows" where to look, because we define it:

// This maps to:
//
// | OS      | PATH                                                            |
// |---------|-----------------------------------------------------------------|
// | Windows | `C:\Users\Alice\AppData\Roaming\Cuprate\cuprate.toml`           |
// | macOS   | `/Users/Alice/Library/Application Support/Cuprate/cuprate.toml` |
// | Linux   | `/home/alice/.config/cuprate/cuprate.toml`                      |
//
//  format  struct  OS location  project folder    sub-dirs  file name (.toml is implied)
//    v     v       v                   v          v         v     
disk::toml!(Config, disk::Dir::Config, "Cuprate", "", "cuprate");
#[derive(Serialize,Deserialize)]
struct Config { /* ... */ }

This library "works" but is rough around the edges as seen with the macro_rules!() instead of a proper #[derive()].

The main question is if this path should even be taken, as opposed to (de)serializing ourselves and using std::fs::write(path, &data) to every file we'd like to make on a per-case basis (possibly more reasonable if we only need to manage a few). Off the top of my head, there is:

  • Config file
  • Crash file
  • Documentation location
  • Cache files
  • Log files
  • Certificate/key files
  • Database
  • Misc state, e.g p2pstate.bin

The benefit of this library is:

  • Everything is defined right on-top of the data in a single declarative line
  • Many file formats supported (bincode, toml, json, etc)
  • (De)serialization is abstracted, no serde_json or toml_edit, just save() and from_file()
  • Bad input safety guards (compile-time error on bad input, panic!() on dangerous operations at runtime)
  • OS differences are abstracted away
  • Many other nice to haves

Making disk "production-ready" would take some time.

...or not to disk

Immediate counter-argument:

  • This is an overly complicated abstraction (aka, way too much "magic")

disk is a macro-heavy mess internally, and will be more so when it moves to #[proc_macro]. If I were to get hit by a bus, or leave Cuprate, maintenance of disk would be very painful. Compile errors (from editing internals, not user-facing) from one of these macros causes walls of unreadable text.

I don't want Cuprate to be dependent on any sole person, so I'm slightly on the side of just doing all of this manually, i.e:

  • dirs +
  • const PATH +
  • serde_json/toml_edit/whatever_format +
  • std::fs::read/write
  • for each file, at each call-site
@Boog900
Copy link
Member

Boog900 commented Dec 24, 2023

I agree, I think Cuprate should go the manual route.

Also can disk even handle dynamic paths, I couldn't see anyway to do this, if it can't and we went with disk it would mean users wouldn't be able to specify custom paths

@hinto-janai
Copy link
Contributor Author

https://docs.rs/disk/latest/disk/trait.Toml.html#method.from_path

// Uses pre-defined location.
let config: Config = Config::from_file()?;

// Uses arbitrary path.
let config: Config = Config::from_path("/any/path")?;

@SyntheticBird45
Copy link
Member

I agree with @Boog900 we should go manual route since it would give us fine control on when and how we write on disk. No need to add more dependencies for something that don't require so much work

@hinto-janai
Copy link
Contributor Author

Going the manual route.

I will find an opportunity to re-write disk someday...

@hinto-janai hinto-janai added the C-discussion General discussion or questions. label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion General discussion or questions.
Projects
None yet
Development

No branches or pull requests

3 participants