Project

General

Profile

Actions

Bug #264

closed

rc.d standardization patch

Added by kevin.kane over 17 years ago. Updated almost 17 years ago.

Status:
Closed
Priority:
Low
Assignee:
-
Category:
-
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:

Description

There is quite a bit of non standardness in the rc.d system and this
aims to get us a little closer to the way NetBSD does things and
towards 'normalcy'.

rename start_vinum -> vinum, enable_quotas -> quotas
remove all instances of _enable
respect configs that use old names with _enable but output a warning
saying _enable is deprecated.
kerberos5, kadmind, kpasswd had the program path in a nonstandard
variable added _program variable which seems to be the standard.
These 3 had _server in the name i removed that as well to make
translation between the old config and new config possible.

here is the diff: http://www.uberstyle.net/~kevin/rcenable.diff

If there are any suggestions for renaming and cleaning up other rcvar
names let me know and ill add them in to this patch.

Actions #1

Updated by corecode about 18 years ago

[..]

@ -219,6 +220,26 @ #
checkyesno() {
+ # this is added for compatibility purposes will
+ # be removed when _enable is deprecated for real
+ eval _value=\$${1}_enable
+ debug "checkyesno: $1 is set to $_value."
+ case $_value in

# "yes", "true", "on", or "1"
+ [Yy][Ee][Ss]|[Tt][Rr][Uu][Ee]|[Oo][Nn]|1)
+ warn "\$${1}_enable is deprecated, use: \$${1} instead."
+ return 0
+ ;;

# "no", "false", "off", or "0"
+ [Nn][Oo]|[Ff][Aa][Ll][Ss][Ee]|[Oo][Ff][Ff]|0)
+ warn "\$${1}_enable is deprecated, use: \$${1} instead."
+ return 1
+ ;;
+ esac
+ # end compatibility section for _enable
+

how about making this
if [ -n "$_value" ]
then
warn "\$${1}_enable is deprecated, use: \$${1} instead."
eval \$${1}=\$${1}_enable
fi

cheers
simon

Actions #2

Updated by kevin.kane about 18 years ago

I just updated patch with a simplified block along the lines you said.
except:
eval ${1}=$_value

http://www.uberstyle.net/~kevin/rcenable.diff

On 7/28/06, Simon 'corecode' Schubert <> wrote:

Kevin L. Kane wrote:

respect configs that use old names with _enable but output a warning
saying _enable is deprecated.

[..]

@ -219,6 +220,26 @ #
checkyesno() {
+ # this is added for compatibility purposes will
+ # be removed when _enable is deprecated for real
+ eval _value=\$${1}_enable
+ debug "checkyesno: $1 is set to $_value."
+ case $_value in

# "yes", "true", "on", or "1"
+ [Yy][Ee][Ss]|[Tt][Rr][Uu][Ee]|[Oo][Nn]|1)
+ warn "\$${1}_enable is deprecated, use: \$${1} instead."
+ return 0
+ ;;

# "no", "false", "off", or "0"
+ [Nn][Oo]|[Ff][Aa][Ll][Ss][Ee]|[Oo][Ff][Ff]|0)
+ warn "\$${1}_enable is deprecated, use: \$${1} instead."
+ return 1
+ ;;
+ esac
+ # end compatibility section for _enable
+

how about making this
if [ -n "$_value" ]
then
warn "\$${1}_enable is deprecated, use: \$${1} instead."
eval \$${1}=\$${1}_enable
fi

cheers
simon

--
Serve - BSD ++ RENT this banner advert ++ ASCII Ribbon /"\
Work - Mac ++ space for low €€€ NOW!1 ++ Campaign \ /
Party Enjoy Relax | http://dragonflybsd.org Against HTML \
Dude 2c 2 the max ! http://golden-apple.biz Mail + News / \

Actions #3

Updated by corecode about 18 years ago

Kevin L. Kane wrote:

I just updated patch with a simplified block along the lines you said.
except:
eval ${1}=$_value

even nicer :)

cheers
simon

Actions #4

Updated by joerg about 18 years ago

On Fri, Jul 28, 2006 at 12:00:08PM -0400, Kevin L. Kane wrote:

kerberos5, kadmind, kpasswd had the program path in a nonstandard
variable added _program variable which seems to be the standard.

I'd just nuke the extra indirection for those.

Joerg

Actions #5

Updated by kevin.kane about 18 years ago

I moved the location of the default command into the rc.d script so we
no longer need the _program variable in defaults/rc.conf. (For
kerberos5, kadmind and kpasswdd)

Updated patch: http://www.uberstyle.net/~kevin/rcenable.diff

On 7/28/06, Joerg Sonnenberger <> wrote:

On Fri, Jul 28, 2006 at 12:00:08PM -0400, Kevin L. Kane wrote:

kerberos5, kadmind, kpasswd had the program path in a nonstandard
variable added _program variable which seems to be the standard.

I'd just nuke the extra indirection for those.

Joerg

Actions #6

Updated by kevin.kane about 18 years ago

Is there any other issues people want me to clear up with this patch,
anymore thoughts on rc.d in general?

-Kevin

On 7/28/06, Kevin L. Kane <> wrote:

I moved the location of the default command into the rc.d script so we
no longer need the _program variable in defaults/rc.conf. (For
kerberos5, kadmind and kpasswdd)

Updated patch: http://www.uberstyle.net/~kevin/rcenable.diff

On 7/28/06, Joerg Sonnenberger <> wrote:

On Fri, Jul 28, 2006 at 12:00:08PM -0400, Kevin L. Kane wrote:

kerberos5, kadmind, kpasswd had the program path in a nonstandard
variable added _program variable which seems to be the standard.

I'd just nuke the extra indirection for those.

Joerg

--
Kevin L. Kane
kevin.kane at gmail.com

Actions #7

Updated by TGEN about 18 years ago

Kevin L. Kane wrote:

Is there any other issues people want me to clear up with this patch,
anymore thoughts on rc.d in general?

Include proper /etc/ifconfig.if support (from NetBSD). And if possible,
make sure that an /etc/rc.d/network stop followed by a start restores
IPv6 link-local addresses.

Cheers,
--
Thomas E. Spanjaard

Actions #8

Updated by joerg about 18 years ago

On Tue, Aug 01, 2006 at 03:18:32PM -0400, Kevin L. Kane wrote:

Is there any other issues people want me to clear up with this patch,
anymore thoughts on rc.d in general?

Sync /etc/rc.subr with NetBSD, adds some nice features which can be used
e.g. by smbd :-)

Joerg

Actions #9

Updated by check+j3df9e00rshfwkvl about 18 years ago

Kevin L. Kane wrote:
> Simon 'corecode' Schubert wrote:
> > Kevin L. Kane wrote:
> > > [...]
> > > + eval _value=\$${1}_enable
> > [...]
> > eval \$${1}=\$${1}_enable
> [...]
> I just updated patch with a simplified block along the lines you said.
> except:
> eval ${1}=$_value

You have to be very careful with "eval" in shell scripts.
It is best to always escape dollar signs on the right, so
they're expanded on the second pass, not on the first, and
enclose the whole expression in escaped double quotes:

eval $1=\"\$_value\"

or:

eval $1='"$_value"'

That's the only way that will always work if the value
of the variable contains white space, semicolons or other
nasty things. If the value comes from a configuration file
and hasn't be checked for errors, then you cannot guarantee
that it's "clean". You probably don't want your rc scripts
abort due to a shell syntax error during boot just because
you accidentally inserted a space in one of the variables.
;-)

Best regards
Oliver

Actions #10

Updated by kevin.kane about 18 years ago

I updated the patch to include ifconfig.if support and man page. I
took olivers suggestion and changed my eval statement some. I also
pulled in some changes from rc.subr in netbsd, I left out a couple
that didnt seem relevant like _systrace support, some perl daemon
stuff that seemed like we already had it, and also some other minor
changes that we already had.

still availabe at:
http://www.uberstyle.net/~kevin/rcenable.diff

On 02 Aug 2006 12:58:35 GMT, Oliver Fromme
<> wrote:

Kevin L. Kane wrote:

Simon 'corecode' Schubert wrote:

Kevin L. Kane wrote:

[...]
+ eval _value=\$${1}_enable

[...]
eval \$${1}=\$${1}_enable

[...]
I just updated patch with a simplified block along the lines you said.
except:
eval ${1}=$_value

You have to be very careful with "eval" in shell scripts.
It is best to always escape dollar signs on the right, so
they're expanded on the second pass, not on the first, and
enclose the whole expression in escaped double quotes:

eval $1=\"\$_value\"

or:

eval $1='"$_value"'

That's the only way that will always work if the value
of the variable contains white space, semicolons or other
nasty things. If the value comes from a configuration file
and hasn't be checked for errors, then you cannot guarantee
that it's "clean". You probably don't want your rc scripts
abort due to a shell syntax error during boot just because
you accidentally inserted a space in one of the variables.
;-)

Best regards
Oliver

--
Oliver Fromme, secnetix GmbH & Co. KG, Marktplatz 29, 85567 Grafing
Dienstleistungen mit Schwerpunkt FreeBSD: http://www.secnetix.de/bsd

Any opinions expressed in this message may be personal to the author
and may not necessarily reflect the opinions of secnetix in any way.

Actions #11

Updated by TGEN about 18 years ago

Kevin L. Kane wrote:

I updated the patch to include ifconfig.if support and man page.

I don't think you can just import NetBSD's /etc/rc.d/network; we have
/etc/rc.d/netif and /etc/network.subr to do such things already. I'll
leave it to you to integrate ifconfig.if(5) support into network.subr
and netif ;).

Cheers,
--
Thomas E. Spanjaard

Actions #12

Updated by victor about 18 years ago

Hi,
I've found a little problem with your patch: if some user made script
does have something like checkyesno foo_enable it will fail. Will work
with foo_enable_enable="YES|NO" and foo_enable="YES|NO", but not
with foo="YES|NO". ie, the new expected behaviour.

I don't think that we should switch to foo="YES|NO", it will require
a lot of documentation changes and will break POLA.

I've created a patch1 inspired by yours that allows to use
foo_enable="YES|NO" and foo="YES|NO" with pkgsrc and base apps.
It's much smaller, don't require documentation changes and gives
us the choice of working the way we want without imposing anything
to the user.

This is a bit hackish, because the place it should be fixed is on
pkgsrc, but i doubt this is doable as of now.

[1]: http://bsdes.net/~victor/dfbsd/rc.subr.patch
--
La prueba más fehaciente de que existe vida inteligente en otros
planetas, es que no han intentado contactar con nosotros.

Actions #13

Updated by swildner about 18 years ago

Victor Balada Diaz wrote:

I've created a patch1 inspired by yours that allows to use
foo_enable="YES|NO" and foo="YES|NO" with pkgsrc and base apps.
It's much smaller, don't require documentation changes and gives
us the choice of working the way we want without imposing anything
to the user.

This is a bit hackish, because the place it should be fixed is on
pkgsrc, but i doubt this is doable as of now.

I like your patch because (even though it's kind of a hack) it gives
everyone the freedom to use the style he prefers with a minimum of effort.

Personally, I prefer "foo_enable" to just "foo". I tend to think we
shouldn't deprecate it just because pkgsrc doesn't properly adapt to the
rc semantics of all its target platforms. :)

I think this should be committed.

Sascha

Actions #14

Updated by joerg about 18 years ago

On Fri, Sep 29, 2006 at 06:18:28PM +0200, Victor Balada Diaz wrote:

I've found a little problem with your patch: if some user made script
does have something like checkyesno foo_enable it will fail. Will work
with foo_enable_enable="YES|NO" and foo_enable="YES|NO", but not
with foo="YES|NO". ie, the new expected behaviour.

Hard-coding foo_enable is already a bug and incorrect use of rcNG
anyway.

Joerg

Actions #15

Updated by victor about 18 years ago

On Fri, Sep 29, 2006 at 06:47:39PM +0200, Joerg Sonnenberger wrote:

Hard-coding foo_enable is already a bug and incorrect use of rcNG
anyway.

We hard code it in some places, take /etc/rc.d/ipfw as an example.
So it's not too hard to think that some user is doing it too.

Actions #16

Updated by TGEN about 18 years ago

Joerg Sonnenberger wrote:

Hard-coding foo_enable is already a bug and incorrect use of rcNG
anyway.

nitpick {
rcNG is incorrect naming anyway ;);
};

Cheers,
--
Thomas E. Spanjaard

Actions #17

Updated by joerg about 18 years ago

On Sat, Sep 30, 2006 at 11:54:19AM +0000, Thomas E. Spanjaard wrote:

Joerg Sonnenberger wrote:

Hard-coding foo_enable is already a bug and incorrect use of rcNG
anyway.

nitpick {
rcNG is incorrect naming anyway ;);
};

Wrong, rcNG is what FreeBSD called and what we originally imported. The
_enable thingy is a FreeBSD-local change.

Joerg

Actions #18

Updated by luxh about 18 years ago

Actions

Also available in: Atom PDF