conn: support /run and /var/run default addresses#424
conn: support /run and /var/run default addresses#424twilly wants to merge 1 commit intogodbus:masterfrom
Conversation
`/var/run` has been depreciated for a while and some Linux filesystem
layouts no longer include the backwards-compatibility link from
`/var/run` to `/run`.
This changes Linux's `getSystemBusPlatformAddress` to probe for the
system bus address in `/var/run` followed by in `/run`. This maintains
compatibility with distributions where /run and /var/run are different
(very rare) and somehow have two system buses (!) while also supporting
distributions where `/var/run` is not linked.
References:
- [dbus: Use ${runstatedir} for system bus instead of ${localstatedir}/run](https://gitlab.freedesktop.org/dbus/dbus/-/issues/180)
- [gdbus: gdbusaddress: Use runstatedir rather than localstatedir](https://gitlab.gnome.org/GNOME/glib/-/commit/b7b9f89417ab547b96dc5000ef5c6be9d0a307d5)
Fixes: godbus#423
9420658 to
5e24245
Compare
kolyshkin
left a comment
There was a problem hiding this comment.
You can probably simplify things by returning "" from getSystemBusPlatformAddress and having a caller error out if the string is empty.
IOW something like
address := getSystemBusPlatformAddress()
if address == "" {
return nil, errors.New("system bus not found")
}
Thanks for taking a look and spending your time on this small feature. :) I don't mind making the change, however there are some consequences. Returning the empty string instead of an error limits to a singular error, departs from the sister call The empty string conveying not-found limits the number of possible error reasons. For example, func getSystemBusPlatformAddress() (string, error) {
if address, ok := os.LookupEnv("DBUS_SYSTEM_BUS_ADDRESS"); ok {
if !validAddress(address) { // assume this does some checks (os.Stat, transport type availability, etc)
return "", errors.New("misconfigured DBUS_SYSTEM_BUS_ADDRESS environment variable")
}
return address, nil
}
for _, path := range defaultSystemBusPaths {
if _, err := os.Stat(path); err == nil {
return "unix:path=" + path, nil
}
}
return "", errors.New("system bus not found")
}Using // SessionBusPrivate returns a new private connection to the session bus.
func SessionBusPrivate(opts ...ConnOption) (*Conn, error) {
return busDial(func() (string, error) { return getSessionBusAddress(true) }, opts...)
}
// SessionBusPrivateNoAutoStartup returns a new private connection to the session bus. If
// the session bus is not already open, do not attempt to launch it.
func SessionBusPrivateNoAutoStartup(opts ...ConnOption) (*Conn, error) {
return busDial(func() (string, error) { return getSessionBusAddress(false) }, opts...)
}
// SystemBusPrivate returns a new private connection to the system bus.
// Note: this connection is not ready to use. One must perform Auth and Hello
// on the connection before it is usable.
func SystemBusPrivate(opts ...ConnOption) (*Conn, error) {
return busDial(getSystemBusPlatformAddress, opts...)
}
func busDial(addrFunc func() (string, error), opts ...ConnOption) (*Conn, error) {
address, err := addrFunc()
if err != nil {
return nil, err
}
return Dial(address, opts...)
} The same refactor goes with ConnectSessionBus and ConnectSystemBus. Here's a direct link within this PR showing how structurally similar they now are: Line 137 in 5e24245 And in regard to the caller's complexity, checking for error versus the empty string is a zero-line difference. IMHO, they're equally complex: same line count, both add one conditional, and both return an error. --- a/conn.go
+++ b/conn.go
@@ -175,9 +175,9 @@ func Connect(address string, opts ...ConnOption) (*Conn, error) {
// Note: this connection is not ready to use. One must perform Auth and Hello
// on the connection before it is usable.
func SystemBusPrivate(opts ...ConnOption) (*Conn, error) {
- address, err := getSystemBusPlatformAddress()
- if err != nil {
- return nil, err
+ address := getSystemBusPlatformAddress()
+ if address = "" {
+ return nil, errors.New("system bus not found")
}
return Dial(address, opts...)
} |
/var/runhas been depreciated for a while and some Linux filesystem layouts no longer include the backwards-compatibility link from/var/runto/run.This changes Linux's
getSystemBusPlatformAddressto probe for the system bus address in/var/runfollowed by in/run. This maintains compatibility with distributions where/runand/var/runare different (very rare) and somehow have two system buses (!) while also supporting distributions where/var/runis not linked.References:
Fixes: #423