svn commit: r337857 - in head: bin/csh bin/sh etc

[Rodney W. Grimes]

> On Wed, Aug 15, 2018 at 10:31:04AM -0700, Rodney W. Grimes wrote:
> > > On Wed, Aug 15, 2018, at 11:17 AM, Ravi Pokala wrote:
> > > > Brad,
> > > > 
> > > > -----Original Message-----
> > > > From: <owner-src-committers at freebsd.org> on behalf of Brad Davis 
> > > > <brd at FreeBSD.org>
> > > > Date: 2018-08-15, Wednesday at 09:22
> > > > To: <src-committers at freebsd.org>, <svn-src-all at freebsd.org>, <svn-src-
> > > > head at freebsd.org>
> > > > Subject: svn commit: r337857 - in head: bin/csh bin/sh etc
> > > > 
> > > > > Author: brd
> > > > > Date: Wed Aug 15 16:22:12 2018
> > > > > New Revision: 337857
> > > > > URL: https://svnweb.freebsd.org/changeset/base/337857
> > > > > 
> > > > > Log:
> > > > >   Fix build after r337849
> > > > >   
> > > > >   This moves the symlink creation to after where the files are installed.
> > > > >   
> > > > >   This also inverts the shell change so that it only happens if MK_TCSH is on.
> > > > ...
> > > > > Modified: head/etc/master.passwd
> > > > > ==============================================================================
> > > > > --- head/etc/master.passwd	Wed Aug 15 16:16:59 2018	(r337856)
> > > > > +++ head/etc/master.passwd	Wed Aug 15 16:22:12 2018	(r337857)
> > > > > @@ -1,6 +1,6 @@
> > > > >  # $FreeBSD$
> > > > >  #
> > > > > -root::0:0::0:0:Charlie &:/root:/bin/csh
> > > > > +root::0:0::0:0:Charlie &:/root:/bin/sh
> > > > >  toor:*:0:0::0:0:Bourne-again Superuser:/root:
> > > > >  daemon:*:1:1::0:0:Owner of many system processes:/root:/usr/sbin/nologin
> > > > >  operator:*:2:5::0:0:System &:/:/usr/sbin/nologin
> > > > 
> > > > Woah! Changing the root shell wasn't mentioned in the change 
> > > > description, has nothing to do with fixing r337849, and is a *HUGE* POLA 
> > > > violation. At the very least, a change of this magnitude needs public 
> > > > discussion, and even if the community agreed, it would also require an 
> > > > UPDATING message and relnote.
> > > > 
> > > > Please revert this change to master.passwd immediately.
> > > 
> > > Hi Ravi,
> > > 
> > > Please, look closer.  It doesn't change what is actually installed in either case, it just inverts the logic.
> > 
> > Though the end results maybe the same, we now have a sed that
> > is running in the normal case, is not run in the exception case,
> > a src file that must be editted to match the distribution file,
> > etc.  I can not express stronly enough how much I object to this.
> > 
> > I have explained in your diffential that this should actually be
> > put back as far as inverted, and that actually bin/csh/Makefile
> > nor bin/sh/Makefile should never even touch etc/master.password,
> > simply the wrong place to be doing it.   This should be in what
> > ever Makefile installs /etc/master.password an no place else.
> 
> As a side observation, this kind of editing at build time can break
> -DNO_ROOT.  See PR 209718 for a different example of the same problem.
> I haven't yet verified that this change has the same issue; my apologies
> if I'm missing something and there is no problem after all.
> 

I do not believe that there are any edits at build time here,
these are in install: type targets, or should be.   None the less,
there could be -DNO_ROOT issues I am not thinking of, as well as
half a dozen other issues.  My concern is we are now at a patch to
a patch to a patch, and that tells me the initial change was not
well thought out and it would probably be best to back out and
rethink the whole thing.

Does anyone have issue with reverting and starting back with
the original differential, fixing it to DTRT ?

-- 
Rod Grimes                                                 rgrimes at freebsd.org