svn commit: r348521 - in head: . etc sys/conf sys/modules usr.bin/login usr.sbin/services_mkdb

[Maxim Sobolev]

https://reviews.freebsd.org/D20520 has been created as a follow-up. Give it
some feedback, thanks!

-Max

On Mon, Jun 3, 2019 at 11:42 AM Rodney W. Grimes <freebsd at gndrsh.dnsmgr.net>
wrote:

> > On Mon, Jun 3, 2019 at 7:28 AM Enji Cooper <yaneurabeya at gmail.com>
> wrote:
> >
> > > Hi Maxim,
> > >
> > >
> > > > Modified: head/Makefile.inc1
> > > >
> > >
> ==============================================================================
> > > > --- head/Makefile.inc1    Sun Jun  2 22:27:26 2019    (r348520)
> > > > +++ head/Makefile.inc1    Sun Jun  2 23:38:19 2019    (r348521)
> > > > @@ -887,8 +887,8 @@ MTREEFLAGS+=    -W
> > > > INSTALLFLAGS+=    -h sha256
> > > > .endif
> > > > .if defined(DB_FROM_SRC) || defined(NO_ROOT)
> > > > -IMAKE_INSTALL=    INSTALL="install ${INSTALLFLAGS}"
> > > > -IMAKE_MTREE=    MTREE_CMD="mtree ${MTREEFLAGS}"
> > > > +IMAKE_INSTALL=    INSTALL="${INSTALL} ${INSTALLFLAGS}"
> > > > +IMAKE_MTREE=    MTREE_CMD="${MTREE_CMD} ${MTREEFLAGS}"
>
> I see the bugger with this change now, it is a loop,
> MTREE_CMD=${MTREE_CMD}... and I do not think MTREE_CMD is defined
> at this point, see below.
>
> > > > .endif
> > >
> > >     Please back out this part of the change as it is unnecessary and
> adds
> > > a dependency on sys.mk?s definitions. $PATH should contain, i.e., be
> > > populated, with these tools when running make installworld. This is
> part of
> > > the logic behind how $ITOOLS, etc, works.
>
> That would be in direct conflict with how preceding tools
> worked,  INSTALL?=install has been there for a very very long time
> and any direct invoke of install in our Makefile/.mk files is just wrong,
> and for the very reasoning that Max is trying to do.
>
> I made the mistake of following someone else earlier that thought
> it was INSTALL that was undefined, and as we known now that was
> not the case, it was MTREE_CMD that was undefined, and that is
> less clear, as iirc when that was originally done mtree was
> contained soly in src/etc/Makefile by design.  It appears as
> if that hos how crept up directly into head/Makefile.inc1, and
> the definition for MTREE_CMD?= did not get moved to a more
> appropriate place.
>
> > >     Thank you very much for your work and the rest of the change
> though!
> > >
> >
> >  Thanks Enji, for a clarification, will do. I still cannot understand why
> > can't we have those tools being re-defined with top-level make
> environment?
>
> You should and must be able to do this.
> Especially for INSTALL, I fully support that part of this change staying.
>
> > What I am striving at is being able to do say, make INSTALL="abcd"
> > installworld. Yes, I understand I can call "abcd" "install" (or make a
> > symlink to it) and put it into PATH, but there are at least one issue
> here,
> > namely what if I need to pass some distinct argument to abcd based on
> usage
> > scenario. E.g. make INSTALL="abcd foo" distributiuon. In your case I
> should
> > be resorting to setting up some global environment variable and hope it
> > won't get erased somewhere in the call chain, but that's bit "how you
> > doing" IMHO. My approach makes certain things so much easier by cutting
> > those un-neded steps.
> >
> > At any rate thank for a feedback and some background info, I will do more
> > work on this. I was thinking more in terms of having some sort of
> > bsd.bootools.mk where this can be defined in a centralized manner.
> >
> > -Max
>
> --
> Rod Grimes
> rgrimes at freebsd.org
>
>