diff --git a/src/conf.c b/src/conf.c index 6b8896fa..f5e87d11 100644 --- a/src/conf.c +++ b/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); - if (cmd && cmd[0] == '-') { + cmd = svc_command(sec, &nowarn); + if (nowarn) addtok(line, sizeof(line), "nowarn"); - cmd++; - } if (dev) { addtok(line, sizeof(line), "%s", dev); diff --git a/system/10-hotplug.conf.in b/system/10-hotplug.conf.in index 873cfe37..f0e8c86c 100644 --- a/system/10-hotplug.conf.in +++ b/system/10-hotplug.conf.in @@ -13,21 +13,25 @@ # command = "syslogd -F" # } # -# with `if = "mdev"` or `if = "mdevd"` and the matching coldplug condition -# for those variants. -# -# This provdes a condition that can act as a barrier for all +# This provides 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" -} -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" + command = { "/lib/systemd/systemd-udevd $UDEVD_ARGS", + "-udevd $UDEVD_ARGS" } } # Wait for udevd to start, then trigger coldplug events and module loading. diff --git a/test/Makefile.am b/test/Makefile.am index 10431cc7..a87995b0 100644 --- a/test/Makefile.am +++ b/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 diff --git a/test/conf-command.sh b/test/conf-command.sh new file mode 100755 index 00000000..4bca34d6 --- /dev/null +++ b/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