FYI This fixes *DIRDEPS* not *META*. The file is misnamed.
I suggest adding @sjg to the review

Thanks for looking.

I may disappear so I'll approve for now; my questions aren't blockers but I think there is an unrelated change that should be fixed first.

I spot checked a few and they looked fine.

I don't like that we have auto generated and manual maintained outside of Makefile as it will get stale fast.

I think the comment from targets/ should be added in too.

# This file is not autogenerated - take care!

What case does this fix exactly?

In D22144#484006, @jhb wrote:

I guess the extra directories don't hurt. I'm less certain about the implications of the missing checks for an empty KERNBUILDDIR.

I only reviewed defs.mk. Looks fine.

In D21654#482501, @ngie wrote:

@bdrewery, @mat, @woodsb02: ping?

In D22059#482035, @imp wrote:

The question is, do we need them to be TARGET_* in this makefile.inc1 usage?
I kinda think it does. Checking into *that* detail...

In D22057#481956, @kevans wrote:

I'd be inclined to explicitly spell it as -I ${KRNLCONFDIR} instead of -I., and then we later rename KRNLCONFDIR to make it more clear. We can't quickly change the cd, at least, because:

In D22057#481900, @avg wrote:

I don't have answers to your questions, sorry.
And I did not intend for the review to center around KRNLCONFDIR vs KERNCONFDIR.
I just want to be able to do what this change allows me to do.
That is, I want to be able to include files from both directories.

In D22057#481894, @bdrewery wrote:

In D22057#481764, @avg wrote:

Just a note, KRNLCONFDIR is the standard location (derived from TARGET) and KERNCONFDIR is the actual, possibly overridden, location.

Sigh. Why. These appear to be the same at quick glance.

In D22057#481764, @avg wrote:

Just a note, KRNLCONFDIR is the standard location (derived from TARGET) and KERNCONFDIR is the actual, possibly overridden, location.

Oops sorry for missing this before. TARGET_ are only useful in Makefile.inc1.

We have use for this at work as well for FreeBSD amd64/32 ABI compat testing since we have our own ABI. Though we just use FreeBSD 10 dist files for the purpose this feature is still nice.

This is great!

I don't know lua yet so cannot review most of this. But I'm OK with expanding lua usage. One "regression" I see is that move from a heredoc to a ton of write_line lines. Can lua not continue a string on the next line? Or maybe we should have some kind of template file instead.

I'm planning to make the syscall files generate *during the build*. It will need to happen quite early. Is lua already an early bootstrap tool?

I like it. But it still comes down to a bikeshed/opinion. Anyway it's a good guidance.

If buildworld and installworld work then it seems fine.

Review findings

My change is at D21295

Something like this is probably needed instead

~/git/freebsd2/lib/libsysdecode # git diff
diff --git lib/libsysdecode/Makefile lib/libsysdecode/Makefile
index 2be244a6da46..9edfe067063c 100644
--- lib/libsysdecode/Makefile
+++ lib/libsysdecode/Makefile
@@ -127,7 +127,8 @@ incdir=${SYSROOT:U${DESTDIR}}${INCLUDEDIR}
 tables.h: mktables ${incdir}/netinet/in.h ${incdir}/netinet/tcp.h \
     ${incdir}/netinet6/in6.h
        sh ${.CURDIR}/mktables ${incdir} ${.TARGET}.tmp && \
-           mv -f ${.TARGET}.tmp ${.TARGET}
+           mv -f ${.TARGET}.tmp ${.TARGET} && \
+           mv -f .depend.${.TARGET}.tmp .depend.${.TARGET}

The hacks to using tables.h.tmp is probably what broke my work.

This already has .depend file support. Why is this needed?

Which package provides clang? I don't have a /usr/local/bin/clang on my system.

This doesn't seem to fix the SIGKILL issue.

~ # procstat -kk 19680
  PID    TID COMM                TDNAME              KSTACK
19680 101200 python2.7           -                   __mtx_lock_sleep+0x118 __mtx_lock_flags+0x102 _sleep+0x334 umtxq_busy+0xb7 do_sem_wait+0x150 __umtx_op_sem_wait+0x6e amd64_syscall+0x2bb fast_syscall_common+0x101
19680 101634 python2.7           -                   mi_switch+0x174 sleepq_switch+0x110 sleepq_catch_signals+0x417 sleepq_wait_sig+0xf _sleep+0x2d0 umtxq_sleep+0x153 do_sem_wait+0x41c __umtx_op_sem_wait+0x6e amd64_syscall+0x2bb fast_syscall_common+0x101
19680 101635 python2.7           -                   _mtx_lock_spin_cookie+0x231 __mtx_lock_spin_flags+0xf8 wakeup_one+0xf do_sem_wait+0x26f __umtx_op_sem_wait+0x6e amd64_syscall+0x2bb fast_syscall_common+0x101
~ # kill -9 19680
~ # procstat -kk 19680
  PID    TID COMM                TDNAME              KSTACK
19680 101200 python2.7           -                   __mtx_lock_flags+0xc1 do_sem_wait+0x218 __umtx_op_sem_wait+0x6e amd64_syscall+0x2bb fast_syscall_common+0x101
19680 101634 python2.7           -                   mi_switch+0x174 sleepq_switch+0x110 sleepq_catch_signals+0x417 sleepq_wait_sig+0xf _sleep+0x2d0 umtxq_sleep+0x153 do_sem_wait+0x41c __umtx_op_sem_wait+0x6e amd64_syscall+0x2bb fast_syscall_common+0x101
19680 101635 python2.7           -                   __mtx_lock_sleep+0x118 __mtx_lock_flags+0x102 do_sem_wait+0x148 __umtx_op_sem_wait+0x6e amd64_syscall+0x2bb fast_syscall_common+0x101
~ # procstat -i 19680|grep -v -- ---|grep KILL
19680 python2.7        KILL     P--

This is a massive POLA violation. What the hell. You took a flag that was intended to make <some effort> to overwrite data and made it SILENTLY DO NOTHING. That's insane.