helsing-ai / buffrs

Modern protobuf package management
https://crates.io/crates/buffrs
Apache License 2.0
214 stars 12 forks source link

Unclear error message for #192 #193

Closed qsantos closed 4 months ago

qsantos commented 11 months ago

From #192:

~% git clone git@github..........
~% cd my-project
~/my-project% buffrs install
Error:   × failed to install dependencies for `my-project`
  ╰─▶ No such file or directory (os error 2)

The error message does not help understanding that the issue comes from the missing proto/vendor/ directory.

qsantos commented 10 months ago

The error message comes from:

https://github.com/helsing-ai/buffrs/blob/4a6d0e9084638f40909052aa1894bc20facbc01b/src/package/store.rs#L53

The core::result::Result returned by the Future is converted into a miette::Result but no context message is attached.

Would it make sense to require that any call to into_diagnostic() be followed by a call to wrap_err()? Maybe we could make a lint for that.

qsantos commented 10 months ago

Note, however, that the full function is:

https://github.com/helsing-ai/buffrs/blob/4a6d0e9084638f40909052aa1894bc20facbc01b/src/package/store.rs#L51-L56

This function will only return Ok(false) if the entry exists but is not a directory. In this case, maybe it would make more sense to do:

    /// Check if this store exists
    async fn exists(&self) -> bool {
        fs::metadata(&self.proto_path()).await.is_ok_and(|meta| meta.is_dir())
    }
gbouv commented 4 months ago

This seems to be fixed right? I tried to repro and now buffrs automatically create the proto/vendor directory. Can this issue be closed?