always add standard kernel configuration include path

[avg]

Summary

This should change nothing for kernel configurations at the standard
locations in the source tree. However, if KERNCONFDIR is used to specify a
custom location for a kernel configuration file (e.g., out of tree), then
both the custom location and the standard location, in this order, will be
used as include paths for config(8). This will allow the kernel
configuration to include files from both locations.

[avg]

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

[bdrewery]

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.

[bdrewery]

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 fact I think it's a bug that there are 2 spellings here.

rS263429 added the -I.
rS82416 "renamed" KRNLDEFDIR to KERNCONFDIR
rS82362 added KERNLDEFDIR

Why should these not be the same?

@@ -434,7 +435,7 @@ buildkernel:
 .if !defined(NO_KERNELCONFIG)
        cd ${KRNLCONFDIR}; \
                PATH=${TMPPATH} \
-                   config ${CONFIGARGS} -d ${KRNLOBJDIR}/${_kernel} ${_kernel}
+                   config ${CONFIGARGS} -d ${KRNLOBJDIR}/${_kernel} ${KRNLDEFDIR}/${_kernel}
 .endif

The old code assumed the kernel was in the directory it ran in from the cd. The change presumably was intended to be a NOP; the cd was pointless after this change wasn't it?

[avg]

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.

[bdrewery]

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.

I understand that. But your change will rely on the questionable code. It could easily break later if someone "fixes" the cd ${KRNLCONFDIR}. I'm hoping to get more feedback from other subscribers on why these should be spelled differently. If it makes sense to keep (which your feature relies on) then we need to make these names more clear. That can happen in a subsequent commit and I'm not asking you to do it.

[kevans]

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:

1.) The magic in config(8) that pulls in DEFAULTS expects to find it in pwd. Trying to change that will likely immediately break any amd64 downstream project that set KERNCONFDIR and expect it to work without setting, for instance, 'machine'.
2.) config(8) uses the fact that it's in sys/${arch}/conf to derive the SRCDIR to set S= in the output Makefile.

#2 is easily remedied by supplying -s to config(8), but #1 is a little trickier. We can probably get away with searching include paths for DEFAULTS if any include paths were set and we don't find it in pwd. That probably works a little better if a downstream wants to override the defaults, but it's unclear to me if there's any legacy use that would suffer because of it.

[bdrewery]

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:

That sounds good to me for now.

[avg]

don't rely on the current directory being the default directory