Re: [PATCH 3/9] buildman: Support #include files in defconfigs

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]

On Fri, Nov 08, 2024 at 08:23:44AM -0700, Simon Glass wrote:

> This is used by some boards in U-Boot and is a convenient way to deal
> with common settings where using a Kconfig files is not desirable.
> 
> Detect #include files and process them as if they were part of the
> original file.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Fixes: https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/30
[snip]
> +defconfig fragments
> +-------------------
> +
> +Buildman provides some initial support for configuration fragments. It can scan
> +these when present in defconfig files and handle the resuiting Kconfig
> +correctly. Thus it is possible to build a board which has a ``#include`` in the
> +defconfig file.
> +
> +For now, Buildman simply includes the files to produce a single output file,
> +using the C preprocessor. It does not call the ``merge_config.sh`` script. The
> +redefined/redundant logic in that script could fairly easily be repeated in
> +Buildman, to detect potential problems. For now it is not clear that this is
> +useful.

I don't like this logic because the whole point of merge_config.sh is
that it IS the canonical way to handle Kconfig config files + fragments
and provides handy feedback like "You expected CONFIG_FOO=y but you
ended up with '# CONFIG_FOO is not set'". It's frankly an at least small
problem of our current cpp rule, but calling that for every defconfig
would be a performance nightmare too.

> +To specify the C preprocessor to use, set the ``CPP`` environment variable. The
> +default is ``cpp``.

Uh, I was hoping it would get the correct CPP and flags from the
Makefile? Otherwise this is going to fall down in some corner cases such
as I expect clang.

> +Note that Buildman does not support adding fragments to existing boards, e.g.
> +like::
> +
> +    make qemu_riscv64_defconfig acpi.config
> +
> +This is partly because there is no way for Buildman to know which fragments are
> +valid on which boards.

That seems like a really weird deficiency and non-sequitur. I don't know
why buildman would be attempting any sort of validation beyond syntax
validation. It's more that we don't have any way to pass additional
arguments to the "make defconfig" part of the build, yes? And then in
turn because buildman reads the defconfig itself too, prior to that
stage?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]