kbuild: kconfig changes

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(-)

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

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.

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

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.

cool..

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.

---

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

index 010600e..274f271 100644
@@ -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:

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

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

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

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

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:

index e95718f..1797226 100644
@@ -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

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

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

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

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

Michal

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

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 <[email protected]>
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.

---
 scripts/kconfig/confdata.c |  102 +++++++++++++++++++++++++++++---------------
 1 files changed, 67 insertions(+), 35 deletions(-)

index f81f263..23d4fa6 100644
@@ -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

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 <[email protected]>
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.

---
 scripts/kconfig/confdata.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

index f81f263..e5d66e4 100644
@@ -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

Your first patch fixes the problem I saw.

Arve Hjønnevåg

Thanks.
I will add the following to the commit:

Sam