🐛 Expose the socket address clients can use #182
+23
−8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jokeyrhyme
commented
Dec 14, 2024
jokeyrhyme
force-pushed
the
default-addresses-and-printing-oh-my
branch
2 times, most recently
from
654a7f5 to
aa8fa3b
Compare
jokeyrhyme
force-pushed
the
default-addresses-and-printing-oh-my
branch
4 times, most recently
from
f8fd47f to
0e6497a
Compare
jokeyrhyme
force-pushed
the
default-addresses-and-printing-oh-my
branch
2 times, most recently
from
21415a9 to
50d093f
Compare
zeenix
reviewed
Jan 6, 2025
jokeyrhyme
force-pushed
the
default-addresses-and-printing-oh-my
branch
from
50d093f to
77fa221
Compare
zeenix
reviewed
Jan 7, 2025
src/bus/mod.rs
Outdated
| #[cfg(unix)] | ||
| use std::{env, path::Path}; | ||
| use std::{str::FromStr, sync::Arc}; | ||
|
|
||
| use anyhow::{bail, Ok, Result}; |
There was a problem hiding this comment.
also this change is unrelated to the commit. :) I wouldn't have even mentioned it but since you're going to update the commit anyway. :)
jokeyrhyme
force-pushed
the
default-addresses-and-printing-oh-my
branch
from
77fa221 to
4bee358
Compare
zeenix
reviewed
Jan 8, 2025
src/bus/mod.rs
Outdated
| @@ -2,6 +2,7 @@ use anyhow::{bail, Ok, Result}; | |||
| #[cfg(unix)] | |||
| use std::{env, path::Path}; | |||
| use std::{str::FromStr, sync::Arc}; | |||
|
|
|||
jokeyrhyme
force-pushed
the
default-addresses-and-printing-oh-my
branch
from
4bee358 to
54d1535
Compare
Previously, we'd take the `--address` or default address, resolve it into the socket address, and start listening on that address. But we need to be able to expose that address to clients, too. So we break `Bus::unix_stream()` apart giving us a separate `Bus::unix_addr()` and access to the intermediate socket address.
Until now, `--print-address` would print the address specification, that is, the input used to determine how and where the server would listen. This would be useful for clients for `unix:path=...`, but for `unix:dir=...` and `unix:tmpdir=...` this would not work. So now we share the socket address that we produce from the address specification.
zeenix
reviewed
Jan 9, 2025
Comment on lines
+67
to
+69
| address = Address::try_from( | ||
| format!("unix:path={}", addr.as_pathname().unwrap().display()).as_str(), | ||
| )?; |
There was a problem hiding this comment.
So sorry but I did point out this before here: You don't (at least should not) need any parsing here.
There was a problem hiding this comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR
takes theaddresses the issue identified here: #175 (comment)BusType,--session, and--systemwork from #159 . And then builds upon that (perhaps unnecessarily) to fixi.e.
--print-addresspreviously didn't always produce a bus address that clients could use.I've confirmed that the output from
--print-addresscan be used withDBUS_SESSION_BUS_ADDRESS=... cargo testin the zbus repository