On Fri, Dec 02, 2022 at 02:08:33PM +0100, Theo Buehler wrote:
> On Fri, Dec 02, 2022 at 01:52:43PM +0100, Marc Espie wrote:
> > On Fri, Dec 02, 2022 at 12:59:02PM +0100, Theo Buehler wrote:
> > > I wanted to override some of the default run-regress-* targets by
> > > declaring them in regress Makefiles. Many tests already do that and
> > > it happens to work as expected.
> > >
> > > However, this relies on behavior in the BUGS section of make. In fact,
> > > espie informs me that gmake behaves the other way around and honors the
> > > last definition (gmake also warns about redefinitions).
> > >
> > > To avoid this, I'd like to have bsd.regress.mk define these default
> > > targets only if they aren't already defined. This matches most targets
> > > in the other *.mk files.
> > >
> > > I kept the .PHONY target outside of the .if block since most regress
> > > Makefiles don't use .PHONY at all although they often should. In fact,
> > > none of the run-regress-* targets I looked at were marked phony.
> > >
> > > Index: share/mk/bsd.regress.mk
> > > ===================================================================
> > > RCS file: /cvs/src/share/mk/bsd.regress.mk,v
> > > retrieving revision 1.24
> > > diff -u -p -r1.24 bsd.regress.mk
> > > --- share/mk/bsd.regress.mk 31 Aug 2021 23:33:05 -0000 1.24
> > > +++ share/mk/bsd.regress.mk 2 Dec 2022 11:54:43 -0000
> > > @@ -32,8 +32,10 @@ _REGRESS_TMP?=/dev/null
> > > _REGRESS_OUT= | tee -a ${REGRESS_LOG} ${_REGRESS_TMP} 2>&1 > /dev/null
> > >
> > > .for p in ${PROG} ${PROGS}
> > > +. if !target(run-regress-$p)
> > > run-regress-$p: $p
> > > ./$p
> > > +. endif
> > > .PHONY: run-regress-$p
> > > .endfor
> > >
> > >
> > >
> > I don't think it's correct, I would separate the dependency on $p,
> > which we almost certainly want anyway, and only do the
> > . if !target dance for the actual commands.
>
> Sorry, this is a bit too cryptic for me. What does 'separate the
> dependency on $p' mean?
>
> > I haven't used it all that much, but we stole
> > .if commands() from NetBSD back in 2012 precisely to allow this kind of
> > stuff
>
> Do you mean this?
>
> Index: bsd.regress.mk
> ===================================================================
> RCS file: /cvs/src/share/mk/bsd.regress.mk,v
> retrieving revision 1.24
> diff -u -p -r1.24 bsd.regress.mk
> --- bsd.regress.mk 31 Aug 2021 23:33:05 -0000 1.24
> +++ bsd.regress.mk 2 Dec 2022 12:59:03 -0000
> @@ -33,7 +33,9 @@ _REGRESS_OUT= | tee -a ${REGRESS_LOG} ${
>
> .for p in ${PROG} ${PROGS}
> run-regress-$p: $p
> +. if !commands(run-regress-$p)
> ./$p
> +. endif
> .PHONY: run-regress-$p
> .endfor
>
>
Yep! Now all it requires is testing it doesn't break a thing.
