Re: problem with LOCAL_MODULES

On Fri, Aug 30, 2019 at 2:26 PM Kyle Evans <kevans_at_freebsd.org> wrote:
>
> On Fri, Aug 30, 2019 at 2:11 PM John Baldwin <jhb_at_freebsd.org> wrote:
> >
> > On 8/30/19 10:42 AM, Kyle Evans wrote:
> > > On Fri, Aug 16, 2019 at 7:38 PM John Baldwin <jhb_at_freebsd.org> wrote:
> > >>
> > >> On 8/16/19 3:05 AM, Gary Jennejohn wrote:
> > >>> I tried to build a kernel today and it failed in modules-all even
> > >>> though I had LOCAL_MODULES="" in /etc/src.conf, as recommended by
> > >>> jhb.
> > >>>
> > >>> That's wrong.  It has to be LOCAL_MODULES=, otherwise
> > >>> /sys/conf/kern.post.mk seems to conclude that there should be a
> > >>> module under /usr/local/sys/modules with the name "".
> > >>
> > >> I think this will permit both versions to work:
> > >>
> > >> Index: sys/conf/kern.post.mk
> > >> ===================================================================
> > >> --- kern.post.mk        (revision 351151)
> > >> +++ kern.post.mk        (working copy)
> > >> _at__at_ -76,6 +76,7 _at__at_ modules-${target}:
> > >>         cd $S/modules; ${MKMODULESENV} ${MAKE} \
> > >>             ${target:S/^reinstall$/install/:S/^clobber$/cleandir/}
> > >>  .endif
> > >> +.if !empty(LOCAL_MODULES)
> > >>  .for module in ${LOCAL_MODULES}
> > >>         _at_${ECHODIR} "===> ${module} (${target:S/^reinstall$/install/:S/^clobber$/cleandir/})"
> > >>         _at_cd ${LOCAL_MODULES_DIR}/${module}; ${MKMODULESENV} ${MAKE} \
> > >> _at__at_ -83,6 +84,7 _at__at_ modules-${target}:
> > >>             ${target:S/^reinstall$/install/:S/^clobber$/cleandir/}
> > >>  .endfor
> > >>  .endif
> > >> +.endif
> > >>  .endfor
> > >>
> > >>  # Handle ports (as defined by the user) that build kernel modules
> > >>
> > >
> > > I think I'd like to see this with !empty(LOCAL_MODULES) &&
> > > EXISTS(${LOCAL_MODULES_DIR}) or maybe just the latter condition to
> > > prevent accidental foot-shooting... I was testing a problem with doing
> > > this stuff in a poudriere build for swills_at_ and set LOCAL_MODULES=""
> > > only to get an error because LOCAL_MODULES_DIR doesn't yet exist on
> > > the machine I was testing with -- which we can trivially avoid.
> >
> > Did this work for you?  Gary said in a followup that it didn't work,
> > so that's why I hadn't committed it.
> >
>
> Hmm... I went back and tested his exact scenario, and no-
> LOCAL_MODULES isn't empty at this point because word processing that
> leaves us with an empty string hasn't yet been done. An .if
> !empty(module) inside the loop is much happier- I'm not seeing any
> modifiers that would give us the behavior we wanted for figuring this
> out before entering the loop.

It turns out I'm silly... this is sufficient for both of our problems
(because it's the same problem) and hopefully my mail client doesn't
goof this up:

diff --git a/sys/conf/kern.post.mk b/sys/conf/kern.post.mk
index ff10daf1a0a..09bfffad095 100644
--- a/sys/conf/kern.post.mk
+++ b/sys/conf/kern.post.mk
_at__at_ -77,10 +77,12 _at__at_ modules-${target}:
            ${target:S/^reinstall$/install/:S/^clobber$/cleandir/}
 .endif
 .for module in ${LOCAL_MODULES}
+.if !empty(module)
        _at_${ECHODIR} "===> ${module}
(${target:S/^reinstall$/install/:S/^clobber$/cleandir/})"
        _at_cd ${LOCAL_MODULES_DIR}/${module}; ${MKMODULESENV} ${MAKE} \
            DIRPRFX="${module}/" \
            ${target:S/^reinstall$/install/:S/^clobber$/cleandir/}
+.endif
 .endfor
 .endif
 .endfor