Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2

[Doug Goldstein <cardoe@xxxxxxxxxx>]

On 11/30/15 7:59 AM, Jan Beulich wrote:
>>>> On 24.11.15 at 18:51, <cardoe@xxxxxxxxxx> wrote:
>> --- /dev/null
>> +++ b/xen/include/linux/kconfig.h
>> @@ -0,0 +1,54 @@
>> +#ifndef __LINUX_KCONFIG_H
>> +#define __LINUX_KCONFIG_H
> 
> Neither placement in the source tree nor guard variable should say
> "Linux".

This file gets removed later in the series. I meant to squash that into
this patch.

> 
>> --- /dev/null
>> +++ b/xen/scripts/Makefile.host
>> @@ -0,0 +1,128 @@
> 
> We already have ways to build host programs. Do we really need a
> second mechanism, the more an abstract one? I'd prefer a more
> minimalistic approach (confined to the xen/scripts/kconfig/ subtree,
> which imo actually should be xen/tools/kconfig/). In any even I'd
> expect, for everything outside of the actual kconfig/ subtree, a
> few words in the commit message towards the rationale for including
> those pieces. That'll help future cleanup by clarifying what certain
> pieces are actually needed for.

I can move it into xen/scripts/kconfig. The Xen make rules to build host
programs leave things to be desired and its actually necessary to use
these to build the different config variants. Earlier in the review
process (or really the RFC) I suggested bringing those parts of Kbuild
into Xen, which would have the effect of providing the ability to build
all the necessary pieces but I was told not to. So I'll just move this
file into xen/scripts/kconfig.

I can also move everything to xen/tools/kconfig.

> 
>> --- /dev/null
>> +++ b/xen/scripts/kconfig/.gitignore
>> @@ -0,0 +1,22 @@
> 
> Does the top level one (perhaps suitably adjusted) not fit the needs?

The whole point here was to bring in the kconfig bits from Linux 4.2
untouched for traceability for where the code came from. By doing it
this way it allows Xen to rebase kconfig support to a newer version much
easier since its all self-contained.

> 
>> --- /dev/null
>> +++ b/xen/scripts/kconfig/Makefile.linux
>> @@ -0,0 +1,317 @@
> 
> This doesn't seem to be referenced anywhere.

Its the central file for building kconfig. Its refereinced in
xen/scripts/kconfig/Makefile on line 63. This is the only file I've
renamed from its original name in the Linux source tree.

> 
>> --- /dev/null
>> +++ b/xen/scripts/kconfig/POTFILES.in
>> @@ -0,0 +1,12 @@
> 
> Do we really need this?

If we don't expect to provide localization of kconfig, then no. Granted
I didn't wire up the other localization bits.

> 
>> +void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const 
>> char *), void *data, int prevtoken)
>> +{
>> +    if (!e) {
>> +            fn(data, NULL, "y");
>> +            return;
>> +    }
>> +
>> +    if (expr_compare_type(prevtoken, e->type) > 0)
>> +            fn(data, NULL, "(");
>> +    switch (e->type) {
>> +    case E_SYMBOL:
>> +            if (e->left.sym->name)
>> +                    fn(data, e->left.sym, e->left.sym->name);
>> +            else
>> +                    fn(data, NULL, "<choice>");
>> +            break;
>> +    case E_NOT:
>> +            fn(data, NULL, "!");
>> +            expr_print(e->left.expr, fn, data, E_NOT);
>> +            break;
>> +    case E_EQUAL:
>> +            if (e->left.sym->name)
>> +                    fn(data, e->left.sym, e->left.sym->name);
>> +            else
>> +                    fn(data, NULL, "<choice>");
>> +            fn(data, NULL, "=");
>> +            fn(data, e->right.sym, e->right.sym->name);
>> +            break;
>> +    case E_LEQ:
>> +    case E_LTH:
>> +            if (e->left.sym->name)
>> +                    fn(data, e->left.sym, e->left.sym->name);
>> +            else
>> +                    fn(data, NULL, "<choice>");
>> +            fn(data, NULL, e->type == E_LEQ ? "<=" : "<");
>> +            fn(data, e->right.sym, e->right.sym->name);
>> +            break;
>> +    case E_GEQ:
>> +    case E_GTH:
>> +            if (e->left.sym->name)
>> +                    fn(data, e->left.sym, e->left.sym->name);
>> +            else
>> +                    fn(data, NULL, "<choice>");
>> +            fn(data, NULL, e->type == E_LEQ ? ">=" : ">");
> 
> Please eliminate the copy-and-paste mistake here right away (see
> Linux commit f6aad2615c).

In an effort to keep the history clean at 4.2, I'll pull this in as an
additional patch to the series.

> 
>> --- /dev/null
>> +++ b/xen/scripts/kconfig/gconf.c
>> @@ -0,0 +1,1521 @@
> 
> I think I had asked before, and with us not wanting any user visible
> prompts for now I wonder even more: Do we really need [gmnq]conf?
> All I think we really need is conf.
> 
> Jan
> 

Well as you (and others) have commented about the fact that KEXEC and
the ARM UARTs should be user configurable which is why I included those.
So I'll continue to leave those in ok?

-- 
Doug Goldstein

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel