-
Notifications
You must be signed in to change notification settings - Fork 28
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
Source on main is at version 3, crates.io is still at version 2.0.2? #62
Comments
Thing is v2 still uses zbus just the sync api which spawns a single thread tokio executor to call the async api. Even worst it pulls in by default both tokio and async-io runtimes... I just wish zbus would have decoupled the transport from the dbus implementation (similar to what diesel did with diesel-async). Right now even if you dont want to impose an async runtime @brotskydotcom you kinda do because it will use tokio under the hood. Though I agree that keyring should remain sync. One could really wish for an STD runtime that would just block on async code 😑 |
Wow, thanks @Sytten for laying that out for me. As you may recall, I don't spend a lot of time in Linux-land so this is really helpful education for me. Does this mean that the current keyring-rs release is also causing its clients to pull in asynchronous runtimes ("under the hood", as we say in the US)? So whether I use v2 or v3 of secret service in the next release of keyring makes no difference in that respect? |
I am not super versed in Linux either but glad I can help with the async /sync stuff. Exactly it pulls in async runtimes under the hood (zbus does not this crate) since its the only way to run async from sync code. I believe that v3 is fully async so would have to do this yourself if you want to keep a async api for keyring. |
@complexspaces Are you open to a contribution (under feature flag, I would expect) that uses dbus rather than zbus? (I seem to remember you suggested as much in some other thread.) |
Unsure if it is a good idea re-reading the code since literally all the api is async now so that would basically mean forking from v2 and writing another lib... I guess you could use the futures crate single threaded executor (https://docs.rs/futures/latest/futures/executor/fn.block_on.html) but it isn't working super well for people calling from tokio I think (https://stackoverflow.com/questions/66035290/how-do-i-await-a-future-inside-a-non-async-method-which-was-called-from-an-async). |
I think a possible course of action is:
That allows downstream users of I started the dbus/libdbus vendoring work in a branch here. Just to see if it's viable. Edit, draft PR here: diwic/dbus-rs#408 |
Hi everyone, sorry for the delay. Let me try and answer everyone's questions:
As I think was discovered above, no, there are not currently a set of feature flags that will let downstream users use the If there are no breaking changes required/planned for the proposal to add another backend, then my plan is to release 3.0, merge some of the cryptography updates (at work, we're stuck on some older versions and I would prefer to have a 3.0 release for use internally that doesn't duplicate a bunch of dependencies) and then release 3.1 with those. I am currently out of town until January for vacation, so I won't be able to do much more then comment on GitHub until then sorry. But once I'm back, a 3.0 release would take 1-2 days at most.
Yes, I am still open to contributions adding this. Given that the crate now has a If possible, it would be nice to give users of the |
Hi all, Thanks so much for the education and context. I think I have a pretty good picture of the situation right now, and here's what I'm planning. All feedback welcome!
This plan seems to me to balance backward-compatibility for keyring-rs (for async-naive clients) with sufficient controls over zbus behavior (for async-aware clients) with sufficient controls over dependencies (for embedded or otherwise zbus-averse clients). And it leaves the door open for an API-compatible release of keyring-rs if anyone ever moves forward with a dbus-based secret-service. Your thoughts? |
After merging diwic/dbus-rs#408, I began trying to get a start on updating the It's looking like a larger lift than I originally anticipated. Since most of the core library is tightly coupled with Ideally the [features]
# Default to using rust crypto backend, force user to choose a Mode
default = ["crypto-rust"]
# Cryptography backends
crypto-rust = ["dep:aes", "dep:block-modes", "dep:sha2", "dep:hkdf"]
crypto-openssl = ["dep:openssl"]
# Modes (Blocking or specifying a runtime)
# The async runtime features mirror those of `zbus` for compatibility.
blocking = ["dep:dbus"]
rt-async-io = ["zbus/async-io"]
rt-tokio = ["zbus/tokio"]
[dependencies]
# Crypto: Rust backend
# TODO: Update these when Rust 1.56 isn't so new.
aes = { version = "0.7.0", optional = true }
block-modes = { version = "0.8.0", optional = true }
hkdf = { version = "0.12.0", optional = true }
sha2 = { version = "0.10.0", optional = true }
# Crypto: Openssl backend
openssl = { version = "^0.10.40", optional = true }
# Mode: Blocking/sync backend
dbus = { git = "https://github.com/diwic/dbus-rs.git", features = ["vendored"], optional = true}
# Mode: Async backend (zbus will pull in the relevant run time)
zbus = { version = "3", default-features = false, optional = true}
... And then the top-level /// Error when a runtime has not been selected
#[cfg(not(any(
feature = "rt-async-io",
feature = "rt-tokio",
feature = "blocking",
)))]
mod error_message {
compile_error!(
"A Mode must be selected, either \"blocking\", \
\"rt-async-io\" or \"rt-tokio\" must be enabled."
);
}
/// Error when a crypto backend has not been selected
#[cfg(not(any(
feature = "crypto-rust",
feature = "crypto-openssl",
)))]
mod error_message {
compile_error!(
"A crypto backend must be selected, either \
\"crypto-rust\" or \"openssl\" must be enabled."
);
}
// Generic modules
mod error;
/// Async mode
#[cfg(any(feature = "rt-async-io", feature = "rt-tokio"))]
pub mod runtime;
/// Sync mode
#[cfg(feature = "blocking")]
pub mod blocking; So I think that your point number 4 may run into issues, since I anticipate potential breaking changes to occur within And a lot of the core code, like the Proxy/ |
Actually it may not affect users of keyring aside from removing the runtime features you're exposing. When/if the swap happens under the hood in So the breaking change would be exposing the runtime features now and later removing them. Your point 2 is definitely a plus for people trying to avoid a runtime entirely. |
Thanks @landhb for the feedback on number 4. I am planning to make introducing a new credential store into keyring-rs fully backwards compatible (with respect to the client API) and thus something that can be done in a dot release. Assuming I succeed, then: if the secret-service blocking API were to change a lot with the move to dbus, or if we needed to split dbus support into a separate sub-module completely, I could handle it in keyring-rs simply by introducing a new credential store that used the new API. What this does make me realize, however, is that I probably should make the error enum returned by keyring be explicitly non-exhaustive, so that new credential stores can introduce new error types without breaking the clients. |
I have published keyring-rs v2 alpha 4 to GitHub so folks can try it out. It can't be published to crates.io until after secret-service v3 is published. |
Hi @complexspaces, Happy New Year! Just checking in on this. It would be great to have a 3.0 release; that will allow me to release keyring 2.0. If you think it will be a while before you do the 3.0 release, that's fine! Just let me know and I will release keyring 2.0 against secret-service 2.0.2 and then update it in a dot once 3.0 comes out. |
I've reverted back to secret service 2.0.2 so I can release keyring-rs v2. I just pushed beta.1 to crates.io. I know exactly what's needed to move back to secret service 3.0 so I can do a dot whenever that happens. |
Hey @brotskydotcom, I'm really sorry for the silence. This month has been far rougher then I expected. I've had to slow down on many non-primary obligations. Given some of the research done above (thank you everyone), I think that we should be good to cut v3 and keep it for a while. The I will do my best to have v3 out by the weekend. Please speak up in the next day or so if you have any issues with the current API. |
Hi @complexspaces, wonderful news and thanks for all your effort! If you can do a release in the next week I will happily use it in keyring 2.0. |
I've now published 3.0 on crates.io. Please let me know if anything about it ended up not working (hopefully nothing! 😅). Regarding the |
Thanks much @complexspaces for doing this release! I will move keyring-rs onto it and release this week. |
@complexspaces I'm finally getting around to the next release of the keyring-rs crate, and I'm trying to figure out which released version of secret-service it should use. I am going to stay with dbus (rather than zbus) because I don't want to force my clients to use an async runtime, so in that sense I could stay with 2.0.2, but then again I'd like to include the unlock improvements of a269ee7 by @jkhsjdhjs, which are only in 3.0. Are you going to release 3.0 to crates.io anytime soon? And will I be able to use feature flags to stay with the dbus interface in 3.0?
cc @Sytten, @landhb
The text was updated successfully, but these errors were encountered: