From owner-freebsd-rc@FreeBSD.ORG Tue Mar 28 14:38:58 2006 Return-Path: X-Original-To: freebsd-rc@freebsd.org Delivered-To: freebsd-rc@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 65E4416A420 for ; Tue, 28 Mar 2006 14:38:58 +0000 (UTC) (envelope-from flz@xbsd.org) Received: from smtp.xbsd.org (xbsd.org [82.233.2.192]) by mx1.FreeBSD.org (Postfix) with ESMTP id B29D943D49 for ; Tue, 28 Mar 2006 14:38:57 +0000 (GMT) (envelope-from flz@xbsd.org) Received: from localhost (localhost.xbsd.org [127.0.0.1]) by smtp.xbsd.org (Postfix) with ESMTP id 792B3117F7; Tue, 28 Mar 2006 16:38:53 +0200 (CEST) Received: from smtp.xbsd.org ([127.0.0.1]) by localhost (srv1.xbsd.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 15177-09; Tue, 28 Mar 2006 16:38:42 +0200 (CEST) Received: from mayday.esat.net (mayday.esat.net [193.95.134.156]) by smtp.xbsd.org (Postfix) with ESMTP id 945B211413; Tue, 28 Mar 2006 16:38:41 +0200 (CEST) From: Florent Thoumie To: Brooks Davis In-Reply-To: <20060327183745.GA19473@odin.ac.hmc.edu> References: <1143202549.16053.145.camel@mayday.esat.net> <20060324205627.GA18100@odin.ac.hmc.edu> <00E087F1-81E4-4580-A655-50F3DD8A471F@xbsd.org> <1143461191.4290.5.camel@mayday.esat.net> <20060327183745.GA19473@odin.ac.hmc.edu> Content-Type: text/plain Date: Tue, 28 Mar 2006 15:38:35 +0100 Message-Id: <1143556715.65237.4.camel@mayday.esat.net> Mime-Version: 1.0 X-Mailer: Evolution 2.4.2.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit X-Virus-Scanned: amavisd-new at xbsd.org Cc: freebsd-rc@freebsd.org Subject: Re: rc.subr / rc.d/sshd patch for review X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Mar 2006 14:38:58 -0000 On Mon, 2006-03-27 at 10:37 -0800, Brooks Davis wrote: > On Mon, Mar 27, 2006 at 01:06:30PM +0100, Florent Thoumie wrote: > > On Sat, 2006-03-25 at 11:06 +0000, Florent Thoumie wrote: > > > On Mar 24, 2006, at 8:56 PM, Brooks Davis wrote: > > > > > > > On Fri, Mar 24, 2006 at 12:15:49PM +0000, Florent Thoumie wrote: > > > >> This is based on Oliver's patch for rc.d/sshd that can be found in > > > >> Gnats. > > > >> > > > >> In load_rc_config, I'm extracting prefix from ${command} (or > > > >> ${name}_program, which part is moved from run_rc_command), and > > > >> setting > > > >> etcdir accordingly. > > > >> > > > >> The point is that some scripts (like rc.d/sshd) can be used for base > > > >> sshd as well as ports sshd, and makes possible to source > > > >> ${prefix}/etc/rc.conf.d/${name}. > > > >> > > > >> This patch also documents ${name}_program above run_rc_command > > > >> (though > > > >> it's actually used in load_rc_config). > > > > > > > > Is command always set? I'm pretty sure it isn't so this may not be > > > > entierly > > > > safe. If it's not set, should we try to guess prefix from $0? > > > > > > Somehow, command gets set to the right value, but you're right, I'm > > > missing a bit here. > > > > Hum, re-reading rc.subr, you were right, so I just did what you > > supposed. > > Thinking about this a bit more, in the guessing frmo $0 case, your proposed > code: > > + prefix=${0%/etc/rc.d/*}/ > > won't work reliably when the user uses a relative path. I think something > like this would be better: > > _tmp=`/bin/realpath $0` > prefix=${_tmp%/etc/rc.d/*}/ Indeed, fixed. > > > > The other issue I see is that instead of: > > > > > > > > if [ -f ${etcdir}/rc.conf.d/"$_command" ]; then > > > > debug "Sourcing ${etcdir}/rc.conf.d/${_command}" > > > > . ${etcdir}/rc.conf.d/"$_command" > > > > fi > > > > > > > > I think we should do: > > > > > > > > if [ -f /etc/rc.conf.d/"$_command" ]; then > > > > debug "Sourcing /etc/rc.conf.d/${_command}" > > > > . /etc/rc.conf.d/"$_command" > > > > fi > > > > if [ "${etcdir}" != "/etc" -a -f ${etcdir}/ > > > > rc.conf.d/"$_command" ]; then > > > > debug "Sourcing ${etcdir}/rc.conf.d/${_command}" > > > > . ${etcdir}/rc.conf.d/"$_command" > > > > fi > > > > > > > > That preserves the old behavior while adding support for > > > > ${prefix}/etc/rc.conf.d. > > > > > > Fair enough, but I'd like to add a note saying that /etc/rc.conf.d/$ > > > {name} is deprecated for ${etcdir} != "/etc". > > The deprecation warning should not be printed in the case that ${etcdir} > is /etc. You should also avoid sourcing the file twice in the /etc > case. The easiest way to do that is probably to make the first case > contingent on ${etcdir} != /etc. Next time I'll test my changes (and sleep more). Did that too, and added a check to test if there's a ${etcdir}/rc.conf.d/${_command} file. Patch updated : http://people.freebsd.org/~flz/local/rc.d-sshd.diff BTW, I think that we should s/_command/_name/ in load_rc_config(), this is a bit confusing. -- Florent Thoumie flz@FreeBSD.org FreeBSD Committer