[GIT] kbuild: kconfig changes

[GIT] kbuild: kconfig changes

[Michal Marek]

Hi Linus,

this is the kconfig part of kbuild. We have four new *config targets:
* oldnoconfig: set all new options to 'n'
* listnewconfig: list all unset config options
* alldefconfig: set all options to their defaults specified in Kconfig
  files
* savedefconfig: write a defconfig file with only the differences from
  an alldefconfig (aka minimal defconfig)

Kconfig also warns when a select statement selects a symbol with unmet
dependencies (which typically results in a broken config). Li Zefan did
quite some usability fixes to the visual config interfaces.

Michal

The following changes since commit 9fe6206f400646a2322096b56c59891d530e8d51:

  Linux 2.6.35 (2010-08-01 15:11:14 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild-2.6.git kconfig

Aristeu Rozanski (1):
      kconfig: introduce nonint_oldconfig and loose_nonint_oldconfig

Catalin Marinas (1):
      kbuild: Warn on selecting symbols with unmet direct dependencies

Jan Beulich (1):
      kconfig: Don't write invisible choice values

Justin P. Mattock (1):
      scripts:conf.c Fix warning: variable 'type' set but not used

Li Zefan (11):
      kconfig: print symbol type in help text
      kconfig: print the range of integer/hex symbol in help text
      kconfig: fix to tag NEW symbols correctly
      menuconfig: improive help text a bit
      gconfig: fix to tag NEW symbols correctly
      gconfig: fix null pointer warning
      xconfig: clean up
      xconfig: remove unused function
      xconfig: add support to show hidden options which have prompts
      menuconfig: fix to center checklist correctly in a corner case
      menuconfig: truncate list items

Michal Marek (1):
      Merge commit 'v2.6.35' into kbuild/kconfig

Peter Korsgaard (1):
      kconfig: make randconfig fair for booleans

Roman Zippel (1):
      kconfig: print more info when we see a recursive dependency

Sam Ravnborg (8):
      kconfig: use long options in conf
      kconfig: rename loose_nonint_oldconfig => oldnoconfig
      kconfig: change nonint_oldconfig to listnewconfig
      kconfig: save location of config symbols
      kconfig: add alldefconfig
      kconfig: refactor code in symbol.c
      kconfig: code refactoring in confdata.c
      kconfig: add savedefconfig

Ulf Magnusson (1):
      kconfig: fix MODULES-related bug in case of no .config

 Documentation/kbuild/kconfig.txt     |    2 +-
 scripts/kconfig/Makefile             |   77 +++++-----
 scripts/kconfig/conf.c               |  181 ++++++++++++---------
 scripts/kconfig/confdata.c           |  221 ++++++++++++++++++--------
 scripts/kconfig/expr.c               |    2 +-
 scripts/kconfig/expr.h               |    3 +
 scripts/kconfig/gconf.c              |    7 +-
 scripts/kconfig/lkc.h                |    2 +
 scripts/kconfig/lkc_proto.h          |    1 +
 scripts/kconfig/lxdialog/checklist.c |   10 +-
 scripts/kconfig/mconf.c              |    2 +-
 scripts/kconfig/menu.c               |   27 +++-
 scripts/kconfig/qconf.cc             |  106 +++++++------
 scripts/kconfig/qconf.h              |   17 ++-
 scripts/kconfig/symbol.c             |  292 ++++++++++++++++++++++++++++++----
 15 files changed, 667 insertions(+), 283 deletions(-)

Re: [GIT] kbuild: kconfig changes

[Linus Torvalds]

 On Wed, Aug 4, 2010 at 5:51 AM, Michal Marek <mmarek@suse.cz> wrote:
>
> this is the kconfig part of kbuild. We have four new *config targets:
> * oldnoconfig: set all new options to 'n'
> * listnewconfig: list all unset config options
> * alldefconfig: set all options to their defaults specified in Kconfig
>  files
> * savedefconfig: write a defconfig file with only the differences from
>  an alldefconfig (aka minimal defconfig)
>
> Kconfig also warns when a select statement selects a symbol with unmet
> dependencies (which typically results in a broken config). Li Zefan did
> quite some usability fixes to the visual config interfaces.

Hmm. This seems to make "make oldconfig" a _lot_ more verbose than it
used to be. In a very annoying way.

I'm used to this (v2.6.35 "make oldconfig" with no changes):

   [torvalds@i5 linux]$ make oldconfig
   scripts/kconfig/conf -o arch/x86/Kconfig
   #
   # configuration written to .config
   #
   [torvalds@i5 linux]$

but now it prints _everything_. The old "oldconfig" only printed
things out when there was something to be asked about.

This is a regression.

              Linus

Re: [GIT] kbuild: kconfig changes

[Justin P. Mattock]

On 08/05/2010 04:33 PM, Linus Torvalds wrote:
>   On Wed, Aug 4, 2010 at 5:51 AM, Michal Marek<mmarek@suse.cz>  wrote:
>>
>> this is the kconfig part of kbuild. We have four new *config targets:
>> * oldnoconfig: set all new options to 'n'
>> * listnewconfig: list all unset config options
>> * alldefconfig: set all options to their defaults specified in Kconfig
>>   files
>> * savedefconfig: write a defconfig file with only the differences from
>>   an alldefconfig (aka minimal defconfig)
>>
>> Kconfig also warns when a select statement selects a symbol with unmet
>> dependencies (which typically results in a broken config). Li Zefan did
>> quite some usability fixes to the visual config interfaces.
>
> Hmm. This seems to make "make oldconfig" a _lot_ more verbose than it
> used to be. In a very annoying way.
>
> I'm used to this (v2.6.35 "make oldconfig" with no changes):
>
>     [torvalds@i5 linux]$ make oldconfig
>     scripts/kconfig/conf -o arch/x86/Kconfig
>     #
>     # configuration written to .config
>     #
>     [torvalds@i5 linux]$
>
> but now it prints _everything_. The old "oldconfig" only printed
> things out when there was something to be asked about.
>
> This is a regression.
>
>                Linus
>

With what I submitted I did not test make oldconfig, only make 
menuconfig due to gcc 4.6.0 giving me a slew of warnings.
 From looking at make oldconfig I do see a more detailed explanation of 
what each option is(last I remember it was more of a line with y/n)but 
then again you could be seeing something completly different.

keep in mind not sure if this is with kconfig merge or not due to me 
seeing make oldconfig react the same way with 2.6.35-rc6-00191-ga2dccdb
and the current(pulled a few minuetes ago)
but look kind of differently with 2.6.34(but could be wrong due to not 
using make oldconfig that much).
Now if this is with what I submitted then I'll have a look at it and see 
if I can get this fixed.

Justin P. Mattock

Re: [GIT] kbuild: kconfig changes

[Linus Torvalds]

On Thu, Aug 5, 2010 at 6:27 PM, Justin P. Mattock
<justinmattock@gmail.com> wrote:
>
> With what I submitted I did not test make oldconfig, only make menuconfig
> due to gcc 4.6.0 giving me a slew of warnings.
> From looking at make oldconfig I do see a more detailed explanation of what
> each option is(last I remember it was more of a line with y/n)but then again
> you could be seeing something completly different.

Just test what I described:  do a "make oldconfig" (do it twice, if
the first one needs to actually get some input). Do it on 2.6.35, and
then on the current tree. Look at the _huge_ difference in output.

> Now if this is with what I submitted then I'll have a look at it and see if I can get this fixed.

I just did a quick git bisect. It's introduced by commit 4062f1a4c030
("kconfig: use long options in conf") by Sam Ravnborg. Apparently that
thing is totally buggy, and doesn't just change the option names, but
actively breaks them.

                 Linus

Re: [GIT] kbuild: kconfig changes

[Justin P. Mattock]

On 08/05/2010 07:08 PM, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 6:27 PM, Justin P. Mattock
> <justinmattock@gmail.com>  wrote:
>>
>> With what I submitted I did not test make oldconfig, only make menuconfig
>> due to gcc 4.6.0 giving me a slew of warnings.
>>  From looking at make oldconfig I do see a more detailed explanation of what
>> each option is(last I remember it was more of a line with y/n)but then again
>> you could be seeing something completly different.
>
> Just test what I described:  do a "make oldconfig" (do it twice, if
> the first one needs to actually get some input). Do it on 2.6.35, and
> then on the current tree. Look at the _huge_ difference in output.
>

yeah I see what your hitting, just experienced what you posted

	make oldconfig
    scripts/kconfig/conf -o arch/x86/Kconfig
    #
    # configuration written to .config
    #

(just sits there)

seemed to not want to go away.(doing a make menuconfig got
make oldconfig to wake up of whatever it was in) as for reproducing
(even though already bisected) seems once make oldconfig starts working 
properly, hitting the above doesn't happen as easily.


>> Now if this is with what I submitted then I'll have a look at it and see if I can get this fixed.
>
> I just did a quick git bisect. It's introduced by commit 4062f1a4c030
> ("kconfig: use long options in conf") by Sam Ravnborg. Apparently that
> thing is totally buggy, and doesn't just change the option names, but
> actively breaks them.
>
>                   Linus
>

cool..

Justin P. Mattock

[PATCH] kconfig: fix make oldconfig

[Sam Ravnborg]

[PATCH] kconfig: fix make oldconfig

Linus wrote:
 This seems to make "make oldconfig" a _lot_ more verbose than it
 used to be. In a very annoying way.

 I just did a quick git bisect. It's introduced by commit 4062f1a4c030
 ("kconfig: use long options in conf") by Sam Ravnborg. Apparently that
 thing is totally buggy, and doesn't just change the option names, but
 actively breaks them.

The old behaviour (from years ago) were reintroduced by accident.
Fix this so we are back to the version that are silent
if there is nothing to ask about.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---

Sorry for this regression. Dunno how I missed it.
I guess I only tested "silentoldconfig".

Following patch seems obviously correct but as I am on the way
out of the door I could not do much testing.

	Sam

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 010600e..274f271 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -599,12 +599,12 @@ int main(int ac, char **av)
 		break;
 	case savedefconfig:
 		break;
-	case oldconfig:
 	case oldaskconfig:
 		rootEntry = &rootmenu;
 		conf(&rootmenu);
 		input_mode = silentoldconfig;
 		/* fall through */
+	case oldconfig:
 	case listnewconfig:
 	case oldnoconfig:
 	case silentoldconfig:

Re: [PATCH] kconfig: fix make oldconfig

[Justin P. Mattock]

> [PATCH] kconfig: fix make oldconfig
>
> Linus wrote:
>   This seems to make "make oldconfig" a _lot_ more verbose than it
>   used to be. In a very annoying way.
>
>   I just did a quick git bisect. It's introduced by commit 4062f1a4c030
>   ("kconfig: use long options in conf") by Sam Ravnborg. Apparently that
>   thing is totally buggy, and doesn't just change the option names, but
>   actively breaks them.
>
> The old behaviour (from years ago) were reintroduced by accident.
> Fix this so we are back to the version that are silent
> if there is nothing to ask about.
>
> Reported-by: Linus Torvalds<torvalds@linux-foundation.org>
> Signed-off-by: Sam Ravnborg<sam@ravnborg.org>
> ---
>
> Sorry for this regression. Dunno how I missed it.
> I guess I only tested "silentoldconfig".
>
> Following patch seems obviously correct but as I am on the way
> out of the door I could not do much testing.
>
> 	Sam
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 010600e..274f271 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -599,12 +599,12 @@ int main(int ac, char **av)
>   		break;
>   	case savedefconfig:
>   		break;
> -	case oldconfig:
>   	case oldaskconfig:
>   		rootEntry =&rootmenu;
>   		conf(&rootmenu);
>   		input_mode = silentoldconfig;
>   		/* fall through */
> +	case oldconfig:
>   	case listnewconfig:
>   	case oldnoconfig:
>   	case silentoldconfig:
>

Id like to be more useful with really hitting this, seems I was for a 
few moments, then something in there tripped and caused oldconfig to 
behave.. anyways only real evidence I have of hitting this is an fpaste 
of my shell output http://fpaste.org/LFew/

Anyways applied your patch below, make oldconfig worked as is no:

  make oldconfig
scripts/kconfig/conf -o arch/x86/Kconfig
#
# configuration written to .config
#

that I was hitting for some reason or another then all of a sudden 
disappeared.

Justin P. Mattock

Re: [PATCH] kconfig: fix make oldconfig

[Michal Marek]

On 6.8.2010 07:13, Sam Ravnborg wrote:
> [PATCH] kconfig: fix make oldconfig
> 
> Linus wrote:
>  This seems to make "make oldconfig" a _lot_ more verbose than it
>  used to be. In a very annoying way.
> 
>  I just did a quick git bisect. It's introduced by commit 4062f1a4c030
>  ("kconfig: use long options in conf") by Sam Ravnborg. Apparently that
>  thing is totally buggy, and doesn't just change the option names, but
>  actively breaks them.
> 
> The old behaviour (from years ago) were reintroduced by accident.
> Fix this so we are back to the version that are silent
> if there is nothing to ask about.
> 
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> 
> Sorry for this regression. Dunno how I missed it.
> I guess I only tested "silentoldconfig".
> 
> Following patch seems obviously correct but as I am on the way
> out of the door I could not do much testing.
> 
> 	Sam
> 
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 010600e..274f271 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -599,12 +599,12 @@ int main(int ac, char **av)
>  		break;
>  	case savedefconfig:
>  		break;
> -	case oldconfig:
>  	case oldaskconfig:
>  		rootEntry = &rootmenu;
>  		conf(&rootmenu);
>  		input_mode = silentoldconfig;
>  		/* fall through */
> +	case oldconfig:
>  	case listnewconfig:
>  	case oldnoconfig:
>  	case silentoldconfig:

Reviewed-by: Michal Marek <mmarek@suse.cz>

The problem is that in the previous code, input_mode could never be set
to 'ask_new' ('oldconfig' in the new code), both oldconfig and
silentoldconfig set the variable to 'ask_silent'. Therefore, the 'case
ask_new:' label had no effect here. The fix is correct, both oldconfig
and silentoldconfig jump to the same place again.

Linus, will you apply this directly or should I apply it to the kbuild
tree and send you a pull request?

Michal

Re: [PATCH] kconfig: fix make oldconfig

[Linus Torvalds]

On Fri, Aug 6, 2010 at 3:21 AM, Michal Marek <mmarek@suse.cz> wrote:
>
> Linus, will you apply this directly or should I apply it to the kbuild
> tree and send you a pull request?

I took it directly, since I spend a lot of time doing "make oldconfig
; make -j16" which is why I found this (after pulling, I tend verify
that at least nothing broke my _own_ configuration)

             Linus

Re: [PATCH] kconfig: fix make oldconfig

[Sam Ravnborg]

On Fri, Aug 06, 2010 at 09:19:23AM -0700, Linus Torvalds wrote:
> On Fri, Aug 6, 2010 at 3:21 AM, Michal Marek <mmarek@suse.cz> wrote:
> >
> > Linus, will you apply this directly or should I apply it to the kbuild
> > tree and send you a pull request?
> 
> I took it directly, since I spend a lot of time doing "make oldconfig
> ; make -j16" which is why I found this (after pulling, I tend verify
> that at least nothing broke my _own_ configuration)

Hmm, I wonder why you call oldconfig explicitly?

A plain "make -j16" executes "silentoldconfig" if there
is any changes in a Kconfig* file or in .config.
Just double checked and it works as I expected.

So you are asked if there is any new options anyway even
if you skip your "oldconfig" step.

The difference between oldconfig and silentoldconfig these days
are just that silentoldconfig updates include/config/*
which oldconfig does not.

It could be as simple as old habits.
Just curious to know if there is something I have missed.

	Sam

Re: [PATCH] kconfig: fix make oldconfig

[Linus Torvalds]

On Fri, Aug 6, 2010 at 10:52 AM, Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hmm, I wonder why you call oldconfig explicitly?
>
> A plain "make -j16" executes "silentoldconfig" if there
> is any changes in a Kconfig* file or in .config.
> Just double checked and it works as I expected.
>
> So you are asked if there is any new options anyway even
> if you skip your "oldconfig" step.

Try this:

   git clean -dqfx
   make -j16 > ../makes

It doesn't work, because "make silentconfig" will say

  ***
  *** You have not yet configured your kernel!
  *** (missing kernel config file ".config")
  ***
  *** Please run some configurator (e.g. "make oldconfig" or
  *** "make menuconfig" or "make xconfig").
  ***

which is why I always run "make oldconfig".

Sure, I could do it only when I need to, but quite frankly, it's much
easier to just always do the thing that works, rather than try
something that doesn't work, do something else, and then re-try the
thing that can fail.

                     Linus

Re: [PATCH] kconfig: fix make oldconfig

[Justin P. Mattock]

On 08/06/2010 11:09 AM, Linus Torvalds wrote:
> On Fri, Aug 6, 2010 at 10:52 AM, Sam Ravnborg<sam@ravnborg.org>  wrote:
>>
>> Hmm, I wonder why you call oldconfig explicitly?
>>
>> A plain "make -j16" executes "silentoldconfig" if there
>> is any changes in a Kconfig* file or in .config.
>> Just double checked and it works as I expected.
>>
>> So you are asked if there is any new options anyway even
>> if you skip your "oldconfig" step.
>
> Try this:
>
>     git clean -dqfx
>     make -j16>  ../makes
>
> It doesn't work, because "make silentconfig" will say
>
>    ***
>    *** You have not yet configured your kernel!
>    *** (missing kernel config file ".config")
>    ***
>    *** Please run some configurator (e.g. "make oldconfig" or
>    *** "make menuconfig" or "make xconfig").
>    ***
>
> which is why I always run "make oldconfig".
>
> Sure, I could do it only when I need to, but quite frankly, it's much
> easier to just always do the thing that works, rather than try
> something that doesn't work, do something else, and then re-try the
> thing that can fail.
>
>                       Linus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


the above command does break over here. using the make oldconfig command 
seems to be working after I did the make menuconfig.

Now before doing all of this I did make a copy of the entire kernel for 
a new system Im building(clfs) so I went into that tree and did a make 
oldconfig and(luckily) hit the non responsive oldconfig thing that Linus 
had originally posted.

here is a strace of when make oldconfig was not working(with git log at 
the top)

http://fpaste.org/317B/

and strace of a git pull today and make oldconfig does not crap out and 
starts asking me y/n options:

http://fpaste.org/QWWn/

hope this helps with debugging and such.

Justin P. Mattock

Re: [GIT] kbuild: kconfig changes

[Arve Hjønnevåg]

On Wed, Aug 4, 2010 at 5:51 AM, Michal Marek <mmarek@suse.cz> wrote:
> Hi Linus,
>
> this is the kconfig part of kbuild. We have four new *config targets:
> * oldnoconfig: set all new options to 'n'
> * listnewconfig: list all unset config options
> * alldefconfig: set all options to their defaults specified in Kconfig
>  files
> * savedefconfig: write a defconfig file with only the differences from
>  an alldefconfig (aka minimal defconfig)
>
> Kconfig also warns when a select statement selects a symbol with unmet
> dependencies (which typically results in a broken config). Li Zefan did
> quite some usability fixes to the visual config interfaces.
>
> Michal
>
> The following changes since commit 9fe6206f400646a2322096b56c59891d530e8d51:
>
>  Linux 2.6.35 (2010-08-01 15:11:14 -0700)
>
> are available in the git repository at:
>  git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild-2.6.git kconfig
>
> Aristeu Rozanski (1):
>      kconfig: introduce nonint_oldconfig and loose_nonint_oldconfig
>
> Catalin Marinas (1):
>      kbuild: Warn on selecting symbols with unmet direct dependencies
>
> Jan Beulich (1):
>      kconfig: Don't write invisible choice values

This change prevents some the minimal defconfig options from working.
Specifically our usb gadget drivers do not get set. Reverting this
part of the changes fixes my problem, but I don't know what other side
effects it may have:

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index e95718f..1797226 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -261,18 +261,14 @@ struct symbol *sym_choice_default(struct symbol *sym)
 static struct symbol *sym_calc_choice(struct symbol *sym)
 {
        struct symbol *def_sym;
-       struct property *prop;
-       struct expr *e;
-
-       /* first calculate all choice values' visibilities */
-       prop = sym_get_choice_prop(sym);
-       expr_list_for_each_sym(prop->expr, e, def_sym)
-               sym_calc_visibility(def_sym);

        /* is the user choice visible? */
        def_sym = sym->def[S_DEF_USER].val;
-       if (def_sym && def_sym->visible != no)
-               return def_sym;
+       if (def_sym) {
+               sym_calc_visibility(def_sym);
+               if (def_sym->visible != no)
+                       return def_sym;
+       }

        def_sym = sym_choice_default(sym);


-- 
Arve Hjønnevåg

Re: [GIT] kbuild: kconfig changes

[Sam Ravnborg]

> 
> This change prevents some the minimal defconfig options from working.
> Specifically our usb gadget drivers do not get set.

Can you help me reproduce this?

I have found an issue with choice values in combination with
tristate logic that fails. I hope this is something similar.

	Sam

Re: [GIT] kbuild: kconfig changes

[Arve Hjønnevåg]

On Fri, Aug 6, 2010 at 9:01 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
>>
>> This change prevents some the minimal defconfig options from working.
>> Specifically our usb gadget drivers do not get set.
>
> Can you help me reproduce this?
>
> I have found an issue with choice values in combination with
> tristate logic that fails. I hope this is something similar.
>

It is probably the same problem. The gadget driver that was not set is
not buildable as a module (it is not in the mainline kernel). If I
select another gadget driver instead it just gets changed to build as
a module instead.

If you create a file, arch/arm/configs/test_defconfig with the following:
CONFIG_MODULES=y
CONFIG_USB_GADGET=y
CONFIG_USB_MASS_STORAGE=y

then "make test_defconfig" results in .config having:
CONFIG_USB_MASS_STORAGE=m

 (at least if you are set up to compile for arm)

-- 
Arve Hjønnevåg

Re: [GIT] kbuild: kconfig changes

[Sam Ravnborg]

On Fri, Aug 06, 2010 at 09:43:24PM -0700, Arve Hj?nnev?g wrote:
> On Fri, Aug 6, 2010 at 9:01 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> >>
> >> This change prevents some the minimal defconfig options from working.
> >> Specifically our usb gadget drivers do not get set.
> >
> > Can you help me reproduce this?
> >
> > I have found an issue with choice values in combination with
> > tristate logic that fails. I hope this is something similar.
> >
> 
> It is probably the same problem. The gadget driver that was not set is
> not buildable as a module (it is not in the mainline kernel). If I
> select another gadget driver instead it just gets changed to build as
> a module instead.
> 
> If you create a file, arch/arm/configs/test_defconfig with the following:
> CONFIG_MODULES=y
> CONFIG_USB_GADGET=y
> CONFIG_USB_MASS_STORAGE=y
> 
> then "make test_defconfig" results in .config having:
> CONFIG_USB_MASS_STORAGE=m
> 
>  (at least if you are set up to compile for arm)

Thanks Arve.

I have it reproduced now with a simple Kconfig:

$ cat Kconfig
config M
	def_bool y
	option modules

choice
	prompt "choice list"

config A
	tristate "a"

config B
	tristate "b"

endchoice

$cat defconfig
CONFIG_M=y
CONFIG_A=y
# CONFIG_B is not set


If I do:

    $scripts/kconfig/conf --defconfig=defconfig Kconfig

with the above input the resulting .config is OK.

But If I drop the line:

    # CONFIG_B is not set

in the defconfig file then I end with CONFIG_A set to m.
And this is not as expected - I cannot see why it should matter
if we specify the value of B or not.

What we see here is that savedefconfig trigger a bug in the
other part of kconfig - a bug which was not exposed before.

The reason why your patch cured it was that we then no
longer triggered the bug (at least I guess so I did not look to close).

I will look into this as time permits. I assume the fix is simple
when I find the reason.

I fear it is in menu_finalize() and that part of kconfig I have
not yet understood.

	Sam

Re: [GIT] kbuild: kconfig changes

[Michal Marek]

On 8.8.2010 17:57, Sam Ravnborg wrote:
> On Fri, Aug 06, 2010 at 09:43:24PM -0700, Arve Hj?nnev?g wrote:
>> On Fri, Aug 6, 2010 at 9:01 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
>>>>
>>>> This change prevents some the minimal defconfig options from working.
>>>> Specifically our usb gadget drivers do not get set.
>>>
>>> Can you help me reproduce this?
>>>
>>> I have found an issue with choice values in combination with
>>> tristate logic that fails. I hope this is something similar.
>>>
>>
>> It is probably the same problem. The gadget driver that was not set is
>> not buildable as a module (it is not in the mainline kernel). If I
>> select another gadget driver instead it just gets changed to build as
>> a module instead.
>>
>> If you create a file, arch/arm/configs/test_defconfig with the following:
>> CONFIG_MODULES=y
>> CONFIG_USB_GADGET=y
>> CONFIG_USB_MASS_STORAGE=y
>>
>> then "make test_defconfig" results in .config having:
>> CONFIG_USB_MASS_STORAGE=m
>>
>>  (at least if you are set up to compile for arm)
> 
> Thanks Arve.
> 
> I have it reproduced now with a simple Kconfig:
> 
> $ cat Kconfig
> config M
> 	def_bool y
> 	option modules
> 
> choice
> 	prompt "choice list"
> 
> config A
> 	tristate "a"
> 
> config B
> 	tristate "b"
> 
> endchoice
> 
> $cat defconfig
> CONFIG_M=y
> CONFIG_A=y
> # CONFIG_B is not set
> 
> 
> If I do:
> 
>     $scripts/kconfig/conf --defconfig=defconfig Kconfig
> 
> with the above input the resulting .config is OK.
> 
> But If I drop the line:
> 
>     # CONFIG_B is not set
> 
> in the defconfig file then I end with CONFIG_A set to m.
> And this is not as expected - I cannot see why it should matter
> if we specify the value of B or not.
> 
> What we see here is that savedefconfig trigger a bug in the
> other part of kconfig - a bug which was not exposed before.
> 
> The reason why your patch cured it was that we then no
> longer triggered the bug (at least I guess so I did not look to close).
> 
> I will look into this as time permits. I assume the fix is simple
> when I find the reason.

I'm looking into it now, but understanding the kconfig internals is not
easy...

Michal

Re: [GIT] kbuild: kconfig changes

[Sam Ravnborg]

> > 
> > The reason why your patch cured it was that we then no
> > longer triggered the bug (at least I guess so I did not look to close).
> > 
> > I will look into this as time permits. I assume the fix is simple
> > when I find the reason.
> 
> I'm looking into it now, but understanding the kconfig internals is not
> easy...
Nope...

What I have understood so far...

If we have a choice then the choice is represented by a symbol.
symbol->def[S_DEF_USER] is set to the selected symbol.
If we read all the choice values then we set:
symbol->flags |= SYMBOL_DEF_USER

If we have a choice where we do not have all choice_values
which is the case with the minimal defconfig then the
missing choice_value results in that SYMBOL_DEF_USER
is not set.
[See confdata:conf_read after the sym_ok label]

Later in symbol.c:sym_calc_value the visibility
is set to mod if we do not have all values.

And therefore the coice value is set to m not y.

I will look a bit more at it tonight.

	Sam

Re: [GIT] kbuild: kconfig changes

[Sam Ravnborg]

On Fri, Aug 06, 2010 at 09:43:24PM -0700, Arve Hj?nnev?g wrote:
> On Fri, Aug 6, 2010 at 9:01 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> >>
> >> This change prevents some the minimal defconfig options from working.
> >> Specifically our usb gadget drivers do not get set.
> >
> > Can you help me reproduce this?
> >
> > I have found an issue with choice values in combination with
> > tristate logic that fails. I hope this is something similar.
> >
> 
> It is probably the same problem. The gadget driver that was not set is
> not buildable as a module (it is not in the mainline kernel). If I
> select another gadget driver instead it just gets changed to build as
> a module instead.
> 
> If you create a file, arch/arm/configs/test_defconfig with the following:
> CONFIG_MODULES=y
> CONFIG_USB_GADGET=y
> CONFIG_USB_MASS_STORAGE=y
> 
> then "make test_defconfig" results in .config having:
> CONFIG_USB_MASS_STORAGE=m
> 
>  (at least if you are set up to compile for arm)

Arve, can you please try the attached patch and check if this
fixes the issue you see.
At least I confirmed that it fixed the minimal test-case I used.

Note: the patch chenges how we interpret a minimal config,
it does not change the content of a minimal config.

Unfortunately the bug I had discovered is another bug.
If I do:
	make ARCH=avr32 atngw100_defconfig
	make ARCH=avr32 savedefconfig
	cp defconfig arch/avr32/configs/atngw100_defconfig
	make ARCH=avr32 atngw100_defconfig
	diff -u .config .config.old

then the diff shows an unexpected difference.

I will continue of that issue.
This is mainly to inform you that we have at least one
additional issue with "savedefconfig".

	Sam

From 6f36cdab76c4ee26bc39c38a2895cac5166e253b Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <sam@ravnborg.org>
Date: Wed, 11 Aug 2010 21:40:23 +0200
Subject: [PATCH] kconfig: fix tristate choice with minimal config

If a minimal config did not specify the value
of all choice values, the resulting configuration
could have wrong values.

Consider following example:
config M
        def_bool y
        option modules
choice
        prompt "choice list"
config A
        tristate "a"
config B
	tristate "b"
endchoice

With a defconfig like this:
CONFIG_M=y
CONFIG_A=y

The resulting configuration would have

    CONFIG_A=m

which was unexpected.

The problem was not not all choice values were set and thus
kconfig calculated a wrong value.

The fix is to set all choice values when we
read a defconfig files.

conf_set_all_new_symbols() is refactored such that
random choice values are now handled by a dedicated function.
And new choice values are set by set_all_choice_values().

This was not the minimal fix, but the fix that resulted
in the most readable code.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Arve Hj?nnev?g <arve@android.com>
---
 scripts/kconfig/confdata.c |  102 +++++++++++++++++++++++++++++---------------
 1 files changed, 67 insertions(+), 35 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index f81f263..23d4fa6 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -919,13 +919,73 @@ void conf_set_changed_callback(void (*fn)(void))
 	conf_changed_callback = fn;
 }
 
+static void randomize_choice_values(struct symbol *csym)
+{
+	struct property *prop;
+	struct symbol *sym;
+	struct expr *e;
+	int cnt, def;
 
-void conf_set_all_new_symbols(enum conf_def_mode mode)
+	/*
+	 * If choice is mod then we may have more items slected
+	 * and if no then no-one.
+	 * In both cases stop.
+	 */
+	if (csym->curr.tri != yes)
+		return;
+
+	prop = sym_get_choice_prop(csym);
+
+	/* count entries in choice block */
+	cnt = 0;
+	expr_list_for_each_sym(prop->expr, e, sym)
+		cnt++;
+
+	/*
+	 * find a random value and set it to yes,
+	 * set the rest to no so we have only one set
+	 */
+	def = (rand() % cnt);
+
+	cnt = 0;
+	expr_list_for_each_sym(prop->expr, e, sym) {
+		if (def == cnt++) {
+			sym->def[S_DEF_USER].tri = yes;
+			csym->def[S_DEF_USER].val = sym;
+		}
+		else {
+			sym->def[S_DEF_USER].tri = no;
+		}
+	}
+	csym->flags |= SYMBOL_DEF_USER;
+	/* clear VALID to get value calculated */
+	csym->flags &= ~(SYMBOL_VALID);
+}
+
+static void set_all_choice_values(struct symbol *csym)
 {
-	struct symbol *sym, *csym;
 	struct property *prop;
+	struct symbol *sym;
 	struct expr *e;
-	int i, cnt, def;
+
+	prop = sym_get_choice_prop(csym);
+
+	/*
+	 * Set all non-assinged choice values to no
+	 */
+	expr_list_for_each_sym(prop->expr, e, sym) {
+		if (!sym_has_value(sym))
+			sym->def[S_DEF_USER].tri = no;
+	}
+	csym->flags |= SYMBOL_DEF_USER;
+	/* clear VALID to get value calculated */
+	csym->flags &= ~(SYMBOL_VALID);
+}
+
+void conf_set_all_new_symbols(enum conf_def_mode mode)
+{
+	struct symbol *sym, *csym;
+	int i, cnt;
 
 	for_all_symbols(i, sym) {
 		if (sym_has_value(sym))
@@ -961,8 +1021,6 @@ void conf_set_all_new_symbols(enum conf_def_mode mode)
 
 	sym_clear_all_valid();
 
-	if (mode != def_random)
-		return;
 	/*
 	 * We have different type of choice blocks.
 	 * If curr.tri equal to mod then we can select several
@@ -977,35 +1035,9 @@ void conf_set_all_new_symbols(enum conf_def_mode mode)
 			continue;
 
 		sym_calc_value(csym);
-
-		if (csym->curr.tri != yes)
-			continue;
-
-		prop = sym_get_choice_prop(csym);
-
-		/* count entries in choice block */
-		cnt = 0;
-		expr_list_for_each_sym(prop->expr, e, sym)
-			cnt++;
-
-		/*
-		 * find a random value and set it to yes,
-		 * set the rest to no so we have only one set
-		 */
-		def = (rand() % cnt);
-
-		cnt = 0;
-		expr_list_for_each_sym(prop->expr, e, sym) {
-			if (def == cnt++) {
-				sym->def[S_DEF_USER].tri = yes;
-				csym->def[S_DEF_USER].val = sym;
-			}
-			else {
-				sym->def[S_DEF_USER].tri = no;
-			}
-		}
-		csym->flags |= SYMBOL_DEF_USER;
-		/* clear VALID to get value calculated */
-		csym->flags &= ~(SYMBOL_VALID);
+		if (mode == def_random)
+			randomize_choice_values(csym);
+		else
+			set_all_choice_values(csym);
 	}
 }
-- 
1.6.0.6

Re: [GIT] kbuild: kconfig changes

[Sam Ravnborg]

> 
> Unfortunately the bug I had discovered is another bug.
> If I do:
> 	make ARCH=avr32 atngw100_defconfig
> 	make ARCH=avr32 savedefconfig
> 	cp defconfig arch/avr32/configs/atngw100_defconfig
> 	make ARCH=avr32 atngw100_defconfig
> 	diff -u .config .config.old
> 
> then the diff shows an unexpected difference.
> 
> I will continue of that issue.
Fixed it - see patch below.

@Michal - I will do a proper patch submission if Arve
can confirm that the problem he saw has been fixed
with these patches.

	Sam

From d93f83bbd650499c146f29a8a4de2b1f98dfbefe Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <sam@ravnborg.org>
Date: Wed, 11 Aug 2010 22:29:05 +0200
Subject: [PATCH] kconfig: fix savedefconfig for tristate choices

savedefconfig failed to save choice symbols equal to 'y'
for tristate choices.
This resulted in this value being lost.

In particular is fixes an issue where

make ARCH=avr32 atngw100_defconfig
make ARCH=avr32 savedefconfig
cp defconfig arch/avr32/configs/atngw100_defconfig
make ARCH=avr32 atngw100_defconfig
diff -u .config .config.old

failed to produce an identical .config.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
 scripts/kconfig/confdata.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index f81f263..e5d66e4 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -497,7 +497,7 @@ int conf_write_defconfig(const char *filename)
 			/*
 			 * If symbol is a choice value and equals to the
 			 * default for a choice - skip.
-			 * But only if value equal to "y".
+			 * But only if value is bool and equal to "y" .
 			 */
 			if (sym_is_choice_value(sym)) {
 				struct symbol *cs;
@@ -506,9 +506,8 @@ int conf_write_defconfig(const char *filename)
 				cs = prop_get_symbol(sym_get_choice_prop(sym));
 				ds = sym_choice_default(cs);
 				if (sym == ds) {
-					if ((sym->type == S_BOOLEAN ||
-					sym->type == S_TRISTATE) &&
-					sym_get_tristate_value(sym) == yes)
+					if ((sym->type == S_BOOLEAN) &&
+					    sym_get_tristate_value(sym) == yes)
 						goto next_menu;
 				}
 			}
-- 
1.6.0.6

Re: [GIT] kbuild: kconfig changes

[Arve Hjønnevåg]

On Wed, Aug 11, 2010 at 1:34 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
>>
>> Unfortunately the bug I had discovered is another bug.
>> If I do:
>>       make ARCH=avr32 atngw100_defconfig
>>       make ARCH=avr32 savedefconfig
>>       cp defconfig arch/avr32/configs/atngw100_defconfig
>>       make ARCH=avr32 atngw100_defconfig
>>       diff -u .config .config.old
>>
>> then the diff shows an unexpected difference.
>>
>> I will continue of that issue.
> Fixed it - see patch below.
>
> @Michal - I will do a proper patch submission if Arve
> can confirm that the problem he saw has been fixed
> with these patches.
>

Your first patch fixes the problem I saw.

-- 
Arve Hjønnevåg

Re: [GIT] kbuild: kconfig changes

[Sam Ravnborg]

On Wed, Aug 11, 2010 at 04:39:35PM -0700, Arve Hj?nnev?g wrote:
> On Wed, Aug 11, 2010 at 1:34 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> >>
> >> Unfortunately the bug I had discovered is another bug.
> >> If I do:
> >>       make ARCH=avr32 atngw100_defconfig
> >>       make ARCH=avr32 savedefconfig
> >>       cp defconfig arch/avr32/configs/atngw100_defconfig
> >>       make ARCH=avr32 atngw100_defconfig
> >>       diff -u .config .config.old
> >>
> >> then the diff shows an unexpected difference.
> >>
> >> I will continue of that issue.
> > Fixed it - see patch below.
> >
> > @Michal - I will do a proper patch submission if Arve
> > can confirm that the problem he saw has been fixed
> > with these patches.
> >
> 
> Your first patch fixes the problem I saw.
Thanks.
I will add the following to the commit:
Reported-by: Arve Hj?nnev?g <arve@android.com>
Tested-by: Arve Hj?nnev?g <arve@android.com>

	Sam