[Buildroot] [PATCH v2 0/6] Update init script style

[Buildroot] [PATCH v2 0/6] Update init script style

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

As discussed on the mailing list [1] init scripts may rely on the
default Buildroot options for Busybox and other base components being
enabled. This allows using long form options to start-stop-daemon in
init scripts for readability, among other things.

This series:

* Documents reliance on Buildroot defaults in the manual (patch 1)

* Updates the reference S01syslogd script to follow this and other
  good practices (patch 2 & 3)

* Updates a few other scripts I've had to do with recently to follow
  the same style.

[1] https://lore.kernel.org/buildroot/6d02d2d5-e71e-45b3-a447-e81521085e7a@mind.be/

Changes v1 -> v2:
* Update the manual paragraph below the reference script to match its
  style (patch 3)
* Use start-stop-daemon --stop --test to check if the service is still
  running during stop (where the service does not delete its PID file
  on exit, patches 2 and 5), instead of poking at procfs

Fiona Klute (WIWA) (6):
  docs/manual: describe relying on default options
  package/busybox: tidy up S01syslogd init script
  docs/manual: include S01syslogd from source
  package/openssh: tidy up init script
  package/dnsmasq: tidy up init script
  package/network-manager: rewrite init script

 .checkpackageignore                        |  1 -
 docs/manual/adding-packages-directory.adoc | 65 ++-------------------
 docs/manual/integration-principles.adoc    | 20 +++++++
 docs/manual/integration.adoc               |  2 +
 docs/manual/manual.mk                      |  5 ++
 package/busybox/S01syslogd                 | 18 ++++--
 package/dnsmasq/S80dnsmasq                 | 64 +++++++++++++-------
 package/network-manager/S45NetworkManager  | 68 ++++++++++++++++++++++
 package/network-manager/S45network-manager | 41 -------------
 package/network-manager/network-manager.mk |  2 +-
 package/openssh/S50sshd                    | 12 ++--
 11 files changed, 163 insertions(+), 135 deletions(-)
 create mode 100644 docs/manual/integration-principles.adoc
 create mode 100644 package/network-manager/S45NetworkManager
 delete mode 100644 package/network-manager/S45network-manager

--
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 1/6] docs/manual: describe relying on default options

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

People can assume that e.g. Busybox options enabled in the package are
enabled when writing code for Buildroot. Anyone who uses custom
configurations that disable default options needs to make sure
relevant scripts etc. still work for themselves.

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
 docs/manual/integration-principles.adoc | 20 ++++++++++++++++++++
 docs/manual/integration.adoc            |  2 ++
 2 files changed, 22 insertions(+)
 create mode 100644 docs/manual/integration-principles.adoc

diff --git a/docs/manual/integration-principles.adoc b/docs/manual/integration-principles.adoc
new file mode 100644
index 0000000000..40c049f0d3
--- /dev/null
+++ b/docs/manual/integration-principles.adoc
@@ -0,0 +1,20 @@
+// -*- mode:doc; -*-
+// vim: set syntax=asciidoc:
+
+[[integration-principles]]
+=== Integration Principles
+
+Some foundational packages like Busybox and uClibc can be configured
+with or without certain features. When writing Buildroot code that
+uses such packages, contributors may assume that the options enabled
+in the Buildroot-provided configurations are enabled. For example,
++package/busybox/busybox.config+ sets
++CONFIG_FEATURE_START_STOP_DAEMON_LONG_OPTIONS=y+, so init scripts
+meant for use with Busybox init may use +start-stop-daemon+ with long
+form options.
+
+People who use custom configurations that disable such default options
+are responsible for making sure that any relevant scripts/packages
+still work, and if not, adapting them accordingly. To follow the
+Busybox example above, disabling long form options will require
+replacing init scripts that use them (in an overlay).
diff --git a/docs/manual/integration.adoc b/docs/manual/integration.adoc
index 1626dd75dd..5f408dca29 100644
--- a/docs/manual/integration.adoc
+++ b/docs/manual/integration.adoc
@@ -9,6 +9,8 @@ level. Buildroot is highly configurable, almost everything discussed
 here can be changed or overridden by xref:rootfs-custom[rootfs overlay
 or custom skeleton] configuration.

+include::integration-principles.adoc[]
+
 include::integration-systemd.adoc[]

 include::integration-selinux-support.adoc[]
--
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/6] docs/manual: describe relying on default options

[Thomas Petazzoni via buildroot]

Hello Fiona,

On Fri, 12 Jul 2024 14:49:51 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> 
> People can assume that e.g. Busybox options enabled in the package are
> enabled when writing code for Buildroot. Anyone who uses custom
> configurations that disable default options needs to make sure
> relevant scripts etc. still work for themselves.
> 
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>

Thanks a lot for this work. I applied your patch, with one change.

> +[[integration-principles]]
> +=== Integration Principles

I wasn't happy with the name "Integration Principles", which was too
vague, while the other sections in this chapter are more specific. So
instead I named this "Configurable packages". Of course, I renamed the
file to match that title change.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 2/6] package/busybox: tidy up S01syslogd init script

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

The manual refers to this script as a reference of how init scripts
should be written. The changes are:

* Use long form options for start-stop-daemon for clarity
* Use --exec on stop to ensure the right process gets stopped
* Avoid --quiet for clearer messages on failure
* Wait for the process to be gone during stop
* Avoid fixed wait between start and stop on restart

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
Changes v1 -> v2:
* Use start-stop-daemon --stop --test to check if the service is still
  running during stop, instead of poking at procfs

 package/busybox/S01syslogd | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/package/busybox/S01syslogd b/package/busybox/S01syslogd
index 15006bc06f..d3530e1f14 100644
--- a/package/busybox/S01syslogd
+++ b/package/busybox/S01syslogd
@@ -9,11 +9,12 @@ SYSLOGD_ARGS=""
 [ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"

 # BusyBox' syslogd does not create a pidfile, so pass "-n" in the command line
-# and use "-m" to instruct start-stop-daemon to create one.
+# and use "--make-pidfile" to instruct start-stop-daemon to create one.
 start() {
 	printf 'Starting %s: ' "$DAEMON"
 	# shellcheck disable=SC2086 # we need the word splitting
-	start-stop-daemon -b -m -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
+	start-stop-daemon --start --background --make-pidfile \
+		--pidfile "$PIDFILE" --exec "/sbin/$DAEMON" \
 		-- -n $SYSLOGD_ARGS
 	status=$?
 	if [ "$status" -eq 0 ]; then
@@ -26,20 +27,27 @@ start() {

 stop() {
 	printf 'Stopping %s: ' "$DAEMON"
-	start-stop-daemon -K -q -p "$PIDFILE"
+	start-stop-daemon --stop --pidfile "$PIDFILE" --exec "/sbin/$DAEMON"
 	status=$?
 	if [ "$status" -eq 0 ]; then
-		rm -f "$PIDFILE"
 		echo "OK"
 	else
 		echo "FAIL"
+		return "$status"
 	fi
+	# Wait for process to be gone, using a loop and --stop --test
+	# because Busybox' start-stop-daemon does not support --retry
+	# (as of 1.36.1).
+	while start-stop-daemon --stop --test --quiet --pidfile "$PIDFILE" \
+		--exec "/sbin/$DAEMON"; do
+		sleep 0.1
+	done
+	rm -f "$PIDFILE"
 	return "$status"
 }

 restart() {
 	stop
-	sleep 1
 	start
 }

--
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 2/6] package/busybox: tidy up S01syslogd init script

[Thomas Petazzoni via buildroot]

Hello Fiona,

On Fri, 12 Jul 2024 14:49:52 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> 
> The manual refers to this script as a reference of how init scripts
> should be written. The changes are:
> 
> * Use long form options for start-stop-daemon for clarity
> * Use --exec on stop to ensure the right process gets stopped
> * Avoid --quiet for clearer messages on failure
> * Wait for the process to be gone during stop
> * Avoid fixed wait between start and stop on restart
> 
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>

Applied, with one change. See below.

> +	# Wait for process to be gone, using a loop and --stop --test
> +	# because Busybox' start-stop-daemon does not support --retry
> +	# (as of 1.36.1).

I dropped this comment, which isn't really useful, because it basically
repeats what the code is doing. The fact that we can't do X or Y or Z
isn't very useful, and I didn't want to see this comment duplicated
into each and every init script.

In fact if you want to include this information, I believe the
Buildroot manual would be a better place, in the section where we
describe a canonical init script.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 3/6] docs/manual: include S01syslogd from source

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

The manual describes package/busybox/S01syslogd as the reference of
how an init script should be written. Include it from source instead
of having a copy in the manual to ensure actual code and manual stay
in sync.

Also use long options in the paragraph after the script to follow the
same style.

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
Changes v1 -> v2:
* Update the manual paragraph below the reference script to match its
  style

 docs/manual/adding-packages-directory.adoc | 65 ++--------------------
 docs/manual/manual.mk                      |  5 ++
 2 files changed, 11 insertions(+), 59 deletions(-)

diff --git a/docs/manual/adding-packages-directory.adoc b/docs/manual/adding-packages-directory.adoc
index 0b7221aae0..0335e5a37c 100644
--- a/docs/manual/adding-packages-directory.adoc
+++ b/docs/manual/adding-packages-directory.adoc
@@ -578,70 +578,17 @@ not start before +S40network+.  The scripts are started in alphabetical
 order, so +S01syslogd+ starts before +S01watchdogd+, and +S02sysctl+
 start thereafter.

+[source,sh]
 ------------------------------
-01: #!/bin/sh
-02:
-03: DAEMON="syslogd"
-04: PIDFILE="/var/run/$DAEMON.pid"
-05:
-06: SYSLOGD_ARGS=""
-07:
-08: # shellcheck source=/dev/null
-09: [ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
-10:
-11: # BusyBox' syslogd does not create a pidfile, so pass "-n" in the command line
-12: # and use "-m" to instruct start-stop-daemon to create one.
-13: start() {
-14: 	printf 'Starting %s: ' "$DAEMON"
-15: 	# shellcheck disable=SC2086 # we need the word splitting
-16: 	start-stop-daemon -b -m -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
-17: 		-- -n $SYSLOGD_ARGS
-18: 	status=$?
-19: 	if [ "$status" -eq 0 ]; then
-20: 		echo "OK"
-21: 	else
-22: 		echo "FAIL"
-23: 	fi
-24: 	return "$status"
-25: }
-26:
-27: stop() {
-28: 	printf 'Stopping %s: ' "$DAEMON"
-29: 	start-stop-daemon -K -q -p "$PIDFILE"
-30: 	status=$?
-31: 	if [ "$status" -eq 0 ]; then
-32: 		rm -f "$PIDFILE"
-33: 		echo "OK"
-34: 	else
-35: 		echo "FAIL"
-36: 	fi
-37: 	return "$status"
-38: }
-39:
-40: restart() {
-41: 	stop
-42: 	sleep 1
-43: 	start
-44: }
-45:
-46: case "$1" in
-47: 	start|stop|restart)
-48: 		"$1";;
-49: 	reload)
-50: 		# Restart, since there is no true "reload" feature.
-51: 		restart;;
-52: 	*)
-53: 		echo "Usage: $0 {start|stop|restart|reload}"
-54: 		exit 1
-55: esac
+include::S01syslogd[]
 ------------------------------

 *Note:* programs that support reloading their configuration in some
 fashion (+SIGHUP+) should provide a +reload()+ function similar to
-+stop()+.  The +start-stop-daemon+ supports +-K -s HUP+ for this.
-It is recommended to always append +-x "/sbin/$DAEMON"+ to all the
-+start-stop-daemon+ commands to ensure signals are set to a PID that
-matches +$DAEMON+.
++stop()+. The +start-stop-daemon+ command supports +--stop --signal
+HUP+ for this. It is recommended to always append +--exec
+"/sbin/$DAEMON"+ to all +start-stop-daemon+ commands to ensure signals
+are set to a PID that matches +$DAEMON+.

 Both start scripts and unit files can source command line arguments from
 +/etc/default/foo+, in general, if such a file does not exist it should
diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
index e851a084ae..ca5a14f838 100644
--- a/docs/manual/manual.mk
+++ b/docs/manual/manual.mk
@@ -7,4 +7,9 @@
 MANUAL_SOURCES = $(sort $(wildcard docs/manual/*.adoc) $(wildcard docs/images/*))
 MANUAL_RESOURCES = $(TOPDIR)/docs/images

+define MANUAL_INIT_SCRIPT_REF
+	cp package/busybox/S01syslogd $(@D)/S01syslogd
+endef
+MANUAL_POST_RSYNC_HOOKS += MANUAL_INIT_SCRIPT_REF
+
 $(eval $(call asciidoc-document))
--
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 3/6] docs/manual: include S01syslogd from source

[Thomas Petazzoni via buildroot]

On Fri, 12 Jul 2024 14:49:53 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> 
> The manual describes package/busybox/S01syslogd as the reference of
> how an init script should be written. Include it from source instead
> of having a copy in the manual to ensure actual code and manual stay
> in sync.
> 
> Also use long options in the paragraph after the script to follow the
> same style.
> 
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
> ---
> Changes v1 -> v2:
> * Update the manual paragraph below the reference script to match its
>   style

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 4/6] package/openssh: tidy up init script

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

* Use long options for start-stop-daemon

* Avoid --quiet, except for reload where the "stopping" message could
  be confusing

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
 package/openssh/S50sshd | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/package/openssh/S50sshd b/package/openssh/S50sshd
index d16a7ad8fc..56db6ebff9 100644
--- a/package/openssh/S50sshd
+++ b/package/openssh/S50sshd
@@ -16,8 +16,8 @@ start() {
 	/usr/bin/ssh-keygen -A

 	printf "Starting %s: " "$DAEMON"
-	start-stop-daemon -S -q -p "$PIDFILE" \
-		-x "/usr/sbin/$DAEMON"
+	start-stop-daemon --start --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON"
 	status=$?
 	if [ "$status" -eq 0 ]; then
 		echo "OK"
@@ -29,8 +29,8 @@ start() {

 stop() {
 	printf "Stopping sshd: "
-	start-stop-daemon -K -q -p "$PIDFILE" \
-		-x "/usr/sbin/$DAEMON"
+	start-stop-daemon --stop --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON"
 	status=$?
 	if [ "$status" -eq 0 ]; then
 		echo "OK"
@@ -51,8 +51,8 @@ restart() {

 reload() {
 	printf "Reloading sshd config: "
-	start-stop-daemon -K --signal HUP -q -p "$PIDFILE" \
-		-x "/usr/sbin/$DAEMON"
+	start-stop-daemon --stop --signal HUP -q --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON"
 	status=$?
 	if [ "$status" -eq 0 ]; then
 		echo "OK"
--
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 4/6] package/openssh: tidy up init script

[Thomas Petazzoni via buildroot]

On Fri, 12 Jul 2024 14:49:54 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> 
> * Use long options for start-stop-daemon
> 
> * Avoid --quiet, except for reload where the "stopping" message could
>   be confusing
> 
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
> ---
>  package/openssh/S50sshd | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 5/6] package/dnsmasq: tidy up init script

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

* Create start/stop/restart functions

* Use long start-stop-daemon options, avoid --quiet

* Return start-stop-daemon exit code from start/stop functions

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
Changes v1 -> v2:
* Use start-stop-daemon --stop --test to check if the service is still
  running during stop, instead of poking at procfs

 package/dnsmasq/S80dnsmasq | 64 +++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 22 deletions(-)

diff --git a/package/dnsmasq/S80dnsmasq b/package/dnsmasq/S80dnsmasq
index f1e1a68585..07fa205b83 100644
--- a/package/dnsmasq/S80dnsmasq
+++ b/package/dnsmasq/S80dnsmasq
@@ -5,30 +5,50 @@ PIDFILE="/var/run/$DAEMON.pid"

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

+start() {
+	printf "Starting dnsmasq: "
+	start-stop-daemon --start --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON" \
+		-- --pid-file="$PIDFILE"
+	status=$?
+	[ "$status" -eq 0 ] && echo "OK" || echo "FAIL"
+	return "$status"
+}
+
+stop() {
+	printf "Stopping dnsmasq: "
+	start-stop-daemon --stop --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+		return "$status"
+	fi
+	# Wait for process to be gone, using a loop and --stop --test
+	# because Busybox' start-stop-daemon does not support --retry
+	# (as of 1.36.1).
+	while start-stop-daemon --stop --test --quiet --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON"; do
+		sleep 0.1
+	done
+	rm -f "$PIDFILE"
+	return "$status"
+}
+
+restart() {
+	stop
+	start
+}
+
 case "$1" in
-	start)
-		printf "Starting dnsmasq: "
-		start-stop-daemon -S -p "$PIDFILE" -x "/usr/sbin/$DAEMON" -- \
-			--pid-file="$PIDFILE"
-		# shellcheck disable=SC2181
-		[ $? = 0 ] && echo "OK" || echo "FAIL"
-		;;
-	stop)
-		printf "Stopping dnsmasq: "
-		start-stop-daemon -K -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON"
-		# shellcheck disable=SC2181
-		[ $? = 0 ] && echo "OK" || echo "FAIL"
-		# wait for dnsmasq process to be gone
-		while true; do
-			pid="$( cat "${PIDFILE}" 2>/dev/null || true )"
-			{ [ -n "${pid}" ] && [ -d "/proc/${pid}" ]; } || break
-			sleep 0.1
-		done
-		rm -f "$PIDFILE"
+	start|stop|restart)
+		"$1"
 		;;
-	restart|reload)
-		$0 stop
-		$0 start
+	reload)
+		# Restart, since there is no true "reload" feature.
+		restart
 		;;
 	*)
 		echo "Usage: $0 {start|stop|restart}"
--
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 5/6] package/dnsmasq: tidy up init script

[Thomas Petazzoni via buildroot]

Hello Fiona,

On Fri, 12 Jul 2024 14:49:55 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> +	# Wait for process to be gone, using a loop and --stop --test
> +	# because Busybox' start-stop-daemon does not support --retry
> +	# (as of 1.36.1).

Applied after dropping this comment, for consistency.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 6/6] package/network-manager: rewrite init script

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

This brings the NetworkManager init script in line with the standard
Buildroot init script pattern, including the script name.

Reload using SIGHUP is also supported now, note that the
NetworkManager documentation cautions that not all parameters can be
changed at runtime (without a full restart).

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
 .checkpackageignore                        |  1 -
 package/network-manager/S45NetworkManager  | 68 ++++++++++++++++++++++
 package/network-manager/S45network-manager | 41 -------------
 package/network-manager/network-manager.mk |  2 +-
 4 files changed, 69 insertions(+), 43 deletions(-)
 create mode 100644 package/network-manager/S45NetworkManager
 delete mode 100644 package/network-manager/S45network-manager

diff --git a/.checkpackageignore b/.checkpackageignore
index 7cb4b98bfc..d39b5bdebd 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -1020,7 +1020,6 @@ package/netsurf/0002-do-not-cross-compile-nsgenbind.patch lib_patch.Upstream
 package/netsurf/0003-fix-compilation-without-curl.patch lib_patch.Upstream
 package/netsurf/0004-framebuffer-Fix-internal-font-generated-source-for-GCC-10.patch lib_patch.Upstream
 package/nettle/0001-disable-testsuite-examples.patch lib_patch.Upstream
-package/network-manager/S45network-manager Shellcheck lib_sysv.ConsecutiveEmptyLines lib_sysv.EmptyLastLine lib_sysv.Variables
 package/nfs-utils/S60nfs Shellcheck lib_sysv.ConsecutiveEmptyLines lib_sysv.Variables
 package/nginx-modsecurity/0001-config-use-pkg-config.patch lib_patch.Upstream
 package/nginx/0001-auto-type-sizeof-rework-autotest-to-be-cross-compila.patch lib_patch.Upstream
diff --git a/package/network-manager/S45NetworkManager b/package/network-manager/S45NetworkManager
new file mode 100644
index 0000000000..9f3398a347
--- /dev/null
+++ b/package/network-manager/S45NetworkManager
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+DAEMON="NetworkManager"
+PIDFILE="/var/run/$DAEMON.pid"
+
+# shellcheck source=/dev/null
+test -r "/etc/default/$DAEMON" && . "/etc/default/$DAEMON"
+
+start() {
+	printf 'Starting %s: ' "$DAEMON"
+	[ ! -d "/var/run/$DAEMON" ] && install -d "/var/run/$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon --start --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON" \
+		-- --pid-file="$PIDFILE" $NETWORKMANAGER_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+stop() {
+	printf 'Stopping %s: ' "$DAEMON"
+	start-stop-daemon --stop --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+		return "$status"
+	fi
+	# NetworkManager deletes its PID file on exit, wait for it to
+	# be gone
+	while [ -f "$PIDFILE" ]; do
+		sleep 0.1
+	done
+	return "$status"
+}
+
+restart() {
+	stop
+	start
+}
+
+reload() {
+	printf 'Reloading %s config: ' "$DAEMON"
+	start-stop-daemon --stop --signal HUP -q --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+case "$1" in
+	start|stop|restart|reload)
+		"$1";;
+	*)
+		echo "Usage: $0 {start|stop|restart|reload}"
+		exit 1
+esac
diff --git a/package/network-manager/S45network-manager b/package/network-manager/S45network-manager
deleted file mode 100644
index bc775d3b20..0000000000
--- a/package/network-manager/S45network-manager
+++ /dev/null
@@ -1,41 +0,0 @@
-#!/bin/sh
-
-# Allow a few customizations from a config file
-test -r /etc/default/NetworkManager && . /etc/default/NetworkManager
-
-PID=`pidof NetworkManager`
-case "$1" in
-	start)
-		printf "Starting NetworkManager ... "
-		[ ! -d /var/run/NetworkManager ] && install -d /var/run/NetworkManager
-		if [ -z "$PID" ]; then
-			/usr/sbin/NetworkManager $NETWORKMANAGER_ARGS
-		fi
-		if [ ! -z "$PID" -o $? -gt 0 ]; then
-			echo "failed!"
-		else
-			echo "done."
-		fi
-		;;
-	stop)
-		printf "Stopping NetworkManager ... "
-			[ ! -z "$PID" ] && kill $PID > /dev/null 2>&1
-		if [ $? -gt 0 ]; then
-			echo "failed!"
-		else
-			echo "done."
-		fi
-		;;
-	restart)
-		$0 stop
-		sleep 1
-		$0 start
-		;;
-	*)
-		echo "usage: $0 {start|stop|restart|sleep|wake}"
-		;;
-esac
-exit 0
-
-
-
diff --git a/package/network-manager/network-manager.mk b/package/network-manager/network-manager.mk
index cbd2ba8839..3937982c5f 100644
--- a/package/network-manager/network-manager.mk
+++ b/package/network-manager/network-manager.mk
@@ -163,7 +163,7 @@ NETWORK_MANAGER_CONF_OPTS += -Dnmcli=false
 endif

 define NETWORK_MANAGER_INSTALL_INIT_SYSV
-	$(INSTALL) -m 0755 -D package/network-manager/S45network-manager $(TARGET_DIR)/etc/init.d/S45network-manager
+	$(INSTALL) -m 0755 -D package/network-manager/S45NetworkManager $(TARGET_DIR)/etc/init.d/S45NetworkManager
 endef

 define NETWORK_MANAGER_INSTALL_INIT_SYSTEMD
--
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 6/6] package/network-manager: rewrite init script

[Thomas Petazzoni via buildroot]

On Fri, 12 Jul 2024 14:49:56 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> 
> This brings the NetworkManager init script in line with the standard
> Buildroot init script pattern, including the script name.
> 
> Reload using SIGHUP is also supported now, note that the
> NetworkManager documentation cautions that not all parameters can be
> changed at runtime (without a full restart).
> 
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
> ---
>  .checkpackageignore                        |  1 -
>  package/network-manager/S45NetworkManager  | 68 ++++++++++++++++++++++
>  package/network-manager/S45network-manager | 41 -------------
>  package/network-manager/network-manager.mk |  2 +-
>  4 files changed, 69 insertions(+), 43 deletions(-)
>  create mode 100644 package/network-manager/S45NetworkManager
>  delete mode 100644 package/network-manager/S45network-manager

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 0/6] Update init script style

[Thomas Petazzoni via buildroot]

Hello Fiona,

On Fri, 12 Jul 2024 14:49:50 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> Fiona Klute (WIWA) (6):
>   docs/manual: describe relying on default options
>   package/busybox: tidy up S01syslogd init script
>   docs/manual: include S01syslogd from source
>   package/openssh: tidy up init script
>   package/dnsmasq: tidy up init script
>   package/network-manager: rewrite init script

Thanks a lot for this, all are applied.

Now, when reviewing this and discussing with Arnout, we thought it
would be nice to have a little bit more documentation. Indeed, while
looking at the different init scripts in this series, we can identify 3
different cases:

- The S01syslogd case, where the daemon doesn't create a PID file, so
  we use --make-pidfile + --background so that start-stop-daemon
  creates the PID file and is in charge of daemonizing the daemon

  We use a loop with start-stop-daemon to check if the service has
  stopped

- The S80dnsmasq case, where the daemon creates its PID file, so we
  don't pass --make-pidfile + --background.

  But dnsmasq doesn't remove its PID file on stop, so we also use a
  start-stop-daemon loop to check if the service has stopped

- The S50sshd or S45NetworkManager cases, where the daemon creates its
  PID file, so we don't pass --make-pidfile + --background (same as
  S80dnsmasq)

  However, those daemons remove their PID file on stop, so we cannot
  use a start-stop-daemon loop, but instead loop until the PID file
  goes away

So essentially, the manual should have guidelines like this:

- If your service doesn't create its own PID file: invoke the daemon in
  foreground mode, and use start-stop-daemon --make-pidfile --background

  snippet of code

- If your service creates its own PID file: pass the --pidfile option
  to both start-stop-daemon *and the daemon itself so they agree on
  where the PID file is

  snippet of code

- If your service removes its PID file on shutdown, use a loop testing
  that the PID file has disappeared on stop, see S50sshd or
  S45NetworkManager for example:
 
  snippet of code

- If your service doesn't remove its PID file on shutdown, use a loop
  of start-stop-daemon, see S01syslogd for example:

  snippet of code

That would be *tremendously* useful to have in the documentation.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot