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

src/Makefile.am: Install D-Bus policy in /usr/share, not /etc #657

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

gioele
Copy link
Contributor

@gioele gioele commented Jul 29, 2023

From https://bugs.debian.org/1006631:

dbus supports policy files in both /usr/share/dbus-1/system.d and
/etc/dbus-1/systemd. [The] recently released dbus 1.14.0, officially
deprecates installing packages' default policies into /etc/dbus-1/systemd,
instead reserving it for the sysadmin. This is the same idea as the
difference between /usr/lib/udev/rules.d and /etc/udev/rules.d.

@arkq
Copy link
Owner

arkq commented Jul 29, 2023

I wonder how this will affect systems with older versions of dbus. Will it read the configuration correctly?

Also, the right place to change the default would be here:
https://github.com/arkq/bluez-alsa/blob/master/configure.ac#L345
According to the dbus-1.pc file on Debian 12: --variable=sysconfdir -> --variable=datadir

But also, it will be required to check whether the datadir variable was provided in older versions of dbus.

And another question is whether we need to provide customization for that directory in the configure.ac after all. Maybe configuration should just read the value from the pkg-config file and that's all...

From https://bugs.debian.org/1006631:

> dbus supports policy files in both `/usr/share/dbus-1/system.d` and
> `/etc/dbus-1/systemd`. [The] recently released dbus 1.14.0, officially
> deprecates installing packages' default policies into `/etc/dbus-1/systemd`,
> instead reserving it for the sysadmin. This is the same idea as the
> difference between `/usr/lib/udev/rules.d` and `/etc/udev/rules.d`.
@gioele
Copy link
Contributor Author

gioele commented Jul 29, 2023

Also, the right place to change the default would be here: https://github.com/arkq/bluez-alsa/blob/master/configure.ac#L345 According to the dbus-1.pc file on Debian 12: --variable=sysconfdir -> --variable=datadir

Right. I amended the PR.

I wonder how this will affect systems with older versions of dbus. Will it read the configuration correctly?

D-Bus reads from /usr since version 1.10, released in 2015: https://sources.debian.org/src/dbus/1.10.32-0%2Bdeb9u1/NEWS/#L483-L499

But also, it will be required to check whether the datadir variable was provided in older versions of dbus.

datadir has been added to dbus-1.pc in 2014: https://salsa.debian.org/utopia-team/dbus/-/commit/95fe17a96d0488da79c5773da9b86b5431fa28c9

@arkq
Copy link
Owner

arkq commented Jul 29, 2023

D-Bus reads from /usr since version 1.10, released in 2015

OK, so I think that we can change the default then.

@codecov
Copy link

codecov bot commented Jul 29, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02% ⚠️

Comparison is base (5534708) 71.11% compared to head (3c915bf) 71.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #657      +/-   ##
==========================================
- Coverage   71.11%   71.10%   -0.02%     
==========================================
  Files          47       47              
  Lines        9011     9011              
==========================================
- Hits         6408     6407       -1     
- Misses       2603     2604       +1     

see 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arkq arkq merged commit 57cd56a into arkq:master Aug 1, 2023
19 checks passed
@gioele gioele deleted the dbus-policy-in-usr branch August 8, 2023 06:50
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