Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 05 Jan 2016 17:27:52 -0700
From:      Ian Lepore <ian@freebsd.org>
To:        Allan Jude <allanjude@freebsd.org>, Devin Teske <dteske@freebsd.org>
Cc:        Warner Losh <imp@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r293227 - head/etc
Message-ID:  <1452040072.1320.49.camel@freebsd.org>
In-Reply-To: <568C5D49.9090502@freebsd.org>
References:  <201601052120.u05LKlQw074919@repo.freebsd.org> <1452038404.1320.46.camel@freebsd.org> <1A1BB09D-2FB4-4E50-9F86-62B772855224@freebsd.org> <568C5D49.9090502@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 2016-01-05 at 19:18 -0500, Allan Jude wrote:
> On 2016-01-05 19:16, Devin Teske wrote:
> > 
> > > On Jan 5, 2016, at 4:00 PM, Ian Lepore <ian@freebsd.org> wrote:
> > > 
> > > On Tue, 2016-01-05 at 21:20 +0000, Warner Losh wrote:
> > > > Author: imp
> > > > Date: Tue Jan  5 21:20:47 2016
> > > > New Revision: 293227
> > > > URL: https://svnweb.freebsd.org/changeset/base/293227
> > > > 
> > > > Log:
> > > >   Use the more proper -f. Leave /bin/rm in place since that's
> > > > what
> > > >   other rc scripts have, though it isn't strictly necessary.
> > > > 
> > > > Modified:
> > > >   head/etc/rc
> > > > 
> > > > Modified: head/etc/rc
> > > > ===============================================================
> > > > ======
> > > > =========
> > > > --- head/etc/rc	Tue Jan  5 21:20:46 2016	(r293226
> > > > )
> > > > +++ head/etc/rc	Tue Jan  5 21:20:47 2016	(r293227
> > > > )
> > > > @@ -132,9 +132,9 @@ done
> > > > # Remove the firstboot sentinel, and reboot if it was
> > > > requested.
> > > > if [ -e ${firstboot_sentinel} ]; then
> > > > 	[ ${root_rw_mount} = "yes" ] || mount -uw /
> > > > -	/bin/rm ${firstboot_sentinel}
> > > > +	/bin/rm -f ${firstboot_sentinel}
> > > > 	if [ -e ${firstboot_sentinel}-reboot ]; then
> > > > -		/bin/rm ${firstboot_sentinel}-reboot
> > > > +		/bin/rm -f ${firstboot_sentinel}-reboot
> > > > 		[ ${root_rw_mount} = "yes" ] || mount -ur /
> > > > 		kill -INT 1
> > > > 	fi
> > > > 
> > > 
> > > Using rm -f to suppress an error message seems like a bad idea
> > > here --
> > > if the sentinel file can't be removed that implies it's going to
> > > do
> > > firstboot behavior every time it boots, and that's the sort of
> > > error
> > > that should be in-your-face.  Especially on the reboot one
> > > because
> > > you're going to be stuck in a reboot loop with no error message.
> > > 
> > 
> > Leaving off -f so that the user gets prompted isn't quite as
> > helpful
> > as, say, using -f but then testing to make sure the file is really
> > gone
> > (if it still exists after a silent "rm -f", put up an informative
> > warning
> > instead of asking the user if they would like to delete it).
> > 
> > The end-result of having something thrown in your face seems
> > desirable. Having a prompt that asks you if you'd like to delete it
> > (even if there is an error immediately above it explaining it could
> > not be deleted) seems nonsensical.
> > 
> 
> More specifically, firstboot is most likely run in situations where
> no 
> one will be at the console, so an interactive prompt stopping the
> system 
> from coming up is bad.
> 

I couldn't possibly disagree more.  If you're not paying attention to
what happens the first time you boot a freshly installed system, you
deserve whatever happens to you.  On the other hand it's about the
worst to have everything silently seem to work right, and the actual
error is going to happen *next* time you boot (which is when it's
really likely you're not paying attention because everything seemed to
be okay the first time).

-- Ian




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1452040072.1320.49.camel>