Re: rc.d script style guide

["J. Lewis Muir" <jlmuir%imca-cat.org@localhost>]

On 01/07, Greg Troxel wrote:
> In working on ups-nut I am noticing several things.  We don't have a
> style guide for pkgsrc-provided rc.d scripts in doc/pkgsrc.
> 
> 1) conditional /etc/rc.subr
> 
> This pattern is in both tor and nut.
> 
>   if [ -f /etc/rc.subr -a -f /etc/rc.conf -a -d /etc/rc.d -a -f /etc/rc.d/DAEMON ]
>   then
>           load_rc_config $name
>           run_rc_command "$1"
>   else
>           eval ${start_cmd}
>   fi
> 
> Often this will start but not stop, from not-really-adequate code reading.
> 
> Is this:
>   - for old NetBSD (and if so how old)
>   - for non-NetBSD with the rc.subr package?  (But, on macOS, that
>     installs /etc/rc.subr)
>   - something else?
> 
> Should it instead just depend on rc.subr?  Let it be and people should
> not expect rc.d scripts to run without rc.subr?
> 
> I am inclined to clean this up, and let it be.

Hi, Greg!

I think it would be great if this got cleaned up.

IMO, it should depend on rc.subr.  Also, it should not be expected that
rc.d scripts will run without rc.subr, so I don't think the conditional
is helpful.

On a related note, I think *lots* of rc.d scripts in pkgsrc assume
rc.subr is at

  /etc/rc.subr

For example:

  https://github.com/NetBSD/pkgsrc/blob/trunk/net/dnsmasq/files/dnsmasq.sh

and (conditionally)

  https://github.com/NetBSD/pkgsrc/blob/trunk/net/djbdns-run/files/dnscache.sh

But that is wrong because the system configuration directory can be
set per pkgsrc instance.  For example, on macOS using binary packages
provided by Joyent

  https://pkgsrc.joyent.com/install-on-macos/

rc.subr is installed at

  /opt/pkg/etc/rc.subr

What this means is that the path to rc.subr in rc.d scripts should be
substituted.  I think adding this to a style guide would be great.  I
also think that a mass fix-up of all rc.d scripts in pkgsrc might be
needed; otherwise, the broken idiom is likely to live on since many
people will copy an existing rc.d script from another package and modify
it for their need.

I have personally run into this problem with the dnsmasq and dnscache
rc.d scripts listed above and with rc.d-boot:

  https://github.com/NetBSD/pkgsrc/blob/trunk/pkgtools/rc.d-boot/files/rc.d-boot

Looking at the rc.d-boot commit log, it seems to have been fixed there
about two weeks ago, but I haven't tried it to confirm that it works
now.

> 2) Strange formatting with then on a new line (see above)
> 
> Is this:
>   - just an odd style choice?
>   - accomodating some ancient sh?
>   - for a good reason I don't understand?
>   - something else?
> 
> I am inclined to align style to base system rc.d scripts as I edit,
> absent a good raeson.

I too prefer the sh conditional style with the trailing "; then":

  if foo; then
    bar
  fi

But I'm not opposed to the other style of "then" on its own line.

And I agree that the test "-a" and "-o" options should not be used as
others have pointed out.

Thanks for working on this!

Lewis