2012-05-15 11:13:05

by Alex Elsayed

[permalink] [raw]
Subject: [PATCH] Add dbus service file that references the systemd unit

This allows bluez to be bus-activated.
---
Makefile.am | 2 ++
configure.ac | 2 +-
src/org.bluez.service.in | 6 ++++++
3 files changed, 9 insertions(+), 1 deletion(-)
create mode 100644 src/org.bluez.service.in

diff --git a/Makefile.am b/Makefile.am
index 44e82c0..82df92e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -27,8 +27,10 @@ include_HEADERS =

if DATAFILES
dbusdir = $(sysconfdir)/dbus-1/system.d
+dbusservicedir = $(datadir)/dbus-1/system-services

dbus_DATA = src/bluetooth.conf
+dbusservice_DATA = src/org.bluez.service

confdir = $(sysconfdir)/bluetooth

diff --git a/configure.ac b/configure.ac
index 44f33ad..0fb3879 100644
--- a/configure.ac
+++ b/configure.ac
@@ -71,4 +71,4 @@ if (test -n "${path_systemdunit}"); then
fi
AM_CONDITIONAL(SYSTEMD, test -n "${path_systemdunit}")

-AC_OUTPUT(Makefile doc/version.xml src/bluetoothd.8 src/bluetooth.service bluez.pc)
+AC_OUTPUT(Makefile doc/version.xml src/bluetoothd.8 src/bluetooth.service src/org.bluez.service bluez.pc)
diff --git a/src/org.bluez.service.in b/src/org.bluez.service.in
new file mode 100644
index 0000000..f8463bb
--- /dev/null
+++ b/src/org.bluez.service.in
@@ -0,0 +1,6 @@
+[D-BUS Service]
+Name=org.bluez
+Exec=@prefix@/sbin/bluetoothd -n
+User=root
+SystemdService=bluetooth.service
+
--
1.7.10.2



2012-05-17 16:43:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2] Add dbus service file that references the systemd unit

Hi Alex,

> > On Wed, May 16, 2012, Alex Elsayed wrote:
> > > This allows bluez to be bus-activated.
> > >
> > > v2: Fix whitespace error
> >
> > This v2 line shouldn't be part of the commit message. You can put stuff
> > like that between the --- line and the diff line (usually before the
> > diffstat).
>
> Alright, I'll fix that
>
> > > +++ b/src/org.bluez.service.in
> > > @@ -0,0 +1,5 @@
> > > +[D-BUS Service]
> > > +Name=org.bluez
> > > +Exec=@prefix@/sbin/bluetoothd -n
> > > +User=root
> > > +SystemdService=bluetooth.service
> >
> > My Fedora 16 installation seems to provide its own org.bluez.service:
> >
> > $ cat /usr/share/dbus-1/system-services/org.bluez.service
> > [D-BUS Service]
> > Name=org.bluez
> > Exec=/bin/false
> > User=root
> > SystemdService=dbus-org.bluez.service
> >
> > Could you explain why it's a bit different from what you're providing
> > (particularly the /bin/false part). I'm not really an expert with with
> > systemd/D-Bus files so you'll need to convince me of why your variant is
> > correct (or more appropriate than the Fedora one) before it goes upstream.
>
> Sure. In a systemd environment the Exec= line is ignored and the
> SystemdService is activated instead iff a SystemdService line exists. In a non-
> systemd environment, however, the Exec= line is used to start the service.
> Since Fedora is standardizing on systemd, they omitted that line, but that
> means that bus activation will fail on non-systemd distros, such as ubuntu and
> debian.

actually non-systemd based distros is something I do not care about
anymore. The only thing that BlueZ should ship is systemd integration.

If a distro wants to deviate from systemd, then they have to go an fix
that up by themselves. This is not something I will allow upstream.

Regards

Marcel



2012-05-17 16:37:42

by Alex Elsayed

[permalink] [raw]
Subject: [PATCH v3] Add dbus service file that references the systemd unit

This allows bluez to be bus-activated.
---
v3: Use alias as SystemdService so 'systemctl enable/disable' works
v2: Fix whitespace error

Makefile.am | 2 ++
configure.ac | 2 +-
src/bluetooth.service.in | 1 +
src/org.bluez.service.in | 5 +++++
4 files changed, 9 insertions(+), 1 deletion(-)
create mode 100644 src/org.bluez.service.in

diff --git a/Makefile.am b/Makefile.am
index 44e82c0..82df92e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -27,8 +27,10 @@ include_HEADERS =

if DATAFILES
dbusdir = $(sysconfdir)/dbus-1/system.d
+dbusservicedir = $(datadir)/dbus-1/system-services

dbus_DATA = src/bluetooth.conf
+dbusservice_DATA = src/org.bluez.service

confdir = $(sysconfdir)/bluetooth

diff --git a/configure.ac b/configure.ac
index 44f33ad..0fb3879 100644
--- a/configure.ac
+++ b/configure.ac
@@ -71,4 +71,4 @@ if (test -n "${path_systemdunit}"); then
fi
AM_CONDITIONAL(SYSTEMD, test -n "${path_systemdunit}")

-AC_OUTPUT(Makefile doc/version.xml src/bluetoothd.8 src/bluetooth.service bluez.pc)
+AC_OUTPUT(Makefile doc/version.xml src/bluetoothd.8 src/bluetooth.service src/org.bluez.service bluez.pc)
diff --git a/src/bluetooth.service.in b/src/bluetooth.service.in
index 8a9edb6..2a576a3 100644
--- a/src/bluetooth.service.in
+++ b/src/bluetooth.service.in
@@ -8,3 +8,4 @@ ExecStart=@prefix@/sbin/bluetoothd -n

[Install]
WantedBy=bluetooth.target
+Alias=dbus-org.bluez.service
diff --git a/src/org.bluez.service.in b/src/org.bluez.service.in
new file mode 100644
index 0000000..64864cd
--- /dev/null
+++ b/src/org.bluez.service.in
@@ -0,0 +1,5 @@
+[D-BUS Service]
+Name=org.bluez
+Exec=@prefix@/sbin/bluetoothd -n
+User=root
+SystemdService=dbus-org.bluez.service
--
1.7.10.2


2012-05-17 16:31:57

by Alex Elsayed

[permalink] [raw]
Subject: Re: [PATCH v2] Add dbus service file that references the systemd unit

On Thursday, May 17, 2012 06:02:37 PM Johan Hedberg wrote:
> Hi Alex,
>
> On Wed, May 16, 2012, Alex Elsayed wrote:
> > This allows bluez to be bus-activated.
> >
> > v2: Fix whitespace error
>
> This v2 line shouldn't be part of the commit message. You can put stuff
> like that between the --- line and the diff line (usually before the
> diffstat).

Alright, I'll fix that

> > +++ b/src/org.bluez.service.in
> > @@ -0,0 +1,5 @@
> > +[D-BUS Service]
> > +Name=org.bluez
> > +Exec=@prefix@/sbin/bluetoothd -n
> > +User=root
> > +SystemdService=bluetooth.service
>
> My Fedora 16 installation seems to provide its own org.bluez.service:
>
> $ cat /usr/share/dbus-1/system-services/org.bluez.service
> [D-BUS Service]
> Name=org.bluez
> Exec=/bin/false
> User=root
> SystemdService=dbus-org.bluez.service
>
> Could you explain why it's a bit different from what you're providing
> (particularly the /bin/false part). I'm not really an expert with with
> systemd/D-Bus files so you'll need to convince me of why your variant is
> correct (or more appropriate than the Fedora one) before it goes upstream.

Sure. In a systemd environment the Exec= line is ignored and the
SystemdService is activated instead iff a SystemdService line exists. In a non-
systemd environment, however, the Exec= line is used to start the service.
Since Fedora is standardizing on systemd, they omitted that line, but that
means that bus activation will fail on non-systemd distros, such as ubuntu and
debian.

However, I probably should use the SystemdService line from Fedora rather than
the one I used, since that allows disabling bus activation. See
https://fedoraproject.org/wiki/Packaging:Systemd#DBus_activation

That requires adding an Alias= to the systemd service, so I'll be sending a v3
of this patch.

2012-05-17 15:02:37

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v2] Add dbus service file that references the systemd unit

Hi Alex,

On Wed, May 16, 2012, Alex Elsayed wrote:
> This allows bluez to be bus-activated.
>
> v2: Fix whitespace error

This v2 line shouldn't be part of the commit message. You can put stuff
like that between the --- line and the diff line (usually before the
diffstat).

> +++ b/src/org.bluez.service.in
> @@ -0,0 +1,5 @@
> +[D-BUS Service]
> +Name=org.bluez
> +Exec=@prefix@/sbin/bluetoothd -n
> +User=root
> +SystemdService=bluetooth.service

My Fedora 16 installation seems to provide its own org.bluez.service:

$ cat /usr/share/dbus-1/system-services/org.bluez.service
[D-BUS Service]
Name=org.bluez
Exec=/bin/false
User=root
SystemdService=dbus-org.bluez.service

Could you explain why it's a bit different from what you're providing
(particularly the /bin/false part). I'm not really an expert with with
systemd/D-Bus files so you'll need to convince me of why your variant is
correct (or more appropriate than the Fedora one) before it goes upstream.

Johan

2012-05-16 20:53:54

by Alex Elsayed

[permalink] [raw]
Subject: [PATCH v2] Add dbus service file that references the systemd unit

This allows bluez to be bus-activated.

v2: Fix whitespace error
---
Makefile.am | 2 ++
configure.ac | 2 +-
src/org.bluez.service.in | 5 +++++
3 files changed, 8 insertions(+), 1 deletion(-)
create mode 100644 src/org.bluez.service.in

diff --git a/Makefile.am b/Makefile.am
index 44e82c0..82df92e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -27,8 +27,10 @@ include_HEADERS =

if DATAFILES
dbusdir = $(sysconfdir)/dbus-1/system.d
+dbusservicedir = $(datadir)/dbus-1/system-services

dbus_DATA = src/bluetooth.conf
+dbusservice_DATA = src/org.bluez.service

confdir = $(sysconfdir)/bluetooth

diff --git a/configure.ac b/configure.ac
index 44f33ad..0fb3879 100644
--- a/configure.ac
+++ b/configure.ac
@@ -71,4 +71,4 @@ if (test -n "${path_systemdunit}"); then
fi
AM_CONDITIONAL(SYSTEMD, test -n "${path_systemdunit}")

-AC_OUTPUT(Makefile doc/version.xml src/bluetoothd.8 src/bluetooth.service bluez.pc)
+AC_OUTPUT(Makefile doc/version.xml src/bluetoothd.8 src/bluetooth.service src/org.bluez.service bluez.pc)
diff --git a/src/org.bluez.service.in b/src/org.bluez.service.in
new file mode 100644
index 0000000..83e2740
--- /dev/null
+++ b/src/org.bluez.service.in
@@ -0,0 +1,5 @@
+[D-BUS Service]
+Name=org.bluez
+Exec=@prefix@/sbin/bluetoothd -n
+User=root
+SystemdService=bluetooth.service
--
1.7.10.2


2012-05-16 08:01:34

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH] Add dbus service file that references the systemd unit

Hi Alex,

On Tue, May 15, 2012, Alex Elsayed wrote:
> This allows bluez to be bus-activated.
> ---
> Makefile.am | 2 ++
> configure.ac | 2 +-
> src/org.bluez.service.in | 6 ++++++
> 3 files changed, 9 insertions(+), 1 deletion(-)
> create mode 100644 src/org.bluez.service.in

The patch looks ok'ish to me but you have to clean it up since it
doesn't apply:

Applying: Add dbus service file that references the systemd unit
/home/jh/src/bluez/.git/rebase-apply/patch:44: new blank line at EOF.
+
fatal: 1 line adds whitespace errors.
Patch failed at 0001 Add dbus service file that references the systemd unit

If you wanna catch this kind of stuff yourself in the future add the
following to your .gitconfig:

[apply]
whitespace = error

Johan