inconvenience when hand editing DEBUG_LL in .config

inconvenience when hand editing DEBUG_LL in .config

[Uwe Kleine-König]

Hello,

usually when I want to change a single kernel option I just open .config
in my editor, remove the line for the config item I want to change and
run oldconfig. For DEBUG_LL (defined in arch/arm/Kconfig.debug) this
doesn't work as usual though.

On 3.3-rc1 the following happens:

	make ARCH=arm at91rm9200_defconfig
	sed -i /CONFIG_DEBUG_LL/d .config
	make ARCH=arm oldconfig

The last command then asks me for DEBUG_LL, after specifying 'y', the
output looks as follows:

	Verbose user fault messages (DEBUG_USER) [N/y/?] n
	Kernel low-level debugging functions (read help!) (DEBUG_LL) [N/y/?] (NEW) y
	  Kernel low-level debugging port
	  > 1. No low-level debugging UART (DEBUG_LL_UART_NONE) (NEW)
	    2. Kernel low-level debugging via EmbeddedICE DCC channel (DEBUG_ICEDCC) (NEW)
	    3. Kernel low-level debugging on rm9200, 9260/9g20, 9261/9g10 and 9rl (AT91_DEBUG_LL_DBGU0) (NEW)
	  choice[1-3]: 1
	  Early printk (EARLY_PRINTK) [N/y/?] (NEW) 

Note that I didn't specify the '1' for the choice even though all items
are marked as new and oldconfig only starts at EARLY_PRINTK to ask
again.

I would have expected to be able to select AT91_DEBUG_LL_DBGU0.

This is not intended, is it?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: inconvenience when hand editing DEBUG_LL in .config

[Russell King - ARM Linux]

On Fri, Jan 20, 2012 at 05:40:36PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> usually when I want to change a single kernel option I just open .config
> in my editor, remove the line for the config item I want to change and
> run oldconfig. For DEBUG_LL (defined in arch/arm/Kconfig.debug) this
> doesn't work as usual though.
> 
> On 3.3-rc1 the following happens:
> 
> 	make ARCH=arm at91rm9200_defconfig
> 	sed -i /CONFIG_DEBUG_LL/d .config

Note that this deletes two entries.  Use /CONFIG_DEBUG_LL\\\>/d to delete
just one (or get rid of two of those \ and use ' to quote it.)

> 	make ARCH=arm oldconfig
> 
> The last command then asks me for DEBUG_LL, after specifying 'y', the
> output looks as follows:
> 
> 	Verbose user fault messages (DEBUG_USER) [N/y/?] n
> 	Kernel low-level debugging functions (read help!) (DEBUG_LL) [N/y/?] (NEW) y
> 	  Kernel low-level debugging port
> 	  > 1. No low-level debugging UART (DEBUG_LL_UART_NONE) (NEW)
> 	    2. Kernel low-level debugging via EmbeddedICE DCC channel (DEBUG_ICEDCC) (NEW)
> 	    3. Kernel low-level debugging on rm9200, 9260/9g20, 9261/9g10 and 9rl (AT91_DEBUG_LL_DBGU0) (NEW)
> 	  choice[1-3]: 1
> 	  Early printk (EARLY_PRINTK) [N/y/?] (NEW) 
> 
> Note that I didn't specify the '1' for the choice even though all items
> are marked as new and oldconfig only starts at EARLY_PRINTK to ask
> again.
> 
> I would have expected to be able to select AT91_DEBUG_LL_DBGU0.

Confirmed here.

> This is not intended, is it?

Nope - and if you delete all the DEBUG choice entries from your .config,
it still defaults to the first and doesn't offer it for you to choose.
Ditto if you delete none.

Re: inconvenience when hand editing DEBUG_LL in .config

[Uwe Kleine-König]

On Fri, Jan 20, 2012 at 07:27:50PM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 20, 2012 at 05:40:36PM +0100, Uwe Kleine-König wrote:
> > Hello,
> > 
> > usually when I want to change a single kernel option I just open .config
> > in my editor, remove the line for the config item I want to change and
> > run oldconfig. For DEBUG_LL (defined in arch/arm/Kconfig.debug) this
> > doesn't work as usual though.
> > 
> > On 3.3-rc1 the following happens:
> > 
> > 	make ARCH=arm at91rm9200_defconfig
> > 	sed -i /CONFIG_DEBUG_LL/d .config
> 
> Note that this deletes two entries.  Use /CONFIG_DEBUG_LL\\\>/d to delete
> just one (or get rid of two of those \ and use ' to quote it.)
After at91rm9200_defconfig CONFIG_DEBUG_LL is not set, so
CONFIG_DEBUG_LL_UART_NONE and the other options from that choice don't
appear in .config and so for me only a single line is removed. But
that's only an uninteresting detail.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: inconvenience when hand editing DEBUG_LL in .config

[Arnaud Lacombe]

Hi,

2012/1/20 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> Hello,
>
> usually when I want to change a single kernel option I just open .config
> in my editor, remove the line for the config item I want to change and
> run oldconfig. For DEBUG_LL (defined in arch/arm/Kconfig.debug) this
> doesn't work as usual though.
>
> On 3.3-rc1 the following happens:
>
>        make ARCH=arm at91rm9200_defconfig
>        sed -i /CONFIG_DEBUG_LL/d .config
>        make ARCH=arm oldconfig
>
> The last command then asks me for DEBUG_LL, after specifying 'y', the
> output looks as follows:
>
>        Verbose user fault messages (DEBUG_USER) [N/y/?] n
>        Kernel low-level debugging functions (read help!) (DEBUG_LL) [N/y/?] (NEW) y
>          Kernel low-level debugging port
>          > 1. No low-level debugging UART (DEBUG_LL_UART_NONE) (NEW)
>            2. Kernel low-level debugging via EmbeddedICE DCC channel (DEBUG_ICEDCC) (NEW)
>            3. Kernel low-level debugging on rm9200, 9260/9g20, 9261/9g10 and 9rl (AT91_DEBUG_LL_DBGU0) (NEW)
>          choice[1-3]: 1
>          Early printk (EARLY_PRINTK) [N/y/?] (NEW)
>
> Note that I didn't specify the '1' for the choice even though all items
> are marked as new and oldconfig only starts at EARLY_PRINTK to ask
> again.
>
> I would have expected to be able to select AT91_DEBUG_LL_DBGU0.
>
> This is not intended, is it?
>
definitively not.

Here is a reduced testcase showing the issue:

config A
        bool "A"
        default y
choice
        prompt "Choice ?"
        depends on A

        config CHOICE_A
                bool "Choice A"

        config CHOICE_B
                bool "Choice B"
endchoice

To trigger the issue, you need a two-step process, as you did:

 1) create a base configuration, lets assume the default one:

% make alldefconfig && cat .config
#
# Automatically generated file; DO NOT EDIT.
# Linux Kernel Configuration
#
CONFIG_A=y
CONFIG_CHOICE_A=y
# CONFIG_CHOICE_B is not set

 2) amend the config:

% sed -i /CONFIG_A/d .config && cat .config

Then, when you run `oldconfig', you will get:

% make ARCH=arm oldconfig
scripts/kconfig/conf --oldconfig Kconfig
*
* Restart config...
*
*
* Linux Kernel Configuration
*
A (A) [Y/n] (NEW) y
  Choice ?
  > 1. Choice A (CHOICE_A)
    2. Choice B (CHOICE_B)
  choice[1-2]: 1
#
# configuration written to .config
#

without being asked for the choice.

This will also happen when a choice's dependency appears between two version.

I would assume that the choice value is known, ie. not marked as 'NEW'
and thus not re-asked. What I think should be done is to reset known
choice value, if the choice dependency is marked as 'NEW'.

 - Arnaud

> Best regards
> Uwe
>
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> --
> 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

[PATCH] kconfig: fix new choices being skipped upon config update

[Arnaud Lacombe]

Running `oldconfig' after any of the following configuration change:

either trivial addition, such as:

config A
	bool "A"

choice
	prompt "Choice ?"
	depends on A

	config CHOICE_B
		bool "Choice B"

	config CHOICE_C
		bool "Choice C"
endchoice

or more tricky change:

OLD KCONFIG                      |  NEW KCONFIG
                                 |
                                 |  config A
                                 |          bool "A"
                                 |
choice                           |  choice
        prompt "Choice ?"        |          prompt "Choice ?"
                                 |
        config CHOICE_C          |          config CHOICE_C
                bool "Choice C"  |                  bool "Choice C"
                                 |
        config CHOICE_D          |          config CHOICE_D
                bool "Choice D"  |                  bool "Choice D"
endchoice                        |
                                 |          config CHOICE_E
                                 |                  bool "Choice E"
                                 |                  depends on A
                                 |  endchoice

will not cause the choice to be considered as NEW, and thus not be asked. The
cause of this behavior is that choice's novelty are computed statically right
after the saved configuration has been read. At this point, the new dependency's
value is still unknown and asserted to be `no'. Moreover, no update to this
decision is made afterward.

Correct this by dynamically evaluating a choice's novelty, and removing the
static evaluation.

Cc: linux-kbuild@vger.kernel.org
Cc: LKML <linux-kernel@vger.kernel.org>,
Cc: Michal Marek <mmarek@suse.cz>
Cc: Uwe Kleine-Konig <u.kleine-koenig@pengutronix.de>
Reported-by: Uwe Kleine-Konig <u.kleine-koenig@pengutronix.de>
Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
---
 scripts/kconfig/confdata.c |   26 ++++++--------------------
 scripts/kconfig/symbol.c   |    9 ++++++++-
 2 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 5a58965..47aed3f 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -344,10 +344,8 @@ int conf_read_simple(const char *name, int def)
 
 int conf_read(const char *name)
 {
-	struct symbol *sym, *choice_sym;
-	struct property *prop;
-	struct expr *e;
-	int i, flags;
+	struct symbol *sym;
+	int i;
 
 	sym_set_change_count(0);
 
@@ -357,7 +355,7 @@ int conf_read(const char *name)
 	for_all_symbols(i, sym) {
 		sym_calc_value(sym);
 		if (sym_is_choice(sym) || (sym->flags & SYMBOL_AUTO))
-			goto sym_ok;
+			continue;
 		if (sym_has_value(sym) && (sym->flags & SYMBOL_WRITE)) {
 			/* check that calculated value agrees with saved value */
 			switch (sym->type) {
@@ -366,30 +364,18 @@ int conf_read(const char *name)
 				if (sym->def[S_DEF_USER].tri != sym_get_tristate_value(sym))
 					break;
 				if (!sym_is_choice(sym))
-					goto sym_ok;
+					continue;
 				/* fall through */
 			default:
 				if (!strcmp(sym->curr.val, sym->def[S_DEF_USER].val))
-					goto sym_ok;
+					continue;
 				break;
 			}
 		} else if (!sym_has_value(sym) && !(sym->flags & SYMBOL_WRITE))
 			/* no previous value and not saved */
-			goto sym_ok;
+			continue;
 		conf_unsaved++;
 		/* maybe print value in verbose mode... */
-	sym_ok:
-		if (!sym_is_choice(sym))
-			continue;
-		/* The choice symbol only has a set value (and thus is not new)
-		 * if all its visible childs have values.
-		 */
-		prop = sym_get_choice_prop(sym);
-		flags = sym->flags;
-		expr_list_for_each_sym(prop->expr, e, choice_sym)
-			if (choice_sym->visible != no)
-				flags &= choice_sym->flags;
-		sym->flags &= flags | ~SYMBOL_DEF_USER;
 	}
 
 	for_all_symbols(i, sym) {
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 071f00c..22a3c40 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -262,11 +262,18 @@ static struct symbol *sym_calc_choice(struct symbol *sym)
 	struct symbol *def_sym;
 	struct property *prop;
 	struct expr *e;
+	int flags;
 
 	/* first calculate all choice values' visibilities */
+	flags = sym->flags;
 	prop = sym_get_choice_prop(sym);
-	expr_list_for_each_sym(prop->expr, e, def_sym)
+	expr_list_for_each_sym(prop->expr, e, def_sym) {
 		sym_calc_visibility(def_sym);
+		if (def_sym->visible != no)
+			flags &= def_sym->flags;
+	}
+
+	sym->flags &= flags | ~SYMBOL_DEF_USER;
 
 	/* is the user choice visible? */
 	def_sym = sym->def[S_DEF_USER].val;
-- 
1.7.9.rc1.23.g51649

Re: [PATCH] kconfig: fix new choices being skipped upon config update

[Uwe Kleine-König]

On Mon, Jan 23, 2012 at 05:29:05PM -0500, Arnaud Lacombe wrote:
> Running `oldconfig' after any of the following configuration change:
> 
> either trivial addition, such as:
> 
> config A
> 	bool "A"
> 
> choice
> 	prompt "Choice ?"
> 	depends on A
> 
> 	config CHOICE_B
> 		bool "Choice B"
> 
> 	config CHOICE_C
> 		bool "Choice C"
> endchoice
> 
> or more tricky change:
> 
> OLD KCONFIG                      |  NEW KCONFIG
>                                  |
>                                  |  config A
>                                  |          bool "A"
>                                  |
> choice                           |  choice
>         prompt "Choice ?"        |          prompt "Choice ?"
>                                  |
>         config CHOICE_C          |          config CHOICE_C
>                 bool "Choice C"  |                  bool "Choice C"
>                                  |
>         config CHOICE_D          |          config CHOICE_D
>                 bool "Choice D"  |                  bool "Choice D"
> endchoice                        |
>                                  |          config CHOICE_E
>                                  |                  bool "Choice E"
>                                  |                  depends on A
>                                  |  endchoice
> 
> will not cause the choice to be considered as NEW, and thus not be asked. The
> cause of this behavior is that choice's novelty are computed statically right
> after the saved configuration has been read. At this point, the new dependency's
> value is still unknown and asserted to be `no'. Moreover, no update to this
> decision is made afterward.
> 
> Correct this by dynamically evaluating a choice's novelty, and removing the
> static evaluation.
> 
> Cc: linux-kbuild@vger.kernel.org
> Cc: LKML <linux-kernel@vger.kernel.org>,
> Cc: Michal Marek <mmarek@suse.cz>
> Cc: Uwe Kleine-Konig <u.kleine-koenig@pengutronix.de>
> Reported-by: Uwe Kleine-Konig <u.kleine-koenig@pengutronix.de>
Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

(Please note the umlaut o in my name. If you have problems with that,
please at least write Kleine-Koenig.)

> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>

Best regards and thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] kconfig: fix new choices being skipped upon config update

[Michal Marek]

On 26.1.2012 10:26, Uwe Kleine-König wrote:
> On Mon, Jan 23, 2012 at 05:29:05PM -0500, Arnaud Lacombe wrote:
[...]
>> will not cause the choice to be considered as NEW, and thus not be asked. The
>> cause of this behavior is that choice's novelty are computed statically right
>> after the saved configuration has been read. At this point, the new dependency's
>> value is still unknown and asserted to be `no'. Moreover, no update to this
>> decision is made afterward.
>>
>> Correct this by dynamically evaluating a choice's novelty, and removing the
>> static evaluation.
>>
>> Cc: linux-kbuild@vger.kernel.org
>> Cc: LKML <linux-kernel@vger.kernel.org>,
>> Cc: Michal Marek <mmarek@suse.cz>
>> Cc: Uwe Kleine-Konig <u.kleine-koenig@pengutronix.de>
>> Reported-by: Uwe Kleine-Konig <u.kleine-koenig@pengutronix.de>
> Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks, applied to kbuild.git#kconfig.


> (Please note the umlaut o in my name. If you have problems with that,
> please at least write Kleine-Koenig.)

Fixed.

Michal