svn commit: r197790 - head/etc

[Doug Barton]

Hiroki Sato wrote:
> Hi Doug,
> 
> Doug Barton <dougb at FreeBSD.org> wrote
>   in <4AD3A722.9060401 at FreeBSD.org>:
> 
> do> I think this is the wrong solution to the problem. In at least two
> do> cases (routed and route6d) where $command is not defined in the rc.d
> do> scripts this change is resulting in $command not being defined at all.
> do>
> do> If you look at the definition of the + parameter substitution this
> do> makes sense:
> do>
> do>      ${parameter:+word}
> do>              Use Alternate Value.  If parameter is unset or null, null
> do>              is substituted; otherwise, the expansion of word is
> do>              substituted.
> do>
> do> I think that what you really wanted to do was:
> 
>  I am sorry for the delay.  Your patch is reasonable to me.  This
>  problem was there for a while, so it should be fixed asap.

Ok, I've committed the fix, thanks for getting back to me.

>  I noticed there was something wrong about ${name}_program but it
>  seems I mistakenly changed it (sorry...).  Then I received a report
>  "it does not work" so I just reverted it.

Understood. I am sure you realize that it's always Ok to ask for help
here on -rc. The rc.d system is not life-threateningly complex but it
does have a lot of "behind the scenes" interactions that are not
always obvious. I certainly don't hesitate to ask for review on
changes myself and I encourage others to do so (as you have done in
the past).

FWIW, what I do object to about your changes in r197144 and r197790
are that in the first case you neglected to mention that you were
changing that part of the code, and in the second you neglected to
mention that you were changing it back to what it was before you
changed it. That made debugging this problem more difficult for me
than (I think) it should have been. You also did not mention that you
were removing $command in your changes to route[6]d, which made
debugging Mark's original complaint harder, but only for about 30
seconds or so. :)

The two most important things about VCS logs are WHAT you did (should
be brief, but thorough) and WHY you did it. My logs are often
obnoxiously long, but you can generally find at least that information
in them. Please think about what people who read the logs years from
now will need to know. Of course, I realize that this is difficult to
do, especially when you are just wrapping up a project and all of the
information is fresh in your head and seems painfully obvious.

>  IMO defining $command in rc.d scripts is not a good practice.
>  "Always use ${name}_program and let load_rc_config() set the
>  $command" would be consistent and useful to avoid this sort of
>  problems.

In the past since there was not a reliable mechanism to allow
${name}_program to override $command, and because rc.subr needs to
have $command defined to work properly (see the description in the
run_rc_command comments in rc.subr) it was necessary to set command in
each script. When yar introduced the current version of the override
code almost 4 years ago he also went through and set command
explicitly in all the scripts that didn't have it, so the situation
that was created here never came up until now.

(FYI, the previous code that he replaced had the same effect as what I
just changed it to but was slightly more complex.)

I'm sort of ambivalent about whether we need to continue encouraging
people to use command in the script or not. As long as what's in the
script matches what's in /etc/defaults/rc.conf we're not hurting
anything, although we are duplicating effort.

My preference at this point is to let the change that I just made
settle for a while, mostly to see if it has any negative interactions
with scripts from ports, then MFC it after 8.0-RELEASE along with the
changes you've made to the IPv6 stuff. After that we can start talking
about ripping command= out of the individual rc.d scripts if people
think that's a good idea.


Doug

PS, Mark you're a slacker for not getting back to me. :)

-- 

	Improve the effectiveness of your Internet presence with
	a domain name makeover!    http://SupersetSolutions.com/