All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/2] dhcpcd: start-up scripts
@ 2017-10-31 22:36 Markus Mayer
  2017-10-31 22:36 ` [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script Markus Mayer
  2017-10-31 22:36 ` [Buildroot] [PATCH 2/2] dhcpcd: add systemd start-up service Markus Mayer
  0 siblings, 2 replies; 20+ messages in thread
From: Markus Mayer @ 2017-10-31 22:36 UTC (permalink / raw)
  To: buildroot

From: Markus Mayer <mmayer@broadcom.com>

System V and systemd start-up scripts for dhcpcd.

Other daemons come with start-up scripts, whereas dhcpcd did not
include them.

Markus Mayer (2):
  dhcpcd: add SysV start-up script
  dhcpcd: add systemd start-up service

 package/dhcpcd/S41dhcpcd      | 34 ++++++++++++++++++++++++++++++++++
 package/dhcpcd/dhcpcd.mk      | 13 +++++++++++++
 package/dhcpcd/dhcpcd.service | 13 +++++++++++++
 3 files changed, 60 insertions(+)
 create mode 100755 package/dhcpcd/S41dhcpcd
 create mode 100644 package/dhcpcd/dhcpcd.service

-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-10-31 22:36 [Buildroot] [PATCH 0/2] dhcpcd: start-up scripts Markus Mayer
@ 2017-10-31 22:36 ` Markus Mayer
  2017-11-02 22:06   ` Thomas Petazzoni
  2017-10-31 22:36 ` [Buildroot] [PATCH 2/2] dhcpcd: add systemd start-up service Markus Mayer
  1 sibling, 1 reply; 20+ messages in thread
From: Markus Mayer @ 2017-10-31 22:36 UTC (permalink / raw)
  To: buildroot

From: Markus Mayer <mmayer@broadcom.com>

Add System V start-up script for dhcpcd that is run after the network
has been brought up.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 package/dhcpcd/S41dhcpcd | 34 ++++++++++++++++++++++++++++++++++
 package/dhcpcd/dhcpcd.mk |  5 +++++
 2 files changed, 39 insertions(+)
 create mode 100755 package/dhcpcd/S41dhcpcd

diff --git a/package/dhcpcd/S41dhcpcd b/package/dhcpcd/S41dhcpcd
new file mode 100755
index 0000000..a2e87ca
--- /dev/null
+++ b/package/dhcpcd/S41dhcpcd
@@ -0,0 +1,34 @@
+#!/bin/sh
+#
+# Start/stop dhcpcd
+#
+
+DAEMON=/sbin/dhcpcd
+CONFIG=/etc/dhcpcd.conf
+PIDFILE=/var/run/dhcpcd.pid
+
+[ -x $DAEMON ] || exit 0
+[ -f $CONFIG ] || exit 0
+
+case "$1" in
+  start)
+	echo "Starting dhcpcd..."
+	start-stop-daemon -S -x "$DAEMON" -p "$PIDFILE" -- -f "$CONFIG"
+	;;
+  stop)
+	echo "Stopping dhcpcd..."
+	start-stop-daemon -K -x "$DAEMON" -p "$PIDFILE" -o
+	;;
+  reload|force-reload)
+	echo "Reloading dhcpcd configuration..."
+	"$DAEMON" -s reload
+	;;
+  restart)
+	"$0" stop
+	sleep 1 # Prevent race condition: ensure dhcpcd stops before start.
+	"$0" start
+	;;
+  *)
+	echo "Usage: $0 {start|stop|restart|reload|force-reload}"
+	exit 1
+esac
diff --git a/package/dhcpcd/dhcpcd.mk b/package/dhcpcd/dhcpcd.mk
index 1d29cda..cf1da4f 100644
--- a/package/dhcpcd/dhcpcd.mk
+++ b/package/dhcpcd/dhcpcd.mk
@@ -36,6 +36,11 @@ define DHCPCD_INSTALL_TARGET_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) install DESTDIR=$(TARGET_DIR)
 endef
 
+define DHCPCD_INSTALL_INIT_SYSV
+	$(INSTALL) -m 755 -D package/dhcpcd/S41dhcpcd \
+		$(TARGET_DIR)/etc/init.d/S41dhcpcd
+endef
+
 # NOTE: Even though this package has a configure script, it is not generated
 # using the autotools, so we have to use the generic package infrastructure.
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 2/2] dhcpcd: add systemd start-up service
  2017-10-31 22:36 [Buildroot] [PATCH 0/2] dhcpcd: start-up scripts Markus Mayer
  2017-10-31 22:36 ` [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script Markus Mayer
@ 2017-10-31 22:36 ` Markus Mayer
  2017-11-02 22:06   ` Thomas Petazzoni
  1 sibling, 1 reply; 20+ messages in thread
From: Markus Mayer @ 2017-10-31 22:36 UTC (permalink / raw)
  To: buildroot

From: Markus Mayer <mmayer@broadcom.com>

Add systemd start-up configuration for dhcpcd that is executed after
the network has been brought up.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 package/dhcpcd/dhcpcd.mk      |  8 ++++++++
 package/dhcpcd/dhcpcd.service | 13 +++++++++++++
 2 files changed, 21 insertions(+)
 create mode 100644 package/dhcpcd/dhcpcd.service

diff --git a/package/dhcpcd/dhcpcd.mk b/package/dhcpcd/dhcpcd.mk
index cf1da4f..dc5c69b 100644
--- a/package/dhcpcd/dhcpcd.mk
+++ b/package/dhcpcd/dhcpcd.mk
@@ -41,6 +41,14 @@ define DHCPCD_INSTALL_INIT_SYSV
 		$(TARGET_DIR)/etc/init.d/S41dhcpcd
 endef
 
+define DHCPCD_INSTALL_INIT_SYSTEMD
+	$(INSTALL) -D -m 0644 package/dhcpcd/dhcpcd.service \
+		$(TARGET_DIR)/usr/lib/systemd/system/dhcpcd.service
+	mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
+	ln -sf ../../../../usr/lib/systemd/system/dhcpcd.service \
+		$(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcpcd.service
+endef
+
 # NOTE: Even though this package has a configure script, it is not generated
 # using the autotools, so we have to use the generic package infrastructure.
 
diff --git a/package/dhcpcd/dhcpcd.service b/package/dhcpcd/dhcpcd.service
new file mode 100644
index 0000000..0552b5c
--- /dev/null
+++ b/package/dhcpcd/dhcpcd.service
@@ -0,0 +1,13 @@
+[Unit]
+Description=DHCP client
+After=network.target
+
+[Service]
+Type=forking
+EnvironmentFile=-/etc/default/dhcpcd
+PIDFile=/var/run/dhcpcd.pid
+ExecStart=/sbin/dhcpcd $DAEMON_ARGS
+Restart=always
+
+[Install]
+WantedBy=multi-user.target
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-10-31 22:36 ` [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script Markus Mayer
@ 2017-11-02 22:06   ` Thomas Petazzoni
  2017-11-02 22:18     ` Yann E. MORIN
  2017-11-03 15:59     ` Peter Korsgaard
  0 siblings, 2 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2017-11-02 22:06 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, 31 Oct 2017 15:36:58 -0700, Markus Mayer wrote:
> From: Markus Mayer <mmayer@broadcom.com>
> 
> Add System V start-up script for dhcpcd that is run after the network
> has been brought up.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>

I've applied, just two comments.

> +DAEMON=/sbin/dhcpcd
> +CONFIG=/etc/dhcpcd.conf
> +PIDFILE=/var/run/dhcpcd.pid
> +
> +[ -x $DAEMON ] || exit 0
> +[ -f $CONFIG ] || exit 0

We have these in a lot of init scripts, so I kept them. But I find
those tests pretty silly in fact:

 - The daemon should definitely be there, as it's installed by the same
   package. And if it turns out not to be there, I'd prefer a loud
   failure than an init script that silently ignores the problem.

 - Same for the configuration file: I prefer a loud error than an init
   script that ignores the problem.

Yann, Peter, Arnout, can we agree that such tests should be removed
entirely, and we let the init script fail if the daemon does not exist,
and the daemon fail if its configuration file does not exist ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 2/2] dhcpcd: add systemd start-up service
  2017-10-31 22:36 ` [Buildroot] [PATCH 2/2] dhcpcd: add systemd start-up service Markus Mayer
@ 2017-11-02 22:06   ` Thomas Petazzoni
  0 siblings, 0 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2017-11-02 22:06 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, 31 Oct 2017 15:36:59 -0700, Markus Mayer wrote:
> From: Markus Mayer <mmayer@broadcom.com>
> 
> Add systemd start-up configuration for dhcpcd that is executed after
> the network has been brought up.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  package/dhcpcd/dhcpcd.mk      |  8 ++++++++
>  package/dhcpcd/dhcpcd.service | 13 +++++++++++++
>  2 files changed, 21 insertions(+)
>  create mode 100644 package/dhcpcd/dhcpcd.service

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-02 22:06   ` Thomas Petazzoni
@ 2017-11-02 22:18     ` Yann E. MORIN
  2017-11-02 22:23       ` Thomas Petazzoni
  2017-11-03 15:59     ` Peter Korsgaard
  1 sibling, 1 reply; 20+ messages in thread
From: Yann E. MORIN @ 2017-11-02 22:18 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2017-11-02 23:06 +0100, Thomas Petazzoni spake thusly:
> On Tue, 31 Oct 2017 15:36:58 -0700, Markus Mayer wrote:
> > From: Markus Mayer <mmayer@broadcom.com>
> > 
> > Add System V start-up script for dhcpcd that is run after the network
> > has been brought up.
> > 
> > Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> 
> I've applied, just two comments.
> 
> > +DAEMON=/sbin/dhcpcd
> > +CONFIG=/etc/dhcpcd.conf
> > +PIDFILE=/var/run/dhcpcd.pid
> > +
> > +[ -x $DAEMON ] || exit 0
> > +[ -f $CONFIG ] || exit 0
> 
> We have these in a lot of init scripts, so I kept them. But I find
> those tests pretty silly in fact:
> 
>  - The daemon should definitely be there, as it's installed by the same
>    package. And if it turns out not to be there, I'd prefer a loud
>    failure than an init script that silently ignores the problem.

Agreed.

>  - Same for the configuration file: I prefer a loud error than an init
>    script that ignores the problem.

Agreed, too.

However:

  - if the config file is mandatory, then this should be a failure.

  - if the config file is optional, then it missing should be silently
    ignored and the service started nonetheless.

> Yann, Peter, Arnout, can we agree that such tests should be removed
> entirely, and we let the init script fail if the daemon does not exist,
> and the daemon fail if its configuration file does not exist ?

Yes, except for the above.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-02 22:18     ` Yann E. MORIN
@ 2017-11-02 22:23       ` Thomas Petazzoni
  2017-11-03 16:10         ` Yann E. MORIN
  2017-11-04 19:56         ` Arnout Vandecappelle
  0 siblings, 2 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2017-11-02 22:23 UTC (permalink / raw)
  To: buildroot

Hello,

On Thu, 2 Nov 2017 23:18:47 +0100, Yann E. MORIN wrote:

> >  - Same for the configuration file: I prefer a loud error than an init
> >    script that ignores the problem.  
> 
> Agreed, too.
> 
> However:
> 
>   - if the config file is mandatory, then this should be a failure.
> 
>   - if the config file is optional, then it missing should be silently
>     ignored and the service started nonetheless.

So you want explicitly handling for the "missing configuration file"
situation in the init script?

I wanted to avoid explicit handling, and just let the daemon whine (or
not) if its config file is missing.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-02 22:06   ` Thomas Petazzoni
  2017-11-02 22:18     ` Yann E. MORIN
@ 2017-11-03 15:59     ` Peter Korsgaard
  1 sibling, 0 replies; 20+ messages in thread
From: Peter Korsgaard @ 2017-11-03 15:59 UTC (permalink / raw)
  To: buildroot

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 >> +[ -x $DAEMON ] || exit 0
 >> +[ -f $CONFIG ] || exit 0

 > We have these in a lot of init scripts, so I kept them. But I find
 > those tests pretty silly in fact:

 >  - The daemon should definitely be there, as it's installed by the same
 >    package. And if it turns out not to be there, I'd prefer a loud
 >    failure than an init script that silently ignores the problem.

 >  - Same for the configuration file: I prefer a loud error than an init
 >    script that ignores the problem.

 > Yann, Peter, Arnout, can we agree that such tests should be removed
 > entirely, and we let the init script fail if the daemon does not exist,
 > and the daemon fail if its configuration file does not exist ?

Sounds sensible to me.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-02 22:23       ` Thomas Petazzoni
@ 2017-11-03 16:10         ` Yann E. MORIN
  2017-11-03 16:17           ` Thomas Petazzoni
  2017-11-04 19:56         ` Arnout Vandecappelle
  1 sibling, 1 reply; 20+ messages in thread
From: Yann E. MORIN @ 2017-11-03 16:10 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2017-11-02 23:23 +0100, Thomas Petazzoni spake thusly:
> On Thu, 2 Nov 2017 23:18:47 +0100, Yann E. MORIN wrote:
> > >  - Same for the configuration file: I prefer a loud error than an init
> > >    script that ignores the problem.  
> > 
> > Agreed, too.
> > 
> > However:
> > 
> >   - if the config file is mandatory, then this should be a failure.
> > 
> >   - if the config file is optional, then it missing should be silently
> >     ignored and the service started nonetheless.
> 
> So you want explicitly handling for the "missing configuration file"
> situation in the init script?
> 
> I wanted to avoid explicit handling, and just let the daemon whine (or
> not) if its config file is missing.

The idea is that the script contains sensible defaults, but scans an
optional file (e.g. /etc/default.d/my-daemon.conf) that a user can
provide to override the defaults.

IIRC, that's what was discussed and concluded a while ago (2 years?).

I would like that users can change the behaviour of a service without
having to sed/replace the init script.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-03 16:10         ` Yann E. MORIN
@ 2017-11-03 16:17           ` Thomas Petazzoni
  2017-11-03 16:31             ` Yann E. MORIN
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Petazzoni @ 2017-11-03 16:17 UTC (permalink / raw)
  To: buildroot

Hello,

On Fri, 3 Nov 2017 17:10:57 +0100, Yann E. MORIN wrote:

> > So you want explicitly handling for the "missing configuration file"
> > situation in the init script?
> > 
> > I wanted to avoid explicit handling, and just let the daemon whine (or
> > not) if its config file is missing.  
> 
> The idea is that the script contains sensible defaults, but scans an
> optional file (e.g. /etc/default.d/my-daemon.conf) that a user can
> provide to override the defaults.
> 
> IIRC, that's what was discussed and concluded a while ago (2 years?).
> 
> I would like that users can change the behaviour of a service without
> having to sed/replace the init script.

I think we're not talking about the same thing. I'm not talking
about /etc/default/foobar containing additional shell variables to
tweak the init script behavior.

I'm talking about the configuration file read by the daemon itself
(/etc/lighttpd/lighttpd.conf for example).

For inadyn, we have:

CONFIG=/etc/inadyn.conf
[ ! -f $CONFIG ] && ( echo "The config file "$CONFIG" is missing...exiting now." && exit 2 )

So here we error out.

And for squid, we have:

[ -f /etc/squid.conf ] || exit 0

So we silently exit if there's no configuration file.

I'd like to have a consistent solution for this, and my proposal is to
do nothing in the init script, i.e start the daemon, and let it
explode/blow up/complain/whine if its configuration file is missing.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-03 16:17           ` Thomas Petazzoni
@ 2017-11-03 16:31             ` Yann E. MORIN
  0 siblings, 0 replies; 20+ messages in thread
From: Yann E. MORIN @ 2017-11-03 16:31 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2017-11-03 17:17 +0100, Thomas Petazzoni spake thusly:
> On Fri, 3 Nov 2017 17:10:57 +0100, Yann E. MORIN wrote:
> 
> > > So you want explicitly handling for the "missing configuration file"
> > > situation in the init script?
> > > 
> > > I wanted to avoid explicit handling, and just let the daemon whine (or
> > > not) if its config file is missing.  
> > 
> > The idea is that the script contains sensible defaults, but scans an
> > optional file (e.g. /etc/default.d/my-daemon.conf) that a user can
> > provide to override the defaults.
> > 
> > IIRC, that's what was discussed and concluded a while ago (2 years?).
> > 
> > I would like that users can change the behaviour of a service without
> > having to sed/replace the init script.
> 
> I think we're not talking about the same thing. I'm not talking
> about /etc/default/foobar containing additional shell variables to
> tweak the init script behavior.
> 
> I'm talking about the configuration file read by the daemon itself
> (/etc/lighttpd/lighttpd.conf for example).

Ah, I see, yes. In that case, the .mk should have installed a default
one, so we should not check.

So yes, the rule is that whatever should be installed by the .mk should
not be checked for by the initscript.

/me hands a lighted match to Thomas...
Kill it with fire!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-02 22:23       ` Thomas Petazzoni
  2017-11-03 16:10         ` Yann E. MORIN
@ 2017-11-04 19:56         ` Arnout Vandecappelle
  2017-11-04 22:03           ` Thomas Petazzoni
  2017-11-05  8:10           ` Yann E. MORIN
  1 sibling, 2 replies; 20+ messages in thread
From: Arnout Vandecappelle @ 2017-11-04 19:56 UTC (permalink / raw)
  To: buildroot



On 02-11-17 23:23, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 2 Nov 2017 23:18:47 +0100, Yann E. MORIN wrote:
> 
>>>  - Same for the configuration file: I prefer a loud error than an init
>>>    script that ignores the problem.  
>>
>> Agreed, too.
>>
>> However:
>>
>>   - if the config file is mandatory, then this should be a failure.
>>
>>   - if the config file is optional, then it missing should be silently
>>     ignored and the service started nonetheless.
> 
> So you want explicitly handling for the "missing configuration file"
> situation in the init script?
> 
> I wanted to avoid explicit handling, and just let the daemon whine (or
> not) if its config file is missing.

 I vaguely remember a recently added package where the daemon would silently
fail to start if the config file is missing, and which doesn't even have a
default config file. So I think checking for existence of the config file in the
init script (if the config file is mandatory) makes sense.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-04 19:56         ` Arnout Vandecappelle
@ 2017-11-04 22:03           ` Thomas Petazzoni
  2017-11-04 22:07             ` Arnout Vandecappelle
  2017-11-05  8:10           ` Yann E. MORIN
  1 sibling, 1 reply; 20+ messages in thread
From: Thomas Petazzoni @ 2017-11-04 22:03 UTC (permalink / raw)
  To: buildroot

Hello,

On Sat, 4 Nov 2017 20:56:55 +0100, Arnout Vandecappelle wrote:

>  I vaguely remember a recently added package where the daemon would silently
> fail to start if the config file is missing, and which doesn't even have a
> default config file. So I think checking for existence of the config file in the
> init script (if the config file is mandatory) makes sense.

So I suppose your suggestion is that the init script should abort with
a failure if the configuration file doesn't exist. Correct?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-04 22:03           ` Thomas Petazzoni
@ 2017-11-04 22:07             ` Arnout Vandecappelle
  2017-11-04 22:22               ` Thomas Petazzoni
  0 siblings, 1 reply; 20+ messages in thread
From: Arnout Vandecappelle @ 2017-11-04 22:07 UTC (permalink / raw)
  To: buildroot



On 04-11-17 23:03, Thomas Petazzoni wrote:
> Hello,
> 
> On Sat, 4 Nov 2017 20:56:55 +0100, Arnout Vandecappelle wrote:
> 
>>  I vaguely remember a recently added package where the daemon would silently
>> fail to start if the config file is missing, and which doesn't even have a
>> default config file. So I think checking for existence of the config file in the
>> init script (if the config file is mandatory) makes sense.
> 
> So I suppose your suggestion is that the init script should abort with
> a failure if the configuration file doesn't exist. Correct?

 Exactly. "Failure" anyway just means an error message on the console, not that
the system fails to boot.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-04 22:07             ` Arnout Vandecappelle
@ 2017-11-04 22:22               ` Thomas Petazzoni
  2017-11-05  8:16                 ` Yann E. MORIN
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Petazzoni @ 2017-11-04 22:22 UTC (permalink / raw)
  To: buildroot

Hello,

On Sat, 4 Nov 2017 23:07:01 +0100, Arnout Vandecappelle wrote:

>  Exactly. "Failure" anyway just means an error message on the console, not that
> the system fails to boot.

Sure, but some scripts currently just "exit 0" with no error message
whatsoever when the configuration file is missing.

So, let's summarize our policy:

 * Never check if the daemon executable is present, because that's
   silly, Buildroot guarantees it's there.

 * If the daemon needs a configuration file, always check if it exists,
   and if not, bail out with an error message.

But... wait, if Buildroot guarantees that it installs a configuration
file for this daemon, having the init script check its presence is
pretty much as silly as checking the presence of the daemon executable
itself.

So?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-04 19:56         ` Arnout Vandecappelle
  2017-11-04 22:03           ` Thomas Petazzoni
@ 2017-11-05  8:10           ` Yann E. MORIN
  2017-11-05 12:18             ` Arnout Vandecappelle
  1 sibling, 1 reply; 20+ messages in thread
From: Yann E. MORIN @ 2017-11-05  8:10 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2017-11-04 20:56 +0100, Arnout Vandecappelle spake thusly:
> On 02-11-17 23:23, Thomas Petazzoni wrote:
> > Hello,
> > 
> > On Thu, 2 Nov 2017 23:18:47 +0100, Yann E. MORIN wrote:
> > 
> >>>  - Same for the configuration file: I prefer a loud error than an init
> >>>    script that ignores the problem.  
> >>
> >> Agreed, too.
> >>
> >> However:
> >>
> >>   - if the config file is mandatory, then this should be a failure.
> >>
> >>   - if the config file is optional, then it missing should be silently
> >>     ignored and the service started nonetheless.
> > 
> > So you want explicitly handling for the "missing configuration file"
> > situation in the init script?
> > 
> > I wanted to avoid explicit handling, and just let the daemon whine (or
> > not) if its config file is missing.
> 
>  I vaguely remember a recently added package where the daemon would silently
> fail to start if the config file is missing, and which doesn't even have a
> default config file. So I think checking for existence of the config file in the
> init script (if the config file is mandatory) makes sense.

Then it is the responsibility of the package.mk to install one if it is
mandatory. Otherwise,that means the packaging is incomplete because it
does not allow the package to work as expected.

And in that case, I side with Thomas' initial suggestion: we should not
check from the init scripts, because we expect the .mk files to be
correct.

Regards,
Yann E. MORIN.

> 
>  Regards,
>  Arnout
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-04 22:22               ` Thomas Petazzoni
@ 2017-11-05  8:16                 ` Yann E. MORIN
  0 siblings, 0 replies; 20+ messages in thread
From: Yann E. MORIN @ 2017-11-05  8:16 UTC (permalink / raw)
  To: buildroot

Thomas, Arnout,  All,

On 2017-11-04 23:22 +0100, Thomas Petazzoni spake thusly:
> On Sat, 4 Nov 2017 23:07:01 +0100, Arnout Vandecappelle wrote:
> >  Exactly. "Failure" anyway just means an error message on the console, not that
> > the system fails to boot.
> 
> Sure, but some scripts currently just "exit 0" with no error message
> whatsoever when the configuration file is missing.
> 
> So, let's summarize our policy:
> 
>  * Never check if the daemon executable is present, because that's
>    silly, Buildroot guarantees it's there.
> 
>  * If the daemon needs a configuration file, always check if it exists,
>    and if not, bail out with an error message.
> 
> But... wait, if Buildroot guarantees that it installs a configuration
> file for this daemon, having the init script check its presence is
> pretty much as silly as checking the presence of the daemon executable
> itself.
> 
> So?

  * Never check if the daemon executable is present, because that's
    silly: Buildroot guarantees it's there;

  * Never check for the daemon configuration file, because that's silly:
    Buildroot guarantees it's there;

  * Check if there is a startup-script override configuration file in
    /etc/default/<foo>, and source it if present.

The /etc/default/<foo> config file is here to override the options pased
to the daemoin, for example to add more options, or to entorely replace
them. Its format is daemon-dependent (we don't want to enforce one, at
least).

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-05  8:10           ` Yann E. MORIN
@ 2017-11-05 12:18             ` Arnout Vandecappelle
  2017-11-05 13:24               ` Thomas Petazzoni
  2017-11-05 13:28               ` Yann E. MORIN
  0 siblings, 2 replies; 20+ messages in thread
From: Arnout Vandecappelle @ 2017-11-05 12:18 UTC (permalink / raw)
  To: buildroot



On 05-11-17 09:10, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2017-11-04 20:56 +0100, Arnout Vandecappelle spake thusly:
>> On 02-11-17 23:23, Thomas Petazzoni wrote:
>>> Hello,
>>>
>>> On Thu, 2 Nov 2017 23:18:47 +0100, Yann E. MORIN wrote:
>>>
>>>>>  - Same for the configuration file: I prefer a loud error than an init
>>>>>    script that ignores the problem.  
>>>>
>>>> Agreed, too.
>>>>
>>>> However:
>>>>
>>>>   - if the config file is mandatory, then this should be a failure.
>>>>
>>>>   - if the config file is optional, then it missing should be silently
>>>>     ignored and the service started nonetheless.
>>>
>>> So you want explicitly handling for the "missing configuration file"
>>> situation in the init script?
>>>
>>> I wanted to avoid explicit handling, and just let the daemon whine (or
>>> not) if its config file is missing.
>>
>>  I vaguely remember a recently added package where the daemon would silently
>> fail to start if the config file is missing, and which doesn't even have a
>> default config file. So I think checking for existence of the config file in the
>> init script (if the config file is mandatory) makes sense.
> 
> Then it is the responsibility of the package.mk to install one if it is
> mandatory. Otherwise,that means the packaging is incomplete because it
> does not allow the package to work as expected.

 So you mean all packages should do like inadyn: install an example
configuration file even if that is guaranteed not to work.

 Regards,
 Arnout

> 
> And in that case, I side with Thomas' initial suggestion: we should not
> check from the init scripts, because we expect the .mk files to be
> correct.
> 
> Regards,
> Yann E. MORIN.
> 
>>
>>  Regards,
>>  Arnout
>>
>> -- 
>> Arnout Vandecappelle                          arnout at mind be
>> Senior Embedded Software Architect            +32-16-286500
>> Essensium/Mind                                http://www.mind.be
>> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
>> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
>> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-05 12:18             ` Arnout Vandecappelle
@ 2017-11-05 13:24               ` Thomas Petazzoni
  2017-11-05 13:28               ` Yann E. MORIN
  1 sibling, 0 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2017-11-05 13:24 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 5 Nov 2017 13:18:29 +0100, Arnout Vandecappelle wrote:

> > Then it is the responsibility of the package.mk to install one if it is
> > mandatory. Otherwise,that means the packaging is incomplete because it
> > does not allow the package to work as expected.  
> 
>  So you mean all packages should do like inadyn: install an example
> configuration file even if that is guaranteed not to work.

Yes, I think it's a good idea if packages install a least some
example/template configuration file, even if indeed it may not work out
of the box in cases such as inadyn.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script
  2017-11-05 12:18             ` Arnout Vandecappelle
  2017-11-05 13:24               ` Thomas Petazzoni
@ 2017-11-05 13:28               ` Yann E. MORIN
  1 sibling, 0 replies; 20+ messages in thread
From: Yann E. MORIN @ 2017-11-05 13:28 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2017-11-05 13:18 +0100, Arnout Vandecappelle spake thusly:
> On 05-11-17 09:10, Yann E. MORIN wrote:
> > Then it is the responsibility of the package.mk to install one if it is
> > mandatory. Otherwise,that means the packaging is incomplete because it
> > does not allow the package to work as expected.
>  So you mean all packages should do like inadyn: install an example
> configuration file even if that is guaranteed not to work.

Yes, if at least because it serves as a base for the user to write their
own or fill-in the blanks.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-11-05 13:28 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-31 22:36 [Buildroot] [PATCH 0/2] dhcpcd: start-up scripts Markus Mayer
2017-10-31 22:36 ` [Buildroot] [PATCH 1/2] dhcpcd: add SysV start-up script Markus Mayer
2017-11-02 22:06   ` Thomas Petazzoni
2017-11-02 22:18     ` Yann E. MORIN
2017-11-02 22:23       ` Thomas Petazzoni
2017-11-03 16:10         ` Yann E. MORIN
2017-11-03 16:17           ` Thomas Petazzoni
2017-11-03 16:31             ` Yann E. MORIN
2017-11-04 19:56         ` Arnout Vandecappelle
2017-11-04 22:03           ` Thomas Petazzoni
2017-11-04 22:07             ` Arnout Vandecappelle
2017-11-04 22:22               ` Thomas Petazzoni
2017-11-05  8:16                 ` Yann E. MORIN
2017-11-05  8:10           ` Yann E. MORIN
2017-11-05 12:18             ` Arnout Vandecappelle
2017-11-05 13:24               ` Thomas Petazzoni
2017-11-05 13:28               ` Yann E. MORIN
2017-11-03 15:59     ` Peter Korsgaard
2017-10-31 22:36 ` [Buildroot] [PATCH 2/2] dhcpcd: add systemd start-up service Markus Mayer
2017-11-02 22:06   ` Thomas Petazzoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.