From owner-freebsd-current@freebsd.org Tue Dec 1 18:30:24 2015 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5D9F9A3E581 for ; Tue, 1 Dec 2015 18:30:24 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 1B9F514F1; Tue, 1 Dec 2015 18:30:24 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 820D0B94C; Tue, 1 Dec 2015 13:30:22 -0500 (EST) From: John Baldwin To: Warner Losh Cc: Sean Bruno , FreeBSD Current Subject: Re: sys/modules "make clean" seems broken again Date: Tue, 01 Dec 2015 10:30:13 -0800 Message-ID: <4121969.dQSZfNjpat@ralph.baldwin.cx> User-Agent: KMail/4.14.3 (FreeBSD/10.2-STABLE; KDE/4.14.3; amd64; ; ) In-Reply-To: References: <564E0322.8050308@freebsd.org> <565CB176.4070906@freebsd.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 01 Dec 2015 13:30:22 -0500 (EST) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Dec 2015 18:30:24 -0000 On Monday, November 30, 2015 02:44:54 PM Warner Losh wrote: > On Mon, Nov 30, 2015 at 1:28 PM, Sean Bruno 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 > > > 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) @@ -208,6 +208,7 @@ # 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 @@ -227,7 +228,7 @@ .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) @@ -372,13 +372,11 @@ # 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) @@ -208,6 +208,7 @@ # 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 @@ -227,7 +228,7 @@ .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) @@ -372,12 +372,16 @@ # 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