diff mbox

[RFC] Kbuild: avoid "make tinyconfig" warnings

Message ID 20160704142608.2069991-1-arnd@arndb.de (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann July 4, 2016, 2:25 p.m. UTC
The introduction of "make *.config" as a shorthand for merging configuration
files unfortunately introduced some build warnings that we see in every
single run of the various build bots testing tinyconfig:

.config:966:warning: override: NOHIGHMEM changes choice state
.config:965:warning: override: SLOB changes choice state
.config:963:warning: override: KERNEL_XZ changes choice state
.config:962:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
.config:933:warning: override: SLOB changes choice state
.config:930:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
.config:870:warning: override: SLOB changes choice state
.config:868:warning: override: KERNEL_XZ changes choice state
.config:867:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state

This changes the behavior for tinyconfig again, so we call the special
allnoconfig target directly, with the fragment as the KCONFIG_ALLCONFIG
file so it gets merged into the configuration. The disadvantage of
this approach is that we cannot pass another KCONFIG_ALLCONFIG on
the command line, that would again have to be done using the mergeconfig
script afterwards.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 63a91033d52e ("kbuild: add generic mergeconfig target, %.config")
Link: https://storage.kernelci.org/mainline/v4.7-rc6/x86-tinyconfig/build.log
---
 scripts/kconfig/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Josh Triplett July 4, 2016, 7:51 p.m. UTC | #1
On Mon, Jul 04, 2016 at 04:25:55PM +0200, Arnd Bergmann wrote:
> The introduction of "make *.config" as a shorthand for merging configuration
> files unfortunately introduced some build warnings that we see in every
> single run of the various build bots testing tinyconfig:
> 
> .config:966:warning: override: NOHIGHMEM changes choice state
> .config:965:warning: override: SLOB changes choice state
> .config:963:warning: override: KERNEL_XZ changes choice state
> .config:962:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> .config:933:warning: override: SLOB changes choice state
> .config:930:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> .config:870:warning: override: SLOB changes choice state
> .config:868:warning: override: KERNEL_XZ changes choice state
> .config:867:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> 
> This changes the behavior for tinyconfig again, so we call the special
> allnoconfig target directly, with the fragment as the KCONFIG_ALLCONFIG
> file so it gets merged into the configuration. The disadvantage of
> this approach is that we cannot pass another KCONFIG_ALLCONFIG on
> the command line, that would again have to be done using the mergeconfig
> script afterwards.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 63a91033d52e ("kbuild: add generic mergeconfig target, %.config")
> Link: https://storage.kernelci.org/mainline/v4.7-rc6/x86-tinyconfig/build.log

Thanks for fixing this.
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

As another possibility, which would preserve the ability to use
KCONFIG_ALLCONFIG, what about adding a parameter to mergeconfig to
disable this warning?  I've seen many other uses of mergeconfig in
production systems that have the same problem.

>  scripts/kconfig/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index ebced77deb9c..35d0e191fe3f 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -125,8 +125,8 @@ xenconfig: xen.config
>  	@:
>  
>  PHONY += tinyconfig
> -tinyconfig:
> -	$(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config
> +tinyconfig: $(obj)/conf
> +	$(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig KCONFIG_ALLCONFIG=$(wildcard $(srctree)/kernel/configs/tiny.config $(srctree)/arch/$(SRCARCH)/configs/tiny.config)
>  
>  # Help text used by make help
>  help:
> -- 
> 2.9.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josh Triplett July 4, 2016, 7:58 p.m. UTC | #2
On Mon, Jul 04, 2016 at 12:51:56PM -0700, Josh Triplett wrote:
> On Mon, Jul 04, 2016 at 04:25:55PM +0200, Arnd Bergmann wrote:
> > The introduction of "make *.config" as a shorthand for merging configuration
> > files unfortunately introduced some build warnings that we see in every
> > single run of the various build bots testing tinyconfig:
> > 
> > .config:966:warning: override: NOHIGHMEM changes choice state
> > .config:965:warning: override: SLOB changes choice state
> > .config:963:warning: override: KERNEL_XZ changes choice state
> > .config:962:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > .config:933:warning: override: SLOB changes choice state
> > .config:930:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > .config:870:warning: override: SLOB changes choice state
> > .config:868:warning: override: KERNEL_XZ changes choice state
> > .config:867:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > 
> > This changes the behavior for tinyconfig again, so we call the special
> > allnoconfig target directly, with the fragment as the KCONFIG_ALLCONFIG
> > file so it gets merged into the configuration. The disadvantage of
> > this approach is that we cannot pass another KCONFIG_ALLCONFIG on
> > the command line, that would again have to be done using the mergeconfig
> > script afterwards.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > Fixes: 63a91033d52e ("kbuild: add generic mergeconfig target, %.config")
> > Link: https://storage.kernelci.org/mainline/v4.7-rc6/x86-tinyconfig/build.log
> 
> Thanks for fixing this.
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> As another possibility, which would preserve the ability to use
> KCONFIG_ALLCONFIG, what about adding a parameter to mergeconfig to
> disable this warning?  I've seen many other uses of mergeconfig in
> production systems that have the same problem.

Actually, I just realized something: does this actually do the right
thing if both tiny.config files exist?  The documentation for
KCONFIG_ALLCONFIG doesn't suggest that it can take two files as
arguments.  And if it can, this also need quotes; otherwise, this will
expand to "... allnoconfig KCONFIG=path/to/a/tiny.config
another/path/to/a/tiny.config".

> >  scripts/kconfig/Makefile | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> > index ebced77deb9c..35d0e191fe3f 100644
> > --- a/scripts/kconfig/Makefile
> > +++ b/scripts/kconfig/Makefile
> > @@ -125,8 +125,8 @@ xenconfig: xen.config
> >  	@:
> >  
> >  PHONY += tinyconfig
> > -tinyconfig:
> > -	$(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config
> > +tinyconfig: $(obj)/conf
> > +	$(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig KCONFIG_ALLCONFIG=$(wildcard $(srctree)/kernel/configs/tiny.config $(srctree)/arch/$(SRCARCH)/configs/tiny.config)
> >  
> >  # Help text used by make help
> >  help:
> > -- 
> > 2.9.0
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Masahiro Yamada July 6, 2016, 2 a.m. UTC | #3
Hi Arnd,


2016-07-04 23:25 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> The introduction of "make *.config" as a shorthand for merging configuration
> files unfortunately introduced some build warnings that we see in every
> single run of the various build bots testing tinyconfig:

I am not convinced with this statement.

Why do you think 63a91033d52e is a bad commit?


With/without 63a91033d52e, I see the same warnings.


$ git log --oneline  -1
63a9103 kbuild: add generic mergeconfig target, %.config
$ make -s mrproper
$ make tinyconfig >/dev/null
.config:871:warning: override: KERNEL_XZ changes choice state
.config:873:warning: override: SLOB changes choice state
.config:874:warning: override: NOHIGHMEM changes choice state
$ git checkout HEAD^
Previous HEAD position was 63a9103... kbuild: add generic mergeconfig
target, %.config
HEAD is now at bc8f8f5... merge_config.sh: rename MAKE to RUNMAKE
$ git log --oneline  -1
bc8f8f5 merge_config.sh: rename MAKE to RUNMAKE
$ make -s mrproper
$ make tinyconfig >/dev/null
.config:871:warning: override: KERNEL_XZ changes choice state
.config:873:warning: override: SLOB changes choice state
.config:874:warning: override: NOHIGHMEM changes choice state







> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index ebced77deb9c..35d0e191fe3f 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -125,8 +125,8 @@ xenconfig: xen.config
>         @:
>
>  PHONY += tinyconfig
> -tinyconfig:
> -       $(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config
> +tinyconfig: $(obj)/conf
> +       $(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig KCONFIG_ALLCONFIG=$(wildcard $(srctree)/kernel/configs/tiny.config $(srctree)/arch/$(SRCARCH)/configs/tiny.config)
>
>  # Help text used by make help
>  help:


The dependency "tinyconfig: $(obj)/conf" is redundant.


It is already specified by:

allnoconfig allyesconfig allmodconfig alldefconfig randconfig: $(obj)/conf
         $< --$@ $(Kconfig)







What is worse, this patch breaks "make tinyconfig" on x86.


$ uname -m
x86_64
$ git log --oneline -2
754e472 Kbuild: avoid "make tinyconfig" warnings
a99cde4 Linux 4.7-rc6
$ make -s mrproper
$ make tinyconfig
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  SHIPPED scripts/kconfig/zconf.tab.c
  SHIPPED scripts/kconfig/zconf.lex.c
  SHIPPED scripts/kconfig/zconf.hash.c
  HOSTCC  scripts/kconfig/zconf.tab.o
  HOSTLD  scripts/kconfig/conf
scripts/kconfig/conf  --allnoconfig Kconfig
#
# configuration written to .config
#
scripts/kconfig/Makefile:115: *** No configuration exists for this
target on this architecture.  Stop.
make[3]: *** [arch/x86/configs/tiny.config] Error 2
make[2]: *** [__build_one_by_one] Error 2
make[1]: *** [tinyconfig] Error 2
make: *** [tinyconfig] Error 2
Arnd Bergmann July 6, 2016, 1:49 p.m. UTC | #4
On Wednesday, July 6, 2016 11:00:01 AM CEST Masahiro Yamada wrote:
> Hi Arnd,
> 
> 
> 2016-07-04 23:25 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> > The introduction of "make *.config" as a shorthand for merging configuration
> > files unfortunately introduced some build warnings that we see in every
> > single run of the various build bots testing tinyconfig:
> 
> I am not convinced with this statement.
> 
> Why do you think 63a91033d52e is a bad commit?
> 
> 
> With/without 63a91033d52e, I see the same warnings.
> 
> 
> $ git log --oneline  -1
> 63a9103 kbuild: add generic mergeconfig target, %.config
> $ make -s mrproper
> $ make tinyconfig >/dev/null
> .config:871:warning: override: KERNEL_XZ changes choice state
> .config:873:warning: override: SLOB changes choice state
> .config:874:warning: override: NOHIGHMEM changes choice state
> $ git checkout HEAD^
> Previous HEAD position was 63a9103... kbuild: add generic mergeconfig
> target, %.config
> HEAD is now at bc8f8f5... merge_config.sh: rename MAKE to RUNMAKE
> $ git log --oneline  -1
> bc8f8f5 merge_config.sh: rename MAKE to RUNMAKE
> $ make -s mrproper
> $ make tinyconfig >/dev/null
> .config:871:warning: override: KERNEL_XZ changes choice state
> .config:873:warning: override: SLOB changes choice state
> .config:874:warning: override: NOHIGHMEM changes choice state
> 

Sorry about, my mistake. I misread the original Makefile contents.

> >  PHONY += tinyconfig
> > -tinyconfig:
> > -       $(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config
> > +tinyconfig: $(obj)/conf
> > +       $(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig KCONFIG_ALLCONFIG=$(wildcard $(srctree)/kernel/configs/tiny.config $(srctree)/arch/$(SRCARCH)/configs/tiny.config)
> >
> >  # Help text used by make help
> >  help:
> 
> 
> The dependency "tinyconfig: $(obj)/conf" is redundant.
> 
> 
> It is already specified by:
> 
> allnoconfig allyesconfig allmodconfig alldefconfig randconfig: $(obj)/conf
>          $< --$@ $(Kconfig)


I don't see that yet. How does this line provide the dependency?
 
> What is worse, this patch breaks "make tinyconfig" on x86.

Right, Josh also pointed this out.

So, let's ignore my incorrect patch for the moment. Do you have
any other idea for how to avoid the warning?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann July 6, 2016, 1:56 p.m. UTC | #5
On Monday, July 4, 2016 12:58:15 PM CEST Josh Triplett wrote:
> On Mon, Jul 04, 2016 at 12:51:56PM -0700, Josh Triplett wrote:
> > On Mon, Jul 04, 2016 at 04:25:55PM +0200, Arnd Bergmann wrote:
> > > The introduction of "make *.config" as a shorthand for merging configuration
> > > files unfortunately introduced some build warnings that we see in every
> > > single run of the various build bots testing tinyconfig:
> > > 
> > > .config:966:warning: override: NOHIGHMEM changes choice state
> > > .config:965:warning: override: SLOB changes choice state
> > > .config:963:warning: override: KERNEL_XZ changes choice state
> > > .config:962:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > > .config:933:warning: override: SLOB changes choice state
> > > .config:930:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > > .config:870:warning: override: SLOB changes choice state
> > > .config:868:warning: override: KERNEL_XZ changes choice state
> > > .config:867:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > > 
> > > This changes the behavior for tinyconfig again, so we call the special
> > > allnoconfig target directly, with the fragment as the KCONFIG_ALLCONFIG
> > > file so it gets merged into the configuration. The disadvantage of
> > > this approach is that we cannot pass another KCONFIG_ALLCONFIG on
> > > the command line, that would again have to be done using the mergeconfig
> > > script afterwards.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > Fixes: 63a91033d52e ("kbuild: add generic mergeconfig target, %.config")
> > > Link: https://storage.kernelci.org/mainline/v4.7-rc6/x86-tinyconfig/build.log
> > 
> > Thanks for fixing this.
> > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > 
> > As another possibility, which would preserve the ability to use
> > KCONFIG_ALLCONFIG, what about adding a parameter to mergeconfig to
> > disable this warning?  I've seen many other uses of mergeconfig in
> > production systems that have the same problem.
> 
> Actually, I just realized something: does this actually do the right
> thing if both tiny.config files exist?  The documentation for
> KCONFIG_ALLCONFIG doesn't suggest that it can take two files as
> arguments.  And if it can, this also need quotes; otherwise, this will
> expand to "... allnoconfig KCONFIG=path/to/a/tiny.config
> another/path/to/a/tiny.config".

You are right, my patch doesn't work.

I can think of a few other ways to work around this, though none seems
perfect:

- add a flag to the kconfig tool to ignore 'choice' symbol overrides and not
  warn about them (as you suggested). This may be problematic if there
  are cases in which the warning is actually helpful.

- drop the feature of allowing multiple tiny.config files to be combined
  and instead have the x86 file include everything from the architecture
  independent file. This would only be a small annoyance, but it doesn't
  solve the other cases you mention in which we see similar warnings.

- merge the fragments first and then use the combined fragment as the
  KCONFIG_ALLCONFIG file.

- change merge_config.sh to do a 'savedefconfig' step before applying
  the fragment, so it doesn't warn about choice statements that are
  overridden from their default, as opposed to having conflicting choices.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josh Triplett July 6, 2016, 6:32 p.m. UTC | #6
On Wed, Jul 06, 2016 at 03:56:25PM +0200, Arnd Bergmann wrote:
> On Monday, July 4, 2016 12:58:15 PM CEST Josh Triplett wrote:
> > On Mon, Jul 04, 2016 at 12:51:56PM -0700, Josh Triplett wrote:
> > > On Mon, Jul 04, 2016 at 04:25:55PM +0200, Arnd Bergmann wrote:
> > > > The introduction of "make *.config" as a shorthand for merging configuration
> > > > files unfortunately introduced some build warnings that we see in every
> > > > single run of the various build bots testing tinyconfig:
> > > > 
> > > > .config:966:warning: override: NOHIGHMEM changes choice state
> > > > .config:965:warning: override: SLOB changes choice state
> > > > .config:963:warning: override: KERNEL_XZ changes choice state
> > > > .config:962:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > > > .config:933:warning: override: SLOB changes choice state
> > > > .config:930:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > > > .config:870:warning: override: SLOB changes choice state
> > > > .config:868:warning: override: KERNEL_XZ changes choice state
> > > > .config:867:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > > > 
> > > > This changes the behavior for tinyconfig again, so we call the special
> > > > allnoconfig target directly, with the fragment as the KCONFIG_ALLCONFIG
> > > > file so it gets merged into the configuration. The disadvantage of
> > > > this approach is that we cannot pass another KCONFIG_ALLCONFIG on
> > > > the command line, that would again have to be done using the mergeconfig
> > > > script afterwards.
> > > > 
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > Fixes: 63a91033d52e ("kbuild: add generic mergeconfig target, %.config")
> > > > Link: https://storage.kernelci.org/mainline/v4.7-rc6/x86-tinyconfig/build.log
> > > 
> > > Thanks for fixing this.
> > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > 
> > > As another possibility, which would preserve the ability to use
> > > KCONFIG_ALLCONFIG, what about adding a parameter to mergeconfig to
> > > disable this warning?  I've seen many other uses of mergeconfig in
> > > production systems that have the same problem.
> > 
> > Actually, I just realized something: does this actually do the right
> > thing if both tiny.config files exist?  The documentation for
> > KCONFIG_ALLCONFIG doesn't suggest that it can take two files as
> > arguments.  And if it can, this also need quotes; otherwise, this will
> > expand to "... allnoconfig KCONFIG=path/to/a/tiny.config
> > another/path/to/a/tiny.config".
> 
> You are right, my patch doesn't work.
> 
> I can think of a few other ways to work around this, though none seems
> perfect:
> 
> - add a flag to the kconfig tool to ignore 'choice' symbol overrides and not
>   warn about them (as you suggested). This may be problematic if there
>   are cases in which the warning is actually helpful.

As long as this remains an option and not the default, I don't think it
would cause problems.

> - drop the feature of allowing multiple tiny.config files to be combined
>   and instead have the x86 file include everything from the architecture
>   independent file. This would only be a small annoyance, but it doesn't
>   solve the other cases you mention in which we see similar warnings.

And I'd rather not duplicate the base config into every
architecture-specific tiny.config; that'll become even more problematic
if other architectures add their own.

> - change merge_config.sh to do a 'savedefconfig' step before applying
>   the fragment, so it doesn't warn about choice statements that are
>   overridden from their default, as opposed to having conflicting choices.

This sounds like a potentially interesting approach.  This would produce
warnings only if overriding explicit configuration, but not when
changing a choice from the implicit default?

- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann July 6, 2016, 7:58 p.m. UTC | #7
On Wednesday, July 6, 2016 11:32:50 AM CEST Josh Triplett wrote:
> On Wed, Jul 06, 2016 at 03:56:25PM +0200, Arnd Bergmann wrote:
> > On Monday, July 4, 2016 12:58:15 PM CEST Josh Triplett wrote:
> > > On Mon, Jul 04, 2016 at 12:51:56PM -0700, Josh Triplett wrote:
> > > > As another possibility, which would preserve the ability to use
> > > > KCONFIG_ALLCONFIG, what about adding a parameter to mergeconfig to
> > > > disable this warning?  I've seen many other uses of mergeconfig in
> > > > production systems that have the same problem.
> > > 
> > > Actually, I just realized something: does this actually do the right
> > > thing if both tiny.config files exist?  The documentation for
> > > KCONFIG_ALLCONFIG doesn't suggest that it can take two files as
> > > arguments.  And if it can, this also need quotes; otherwise, this will
> > > expand to "... allnoconfig KCONFIG=path/to/a/tiny.config
> > > another/path/to/a/tiny.config".
> > 
> > You are right, my patch doesn't work.
> > 
> > I can think of a few other ways to work around this, though none seems
> > perfect:
> > 
> > - add a flag to the kconfig tool to ignore 'choice' symbol overrides and not
> >   warn about them (as you suggested). This may be problematic if there
> >   are cases in which the warning is actually helpful.
> 
> As long as this remains an option and not the default, I don't think it
> would cause problems.
> 
> > - drop the feature of allowing multiple tiny.config files to be combined
> >   and instead have the x86 file include everything from the architecture
> >   independent file. This would only be a small annoyance, but it doesn't
> >   solve the other cases you mention in which we see similar warnings.
> 
> And I'd rather not duplicate the base config into every
> architecture-specific tiny.config; that'll become even more problematic
> if other architectures add their own.

Fair enough.

> > - change merge_config.sh to do a 'savedefconfig' step before applying
> >   the fragment, so it doesn't warn about choice statements that are
> >   overridden from their default, as opposed to having conflicting choices.
> 
> This sounds like a potentially interesting approach.  This would produce
> warnings only if overriding explicit configuration, but not when
> changing a choice from the implicit default?

Yes, that is the idea. Actually after looking at merge_config.sh I think
this is not the best approach though, based on the way it works: it
looks at all the fragments and merges them together first, then applies
the combined fragment as a single override file, prioritizing this
override over ${objdir}/config, which in turn looks at either
the "olddefconfig" or "allnoconfig" defaults as a fallback.

Our "%.config" Makefile rule in turn passes the current .config into
mergeconfig.sh, so the current defaults get passed in twice. What I
think would work better here is to have an unchanged mergeconfig.sh,
and then change the Makefile to call 'savedefconfig' first, remove
the .config file and then merge the saved defconfig with the other
fragments. It should not be hard to do, but I don't think I can
implement this correctly at the moment for lack of attention.

If any of the folks on Cc could give this a try, that would be very
nice, otherwise I can try to come up with something next week.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Masahiro Yamada July 8, 2016, 1:41 a.m. UTC | #8
Hi Arnd,

2016-07-06 22:49 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:

>>
>>
>> The dependency "tinyconfig: $(obj)/conf" is redundant.
>>
>>
>> It is already specified by:
>>
>> allnoconfig allyesconfig allmodconfig alldefconfig randconfig: $(obj)/conf
>>          $< --$@ $(Kconfig)
>
>
> I don't see that yet. How does this line provide the dependency?


"make tinyconfig" works like this:


[1] In the top Makefile, Kbuild handles "make tinyconfig"
    with the following rule:

%config: scripts_basic outputmakefile FORCE
        $(Q)$(MAKE) $(build)=scripts/kconfig $@


So, Kbuild will descend into scripts/kconfig.



[2] In scripts/kconfig/Makefile, Kbuild handles "make tinyconfig"
    with the following rule:


tinyconfig:
        $(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config


-f $(srctree)/Makefile   means "use the top Makefile".

So, Kbuild will go back to the top Makefile and run "make allnoconfig
tiny.config" there.



[3] In the top Makefile, Kbuild handles "make allnoconfig tiny.config"
     with the following rule  (mixed targets):


__build_one_by_one:
        $(Q)set -e; \
        for i in $(MAKECMDGOALS); do \
                 $(MAKE) -f $(srctree)/Makefile $$i; \
        done


This means, run "make allnoconfig", then "make tiny.config"
in this order.


[4] In the top Makefile, Kbuild handles "make allnoconfig"
    with the following rule:

%config: scripts_basic outputmakefile FORCE
        $(Q)$(MAKE) $(build)=scripts/kconfig $@

Kbuild will descend into scripts/kconfig.


[5] In scripts/kconfig/Makefile, Kbuild handles "make allnoconfig"
     with the following rule:

simple-targets := oldconfig allnoconfig allyesconfig allmodconfig \
         alldefconfig randconfig listnewconfig olddefconfig
PHONY += $(simple-targets)

$(simple-targets): $(obj)/conf
          $< $(silent) --$@ $(Kconfig)


At this point, $(obj)/conf is generated and then, Kbuild runs allnoconfig.



[6] In the top Makefile, Kbuild handles "make tiny.config"
    with the following rule:

%config: scripts_basic outputmakefile FORCE
        $(Q)$(MAKE) $(build)=scripts/kconfig $@

Kbuild will descend into scripts/kconfig.


[7] In scripts/kconfig/Makefile, Kbuild handles "make tiny.config"
    with the following rule:

%.config: $(obj)/conf
       $(if $(call configfiles),, $(error No configuration exists for
this target on this architecture))
       $(Q)$(CONFIG_SHELL) $(srctree)/scripts/kconfig/merge_config.sh
-m .config $(configfiles)
       +$(Q)yes "" | $(MAKE) -f $(srctree)/Makefile oldconfig



I hope this is helpful.





>
> So, let's ignore my incorrect patch for the moment. Do you have
> any other idea for how to avoid the warning?


Of your four ideas, I do not like the first two,
but the others sound reasonable.




> - merge the fragments first and then use the combined fragment as the
>   KCONFIG_ALLCONFIG file.

This is a strait-forward solution.
I thought of this one, too.


> - change merge_config.sh to do a 'savedefconfig' step before applying
>   the fragment, so it doesn't warn about choice statements that are
>   overridden from their default, as opposed to having conflicting choices.

This sounds interesting.





BTW, I have been wondering if we could support merge_config
as a native feature of Kconfig instead of by a separate shell script.

If we could support KCONFIG_ALLCONFIG for "make oldconfig"
perhaps merge_config.sh will go away?
Maybe I am missing something, though.
Arnd Bergmann July 8, 2016, 11:50 a.m. UTC | #9
On Friday, July 8, 2016 10:41:57 AM CEST Masahiro Yamada wrote: 
> 2016-07-06 22:49 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> >> It is already specified by:
> >>
> >> allnoconfig allyesconfig allmodconfig alldefconfig randconfig: $(obj)/conf
> >>          $< --$@ $(Kconfig)
> >
> >
> > I don't see that yet. How does this line provide the dependency?
> 
> 
> "make tinyconfig" works like this:
> 
> This means, run "make allnoconfig", then "make tiny.config"
> in this order.

Ok, got it, sorry for being slow here.

> > So, let's ignore my incorrect patch for the moment. Do you have
> > any other idea for how to avoid the warning?
> 
> 
> Of your four ideas, I do not like the first two,
> but the others sound reasonable.

ok

> > - merge the fragments first and then use the combined fragment as the
> >   KCONFIG_ALLCONFIG file.
> 
> This is a strait-forward solution.
> I thought of this one, too.
> 
> 
> > - change merge_config.sh to do a 'savedefconfig' step before applying
> >   the fragment, so it doesn't warn about choice statements that are
> >   overridden from their default, as opposed to having conflicting choices.
> 
> This sounds interesting.
> 
> 
> 
> 
> 
> BTW, I have been wondering if we could support merge_config
> as a native feature of Kconfig instead of by a separate shell script.
> 
> If we could support KCONFIG_ALLCONFIG for "make oldconfig"
> perhaps merge_config.sh will go away?
> Maybe I am missing something, though.

I agree this sounds really nice, but I see two problems with it:

- at the moment, we always read exactly one input file from the
  kconfig tool, and that can be one of
	.config
	arch/${ARCH}/defconfig
	arch/${ARCH}/configs/*_defconfig
	the KCONFIG_ALLCONFIG contents
	./all{no,yes,mod,def,random}.config
  Teaching the tool to have two input files basically means reimplementing
  merge_config.sh in C. Definitely doable, and probably nicer than what
  we have, but not trivial.
- It doesn't actually solve the problem of the warnings that I'm trying to
  address, that would still have to be done on top of the reimplementation.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index ebced77deb9c..35d0e191fe3f 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -125,8 +125,8 @@  xenconfig: xen.config
 	@:
 
 PHONY += tinyconfig
-tinyconfig:
-	$(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config
+tinyconfig: $(obj)/conf
+	$(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig KCONFIG_ALLCONFIG=$(wildcard $(srctree)/kernel/configs/tiny.config $(srctree)/arch/$(SRCARCH)/configs/tiny.config)
 
 # Help text used by make help
 help: