Use dbus-addr crate #562
Use dbus-addr crate #562
Conversation
Its usage is gone.
error[E0423]: expected value, found built-in attribute `path`
Simplify a bit the code.
As a target may have multiple addresses in the following changes, we should return the actual associated guid.
Offload the D-Bus address parsing task to a dedicated crate. zbus code transition in the following changes.
Compared to the existing implementation, this will take the family into account (except with tokio), as well as multiple host resolution.
Parsing can be done with legacy Address and DBusAddr
So I reject your work for an alternative impl and you think putting it in a separate crate under your maintenance would convince me to be more open to those changes? 😆 This is a pretty big change so close to a major release that's been in the making for months. Even if I'd be fine with the API of the new crate (I haven't looked but as you said it's based on your work and I didn't agree with your approaches there), I think we need to postpone this to 5.0.
What does it really achieve? We can put many things in separate crate but it's only useful if people need to parse dbus addresses separately from zbus but I seriously doubt there are sufficient use cases of that. Typically people don't even directly use our Besides we already considered use of an external crate for address handling but in the end, I decided (w/o objections from you) that it's not worth it. I think the rationale to close that issue still applies here.
I'll have to look closer to know what that implies and if I agree with it but neither of the advantages you specified requires splitting into a separate crate.
Exactly and hence no compelling reason to split. P.S. I see some fixes that are independent of this PR here. Please split them into another PR so they can already go in. |
|
I didn't remember someone suggested we use an existing crate. There are many differences between dbus-addr and dbus-server-address-parser. dbus-addr is rooted in zbus, and uses Cow everywhere it possibly can, and parses things strictly. Your Address changes are also very recent (more recent than mine!), so this argument doesn't hold much. Imho splitting code in separate crate is a good practice, even if there is only a single user so far (the crate was released yesterday...) |
Those changes are based on what we discussed and agreed many months ago (although they don't include all the changes we agreed on). Also, I was doing almost all the work for 4.0 so they were not disruptive to my planning. Also, I didn't introduce a completely new crate at the last minute.
Let's be a bit specific. Is it useful for anyone not using zbus, in practice (i can see this being true in theory)? If not, I don't see why we need to do this now. We can do it if/when the use case presents itself. To be very honest, this split seems like your attempt to workaround my objections/concerns to your PR. That is not a good rationale for me to accept this, sorry. |
|
"completely new" is not correct, it's rooted on zbus code, the same tests are used (with a few more tests). The fact that there are already crates parsing dbus addresses shows that there is interest in having independent crate for it. And again, if code can be split in a separate crate, it is generally a good idea to do it. Many rust crates are neatly divided, even if they belong and are used only by to a top project. I recall your main objection was that you didn't want to break API, but now you did. |
We can't assume that. Are those crates being actually used by other dbus crates to be useful? Please point to specific use cases this will satisfy.
That was not my objection at all. I in fact told you the exact opposite. :) Before #430, we agreed on Address becoming a struct and that is a clear API-break. So I don't know how you got this very wrong impression. Please see the logs and my comments your old PR again. Bottom line, I'm not objecting to split per say but:
|
Address parsing without connection? That would be for example configuration file handling. It could also be testing tools that just want to connect to a dbus server without the need for dbus implementations (fuzzers etc). Or simple proxy code for ex.
"split of our own API" ? what does this mean?
The main compelling reasons are:
|
Can you show me one example in any language of this? If you're going to connect to dbus, why would you not just use the full D-Bus API? You continue to just come up with random theoretical possibilities as the rationale, Please be specific and give practical examples.
A simple (mostly) verbatim move. This will allow us to be able to do that even in a stable release, using re-exports. So we finalize the API in zbus first and then split it.
Given that you addressed this in your PR, you know that this is not relevant to splitting so why present it as an argument for that?
My question was, what's the compelling reason to do this now?
Both are not relevant to splitting. The same could be achieved w/o the split. |
|
I spent enough time on this already and you didn't succeed in convincing me that this is something we need to do right now. I'm closing this with the bottom line above. Sorry! I'll also now unsubscribe from this issue. Feel free to provide PRs for improvements to the Address API for 4.0 (I will hold it off till Sun for you) and create PRs for address split after 4.0. Oh and I would want to be a co-maintainer of such a split and it should remain in zbus umbrella and repo (i-e |
Those are not random, they are practical. Haven't you ever seen address parsing or url or email without actually implementing a browser or a mail client. You are not considering seriously my argument. And even then, I don't agree with you that it needs different users to have code split in a different crate.
It's not a move, I wrote the code from scratch before your implementation. I am not going to justify every choice in implementation I made, (you didn't have to justify yours btw... since nobody reviewed)
Because dbus-addr handles it with this PR, and this is compelling to be compliant with dbus address handling.
Actually, I proposed to do it in the original PR. And you pressed me to release my code somewhere to not just have to trash it...
Well it's done in dbus-addr and with this PR. But you don't want to consider it seriously. |
I published https://crates.io/crates/dbus-addr based on my work from #430.
zbus can rely on it instead of providing its own Address implementation.
This allows to cleanup some code (mixing code parsing and stream connection etc)..
The other advantage of this series is the support for ;-separated address.
This change is mostly unnoticeable from a typical zbus API user, who rely on session() most of the time, or DBus addresses as string.