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

Support Sync trait for rpm::Error #191

Closed
marxin opened this issue Oct 29, 2023 · 11 comments
Closed

Support Sync trait for rpm::Error #191

marxin opened this issue Oct 29, 2023 · 11 comments

Comments

@marxin
Copy link
Contributor

marxin commented Oct 29, 2023

I would like to use a return type equal to anyhow::Result<RPMFile> for a multi-threaded code and I face the following issue:

        let header = rpm::PackageMetadata::parse(&mut buf_reader).context("RPM parsing failed")?;
error[E0599]: the method `context` exists for enum `Result<PackageMetadata, Error>`, but its trait bounds were not satisfied
   --> src/main.rs:140:67
    |
140 |         let header = rpm::PackageMetadata::parse(&mut buf_reader).context("RPM parsing failed")?;
    |                                                                   ^^^^^^^ method cannot be called on `Result<PackageMetadata, Error>` due to unsatisfied trait bounds
    |
   ::: /home/marxin/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/result.rs:502:1
    |
502 | pub enum Result<T, E> {
    | --------------------- doesn't satisfy `_: Context<PackageMetadata, Error>`
    |
   ::: /home/marxin/Programming/rpm-rs/src/errors.rs:9:1
    |
9   | pub enum Error {
    | --------------
    | |
    | doesn't satisfy `rpm::Error: Sync`
    | doesn't satisfy `rpm::Error: anyhow::context::ext::StdError`
    | doesn't satisfy `rpm::Error: std::marker::Send`
    |
    = note: the following trait bounds were not satisfied:
            `rpm::Error: anyhow::context::ext::StdError`
            which is required by `Result<PackageMetadata, rpm::Error>: anyhow::Context<PackageMetadata, rpm::Error>`
            `rpm::Error: std::marker::Send`
            which is required by `Result<PackageMetadata, rpm::Error>: anyhow::Context<PackageMetadata, rpm::Error>`
            `rpm::Error: Sync`
            which is required by `Result<PackageMetadata, rpm::Error>: anyhow::Context<PackageMetadata, rpm::Error>`
@marxin
Copy link
Contributor Author

marxin commented Nov 7, 2023

@dralley Is there a straightforward way we can take to implement this?

@dralley
Copy link
Collaborator

dralley commented Nov 7, 2023

Dunno, you could try annotating the error type with Send and Sync and seeing which variants the compiler complains about.

@marxin
Copy link
Contributor Author

marxin commented Nov 8, 2023

One can annotate neither Sync nor Send, however, I noticed the problem goes away if I remove usage of all the Box<dyn std::error::Error> members in the error enum variants. Can't we just replace all the inner errors with String fields that will be constructed with inner.to_string() ?
To be honest, I'm just learning Rust, but that's definitely something that makes the library more usable with anyhow and multi-threaded execution.

@marxin
Copy link
Contributor Author

marxin commented Nov 8, 2023

Which might not be possible due to:

error[E0599]: the method `as_dyn_error` exists for reference `&String`, but its trait bounds were not satisfied
   --> src/errors.rs:69:9

which is caused by fact that the GPG errors use dyn Error.

@dralley
Copy link
Collaborator

dralley commented Nov 8, 2023

Apologies, I won't be able to look at this much until the weekend (really busy at the moment) but I'm definitely interested in making it easier to use.

@cmeister2
Copy link
Collaborator

You can do it like this if you like: e61be9b

@marxin
Copy link
Contributor Author

marxin commented Nov 9, 2023

You can do it like this if you like: e61be9b

Great, it works for me! Thank you and can you please make a pull request with the changes?

@cmeister2
Copy link
Collaborator

cmeister2 commented Nov 9, 2023 via email

udoprog added a commit to udoprog/rpm that referenced this issue Dec 3, 2023
udoprog added a commit to udoprog/rpm that referenced this issue Dec 3, 2023
@dralley
Copy link
Collaborator

dralley commented Dec 5, 2023

@cmeister2 @marxin Are you OK with the solution in #198 ?

@cmeister2
Copy link
Collaborator

@cmeister2 @marxin Are you OK with the solution in #198 ?

Yes, it looks very elegant.

@marxin
Copy link
Contributor Author

marxin commented Dec 5, 2023

@cmeister2 @marxin Are you OK with the solution in #198 ?

Yep, I like it much.

@dralley dralley closed this as completed in 9d76461 Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants