[2/3] package/mosquitto: Switch systemd unit to Type=notify and depend on network
diff mbox series

Message ID 20190718100636.27290-3-titouan.christophe@railnova.eu
State Changes Requested
Headers show
Series
  • Update package mosquitto
Related show

Commit Message

Titouan Christophe July 18, 2019, 10:06 a.m. UTC
Signed-off-by: Titouan Christophe <titouan.christophe@railnova.eu>
---
RFC: Would it be better to use the mosquitto.service file provided upstream ?
https://github.com/eclipse/mosquitto/blob/master/service/systemd/mosquitto.service.notify
---
 package/mosquitto/mosquitto.mk      | 4 ++++
 package/mosquitto/mosquitto.service | 3 +++
 2 files changed, 7 insertions(+)

Comments

Peter Korsgaard Aug. 1, 2019, 10:27 a.m. UTC | #1
>>>>> "Titouan" == Titouan Christophe <titouan.christophe@railnova.eu> writes:

 > Signed-off-by: Titouan Christophe <titouan.christophe@railnova.eu>

It would be great if your commit message would explain _WHY_ this is
done, not just _WHAT_ the change is - E.G. what is the advantage of type=notify?

 > ---
 > RFC: Would it be better to use the mosquitto.service file provided upstream ?
 > https://github.com/eclipse/mosquitto/blob/master/service/systemd/mosquitto.service.notify

Not knowing much about systemd, but I think so. The differences look
quite small.

> ---
 >  package/mosquitto/mosquitto.mk      | 4 ++++
 >  package/mosquitto/mosquitto.service | 3 +++
 >  2 files changed, 7 insertions(+)

 > diff --git a/package/mosquitto/mosquitto.mk b/package/mosquitto/mosquitto.mk
 > index ed72af754a..333cce35c2 100644
 > --- a/package/mosquitto/mosquitto.mk
 > +++ b/package/mosquitto/mosquitto.mk
 > @@ -23,6 +23,10 @@ else
 >  MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no WITH_SHARED_LIBRARIES=yes
 >  endif
 
 > +ifeq ($(BR2_PACKAGE_SYSTEMD),y)
 > +MOSQUITTO_MAKE_OPTS += WITH_SYSTEMD=yes

This is not mentioned in the commit message. I see this causes mosquitto
to link with -lsystemd, so you need to add systemd to
MOSQUITTO_DEPENDENCIES.

Care to send an updated patch with a more descriptive commit message,
reusing the upstream service file and adding systemd to _DEPENDENCIES?
Titouan Christophe Aug. 2, 2019, 8:51 a.m. UTC | #2
Hello Peter,

On 8/1/19 12:27 PM, Peter Korsgaard wrote:
>>>>>> "Titouan" == Titouan Christophe <titouan.christophe@railnova.eu> writes:
> 
>   > Signed-off-by: Titouan Christophe <titouan.christophe@railnova.eu>
> 
> It would be great if your commit message would explain _WHY_ this is
> done, not just _WHAT_ the change is - E.G. what is the advantage of type=notify?

Sure !

> 
>   > ---
>   > RFC: Would it be better to use the mosquitto.service file provided upstream ?
>   > https://github.com/eclipse/mosquitto/blob/master/service/systemd/mosquitto.service.notify
> 
> Not knowing much about systemd, but I think so. The differences look
> quite small.
Yes, that's why I think it would be better to use the upstreamed one for 
that. I'm not sure of Buildroot's policy regarding config files provided 
by the packages themselves, hence the question.

> 
>> ---
>   >  package/mosquitto/mosquitto.mk      | 4 ++++
>   >  package/mosquitto/mosquitto.service | 3 +++
>   >  2 files changed, 7 insertions(+)
> 
>   > diff --git a/package/mosquitto/mosquitto.mk b/package/mosquitto/mosquitto.mk
>   > index ed72af754a..333cce35c2 100644
>   > --- a/package/mosquitto/mosquitto.mk
>   > +++ b/package/mosquitto/mosquitto.mk
>   > @@ -23,6 +23,10 @@ else
>   >  MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no WITH_SHARED_LIBRARIES=yes
>   >  endif
>   
>   > +ifeq ($(BR2_PACKAGE_SYSTEMD),y)
>   > +MOSQUITTO_MAKE_OPTS += WITH_SYSTEMD=yes
> 
> This is not mentioned in the commit message. I see this causes mosquitto
> to link with -lsystemd, so you need to add systemd to
> MOSQUITTO_DEPENDENCIES.

Ok

> 
> Care to send an updated patch with a more descriptive commit message,
> reusing the upstream service file and adding systemd to _DEPENDENCIES?
> 

Sure, I'll respin that one

Regards,
Titouan

Patch
diff mbox series

diff --git a/package/mosquitto/mosquitto.mk b/package/mosquitto/mosquitto.mk
index ed72af754a..333cce35c2 100644
--- a/package/mosquitto/mosquitto.mk
+++ b/package/mosquitto/mosquitto.mk
@@ -23,6 +23,10 @@  else
 MOSQUITTO_MAKE_OPTS += WITH_STATIC_LIBRARIES=no WITH_SHARED_LIBRARIES=yes
 endif
 
+ifeq ($(BR2_PACKAGE_SYSTEMD),y)
+MOSQUITTO_MAKE_OPTS += WITH_SYSTEMD=yes
+endif
+
 # adns uses getaddrinfo_a
 ifeq ($(BR2_TOOLCHAIN_USES_GLIBC),y)
 MOSQUITTO_MAKE_OPTS += WITH_ADNS=yes
diff --git a/package/mosquitto/mosquitto.service b/package/mosquitto/mosquitto.service
index 2d1939d1c7..24c9d8e948 100644
--- a/package/mosquitto/mosquitto.service
+++ b/package/mosquitto/mosquitto.service
@@ -1,7 +1,10 @@ 
 [Unit]
 Description=Mosquitto MQTT broker
+After=network-online.target
+Wants=network-online.target
 
 [Service]
+Type=notify
 ExecStart=/usr/sbin/mosquitto -c /etc/mosquitto/mosquitto.conf
 ExecReload=/bin/kill -HUP $MAINPID
 Restart=always