On Thu, Oct 06, 2016 at 03:09:49PM +0200, Martin Natano wrote:
> On Thu, Oct 06, 2016 at 02:02:41PM +0200, Theo Buehler wrote:
> > Before calling 'make includes' during 'make build', the first two steps
> > are repetitions of what a direct 'make includes' does anyway, so move
> > the privdrop up there and simply call 'make includes' from 'make build'.
> > 
> > While there, use the exec idiom also for 'make cleandirs'.
> 
> Seems right to me. I wonder whether we should enforce root for includes
> like we do for build. Either in the command list of the target, or even
> something like this:

I'm somewhat torn.  Most, if not all, of the targets require root at
some point, don't they?  Do we want to annotate all of them?  How much
handholding do we really need to add to the Makefile?

Adding the warning to 'make build' and 'make release' made sense since
these are targets that are used by many and they are going through a
drastic transformation.

Regardless, a unified way of spewing that warning probably makes sense,
if we're going to add it to more places than we have now.  I would
probably go for the require_root target in bsd.own.mk first for its
simplicity and because it adds the least clutter to the Makefiles
themselves.

Unfortunately, your suggested solution will always print

require_root must be called by root

so that doesn't work quite as nicely as intended.

> 
> UID!= id -u
> .if ${UID} == 0
> includes:
>       # actual code
> 
> build:
>       # actual code
> .else
> build includes:
>       @echo make $@ must be called by root >&2
>       @false
> .endif
> 
> 
> 
> or this:
> 
> (in bsd.own.mk)
> require_root:
>       @if [[ `id -u` -ne 0 ]]; then \
>               echo $@ must be called by root >&2; \
>               false; \
>       fi
> .PHONY: require_root
> 
> (in src/Makefile)
> include: require_root
>       # actual code
> 
> build: require_root
>       # actual code
> 
> 
> I thinke a require_root target in bsd.own.mk could be useful in other
> parts of the tree too and it helps to keep the Makefile's clean from
> those checks. What do you think?

> 
> natano
> 
> 
> > 
> > ok?
> > 
> > Index: Makefile
> > ===================================================================
> > RCS file: /var/cvs/src/Makefile,v
> > retrieving revision 1.127
> > diff -u -p -r1.127 Makefile
> > --- Makefile        5 Oct 2016 18:00:41 -0000       1.127
> > +++ Makefile        6 Oct 2016 11:31:46 -0000
> > @@ -50,7 +50,9 @@ regression-tests:
> >     @cd ${.CURDIR}/regress && ${MAKE} depend && exec ${MAKE} regress
> >  
> >  includes:
> > -   cd ${.CURDIR}/include && ${MAKE} prereq && exec ${MAKE} includes
> > +   cd ${.CURDIR}/include && \
> > +           su ${BUILDUSER} -c 'exec ${MAKE} prereq' && \
> > +           exec ${MAKE} includes
> >  
> >  beforeinstall:
> >     cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} distrib-dirs
> > @@ -77,10 +79,8 @@ build:
> >             false; \
> >     fi
> >     cd ${.CURDIR}/share/mk && exec ${MAKE} install
> > -   cd ${.CURDIR}/include && \
> > -       su ${BUILDUSER} -c 'exec ${MAKE} prereq' && \
> > -       exec ${MAKE} includes
> > -   ${MAKE} cleandir
> > +   exec ${MAKE} includes
> > +   exec ${MAKE} cleandir
> >     cd ${.CURDIR}/lib && \
> >         su ${BUILDUSER} -c '${MAKE} depend && exec ${MAKE}' && \
> >         NOMAN=1 exec ${MAKE} install
> > 
> 

Reply via email to