[PATCH] kbuild: Enable building defconfigs from Kconfig files

[PATCH] kbuild: Enable building defconfigs from Kconfig files

[Grant Likely]

[cc'ing rmk and linux-arm-kernel]

On Mon, Jul 12, 2010 at 7:43 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> After this change, doing a "make xxx_defconfig" will check first for
> a file called arch/<arch>/configs/Kconfig.xxx and use that to generate
> the .config (effectively starting from an allnoconfig). ?If that file
> doesn't exist, it will use arch/<ARCH>/configs/xxx_defconfig as now.

Oops, I hadn't seen this patch when I wrote mine this afternoon[1].
:-)  A few minor differences, but essentially the same solution.

[1] http://patchwork.ozlabs.org/patch/58823/

I chose to use -D /dev/null (defconfig from an empty file) instead of
-n (allnoconfig) so that default values in Kconfig would get
respected.  For the benefit of everyone else, here's an excerpt from
our IRC conversation this afternoon:

19:49 < gcl> sfr: [...] Your patch and my patch are
             essentially doing exactly the same thing, except that I used '-d'
             and you used '-n'.
19:50 < gcl> s/-d/-D/
19:55 < sfr> right
19:55 < sfr> Linus wanted us to use -n
19:55 < sfr> because that way you get what you asked for, not what the defaults
             say ...
19:58 < gcl> I suppose we don't currently have a way to say "select FOO=n", so
             I suppose that makes sense
19:58 < gcl> although I think using the defaults unless told not to is a better
             approach in the long run
19:59 < gcl> most of the time I *don't want* to ask for something in the
             defconfig.  :-)
20:00 < gcl> I just want TheBestOrCorrectAnswer to be chosen for me
20:04 < sfr> gcl: go read Linus' vision :-)

> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> ?scripts/kconfig/Makefile | ? 14 +++++++++++++-
> ?1 files changed, 13 insertions(+), 1 deletions(-)
>
> Hi Linus,
>
> Is this more the direction you want to take?
>
> There are still 2 main problems with is approach:
>
> ? ? ? ?- there are some config options that are globally and
> unconditionally enabled that some platforms may not want. ?The only way
> currently to turn them off is to reproduce the config entry with the
> different default. ?I am not sure if we need a wa to turn them off or to
> just change them to being neede to be selected by those that do want them.
> ? ? ? ?- we have no way to select options that are neither bool or
> tristate to suitable values. ?Again the only way to do that currently is
> to reproduce the config entry with a different default value.

For both of the above problems, what if we added syntax like the
following to Kconfig?

config FOO
        bool
        select BAR = n
        select FOO_VALUE = 54

> I am currently working towards using this to recreate the PowerPC
> defconfigs, but it is a slow process as they have some much stuff enabled
> in them and some of it is probably actually not relevant.

If the trimmed configs are merged, then there is no rush on this.  We
can keep them around and switch them over as people want to make
changes.

> This process is made easier by the recent commit "kbuild: Warn on
> selecting symbols with unmet direct dependencies" that is in the kbuild
> tree (and linux-next).

Ah, I didn't know that change was being merged.  That indeed makes
things easier, and eliminates the post-test that I do to make sure
that the resulting config is actually valid.

> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index 7ea649d..1ab8f45 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -117,9 +117,21 @@ else
> ? ? ? ?$(Q)$< -D arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) $(Kconfig)
> ?endif
>
> -%_defconfig: $(obj)/conf
> +configs_dir := $(srctree)/arch/$(SRCARCH)/configs
> +# We check a level of sub directories because arch/powerpc
> +# has its defconfig files arranged that way
> +old_defconfigs := $(patsubst $(configs_dir)/%,%,\
> + ? ? ? $(wildcard $(configs_dir)/*_defconfig) \
> + ? ? ? $(wildcard $(configs_dir)/*/*_defconfig))
> +defconfigs := $(patsubst $(configs_dir)/Kconfig.%,%_defconfig,\
> + ? ? ? $(wildcard $(configs_dir)/Kconfig.*))
> +

Ugh.  My first impression is that all this shouldn't be necessary, and
it should be okay to just make the two following rules include a
pattern dependency for the source file.  However, as I play with it, I
cannot seem to get the rules right to handle the sub directories.  The
$(defconfigs) patsubst at least could be done solely with dependencies
on the target rule if the files were named *.Kconfig instead of
Kconfig.* (because subdirectories are not handled in that case).  For
example:

%_defconfig: $(obj)/conf arch/$(SRCARCH)/configs/%.Kconfig
        $(Q)$< -n arch/$(SRCARCH)/configs/$*.Kconfig

> +$(old_defconfigs): %_defconfig: $(obj)/conf
> ? ? ? ?$(Q)$< -D arch/$(SRCARCH)/configs/$@ $(Kconfig)
>
> +$(defconfigs): %_defconfig: $(obj)/conf
> + ? ? ? $(Q)$< -n arch/$(SRCARCH)/configs/Kconfig.$*
> +
> ?# Help text used by make help
> ?help:
> ? ? ? ?@echo ?' ?config ? ? ? ? ?- Update current config utilising a line-oriented program'

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

[PATCH] kbuild: Enable building defconfigs from Kconfig files

[Linus Torvalds]

On Tue, Jul 13, 2010 at 7:26 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>
> I chose to use -D /dev/null (defconfig from an empty file) instead of
> -n (allnoconfig) so that default values in Kconfig would get
> respected. ?For the benefit of everyone else, here's an excerpt from
> our IRC conversation this afternoon:
>
> 19:49 < gcl> sfr: [...] Your patch and my patch are
> ? ? ? ? ? ? essentially doing exactly the same thing, except that I used '-d'
> ? ? ? ? ? ? and you used '-n'.
> 19:50 < gcl> s/-d/-D/
> 19:55 < sfr> right
> 19:55 < sfr> Linus wanted us to use -n

Just a note: Linus doesn't really care.

IOW, I used -n not because of any fundamental belief that it is
correct, but just because ti happened to be how I happened to decide
to solve it. It's entirely possible that starting from the Kconfig
defaults (rather than "no") is the right way to go.

I think either approach is likely fine. The -D /dev/null approach
would presumably give smaller Kconfig.xyz files, assuming our defaults
are sane (an maybe that could be at least a partial validation of the
defaults we do have). While the -n approach is in some ways more
stable, in that the resulting Kconfig.xyz entires would presumably be
more stand-alone, and there wouldn't be any subtle interactions when
somebody changes a default value int he Kconfig file.

So I can see advantages to either model. And either model clearly
would want the improvements to "select" that are independent (ie warn
about unsatisfied 'depends on' constraints, and select recursively.
Maybe we already fixed the recursive select thing, I forget).

I also think we need to allow setting of actual values. I don't know
what the syntax would be. A "set" statement that overrides a default
in the Kconfig file, so that you can do

          set NODES_SHIFT 10

which would basically be equivalent to a "select" of a tristate
variable, but instead set the actual value? I dunno.

And quite frankly, maybe somebody comes up with a better model
entirely. I like the Kconfig.xyz model, in that it should be
human-readable/writable and shouldn't introduce any fundamentally new
concepts (except the fairly simple extensions to the Kconfig
language), but maybe there are better models.

Regardless, I don't have anything against either set of patches
(Grant's or Stephen's).

                       Linus

[PATCH] kbuild: Enable building defconfigs from Kconfig files

[Grant Likely]

On Tue, Jul 13, 2010 at 10:04 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Jul 13, 2010 at 7:26 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>>
>> I chose to use -D /dev/null (defconfig from an empty file) instead of
>> -n (allnoconfig) so that default values in Kconfig would get
>> respected. ?For the benefit of everyone else, here's an excerpt from
>> our IRC conversation this afternoon:
>>
>> 19:49 < gcl> sfr: [...] Your patch and my patch are
>> ? ? ? ? ? ? essentially doing exactly the same thing, except that I used '-d'
>> ? ? ? ? ? ? and you used '-n'.
>> 19:50 < gcl> s/-d/-D/
>> 19:55 < sfr> right
>> 19:55 < sfr> Linus wanted us to use -n
>
> Just a note: Linus doesn't really care.
>
> IOW, I used -n not because of any fundamental belief that it is
> correct, but just because ti happened to be how I happened to decide
> to solve it. It's entirely possible that starting from the Kconfig
> defaults (rather than "no") is the right way to go.
>
> I think either approach is likely fine. The -D /dev/null approach
> would presumably give smaller Kconfig.xyz files, assuming our defaults
> are sane (an maybe that could be at least a partial validation of the
> defaults we do have). While the -n approach is in some ways more
> stable, in that the resulting Kconfig.xyz entires would presumably be
> more stand-alone, and there wouldn't be any subtle interactions when
> somebody changes a default value int he Kconfig file.

Okay, well I advocate for the -D /dev/null approach then.  I think
that validating our defaults, and looking for the subtle interactions
are exactly what we want to be doing when it comes to defconfigs.  The
fact that a defconfig does *not* want the default value is exactly
what the defconfigs should be capturing.

> So I can see advantages to either model. And either model clearly
> would want the improvements to "select" that are independent (ie warn
> about unsatisfied 'depends on' constraints, and select recursively.
> Maybe we already fixed the recursive select thing, I forget).
>
> I also think we need to allow setting of actual values. I don't know
> what the syntax would be. A "set" statement that overrides a default
> in the Kconfig file, so that you can do
>
> ? ? ? ? ?set NODES_SHIFT 10
>
> which would basically be equivalent to a "select" of a tristate
> variable, but instead set the actual value? I dunno.

I'm partial to extending select statements myself because it fits
nicely into the existing grammer; but I can see value in having a set
statement too.  It would eliminate the temporary config options that
both my and Stephen's patch would add.

> And quite frankly, maybe somebody comes up with a better model
> entirely. I like the Kconfig.xyz model, in that it should be
> human-readable/writable and shouldn't introduce any fundamentally new
> concepts (except the fairly simple extensions to the Kconfig
> language), but maybe there are better models.

Perhaps, but I can't think of anything and this one is simple,
elegant, and easy to implement.

> Regardless, I don't have anything against either set of patches
> (Grant's or Stephen's).

I think we should run with this.  Since the patch has been merged to
warn on unmet Kconfig dependences, the only major hole left is being
able to do negative selects and to select specific values.  Stephen,
I'm happy to either keep working on this, or drop my patch in favor of
yours.  Whichever you prefer.  I'll try to find some time to look at
the Kconfig grammer.

The solver would also be useful and could further reduce the size of
the Kconfig fragments, but it isn't necessary so we don't need to wait
for it to get implemented to take this approach..

Cheers,
g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

[PATCH] kbuild: Enable building defconfigs from Kconfig files

[Linus Torvalds]

On Tue, Jul 13, 2010 at 10:47 PM, Grant Likely
<grant.likely@secretlab.ca> wrote:
>
> Okay, well I advocate for the -D /dev/null approach then. ?I think
> that validating our defaults, and looking for the subtle interactions
> are exactly what we want to be doing when it comes to defconfigs. ?The
> fact that a defconfig does *not* want the default value is exactly
> what the defconfigs should be capturing.

Sure. That does have the downside that it absolutely _requires_
changes to the Kconfig language. Because currently "select" can only
set a config variable, it cannot clear it or downgrade it from the
defaults - so if you start from anything but a noconfig, you can't
unset anything.

But that's not a big downside, since honestly either approach will
want it anyway (even -n wants it for setting integer or string options
- never mind the whole issue of making it easier/cleaner to write the
files in the first place).

>> ? ? ? ? ?set NODES_SHIFT 10
>
> I'm partial to extending select statements myself because it fits
> nicely into the existing grammer; but I can see value in having a set
> statement too.

If it turns out easier to just implement the "set value" as a "select
with a value", that's fine by me too. If the syntax for setting a
particular value ends up being

   select NODES_SHIFT 10

that still works (with or without a "=" there). It looks a bit odd to
me, but that's probably just because it's new. It doesn't look _wrong_
by any stretch.

>?It would eliminate the temporary config options that
> both my and Stephen's patch would add.

I don't think there is anything wrong with having the extra config
option just to drive the "select" syntax. In fact, in many cases I
think it would be "documentation", because I think the extra config
option should generally be named by the platform that you're actually
selecting for - even if that name may not be used for anything else.

So having the basic syntax for a Kconfig.xyz file be something like

         config SIDEWINDER_PLATFORM
              def_bool y
              select ...

where that initial config/def_bool is strictly not needed for the
actual end result, but is used to get to the select statements, and to
include that "SIDEWINDER_PLATFORM=y" in the resulting .config as a
kind of top-level comment all looks fine.

                 Linus