Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"
diff mbox

Message ID 1372196264-3407-1-git-send-email-yann.morin.1998@free.fr
State New, archived
Headers show

Commit Message

Yann E. MORIN June 25, 2013, 9:37 p.m. UTC
From: "Yann E. MORIN" <yann.morin.1998@free.fr>

This reverts commit 8357b48549e17b3e4e402c7f977b65708922e60f.

It breaks more stuff than it fixes.

Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Michal Marek <mmarek@suse.cz>
Cc: Fengguang Wu <fengguang.wu@intel.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: Matt Porter <mporter@kernel.crashing.org>

---
Michal, here is the revert patch if you want it.
Regards,
Yann E. MORIN.
---
 scripts/kconfig/confdata.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Michal Marek June 26, 2013, 1:48 p.m. UTC | #1
On Tue, Jun 25, 2013 at 11:37:44PM +0200, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> This reverts commit 8357b48549e17b3e4e402c7f977b65708922e60f.
> 
> It breaks more stuff than it fixes.

Thanks, I applied it to kbuild.git#kconfig, after verifying that the
issue is gone.

Michal
--
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
Yann E. MORIN June 26, 2013, 9:44 p.m. UTC | #2
Michal, All,

On 2013-06-26 15:48 +0200, Michal Marek spake thusly:
> On Tue, Jun 25, 2013 at 11:37:44PM +0200, Yann E. MORIN wrote:
> > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > 
> > This reverts commit 8357b48549e17b3e4e402c7f977b65708922e60f.
> > 
> > It breaks more stuff than it fixes.
> 
> Thanks, I applied it to kbuild.git#kconfig, after verifying that the
> issue is gone.

Yes, thanks. I'm sorry this second attempt was a (slightly different!)
failure again. :-(

The fix is working for boolean choices, but exposes a latent bug on
tristate changes. Obviously, we must fix that latent bug first, but I
can't come to a sane solution.

I'll let it mature in a corner of my head for one more night. Maybe I'll
see things clearer tomorrow. ;-)

Regards,
Yann E. MORIN.
Sedat Dilek June 27, 2013, 6:23 a.m. UTC | #3
On Wed, Jun 26, 2013 at 11:44 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Michal, All,
>
> On 2013-06-26 15:48 +0200, Michal Marek spake thusly:
>> On Tue, Jun 25, 2013 at 11:37:44PM +0200, Yann E. MORIN wrote:
>> > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> >
>> > This reverts commit 8357b48549e17b3e4e402c7f977b65708922e60f.
>> >
>> > It breaks more stuff than it fixes.
>>
>> Thanks, I applied it to kbuild.git#kconfig, after verifying that the
>> issue is gone.
>
> Yes, thanks. I'm sorry this second attempt was a (slightly different!)
> failure again. :-(
>
> The fix is working for boolean choices, but exposes a latent bug on
> tristate changes. Obviously, we must fix that latent bug first, but I
> can't come to a sane solution.
>
> I'll let it mature in a corner of my head for one more night. Maybe I'll
> see things clearer tomorrow. ;-)
>

Just as a side-effect:
Yesterday, I did some git-bisecting which has shown a char-misc-next issue.
Unfortunately, I had to revert this change in several runs to do a
sane bisecting.

NOTE:
Can you please write up some cooler changelog next time why this
revert was done?
In one of your emails you did some nice analysis.
Such informations belong IMHO into the revert's changelog.

- Sedat -

> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
--
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
Yann E. MORIN June 28, 2013, 5:19 p.m. UTC | #4
Sedat, All,

On 2013-06-27 08:23 +0200, Sedat Dilek spake thusly:
> On Wed, Jun 26, 2013 at 11:44 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > Michal, All,
> >
> > On 2013-06-26 15:48 +0200, Michal Marek spake thusly:
> >> On Tue, Jun 25, 2013 at 11:37:44PM +0200, Yann E. MORIN wrote:
> >> > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >> >
> >> > This reverts commit 8357b48549e17b3e4e402c7f977b65708922e60f.
> >> >
> >> > It breaks more stuff than it fixes.
> >>
> >> Thanks, I applied it to kbuild.git#kconfig, after verifying that the
> >> issue is gone.
> >
> > Yes, thanks. I'm sorry this second attempt was a (slightly different!)
> > failure again. :-(
[--SNIP--]
> Can you please write up some cooler changelog next time why this
> revert was done?
> In one of your emails you did some nice analysis.
> Such informations belong IMHO into the revert's changelog.

Yes, you are right, I'll do next time I'll have to send a revert patch
(which I hope not to have to for some time! ;-) )

Regards,
Yann E. MORIN.

Patch
diff mbox

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 3e39208..c55c227 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -288,6 +288,8 @@  load:
 	for_all_symbols(i, sym) {
 		sym->flags |= SYMBOL_CHANGED;
 		sym->flags &= ~(def_flags|SYMBOL_VALID);
+		if (sym_is_choice(sym))
+			sym->flags |= def_flags;
 		switch (sym->type) {
 		case S_INT:
 		case S_HEX:
@@ -377,13 +379,13 @@  setsym:
 			case mod:
 				if (cs->def[def].tri == yes) {
 					conf_warning("%s creates inconsistent choice state", sym->name);
+					cs->flags &= ~def_flags;
 				}
 				break;
 			case yes:
 				if (cs->def[def].tri != no)
 					conf_warning("override: %s changes choice state", sym->name);
 				cs->def[def].val = sym;
-				cs->flags |= def_flags;
 				break;
 			}
 			cs->def[def].tri = EXPR_OR(cs->def[def].tri, sym->def[def].tri);
@@ -789,8 +791,6 @@  int conf_write(const char *name)
 			sym_calc_value(sym);
 			if (!(sym->flags & SYMBOL_WRITE))
 				goto next;
-			if (sym_is_choice_value(sym) && !menu_is_visible(menu->parent))
-				goto next;
 			sym->flags &= ~SYMBOL_WRITE;
 
 			conf_write_symbol(out, sym, &kconfig_printer_cb, NULL);