-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Ensure dbus detects new services #20871
Conversation
@layus, thanks for your PR! By analyzing the history of the files in this pull request, we identified @wkennington, @edolstra and @peterhoeg to be potential reviewers. |
@layus Tested this patch but I still get the security policy error:
|
@tomberek Just to be sure, you applied the patch and rebooted before testing ? To test the patch, you need first to apply the patch, disable avahi and reboot. On a machine booted with the patch (and avahi disabled), you can now enable avahi with |
@layus Yes. I did a fresh re-creation of the environment and images. The above is the result of a
|
I will need more time than I currently have to debug this. Maybe tomorrow morning. |
Status update? |
DBus daemon now loads its config from /run/current-system/dbus. Reloading the daemon makes it re-read that file and catch the updates after a system upgrade.
@tomberek I just found a way to make it work correctly. Do you mind to try the new patch ? |
@peterhoeg @domenkozar Would you mind testing and/or reviewing this PR ? It's working for me, but dbus is a core component. Better safe than sorry. You both worked on dbus in the past. |
I will definitely have a look but realistically it will only be this weekend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks fine, going to test this in the next few days.
@layus, I'm sorry, I haven't had the time to try this out... So 👎 to me (not your changes)... |
ping @fpletz, have you tested this, yet? |
This looks great! With that said, would somewhere under |
@cstrahan |
I guess I'm just surprised that we didn't constrain the changes in this PR to something like: - "${lib.getBin pkgs.dbus}/bin/dbus-daemon --config-file=${configDir}/system.conf ${daemonArgs}"
+ "${lib.getBin pkgs.dbus}/bin/dbus-daemon --config-file=/etc/dbus-1/system.conf ${daemonArgs}" It would seem that'd have the same effect. |
I feel that Anyway, in this case |
@abbradar, @cstrahan I explained above why /etc is not a good default choice. Now, I have overlooked the importance of session instances in this issue. If session dbus instances can be started by anyone, then /etc becomes the right place to use. |
The |
@vcunat I too feel that discoverability is important. But I would rather teach NixOS users the |
I don't have any evidence but I think this follows a pattern same to for example systemd -- |
From the dbus introduction:
This patch sets 's to /run/current-system/sw/... and bundles
all the provided dbus schemas in that location. System updates are
automatically catched by dbus.
Fixes #19034.