usr.sbin/service: Fixes -j to not be order dependant

[dor]

Summary

This PR corrects -j behaviour to not be order dependant.
We also simplify the feature by removing the unnecessary double parsing of getopts.

Test Plan

Currently, this has been tested locally against a few of my jails, by passing
various arguments in random orders, both operating on the host and a jail.
Due to the passing of the -j argument, it was necessary to update the
service(8) command in the target jails before testing.

kevans has expressed a few ideas on some automated testing of service(8) to
ensure that the arguments do not become order dependant in the future.

[kevans]

As indicated on IRC, I think I like this approach given that it doesn't add any opt parsing complexity if it's executing in a jailed context, leaving less room to break since it's generally executing the exact same path as before these changes.

However, I'm not necessarily a fan of how it passes the -j bits along to the jail, making -j effectively useless until both jail host and jail have been updated with the new service(8)- especially given that there's really no reason for the limitation.

[dor]

The following is a diff to the above, playing with another technique for removing -j JAIL from the arguments before passing them to jexec(8).

Edit: Removed diff here to avoid confusion when reading this issue.

[jilles]

getopts also allows option-arguments in the same argument as the option, like -jGAOL, or even -ejGAOL (for a jail named GAOL). Therefore, extracting the jail option out without rebuilding the arguments completely is rather hard.

[dor]

This new diff now rebuilds the command line arguments before passing them into the jail.

[dor]

This properly handles arguments in the style jilles outlined in his last comment:

# ./service.sh -vjnginx nginx status
nginx is located in /usr/local/etc/rc.d
nginx is running as pid 62587.

[kevans]

I think this is reasonable- just one nit from me. =)

usr.sbin/service/service.sh

52 ↗

(On Diff #38139)

Go ahead and undo this accepted_argstr= completely and replace ${accepted_argstr} below with the literal. This isn't baggage we should bring forth if we're back to only parsing opts once.

[dor]

Takes care of the vestigial accepted_argstr variable.

[kevans]

I think it's good- @jilles ?

While I'm not 100% a fan of rebuilding args in general, I think service(8) is fairly stable as far as arguments/options go and it's small enough that it probably won't cause any maintenance headache later on.

[jilles]

usr.sbin/service/service.sh

66–67 ↗

(On Diff #38143)

The check for jailed seems unnecessary with the new approach, and will break this functionality with nested jails.

69–73 ↗

(On Diff #38143)

Do we need this? It looks like jexec already prints an error message and returns with status 1 in this case.

86 ↗

(On Diff #38143)

This can use "$@" instead of $@ in order to avoid word splitting and pathname generation (even though this is performed inappropriately elsewhere in this script and rc.subr).

[dor]

Takes care of comments by jilles.

[dor]

usr.sbin/service/service.sh

69–73 ↗

(On Diff #38143)

I was duplicating the jexec(8) behaviour here for some reason. I think I had a reason for that long ago, I no longer remember it. Removed.