rc: Honor ${name}_env when a custom *_cmd is defined (e.g., start_cmd)

[0mp]

Summary

rc: Honor ${name}_env when a custom *_cmd is defined (e.g., start_cmd)

A user may set ${name}_env variable in rc.conf(5) in order to set additional
environment variables for a service command. Unfortunately, at the moment this
variable is only honored when the command is specified via the command
variable. Those additional environment variables are never set if the service
is started via the ${rc_arg}_cmd variable (for example start_cmd).

PR:		239692

• https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=239692

Test Plan

1. Save the following script as /usr/local/etc/rc.d/teststartcmd and make it executable:

#!/bin/sh
# PROVIDE: teststartcmd
. /etc/rc.subr
name=teststartcmd
load_rc_config $name
start_cmd="teststartcmd_start"
teststartcmd_start() {
	env
}
run_rc_command "$1"

2. Run sysrc teststartcmd_env="TESTVARIABLE=foo" to modify rc.conf.

3. Run service teststartcmd onestart and observe that TESTVARIABLE is not in the output.

4. Save the following script as /usr/local/etc/rc.d/testcommand and make it executable:

#!/bin/sh
# PROVIDE: testcommand
. /etc/rc.subr
name=testcommand
load_rc_config $name
command="/usr/bin/env"
run_rc_command "$1"

5. Run sysrc testcommand_env="TESTVARIABLE=foo" to modify rc.conf.

6. Run service testcommand onestart and observe that TESTVARIABLE is in the output this time.

With the patch, both outputs should contain TESTVARIABLE=foo.

[bcr]

Minor man page fix.

share/man/man8/rc.subr.8

573

s/the/then/
(You can also leave out the "the" completely)

[0mp]

Fix a typo (thanks @bcr)

[bcr]

OK from manpages.
Nice feature, BTW.

[0mp]

Otherwise, we may just document that ${name}_env only works if the ${rc_arg}_cmd is not defined.

[jilles]

This change looks reasonable, although the exact logic can be improved as noted. It may have been the original intent that $start_cmd use $_env by itself but that would involve replicating error-prone code.

libexec/rc/rc.subr

1039–1041

This should have an effect as close as possible to the effect without a _cmd. In that case, env plus the value of _env is prepended to the command and this is passed to eval. The code above first applies word splitting and pathname generation to the value of _env, concatenates it with spaces and evals it as shell variable assignments.

So I suggest instead:

if [ -n "$_env" ]; then
    eval "export -- $_env"
fi

This applies exactly one expansion pass, a full shell parse and expansion.

The -- will cause any attempt to use options to fail. There are no options that agree between export and env.

Differently from env, export will not set environment variables that do not have a proper name (consisting solely of alphabetics, numerics, and underscores, and starting with an alphabetic or an underscore). There is no way the shell allows this, so this difference cannot be fixed.

[0mp]

• Apply changes to rc.subr suggested by jilles
• Update the manpage patch to reflect latest changes

[0mp]

• Refactor manpage changes a little bit

[0mp]

Just one last question: do we want to MFC this change?