conf: support list of candidate commands for services

A daemon known under more than one name had to be declared once per
name, each stanza carrying nowarn so the ones that were not installed
were skipped quietly.  That leaned on the line-based format having no
titles.  With a title as the service identity the repetition is no
longer available, so the candidates move inside the block:

    command = { "/lib/systemd/systemd-udevd", "-udevd" }

Finit starts the first one whose binary resolves.  A candidate that is
not installed is expected here, so nothing is logged for the ones that
lose, and only the winner is looked up again by service_register().
The leading '-' keeps its meaning and is read from the candidate that
wins, or from the last one when none resolve.

libconfuse accepts a bare string for a list option, so the common
`command = "prog args"` is unchanged, and whichp() already skips any
arguments to the command, so the candidate goes to it as written.

tty blocks take the same list.  Their command is the getty to run, and
it has the same reason to name alternatives.  Both block types now
share svc_command(), which also puts the leading '-' rule back in one
place: tty_translate() had its own copy.  A tty needs no command at
all, it may name a device or notty instead, so svc_command() returns
NULL for an empty section and each caller decides whether that is an
error.

system/10-hotplug.conf.in returns to one udevd block, which is what it
meant all along.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

src/conf.c

@@ -232,7 +232,12 @@ static cfg_opt_t log_opts[] = {
 static cfg_opt_t svc_opts[] = {
 	CFG_STR     ("description",  NULL, CFGF_NODEFAULT),
 	CFG_STR     ("desc",         NULL, CFGF_NODEFAULT),	/* alias */
-	CFG_STR     ("command",      NULL, CFGF_NODEFAULT),
+	/*
+	 * A list, but libconfuse takes a bare string for one as well, so
+	 * the common `command = "prog args"` is unaffected.  See
+	 * svc_command().
+	 */
+	CFG_STR_LIST("command",      NULL, CFGF_NODEFAULT),
 	CFG_STR     ("runlevel",     NULL, CFGF_NODEFAULT),
 	CFG_STR_LIST("conditions",   NULL, CFGF_NODEFAULT),
 	CFG_STR_LIST("cond",         NULL, CFGF_NODEFAULT),	/* alias */
@@ -307,7 +312,7 @@ static cfg_opt_t tty_opts[] = {
 	CFG_BOOL    ("noclear",   cfg_false, CFGF_NODEFAULT),
 	CFG_BOOL    ("nowait",    cfg_false, CFGF_NODEFAULT),
 	CFG_BOOL    ("nologin",   cfg_false, CFGF_NODEFAULT),
-	CFG_STR     ("command",   NULL, CFGF_NODEFAULT),
+	CFG_STR_LIST("command",   NULL, CFGF_NODEFAULT),	/* candidates, see svc_command() */
 	CFG_BOOL    ("notty",     cfg_false, CFGF_NODEFAULT),
 	CFG_BOOL    ("rescue",    cfg_false, CFGF_NODEFAULT),
 	CFG_END()
@@ -1105,6 +1110,46 @@ static int sec_getbool(cfg_t *sec, const char *key, const char *alias)
 	return cfg_getbool(sec, key) == cfg_true;
 }
 
+/*
+ * Pick the command for a service.  Several may be listed, in which
+ * case they are candidates for the same service and the first one
+ * whose binary resolves wins:
+ *
+ *     command = { "/lib/systemd/systemd-udevd", "udevd" }
+ *
+ * The line-based format could only spell this as one stanza per
+ * candidate, relying on nowarn to skip the ones that were not
+ * installed.  A candidate not being there is not an error here, so
+ * only the winner is looked up again by service_register().
+ *
+ * A leading '-' still says a missing binary is expected, cf. systemd
+ * ExecStart=-.
+ *
+ * Returns NULL when the section has no command at all, which is an
+ * error for a service but not for a tty, so the caller decides.
+ */
+static const char *svc_command(cfg_t *sec, int *nowarn)
+{
+	const char *cand = NULL;
+	unsigned int i, num;
+
+	num = cfg_size(sec, "command");
+	if (!num)
+		return NULL;
+
+	for (i = 0; i < num; i++) {
+		cand = cfg_getnstr(sec, "command", i);
+		*nowarn = cand[0] == '-';
+
+		/* which() skips any arguments to the command */
+		if (whichp(&cand[*nowarn]))
+			break;
+	}
+
+	/* the winner, or the last candidate when none of them resolve */
+	return &cand[*nowarn];
+}
+
 /*
  * Append token to the one-liner being built, space separated.
  */
@@ -1399,18 +1444,13 @@ static void svc_translate(cfg_t *sec, int type, struct rlimit rlimit[], char *fi
 	svc_t *svc;
 	char *id;
 
-	cmd = sec_getstr(sec, "command", NULL);
+	cmd = svc_command(sec, &nowarn);
 	if (!cmd) {
 		logit(LOG_ERR, "%s: section '%s' missing command, skipping",
 		      file, cfg_title(sec));
 		return;
 	}
 
-	/* a leading - tolerates a missing binary, cf. systemd ExecStart=- */
-	nowarn = cmd[0] == '-';
-	if (nowarn)
-		cmd++;
-
 	if ((str = sec_getstr(sec, "runlevel", NULL)))
 		addtok(line, sizeof(line), "[%s]", str);
 
@@ -1614,6 +1654,7 @@ static void tty_translate(cfg_t *sec, struct rlimit rlimit[], char *file)
 {
 	char line[LINE_SIZE] = "";
 	const char *str, *dev, *cmd;
+	int nowarn = 0;
 	char buf[512];
 
 	if ((str = sec_getstr(sec, "runlevel", NULL)))
@@ -1623,11 +1664,9 @@ static void tty_translate(cfg_t *sec, struct rlimit rlimit[], char *file)
 		addtok(line, sizeof(line), "<%s>", buf);
 
 	dev = sec_getstr(sec, "device", NULL);
-	cmd = sec_getstr(sec, "command", NULL);
+	cmd = svc_command(sec, &nowarn);
-	if (cmd && cmd[0] == '-') {
+	if (nowarn)
 		addtok(line, sizeof(line), "nowarn");
-		cmd++;
-	}
 
 	if (dev) {
 		addtok(line, sizeof(line), "%s", dev);

system/10-hotplug.conf.in

@@ -13,21 +13,25 @@
 #        command     = "syslogd -F"
 #    }
 #
-# with `if = "mdev"` or `if = "mdevd"` and the matching coldplug condition
+# This provides a pid/syslogd condition that can act as a barrier for all
-# for those variants.
-#
-# This provdes a <pid/syslogd> condition that can act as a barrier for all
 # other services.  Notice the `if` setting: the condition for starting
 # syslogd is only considered if either udevd or mdev (service) is loaded and
 # is guaranteed to run after each respective run stanza have completed.
 #
+# The mdev and mdevd systems wait for a different coldplug condition, so they
+# need their own block, and a block title is a service identity: two blocks
+# titled syslogd in one file are two declarations of one service, which Finit
+# rejects.  Give each variant its own file, or keep them as one-liners in the
+# line-based format, which has no titles and allows the repetition.  See
+# doc/config/migration.md for both.
+#
 # Override this file by copying it to /etc/finit.d/, using the same name, then
 # change the contents any way you like, it can even be empty.
-#
-# The leading '-' on every command says a missing binary is expected here, so
-# the block is skipped quietly and the next candidate gets its turn.
 
 # Check for systemd-udevd and eudev, if we find both, we opt for the latter.
+# Both spellings are candidates for the same service, Finit starts the first
+# one it finds.  A leading '-' on the last says it is fine if neither is
+# installed, we fall through to mdevd or mdev below.
 service udevd {
 	description    = "Device event daemon (udev)"
 	runlevel       = "S12345789"
@@ -37,18 +41,8 @@ service udevd {
 	pidfile-create = true
 	log { }
 	cgroup system { name = "udevd" }
-	command        = "-/lib/systemd/systemd-udevd $UDEVD_ARGS"
+	command        = { "/lib/systemd/systemd-udevd $UDEVD_ARGS",
-}
+			   "-udevd $UDEVD_ARGS" }
-service udevd {
-	description    = "Device event daemon (udev)"
-	runlevel       = "S12345789"
-	notify         = "none"
-	envfile        = "-/etc/default/udevd"
-	pidfile        = "udevd"
-	pidfile-create = true
-	log { }
-	cgroup system { name = "udevd" }
-	command        = "-udevd $UDEVD_ARGS"
 }
 
 # Wait for udevd to start, then trigger coldplug events and module loading.

test/Makefile.am

@@ -32,6 +32,7 @@ EXTRA_DIST		+= add-remove-dynamic-service-sub-config.sh
 EXTRA_DIST		+= bootstrap-crash.sh
 EXTRA_DIST		+= cond-start-task.sh
 EXTRA_DIST		+= conf-format.sh
+EXTRA_DIST		+= conf-command.sh
 EXTRA_DIST		+= conf-dup-title.sh
 EXTRA_DIST		+= conf-dirs.sh
 EXTRA_DIST		+= conf-if.sh
@@ -82,6 +83,7 @@ TESTS			+= add-remove-dynamic-service-sub-config.sh
 TESTS			+= bootstrap-crash.sh
 TESTS			+= cond-start-task.sh
 TESTS			+= conf-format.sh
+TESTS			+= conf-command.sh
 TESTS			+= conf-dup-title.sh
 TESTS			+= conf-dirs.sh
 TESTS			+= conf-if.sh

test/conf-command.sh

@@ -0,0 +1,100 @@
+#!/bin/sh
+# Verify a command list.  The entries are candidates for the same
+# service and the first one whose binary resolves wins, which the
+# line-based format could only express as one stanza per candidate.
+set -eu
+
+TEST_DIR=$(dirname "$0")
+
+# shellcheck disable=SC2034
+# The first candidate is not installed, so the second one runs.  This
+# is the udevd case from system/10-hotplug.conf, where the systemd
+# build of the daemon is tried before the standalone one.
+BOOTSTRAP="service pick {
+    runlevel = \"S12345\"
+    command  = { \"/no/such/binary --daemon\", \"serv -np -i pick\" }
+}"
+
+# The chosen candidate is what Finit runs, so assert on that rather
+# than on process names: pgrep matches a substring, and 'serv' is one
+# of 'service.sh'.
+assert_cmd()
+{
+    assert "Service $1 command == $2" \
+	   "$(texec initctl status "$1" | grep 'Command' | sed 's/.*Command : //')" = "$2"
+}
+
+test_teardown()
+{
+    say "Running test teardown."
+    run "rm -f $FINIT_CONF"
+}
+
+# shellcheck source=/dev/null
+. "$TEST_DIR/lib/setup.sh"
+
+say 'A missing first candidate hands the service to the next one'
+retry 'assert_num_children 1 serv'
+assert_cmd pick "serv -np -i pick"
+
+# With a BOOTSTRAP the test is released in runlevel S, where a reload
+# is ignored, and the rewritten file would then be picked up by the
+# runlevel change instead of the reload under test.
+say 'Waiting for bootstrap to finish before rewriting the configuration'
+retry "test \"\$(texec sh -c \"initctl runlevel | awk '{print \\\$2;}'\")\" = 2" 20 1
+
+# Both binaries exist here, so a plain "does it resolve" check would
+# be satisfied by either.  Only order decides.
+say 'When several candidates resolve, the first one wins'
+run "echo 'service pick {'                                    >  $FINIT_CONF"
+run "echo '    command = { \"serv -np -i pick\", \"service.sh\" }' >> $FINIT_CONF"
+run "echo '}'                                                 >> $FINIT_CONF"
+run "initctl reload"
+
+retry 'assert_cmd pick "serv -np -i pick"'
+assert_num_children 1 serv
+
+say 'The order is honoured the other way around too'
+run "echo 'service pick {'                                    >  $FINIT_CONF"
+run "echo '    command = { \"service.sh\", \"serv -np -i pick\" }' >> $FINIT_CONF"
+run "echo '}'                                                 >> $FINIT_CONF"
+run "initctl reload"
+
+retry 'assert_cmd pick "service.sh"'
+assert_num_children 1 service.sh
+
+# libconfuse takes a bare string for a list option, so the spelling
+# every other .conf uses has to keep working after command became one.
+say 'A single command needs no braces'
+run "echo 'service pick {'                         >  $FINIT_CONF"
+run "echo '    description = \"Single command\"'   >> $FINIT_CONF"
+run "echo '    command     = \"serv -np -i pick\"' >> $FINIT_CONF"
+run "echo '}'                                      >> $FINIT_CONF"
+run "initctl reload"
+
+retry 'assert_cmd pick "serv -np -i pick"'
+assert_desc "Single command" pick
+assert_num_children 1 serv
+
+# A tty block takes the same list, its command is the getty to run.
+# Asserting it is loaded is enough here, a getty in the test namespace
+# has no terminal to attach to.
+say 'A tty block takes candidates too'
+run "echo 'tty console {'                                  >  $FINIT_CONF"
+run "echo '    runlevel = \"12345\"'                       >> $FINIT_CONF"
+run "echo '    command  = { \"/no/such/getty\", \"serv -np -i ttypick\" }' >> $FINIT_CONF"
+run "echo '}'                                              >> $FINIT_CONF"
+run "initctl reload"
+
+retry 'assert_cmd tty "serv -np -i ttypick"'
+
+# Nothing resolves, so the block is skipped the same way a single
+# missing command is.  The leading - only decides whether that is
+# quiet, service_register() bails before svc_new() either way.
+say 'A block whose candidates are all missing is skipped'
+run "echo 'service ghost {'                                >  $FINIT_CONF"
+run "echo '    command = { \"-/no/such/one\", \"-/no/such/two\" }' >> $FINIT_CONF"
+run "echo '}'                                              >> $FINIT_CONF"
+run "initctl reload"
+
+assert_num_services 0 ghost