Re: sys/modules "make clean" seems broken again

On Monday, November 30, 2015 02:44:54 PM Warner Losh wrote:
> On Mon, Nov 30, 2015 at 1:28 PM, Sean Bruno <sbruno_at_freebsd.org> wrote:
> 
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA512
> >
> >
> >
> > On 11/19/15 15:42, Warner Losh wrote:
> > >
> > >
> > > On Thu, Nov 19, 2015 at 11:12 AM, John Baldwin <jhb_at_freebsd.org
> > > <mailto:jhb_at_freebsd.org>> wrote:
> > >
> > > On Thursday, November 19, 2015 09:13:06 AM Sean Bruno wrote:
> > >> I thought I fixed this a year or two ago, but now a "make clean"
> > >> in sys/modules seems to leave bus_if.h device_if.h and pci_if.h
> > >> in the directory.
> > >>
> > >> Should I just add these to the clean targets?
> > >
> > > Blame Warner as his MFILES changes broke this.  In particular, see
> > > r287263 which turned off cleaning these up.  I'm not sure what that
> > > broke that caused it to be disabled.
> > >
> > > Your change is a gross hack though.
> > >
> > >
> > > Yes. The reason I had to break it was that there were a few files
> > > named _if.c and _if.h in the tree that would get deleted when make
> > > clean was run in the modules.
> > >
> > > I'll take another run at fixing this. Sean's "fix" is a horrible
> > > hack.
> > >
> > > Warner
> >
> >
> > Just bouncing a ping here.  I haven't actually checked to see if the
> > last weeks' commits to head fixed this, but I suspect this hasn't been
> > addressed.  What do we want to do different if not this hackery I propose?
> >
> 
> Your hackery wouldn't actually fix it completely, just mostly for a common
> case.

So which files broke before?  Hmm, I guess just these:

./netpfil/pf/pf_if.c
./dev/oce/oce_if.c
./contrib/vchiq/interface/vchiq_arm/vchiq_if.h
./dev/oce/oce_if.h
./dev/mlx5/mlx5_rdma_if.h

Honestly, I still think just having a single global list of MFILES
in sys/conf/foo.mk instead of doing the find trick isn't that bad.

Actually, can't we use __MPATH to do this instead?

This seems to work:

Index: conf/kern.pre.mk
===================================================================
--- conf/kern.pre.mk	(revision 291495)
+++ conf/kern.pre.mk	(working copy)
_at__at_ -208,6 +208,7 _at__at_
 # Calculate path for .m files early, if needed.
 .if !defined(_MPATH)
 __MPATH!=find ${S:tA}/ -name \*_if.m
+_MFILES=${__MPATH:T:O}
 _MPATH=${__MPATH:H:O:u}
 .endif
 
_at__at_ -227,7 +228,7 _at__at_
 .if defined(DEBUG)
 MKMODULESENV+=	DEBUG_FLAGS="${DEBUG}"
 .endif
-MKMODULESENV+=	_MPATH="${_MPATH}"
+MKMODULESENV+=	_MPATH="${_MPATH}" _MFILES="${_MFILES}"
 
 # Architecture and output format arguments for objdump to convert image to
 # object file
Index: conf/kmod.mk
===================================================================
--- conf/kmod.mk	(revision 291495)
+++ conf/kmod.mk	(working copy)
_at__at_ -372,13 +372,11 _at__at_
 # Build _if.[ch] from _if.m, and clean them when we're done.
 .if !defined(_MPATH)
 __MPATH!=find ${SYSDIR:tA}/ -name \*_if.m
+_MFILES=${__MPATH:T:O}
 _MPATH=${__MPATH:H:O:u}
 .endif
 .PATH.m: ${_MPATH}
-.for _i in ${SRCS:M*_if.[ch]}
-#removes too much, comment out until it's more constrained.
-#CLEANFILES+=	${_i}
-.endfor # _i
+CLEANFILES+=	${_MFILES:R:S/$/.c/} ${_MFILES:R:S/$/.h/}
 .m.c:	${SYSDIR}/tools/makeobjops.awk
 	${AWK} -f ${SYSDIR}/tools/makeobjops.awk ${.IMPSRC} -c
 
It bloats CLEANFILES a bit because it always cleans all generated m files
even if they aren't present.  If you wanted to be picky about it you could
have a for loop that trims to just the used ones.  This seems to work:

% pwd
/usr/home/john/work/freebsd/clean/sys/modules/oce
% make -V CLEANFILES
export_syms machine x86 if_oce.ko if_oce.kld oce_if.o oce_hw.o oce_mbox.o oce_util.o oce_queue.o oce_sysctl.o opt_inet.h opt_inet6.h bus_if.h device_if.h pci_if.h

Index: conf/kern.pre.mk
===================================================================
--- conf/kern.pre.mk	(revision 291495)
+++ conf/kern.pre.mk	(working copy)
_at__at_ -208,6 +208,7 _at__at_
 # Calculate path for .m files early, if needed.
 .if !defined(_MPATH)
 __MPATH!=find ${S:tA}/ -name \*_if.m
+_MFILES=${__MPATH:T:O}
 _MPATH=${__MPATH:H:O:u}
 .endif
 
_at__at_ -227,7 +228,7 _at__at_
 .if defined(DEBUG)
 MKMODULESENV+=	DEBUG_FLAGS="${DEBUG}"
 .endif
-MKMODULESENV+=	_MPATH="${_MPATH}"
+MKMODULESENV+=	_MPATH="${_MPATH}" _MFILES="${_MFILES}"
 
 # Architecture and output format arguments for objdump to convert image to
 # object file
Index: conf/kmod.mk
===================================================================
--- conf/kmod.mk	(revision 291495)
+++ conf/kmod.mk	(working copy)
_at__at_ -372,12 +372,16 _at__at_
 # Build _if.[ch] from _if.m, and clean them when we're done.
 .if !defined(_MPATH)
 __MPATH!=find ${SYSDIR:tA}/ -name \*_if.m
+_MFILES=${__MPATH:T:O}
 _MPATH=${__MPATH:H:O:u}
 .endif
 .PATH.m: ${_MPATH}
 .for _i in ${SRCS:M*_if.[ch]}
-#removes too much, comment out until it's more constrained.
-#CLEANFILES+=	${_i}
+_MATCH=M${_i:R:S/$/.m/}
+_MATCHES=${_MFILES:${_MATCH}}
+.if !empty(_MATCHES)
+CLEANFILES+=	${_i}
+.endif
 .endfor # _i
 .m.c:	${SYSDIR}/tools/makeobjops.awk
 	${AWK} -f ${SYSDIR}/tools/makeobjops.awk ${.IMPSRC} -c

I've put this last one up at D4336.

-- 
John Baldwin