librpm.rs icon indicating copy to clipboard operation
librpm.rs copied to clipboard

When not configured, no Rust Error happens and configuration is not enforced

Open mkpankov opened this issue 5 years ago • 1 comments

fn main() {
    use librpm::Index;

    // commented out intentionally
    // librpm::config::read_file(None).unwrap();

    let mut matches = Index::Name.find("rpm-devel");
    let package = matches.next();
    println!("{:?}", if package.is_some() {
        true
    } else {
        false
    });
}

[root@6bf893578ee6 work]# cargo run --target-dir docker-target
   Compiling rpmp v0.1.0 (/work)
    Finished dev [unoptimized + debuginfo] target(s) in 0.40s
     Running `docker-target/debug/rpmp`
error: no dbpath has been set
error: cannot open Packages database in 
error: no dbpath has been set
error: cannot open Packages database in 
false

Reproduced on a branch in repository.

This is poor design for 2 reasons:

  1. Error is impossible to handle in the program. There are just some messages emitted to stderr. And you can't discriminate between an error or package not installed.
  2. This is not idiomatic Rust as it doesn't enforce configuration. The way it should be is, reading config produces a type which has all the DB methods defined. DB methods shouldn't be defined as free functions in isolation.

mkpankov avatar Nov 20 '19 08:11 mkpankov

While I agree with everything you're saying, note that the error messages, API, and inability to handle the misconfiguration are all artifacts of the librpm API itself and don't have anything to do with the strategy chosen by this particular crate.

Configuration is managed here, but note that librpm stores its own configuration using global singletons, and is extremely inflexible about how it's loaded:

https://github.com/RustRPM/librpm.rs/blob/master/src/config.rs

If you have a concrete suggestion for a better way to encapsulate this, then by all means please propose it.

tarcieri avatar Nov 20 '19 15:11 tarcieri