Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Masahiro Yamada]

2018-02-22 1:57 GMT+09:00 Arnd Bergmann :
> On Tue, Feb 20, 2018 at 6:04 PM, Masahiro Yamada
>  wrote:
>> 2018-02-20 18:59 GMT+09:00 Arnd Bergmann :
>>> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
>>>  wrote:
 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
>>>
> @@ -621,14 +622,24 @@ int main(int ac, char **av)
> case randconfig:name = "allrandom.config"; break;
> default: break;
> }
> -   if (conf_read_simple(name, S_DEF_USER) &&
> -   conf_read_simple("all.config", S_DEF_USER)) {
> -   fprintf(stderr,
> -   _("*** KCONFIG_ALLCONFIG set, but no 
> \"%s\" or \"all.config\" file found\n"),
> -   name);
> -   exit(1);
> +   /* try ./name, arch/$(ARCH)/configs/name and 
> kernel/config/name */
> +   if (!conf_read_simple(name, S_DEF_USER))
> +   break;
> +   arch = getenv("ARCH");
> +   if (arch) {
> +   snprintf(fullname, sizeof(fullname), 
> "arch/%s/configs/%s",
> +arch, name);
> +   if (!conf_read_simple(fullname, S_DEF_USER))
> +   break;
> }


 I am not a big fan of hard-coding the kernel directory structure.

 We already do this [1], but I am thinking of kicking this out.
 [1] 
 https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33
>>>
>>> Ok, I see. How about adding a way to detect that we are build-testing
>>> with randconfig instead?
>>
>>
>> How about implementing something in scripts/kconfig/Makefile?
>>
>> merge_config collects config fragments into 'configfiles'
>>
>> I was thinking of a similar thing.
>>
>>
>> If KCONFIG_ALLCONFIG is 1,
>> scripts/kconfig/Makefile searches
>> './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/'
>> and sets the found file in it,
>>
>> If KCONFIG_ALLCONFIG already contains a file name, it is used as-is.
>>
>> scripts/kconfig/conf.c will be simplified.
>
> Yes, good idea. I'm struggling a bit with the implementation (fitting
> it into the
> simple-tagets rule), can you come up with a prototype?


Just a prototype for the Makefile part.
(We need to agree with the search order specification, though)


>8-
configs_dirs := $(srctree)/kernel/configs/ $(srctree)/arch/$(SRCARCH)/configs/

allconfig_files = $(if $(filter 1, $(KCONFIG_ALLCONFIG)),$(firstword \
$(foreach f, $(1), $(wildcard $(addsuffix $(f), ./ $(srctree)/))) \
$(foreach f, $(1), $(subst $(space),$(comma),$(wildcard
$(addsuffix $(f), $(configs_dirs)), \
$(KCONFIG_ALLCONFIG))

allyesconfig: override KCONFIG_ALLCONFIG := $(call allconfig_files,
allyes.config all.config)
allmodconfig: override KCONFIG_ALLCONFIG := $(call allconfig_files,
allmoyes.config all.config)
 (likewise for allnoconfig, alldefconfig)
randconfig: override KCONFIG_ALLCONFIG := $(call allconfig_files,
allrandom.config all.config)
->8---


If a user specifies a file path in KCONFIG_ALLCONFIG, it is used as-is.

IF KCONFIG_ALLCONFIG=1, the input file is searched in the following order.
( [1] - [4] is the same as the current specification)

[1] ./allrandom.config
[2] $(srctree)/allrandom.config
[3] ./all.config
[4] $(srctree)/all.config
[5] $(srctree)/kernel/configs/allrandom.config +
$(srctree)/arch/$(SRCARCH)/configs/allrandom.config
[6] $(srctree)/kernel/configs/all.config +
$(srctree)/arch/$(SRCARCH)/configs/all.config

For [5] and [6], if multiple files are found, KCONFIG_ALLCONFIG
is set to comma-separated file paths.

scripts/kconfig/conf.c must be tweaked a little bit
if we need to read multiple config fragments.

Currently, tinyconfig for x86 comes from common part and arch-specific part.
./kernel/configs/tiny.config
./arch/x86/configs/tiny.config


What do you think?







> We also need to decide what happens when e.g. both a ./all.config and
> arch/${SRCARCH}/configs/allrand.config file exist, should the first path
> take precedence or the more specific file?
>



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Masahiro Yamada]

2018-02-22 1:57 GMT+09:00 Arnd Bergmann :
> On Tue, Feb 20, 2018 at 6:04 PM, Masahiro Yamada
>  wrote:
>> 2018-02-20 18:59 GMT+09:00 Arnd Bergmann :
>>> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
>>>  wrote:
 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
>>>
> @@ -621,14 +622,24 @@ int main(int ac, char **av)
> case randconfig:name = "allrandom.config"; break;
> default: break;
> }
> -   if (conf_read_simple(name, S_DEF_USER) &&
> -   conf_read_simple("all.config", S_DEF_USER)) {
> -   fprintf(stderr,
> -   _("*** KCONFIG_ALLCONFIG set, but no 
> \"%s\" or \"all.config\" file found\n"),
> -   name);
> -   exit(1);
> +   /* try ./name, arch/$(ARCH)/configs/name and 
> kernel/config/name */
> +   if (!conf_read_simple(name, S_DEF_USER))
> +   break;
> +   arch = getenv("ARCH");
> +   if (arch) {
> +   snprintf(fullname, sizeof(fullname), 
> "arch/%s/configs/%s",
> +arch, name);
> +   if (!conf_read_simple(fullname, S_DEF_USER))
> +   break;
> }


 I am not a big fan of hard-coding the kernel directory structure.

 We already do this [1], but I am thinking of kicking this out.
 [1] 
 https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33
>>>
>>> Ok, I see. How about adding a way to detect that we are build-testing
>>> with randconfig instead?
>>
>>
>> How about implementing something in scripts/kconfig/Makefile?
>>
>> merge_config collects config fragments into 'configfiles'
>>
>> I was thinking of a similar thing.
>>
>>
>> If KCONFIG_ALLCONFIG is 1,
>> scripts/kconfig/Makefile searches
>> './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/'
>> and sets the found file in it,
>>
>> If KCONFIG_ALLCONFIG already contains a file name, it is used as-is.
>>
>> scripts/kconfig/conf.c will be simplified.
>
> Yes, good idea. I'm struggling a bit with the implementation (fitting
> it into the
> simple-tagets rule), can you come up with a prototype?


Just a prototype for the Makefile part.
(We need to agree with the search order specification, though)


>8-
configs_dirs := $(srctree)/kernel/configs/ $(srctree)/arch/$(SRCARCH)/configs/

allconfig_files = $(if $(filter 1, $(KCONFIG_ALLCONFIG)),$(firstword \
$(foreach f, $(1), $(wildcard $(addsuffix $(f), ./ $(srctree)/))) \
$(foreach f, $(1), $(subst $(space),$(comma),$(wildcard
$(addsuffix $(f), $(configs_dirs)), \
$(KCONFIG_ALLCONFIG))

allyesconfig: override KCONFIG_ALLCONFIG := $(call allconfig_files,
allyes.config all.config)
allmodconfig: override KCONFIG_ALLCONFIG := $(call allconfig_files,
allmoyes.config all.config)
 (likewise for allnoconfig, alldefconfig)
randconfig: override KCONFIG_ALLCONFIG := $(call allconfig_files,
allrandom.config all.config)
->8---


If a user specifies a file path in KCONFIG_ALLCONFIG, it is used as-is.

IF KCONFIG_ALLCONFIG=1, the input file is searched in the following order.
( [1] - [4] is the same as the current specification)

[1] ./allrandom.config
[2] $(srctree)/allrandom.config
[3] ./all.config
[4] $(srctree)/all.config
[5] $(srctree)/kernel/configs/allrandom.config +
$(srctree)/arch/$(SRCARCH)/configs/allrandom.config
[6] $(srctree)/kernel/configs/all.config +
$(srctree)/arch/$(SRCARCH)/configs/all.config

For [5] and [6], if multiple files are found, KCONFIG_ALLCONFIG
is set to comma-separated file paths.

scripts/kconfig/conf.c must be tweaked a little bit
if we need to read multiple config fragments.

Currently, tinyconfig for x86 comes from common part and arch-specific part.
./kernel/configs/tiny.config
./arch/x86/configs/tiny.config


What do you think?







> We also need to decide what happens when e.g. both a ./all.config and
> arch/${SRCARCH}/configs/allrand.config file exist, should the first path
> take precedence or the more specific file?
>



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Masahiro Yamada]

2018-02-22 1:57 GMT+09:00 Arnd Bergmann :
> On Tue, Feb 20, 2018 at 6:04 PM, Masahiro Yamada
>  wrote:
>> 2018-02-20 18:59 GMT+09:00 Arnd Bergmann :
>>> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
>>>  wrote:
 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
>>>
> @@ -621,14 +622,24 @@ int main(int ac, char **av)
> case randconfig:name = "allrandom.config"; break;
> default: break;
> }
> -   if (conf_read_simple(name, S_DEF_USER) &&
> -   conf_read_simple("all.config", S_DEF_USER)) {
> -   fprintf(stderr,
> -   _("*** KCONFIG_ALLCONFIG set, but no 
> \"%s\" or \"all.config\" file found\n"),
> -   name);
> -   exit(1);
> +   /* try ./name, arch/$(ARCH)/configs/name and 
> kernel/config/name */
> +   if (!conf_read_simple(name, S_DEF_USER))
> +   break;
> +   arch = getenv("ARCH");
> +   if (arch) {
> +   snprintf(fullname, sizeof(fullname), 
> "arch/%s/configs/%s",
> +arch, name);
> +   if (!conf_read_simple(fullname, S_DEF_USER))
> +   break;
> }


 I am not a big fan of hard-coding the kernel directory structure.

 We already do this [1], but I am thinking of kicking this out.
 [1] 
 https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33
>>>
>>> Ok, I see. How about adding a way to detect that we are build-testing
>>> with randconfig instead?
>>
>>
>> How about implementing something in scripts/kconfig/Makefile?
>>
>> merge_config collects config fragments into 'configfiles'
>>
>> I was thinking of a similar thing.
>>
>>
>> If KCONFIG_ALLCONFIG is 1,
>> scripts/kconfig/Makefile searches
>> './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/'
>> and sets the found file in it,
>>
>> If KCONFIG_ALLCONFIG already contains a file name, it is used as-is.
>>
>> scripts/kconfig/conf.c will be simplified.
>
> Yes, good idea. I'm struggling a bit with the implementation (fitting
> it into the
> simple-tagets rule), can you come up with a prototype?
>
> We also need to decide what happens when e.g. both a ./all.config and
> arch/${SRCARCH}/configs/allrand.config file exist, should the first path
> take precedence or the more specific file?
>

Hmm, maybe the following order?

[1] ./allrandom.config
[2] $(srctree)/allrandom.config
[3] ./all.config
[4] $(srctree)/all.config
[5] kernel/configs/allrandom.config + arch/$(SRCARCH)/configs/allrandom.config


I think you want to include
both kernel/configs/allrandom.config and
arch/$(SRCARCH)/configs/allrandom.config
if exist.

Some options are common, some are arch-specific.

This is how tinyconfig works.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Masahiro Yamada]

2018-02-22 1:57 GMT+09:00 Arnd Bergmann :
> On Tue, Feb 20, 2018 at 6:04 PM, Masahiro Yamada
>  wrote:
>> 2018-02-20 18:59 GMT+09:00 Arnd Bergmann :
>>> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
>>>  wrote:
 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
>>>
> @@ -621,14 +622,24 @@ int main(int ac, char **av)
> case randconfig:name = "allrandom.config"; break;
> default: break;
> }
> -   if (conf_read_simple(name, S_DEF_USER) &&
> -   conf_read_simple("all.config", S_DEF_USER)) {
> -   fprintf(stderr,
> -   _("*** KCONFIG_ALLCONFIG set, but no 
> \"%s\" or \"all.config\" file found\n"),
> -   name);
> -   exit(1);
> +   /* try ./name, arch/$(ARCH)/configs/name and 
> kernel/config/name */
> +   if (!conf_read_simple(name, S_DEF_USER))
> +   break;
> +   arch = getenv("ARCH");
> +   if (arch) {
> +   snprintf(fullname, sizeof(fullname), 
> "arch/%s/configs/%s",
> +arch, name);
> +   if (!conf_read_simple(fullname, S_DEF_USER))
> +   break;
> }


 I am not a big fan of hard-coding the kernel directory structure.

 We already do this [1], but I am thinking of kicking this out.
 [1] 
 https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33
>>>
>>> Ok, I see. How about adding a way to detect that we are build-testing
>>> with randconfig instead?
>>
>>
>> How about implementing something in scripts/kconfig/Makefile?
>>
>> merge_config collects config fragments into 'configfiles'
>>
>> I was thinking of a similar thing.
>>
>>
>> If KCONFIG_ALLCONFIG is 1,
>> scripts/kconfig/Makefile searches
>> './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/'
>> and sets the found file in it,
>>
>> If KCONFIG_ALLCONFIG already contains a file name, it is used as-is.
>>
>> scripts/kconfig/conf.c will be simplified.
>
> Yes, good idea. I'm struggling a bit with the implementation (fitting
> it into the
> simple-tagets rule), can you come up with a prototype?
>
> We also need to decide what happens when e.g. both a ./all.config and
> arch/${SRCARCH}/configs/allrand.config file exist, should the first path
> take precedence or the more specific file?
>

Hmm, maybe the following order?

[1] ./allrandom.config
[2] $(srctree)/allrandom.config
[3] ./all.config
[4] $(srctree)/all.config
[5] kernel/configs/allrandom.config + arch/$(SRCARCH)/configs/allrandom.config


I think you want to include
both kernel/configs/allrandom.config and
arch/$(SRCARCH)/configs/allrandom.config
if exist.

Some options are common, some are arch-specific.

This is how tinyconfig works.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Arnd Bergmann]

On Tue, Feb 20, 2018 at 6:04 PM, Masahiro Yamada
 wrote:
> 2018-02-20 18:59 GMT+09:00 Arnd Bergmann :
>> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
>>  wrote:
>>> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
>>
 @@ -621,14 +622,24 @@ int main(int ac, char **av)
 case randconfig:name = "allrandom.config"; break;
 default: break;
 }
 -   if (conf_read_simple(name, S_DEF_USER) &&
 -   conf_read_simple("all.config", S_DEF_USER)) {
 -   fprintf(stderr,
 -   _("*** KCONFIG_ALLCONFIG set, but no 
 \"%s\" or \"all.config\" file found\n"),
 -   name);
 -   exit(1);
 +   /* try ./name, arch/$(ARCH)/configs/name and 
 kernel/config/name */
 +   if (!conf_read_simple(name, S_DEF_USER))
 +   break;
 +   arch = getenv("ARCH");
 +   if (arch) {
 +   snprintf(fullname, sizeof(fullname), 
 "arch/%s/configs/%s",
 +arch, name);
 +   if (!conf_read_simple(fullname, S_DEF_USER))
 +   break;
 }
>>>
>>>
>>> I am not a big fan of hard-coding the kernel directory structure.
>>>
>>> We already do this [1], but I am thinking of kicking this out.
>>> [1] 
>>> https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33
>>
>> Ok, I see. How about adding a way to detect that we are build-testing
>> with randconfig instead?
>
>
> How about implementing something in scripts/kconfig/Makefile?
>
> merge_config collects config fragments into 'configfiles'
>
> I was thinking of a similar thing.
>
>
> If KCONFIG_ALLCONFIG is 1,
> scripts/kconfig/Makefile searches
> './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/'
> and sets the found file in it,
>
> If KCONFIG_ALLCONFIG already contains a file name, it is used as-is.
>
> scripts/kconfig/conf.c will be simplified.

Yes, good idea. I'm struggling a bit with the implementation (fitting
it into the
simple-tagets rule), can you come up with a prototype?

We also need to decide what happens when e.g. both a ./all.config and
arch/${SRCARCH}/configs/allrand.config file exist, should the first path
take precedence or the more specific file?

 Arnd

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Arnd Bergmann]

On Tue, Feb 20, 2018 at 6:04 PM, Masahiro Yamada
 wrote:
> 2018-02-20 18:59 GMT+09:00 Arnd Bergmann :
>> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
>>  wrote:
>>> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
>>
 @@ -621,14 +622,24 @@ int main(int ac, char **av)
 case randconfig:name = "allrandom.config"; break;
 default: break;
 }
 -   if (conf_read_simple(name, S_DEF_USER) &&
 -   conf_read_simple("all.config", S_DEF_USER)) {
 -   fprintf(stderr,
 -   _("*** KCONFIG_ALLCONFIG set, but no 
 \"%s\" or \"all.config\" file found\n"),
 -   name);
 -   exit(1);
 +   /* try ./name, arch/$(ARCH)/configs/name and 
 kernel/config/name */
 +   if (!conf_read_simple(name, S_DEF_USER))
 +   break;
 +   arch = getenv("ARCH");
 +   if (arch) {
 +   snprintf(fullname, sizeof(fullname), 
 "arch/%s/configs/%s",
 +arch, name);
 +   if (!conf_read_simple(fullname, S_DEF_USER))
 +   break;
 }
>>>
>>>
>>> I am not a big fan of hard-coding the kernel directory structure.
>>>
>>> We already do this [1], but I am thinking of kicking this out.
>>> [1] 
>>> https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33
>>
>> Ok, I see. How about adding a way to detect that we are build-testing
>> with randconfig instead?
>
>
> How about implementing something in scripts/kconfig/Makefile?
>
> merge_config collects config fragments into 'configfiles'
>
> I was thinking of a similar thing.
>
>
> If KCONFIG_ALLCONFIG is 1,
> scripts/kconfig/Makefile searches
> './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/'
> and sets the found file in it,
>
> If KCONFIG_ALLCONFIG already contains a file name, it is used as-is.
>
> scripts/kconfig/conf.c will be simplified.

Yes, good idea. I'm struggling a bit with the implementation (fitting
it into the
simple-tagets rule), can you come up with a prototype?

We also need to decide what happens when e.g. both a ./all.config and
arch/${SRCARCH}/configs/allrand.config file exist, should the first path
take precedence or the more specific file?

 Arnd

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Masahiro Yamada]

2018-02-20 18:59 GMT+09:00 Arnd Bergmann :
> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
>  wrote:
>> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
>
>>> @@ -621,14 +622,24 @@ int main(int ac, char **av)
>>> case randconfig:name = "allrandom.config"; break;
>>> default: break;
>>> }
>>> -   if (conf_read_simple(name, S_DEF_USER) &&
>>> -   conf_read_simple("all.config", S_DEF_USER)) {
>>> -   fprintf(stderr,
>>> -   _("*** KCONFIG_ALLCONFIG set, but no \"%s\" 
>>> or \"all.config\" file found\n"),
>>> -   name);
>>> -   exit(1);
>>> +   /* try ./name, arch/$(ARCH)/configs/name and 
>>> kernel/config/name */
>>> +   if (!conf_read_simple(name, S_DEF_USER))
>>> +   break;
>>> +   arch = getenv("ARCH");
>>> +   if (arch) {
>>> +   snprintf(fullname, sizeof(fullname), 
>>> "arch/%s/configs/%s",
>>> +arch, name);
>>> +   if (!conf_read_simple(fullname, S_DEF_USER))
>>> +   break;
>>> }
>>
>>
>> I am not a big fan of hard-coding the kernel directory structure.
>>
>> We already do this [1], but I am thinking of kicking this out.
>> [1] 
>> https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33
>
> Ok, I see. How about adding a way to detect that we are build-testing
> with randconfig instead?


How about implementing something in scripts/kconfig/Makefile?

merge_config collects config fragments into 'configfiles'

I was thinking of a similar thing.


If KCONFIG_ALLCONFIG is 1,
scripts/kconfig/Makefile searches
'./', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/'
and sets the found file in it,

If KCONFIG_ALLCONFIG already contains a file name, it is used as-is.

scripts/kconfig/conf.c will be simplified.



>> BTW, I am trying to compiler capability check to Kconfig.
>> If this work is done, I hope some "depends on !COMPILE_TEST" will go
>> away (but not all?)
>> https://patchwork.kernel.org/patch/10225375/
>
> I think most of the 'depends on !COMPILE_TEST' are for cases that
> impact compile-testing in some way (more false-positive warnings,
> fewer real warnings, much slower compile speed), rather than a lack
> of check for a specific compiler version.
>
> We might be able to replace the CONFIG_STANDALONE and
> CONFIG_PREVENT_FIRMWARE_BUILD with Kconfig $(shell)
> checks if you think that leads to an improvement.
>

OK, then your kernel/configs/allrandom.config approach will be useful.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Masahiro Yamada]

2018-02-20 18:59 GMT+09:00 Arnd Bergmann :
> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
>  wrote:
>> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
>
>>> @@ -621,14 +622,24 @@ int main(int ac, char **av)
>>> case randconfig:name = "allrandom.config"; break;
>>> default: break;
>>> }
>>> -   if (conf_read_simple(name, S_DEF_USER) &&
>>> -   conf_read_simple("all.config", S_DEF_USER)) {
>>> -   fprintf(stderr,
>>> -   _("*** KCONFIG_ALLCONFIG set, but no \"%s\" 
>>> or \"all.config\" file found\n"),
>>> -   name);
>>> -   exit(1);
>>> +   /* try ./name, arch/$(ARCH)/configs/name and 
>>> kernel/config/name */
>>> +   if (!conf_read_simple(name, S_DEF_USER))
>>> +   break;
>>> +   arch = getenv("ARCH");
>>> +   if (arch) {
>>> +   snprintf(fullname, sizeof(fullname), 
>>> "arch/%s/configs/%s",
>>> +arch, name);
>>> +   if (!conf_read_simple(fullname, S_DEF_USER))
>>> +   break;
>>> }
>>
>>
>> I am not a big fan of hard-coding the kernel directory structure.
>>
>> We already do this [1], but I am thinking of kicking this out.
>> [1] 
>> https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33
>
> Ok, I see. How about adding a way to detect that we are build-testing
> with randconfig instead?


How about implementing something in scripts/kconfig/Makefile?

merge_config collects config fragments into 'configfiles'

I was thinking of a similar thing.


If KCONFIG_ALLCONFIG is 1,
scripts/kconfig/Makefile searches
'./', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/'
and sets the found file in it,

If KCONFIG_ALLCONFIG already contains a file name, it is used as-is.

scripts/kconfig/conf.c will be simplified.



>> BTW, I am trying to compiler capability check to Kconfig.
>> If this work is done, I hope some "depends on !COMPILE_TEST" will go
>> away (but not all?)
>> https://patchwork.kernel.org/patch/10225375/
>
> I think most of the 'depends on !COMPILE_TEST' are for cases that
> impact compile-testing in some way (more false-positive warnings,
> fewer real warnings, much slower compile speed), rather than a lack
> of check for a specific compiler version.
>
> We might be able to replace the CONFIG_STANDALONE and
> CONFIG_PREVENT_FIRMWARE_BUILD with Kconfig $(shell)
> checks if you think that leads to an improvement.
>

OK, then your kernel/configs/allrandom.config approach will be useful.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Arnd Bergmann]

On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
 wrote:
> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :

>> @@ -621,14 +622,24 @@ int main(int ac, char **av)
>> case randconfig:name = "allrandom.config"; break;
>> default: break;
>> }
>> -   if (conf_read_simple(name, S_DEF_USER) &&
>> -   conf_read_simple("all.config", S_DEF_USER)) {
>> -   fprintf(stderr,
>> -   _("*** KCONFIG_ALLCONFIG set, but no \"%s\" 
>> or \"all.config\" file found\n"),
>> -   name);
>> -   exit(1);
>> +   /* try ./name, arch/$(ARCH)/configs/name and 
>> kernel/config/name */
>> +   if (!conf_read_simple(name, S_DEF_USER))
>> +   break;
>> +   arch = getenv("ARCH");
>> +   if (arch) {
>> +   snprintf(fullname, sizeof(fullname), 
>> "arch/%s/configs/%s",
>> +arch, name);
>> +   if (!conf_read_simple(fullname, S_DEF_USER))
>> +   break;
>> }
>
>
> I am not a big fan of hard-coding the kernel directory structure.
>
> We already do this [1], but I am thinking of kicking this out.
> [1] 
> https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33

Ok, I see. How about adding a way to detect that we are build-testing
with randconfig instead?

> BTW, I am trying to compiler capability check to Kconfig.
> If this work is done, I hope some "depends on !COMPILE_TEST" will go
> away (but not all?)
> https://patchwork.kernel.org/patch/10225375/

I think most of the 'depends on !COMPILE_TEST' are for cases that
impact compile-testing in some way (more false-positive warnings,
fewer real warnings, much slower compile speed), rather than a lack
of check for a specific compiler version.

We might be able to replace the CONFIG_STANDALONE and
CONFIG_PREVENT_FIRMWARE_BUILD with Kconfig $(shell)
checks if you think that leads to an improvement.

Arnd

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Arnd Bergmann]

On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada
 wrote:
> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann :

>> @@ -621,14 +622,24 @@ int main(int ac, char **av)
>> case randconfig:name = "allrandom.config"; break;
>> default: break;
>> }
>> -   if (conf_read_simple(name, S_DEF_USER) &&
>> -   conf_read_simple("all.config", S_DEF_USER)) {
>> -   fprintf(stderr,
>> -   _("*** KCONFIG_ALLCONFIG set, but no \"%s\" 
>> or \"all.config\" file found\n"),
>> -   name);
>> -   exit(1);
>> +   /* try ./name, arch/$(ARCH)/configs/name and 
>> kernel/config/name */
>> +   if (!conf_read_simple(name, S_DEF_USER))
>> +   break;
>> +   arch = getenv("ARCH");
>> +   if (arch) {
>> +   snprintf(fullname, sizeof(fullname), 
>> "arch/%s/configs/%s",
>> +arch, name);
>> +   if (!conf_read_simple(fullname, S_DEF_USER))
>> +   break;
>> }
>
>
> I am not a big fan of hard-coding the kernel directory structure.
>
> We already do this [1], but I am thinking of kicking this out.
> [1] 
> https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33

Ok, I see. How about adding a way to detect that we are build-testing
with randconfig instead?

> BTW, I am trying to compiler capability check to Kconfig.
> If this work is done, I hope some "depends on !COMPILE_TEST" will go
> away (but not all?)
> https://patchwork.kernel.org/patch/10225375/

I think most of the 'depends on !COMPILE_TEST' are for cases that
impact compile-testing in some way (more false-positive warnings,
fewer real warnings, much slower compile speed), rather than a lack
of check for a specific compiler version.

We might be able to replace the CONFIG_STANDALONE and
CONFIG_PREVENT_FIRMWARE_BUILD with Kconfig $(shell)
checks if you think that leads to an improvement.

Arnd

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Masahiro Yamada]

2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
> The kernel currently supports two methods of dealing with config
> fragments in the tree:
>
> a) Running "make foo.config" looks for arch/$(ARCH)/configs/foo.config
>and kernel/configs/foo.config, and applies the defaults from those
>files on top of the current configuration.
>
> b) Running "KCONFIG_ALLCONFIG=1 make randconfig" (or the equivalent
>allmodconfig/allnoconfig/allyesconfig/alldefconfig) will look
>for a "allrandconfig.config" file in the current directory or the
>top of the $(srctree). These are used as defaults before we generate
>the remaining options.
>
> This is rather inconsistent, and prevents us from easily shipping
> good defaults for "randconfig". I'm extending the logic here so that
> the second case also looks for the hardcoded file names in the standard
> directories (first arch/$(ARCH)/configs/, then kernel/configs) in the
> source tree.
>
> Signed-off-by: Arnd Bergmann 
> ---
>  scripts/kconfig/conf.c | 27 +++
>  1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 866369f10ff8..848bf4d15e9a 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -493,8 +493,9 @@ int main(int ac, char **av)
>  {
> const char *progname = av[0];
> int opt;
> -   const char *name, *defconfig_file = NULL /* gcc uninit */;
> +   const char *arch, *name, *defconfig_file = NULL /* gcc uninit */;
> struct stat tmpstat;
> +   char fullname[PATH_MAX+1];
>
> setlocale(LC_ALL, "");
> bindtextdomain(PACKAGE, LOCALEDIR);
> @@ -621,14 +622,24 @@ int main(int ac, char **av)
> case randconfig:name = "allrandom.config"; break;
> default: break;
> }
> -   if (conf_read_simple(name, S_DEF_USER) &&
> -   conf_read_simple("all.config", S_DEF_USER)) {
> -   fprintf(stderr,
> -   _("*** KCONFIG_ALLCONFIG set, but no \"%s\" 
> or \"all.config\" file found\n"),
> -   name);
> -   exit(1);
> +   /* try ./name, arch/$(ARCH)/configs/name and 
> kernel/config/name */
> +   if (!conf_read_simple(name, S_DEF_USER))
> +   break;
> +   arch = getenv("ARCH");
> +   if (arch) {
> +   snprintf(fullname, sizeof(fullname), 
> "arch/%s/configs/%s",
> +arch, name);
> +   if (!conf_read_simple(fullname, S_DEF_USER))
> +   break;
> }


I am not a big fan of hard-coding the kernel directory structure.

We already do this [1], but I am thinking of kicking this out.
[1] 
https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33



BTW, I am trying to compiler capability check to Kconfig.
If this work is done, I hope some "depends on !COMPILE_TEST" will go
away (but not all?)
https://patchwork.kernel.org/patch/10225375/



> -   break;
> +   snprintf(fullname, sizeof(fullname), "kernel/configs/%s", 
> name);
> +   if (!conf_read_simple(fullname, S_DEF_USER))
> +   break;
> +
> +   fprintf(stderr,
> +   _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or 
> \"all.config\" file found\n"),
> +   name);
> +   exit(1);
> default:
> break;
> }
> --
> 2.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] Kconfig: improve handling for all{rand,yes,no,}.config fragments

[Masahiro Yamada]

2018-02-17 6:41 GMT+09:00 Arnd Bergmann :
> The kernel currently supports two methods of dealing with config
> fragments in the tree:
>
> a) Running "make foo.config" looks for arch/$(ARCH)/configs/foo.config
>and kernel/configs/foo.config, and applies the defaults from those
>files on top of the current configuration.
>
> b) Running "KCONFIG_ALLCONFIG=1 make randconfig" (or the equivalent
>allmodconfig/allnoconfig/allyesconfig/alldefconfig) will look
>for a "allrandconfig.config" file in the current directory or the
>top of the $(srctree). These are used as defaults before we generate
>the remaining options.
>
> This is rather inconsistent, and prevents us from easily shipping
> good defaults for "randconfig". I'm extending the logic here so that
> the second case also looks for the hardcoded file names in the standard
> directories (first arch/$(ARCH)/configs/, then kernel/configs) in the
> source tree.
>
> Signed-off-by: Arnd Bergmann 
> ---
>  scripts/kconfig/conf.c | 27 +++
>  1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 866369f10ff8..848bf4d15e9a 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -493,8 +493,9 @@ int main(int ac, char **av)
>  {
> const char *progname = av[0];
> int opt;
> -   const char *name, *defconfig_file = NULL /* gcc uninit */;
> +   const char *arch, *name, *defconfig_file = NULL /* gcc uninit */;
> struct stat tmpstat;
> +   char fullname[PATH_MAX+1];
>
> setlocale(LC_ALL, "");
> bindtextdomain(PACKAGE, LOCALEDIR);
> @@ -621,14 +622,24 @@ int main(int ac, char **av)
> case randconfig:name = "allrandom.config"; break;
> default: break;
> }
> -   if (conf_read_simple(name, S_DEF_USER) &&
> -   conf_read_simple("all.config", S_DEF_USER)) {
> -   fprintf(stderr,
> -   _("*** KCONFIG_ALLCONFIG set, but no \"%s\" 
> or \"all.config\" file found\n"),
> -   name);
> -   exit(1);
> +   /* try ./name, arch/$(ARCH)/configs/name and 
> kernel/config/name */
> +   if (!conf_read_simple(name, S_DEF_USER))
> +   break;
> +   arch = getenv("ARCH");
> +   if (arch) {
> +   snprintf(fullname, sizeof(fullname), 
> "arch/%s/configs/%s",
> +arch, name);
> +   if (!conf_read_simple(fullname, S_DEF_USER))
> +   break;
> }


I am not a big fan of hard-coding the kernel directory structure.

We already do this [1], but I am thinking of kicking this out.
[1] 
https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33



BTW, I am trying to compiler capability check to Kconfig.
If this work is done, I hope some "depends on !COMPILE_TEST" will go
away (but not all?)
https://patchwork.kernel.org/patch/10225375/



> -   break;
> +   snprintf(fullname, sizeof(fullname), "kernel/configs/%s", 
> name);
> +   if (!conf_read_simple(fullname, S_DEF_USER))
> +   break;
> +
> +   fprintf(stderr,
> +   _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or 
> \"all.config\" file found\n"),
> +   name);
> +   exit(1);
> default:
> break;
> }
> --
> 2.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada