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

🐛 Expose the socket address clients can use #182

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jokeyrhyme
Copy link
Contributor

@jokeyrhyme jokeyrhyme commented Dec 14, 2024

This PR takes the BusType, --session, and --system work from #159 . And then builds upon that (perhaps unnecessarily) to fix the issue identified here: #175 (comment)

i.e. --print-address previously didn't always produce a bus address that clients could use.

I've confirmed that the output from --print-address can be used with DBUS_SESSION_BUS_ADDRESS=... cargo test in the zbus repository

We'll borrow this from dbus2#159 so that we can be clearer about the default
address.
Address::try_from(bus_type)?
};

let mut bus = bus::Bus::for_address(&format!("{address}")).await?;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I'm a little anxious about all the back and forth between Address and &str / String we have here now

It only happens during start up, so I'm not sure whether it's worth trying to optimise this now, or do this holistically once Config and other functionality is working

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I'm a little anxious about all the back and forth between Address and &str / String we have here now

We shouldn't need to parse any address string and we most definitely don't need to care for session/system here. At this point in the code, we should already have a zbus::Address (either from the config, cli arg or a default).

Also, you can construct a zbus::Address through their properties directly (if not, we need to add more API to zbus first) when/where needed.

@jokeyrhyme jokeyrhyme force-pushed the default-addresses-and-printing-oh-my branch from 1a91de1 to 654a7f5 Compare December 14, 2024 06:23
We'll use this later when we need the default bus address for each type.
We can rely on `TryFrom<BusType> for Address` to produce the default
address for `--session` and `--system`.
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 is produced from the address
specification.
@jokeyrhyme jokeyrhyme force-pushed the default-addresses-and-printing-oh-my branch from 654a7f5 to aa8fa3b Compare December 14, 2024 06:28
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

Successfully merging this pull request may close these issues.

2 participants