diff mbox

[1/3] kconfig: do not special-case 'MODULES' symbol

Message ID 1378242287-12759-1-git-send-email-yann.morin.1998@free.fr (mailing list archive)
State New, archived
Headers show

Commit Message

Yann E. MORIN Sept. 3, 2013, 9:04 p.m. UTC
From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Currently, the 'MODULES' symbol is hard-coded to be the default symbol
that enables/disables tristates, if no other symbol was declared with
'option modules'.

While this used to be needed for the Linux kernel, we now have an
explicit 'option modules' attached to the 'MODULES' symbol (since
cset 11097a036), so we no longer need to special-case it in the
kconfig code.

Furthermore, kconfig is extensively used out of the Linux kernel, and
other projects may have another meaning for a symbol named 'MODULES'.

This patch changes the way we enable/disable tristates: if a symbol was
found with 'option modules' attached to it, then that symbol controls
enabling tristates. Otherwise, tristates are disabled, even if a symbol
named 'MODULES' exists.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Michal Marek <mmarek@suse.cz>
---
 scripts/kconfig/menu.c  |  5 +----
 scripts/kconfig/zconf.y | 11 ++---------
 2 files changed, 3 insertions(+), 13 deletions(-)

Comments

Yann E. MORIN Sept. 3, 2013, 9:12 p.m. UTC | #1
Michal, All,

On 2013-09-03 23:04 +0200, Yann E. MORIN spake thusly:
> Currently, the 'MODULES' symbol is hard-coded to be the default symbol
> that enables/disables tristates, if no other symbol was declared with
> 'option modules'.
> 
> While this used to be needed for the Linux kernel, we now have an
> explicit 'option modules' attached to the 'MODULES' symbol (since
> cset 11097a036), so we no longer need to special-case it in the
> kconfig code.
> 
> Furthermore, kconfig is extensively used out of the Linux kernel, and
> other projects may have another meaning for a symbol named 'MODULES'.
> 
> This patch changes the way we enable/disable tristates: if a symbol was
> found with 'option modules' attached to it, then that symbol controls
> enabling tristates. Otherwise, tristates are disabled, even if a symbol
> named 'MODULES' exists.

Doh... I forgot to send an intro mail first. Seems holidays really were
a good break! :-)

Anyway, this series might be a bit late to go in for 3.12, but it has
previously been discussed with you and Sam.

If you feel it's too touchy for 3.12, then I believe it can easily wait
for 3.13.

If you want it for 3.12, I can send a pull-request if that's easier for
you.

Regards,
Yann E. MORIN.
Michal Marek Sept. 4, 2013, 8:22 a.m. UTC | #2
Dne 3.9.2013 23:12, Yann E. MORIN napsal(a):
> Doh... I forgot to send an intro mail first. Seems holidays really were
> a good break! :-)

:-)


> Anyway, this series might be a bit late to go in for 3.12, but it has
> previously been discussed with you and Sam.
> 
> If you feel it's too touchy for 3.12, then I believe it can easily wait
> for 3.13.
> 
> If you want it for 3.12, I can send a pull-request if that's easier for
> you.

Please send me a pull request regardless, so that the second commit's
changelog references the right commit id. I'll play with this and will
merge it into kbuild.git if there are no problems. It's two days into
the merge window, so no big deal (shh!).

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
Michal Marek Sept. 5, 2013, 9:38 a.m. UTC | #3
On 3.9.2013 23:04, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Currently, the 'MODULES' symbol is hard-coded to be the default symbol
> that enables/disables tristates, if no other symbol was declared with
> 'option modules'.
> 
> While this used to be needed for the Linux kernel, we now have an
> explicit 'option modules' attached to the 'MODULES' symbol (since
> cset 11097a036), so we no longer need to special-case it in the
> kconfig code.
> 
> Furthermore, kconfig is extensively used out of the Linux kernel, and
> other projects may have another meaning for a symbol named 'MODULES'.
> 
> This patch changes the way we enable/disable tristates: if a symbol was
> found with 'option modules' attached to it, then that symbol controls
> enabling tristates. Otherwise, tristates are disabled, even if a symbol
> named 'MODULES' exists.

Wasn't this change supposed to fix allmodconfig with KCONFIG_ALLCONFIG
(http://lkml.org/lkml/2013/8/8/573)? Or is there still more to do?
Because I'm still getting:

$ touch empty
$ make KCONFIG_ALLCONFIG=empty allmodconfig
  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 --allmodconfig Kconfig
#
# configuration written to .config
#
$ grep MODULES .config
CONFIG_MODULES_USE_ELF_RELA=y
# CONFIG_MODULES is not set
$

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
Michal Marek Sept. 5, 2013, 10:02 a.m. UTC | #4
On 5.9.2013 11:38, Michal Marek wrote:
> On 3.9.2013 23:04, Yann E. MORIN wrote:
>> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> This patch changes the way we enable/disable tristates: if a symbol was
>> found with 'option modules' attached to it, then that symbol controls
>> enabling tristates. Otherwise, tristates are disabled, even if a symbol
>> named 'MODULES' exists.
> 
> Wasn't this change supposed to fix allmodconfig with KCONFIG_ALLCONFIG
> (http://lkml.org/lkml/2013/8/8/573)? Or is there still more to do?
> Because I'm still getting:
> [...]
> $ grep MODULES .config
> CONFIG_MODULES_USE_ELF_RELA=y
> # CONFIG_MODULES is not set
> $

Otherwise, I have no problem merging this series. I'm just curious what
to do with the allmodconfig with KCONFIG_ALLCONFIG bug.

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 Sept. 6, 2013, 9:47 a.m. UTC | #5
Michal, All,

On 2013-09-05 11:38 +0200, Michal Marek spake thusly:
> On 3.9.2013 23:04, Yann E. MORIN wrote:
> > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > 
> > Currently, the 'MODULES' symbol is hard-coded to be the default symbol
> > that enables/disables tristates, if no other symbol was declared with
> > 'option modules'.
> > 
> > While this used to be needed for the Linux kernel, we now have an
> > explicit 'option modules' attached to the 'MODULES' symbol (since
> > cset 11097a036), so we no longer need to special-case it in the
> > kconfig code.
> > 
> > Furthermore, kconfig is extensively used out of the Linux kernel, and
> > other projects may have another meaning for a symbol named 'MODULES'.
> > 
> > This patch changes the way we enable/disable tristates: if a symbol was
> > found with 'option modules' attached to it, then that symbol controls
> > enabling tristates. Otherwise, tristates are disabled, even if a symbol
> > named 'MODULES' exists.
> 
> Wasn't this change supposed to fix allmodconfig with KCONFIG_ALLCONFIG
> (http://lkml.org/lkml/2013/8/8/573)? Or is there still more to do?

This is part of the fix, and there are still things to change, yes.

I'm not done with it yet, since I was unavailable for some time, and I
just wanted this part to be reviewed/upstreamed before continuing, just
in case I was going the wrong way.

I'll be further working on this, and hope to have something at the
beginning of next week.

Regards,
Yann E. MORIN.
Yann E. MORIN Sept. 6, 2013, 9:48 a.m. UTC | #6
Michal, All,

On 2013-09-05 12:02 +0200, Michal Marek spake thusly:
> On 5.9.2013 11:38, Michal Marek wrote:
> > On 3.9.2013 23:04, Yann E. MORIN wrote:
> >> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >> This patch changes the way we enable/disable tristates: if a symbol was
> >> found with 'option modules' attached to it, then that symbol controls
> >> enabling tristates. Otherwise, tristates are disabled, even if a symbol
> >> named 'MODULES' exists.
> > 
> > Wasn't this change supposed to fix allmodconfig with KCONFIG_ALLCONFIG
> > (http://lkml.org/lkml/2013/8/8/573)? Or is there still more to do?
> > Because I'm still getting:
> > [...]
> > $ grep MODULES .config
> > CONFIG_MODULES_USE_ELF_RELA=y
> > # CONFIG_MODULES is not set
> > $
> 
> Otherwise, I have no problem merging this series.

Good, thanks!

> I'm just curious what
> to do with the allmodconfig with KCONFIG_ALLCONFIG bug.

Working on it! ;-)

Regards,
Yann E. MORIN.
Michal Marek Sept. 6, 2013, 10:02 a.m. UTC | #7
On 6.9.2013 11:47, Yann E. MORIN wrote:
> Michal, All,
> 
> On 2013-09-05 11:38 +0200, Michal Marek spake thusly:
>> On 3.9.2013 23:04, Yann E. MORIN wrote:
>>> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>
>>> Currently, the 'MODULES' symbol is hard-coded to be the default symbol
>>> that enables/disables tristates, if no other symbol was declared with
>>> 'option modules'.
>>>
>>> While this used to be needed for the Linux kernel, we now have an
>>> explicit 'option modules' attached to the 'MODULES' symbol (since
>>> cset 11097a036), so we no longer need to special-case it in the
>>> kconfig code.
>>>
>>> Furthermore, kconfig is extensively used out of the Linux kernel, and
>>> other projects may have another meaning for a symbol named 'MODULES'.
>>>
>>> This patch changes the way we enable/disable tristates: if a symbol was
>>> found with 'option modules' attached to it, then that symbol controls
>>> enabling tristates. Otherwise, tristates are disabled, even if a symbol
>>> named 'MODULES' exists.
>>
>> Wasn't this change supposed to fix allmodconfig with KCONFIG_ALLCONFIG
>> (http://lkml.org/lkml/2013/8/8/573)? Or is there still more to do?
> 
> This is part of the fix, and there are still things to change, yes.
>
> I'm not done with it yet, since I was unavailable for some time, and I
> just wanted this part to be reviewed/upstreamed before continuing, just
> in case I was going the wrong way.

Understood. I will merge this series now and send the whole branch to
Linus later.


> I'll be further working on this, and hope to have something at the
> beginning of next week.

OK, thanks!

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
diff mbox

Patch

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 7e233a6..3a9c674 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -197,12 +197,9 @@  void menu_add_symbol(enum prop_type type, struct symbol *sym, struct expr *dep)
 
 void menu_add_option(int token, char *arg)
 {
-	struct property *prop;
-
 	switch (token) {
 	case T_OPT_MODULES:
-		prop = prop_alloc(P_DEFAULT, modules_sym);
-		prop->expr = expr_alloc_symbol(current_entry->sym);
+		modules_sym = current_entry->sym;
 		break;
 	case T_OPT_DEFCONFIG_LIST:
 		if (!sym_defconfig_list)
diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y
index 864da07..0653886 100644
--- a/scripts/kconfig/zconf.y
+++ b/scripts/kconfig/zconf.y
@@ -493,9 +493,6 @@  void conf_parse(const char *name)
 
 	sym_init();
 	_menu_init();
-	modules_sym = sym_lookup(NULL, 0);
-	modules_sym->type = S_BOOLEAN;
-	modules_sym->flags |= SYMBOL_AUTO;
 	rootmenu.prompt = menu_add_prompt(P_MENU, "Linux Kernel Configuration", NULL);
 
 	if (getenv("ZCONF_DEBUG"))
@@ -503,12 +500,8 @@  void conf_parse(const char *name)
 	zconfparse();
 	if (zconfnerrs)
 		exit(1);
-	if (!modules_sym->prop) {
-		struct property *prop;
-
-		prop = prop_alloc(P_DEFAULT, modules_sym);
-		prop->expr = expr_alloc_symbol(sym_lookup("MODULES", 0));
-	}
+	if (!modules_sym)
+		modules_sym = sym_find( "n" );
 
 	rootmenu.prompt->text = _(rootmenu.prompt->text);
 	rootmenu.prompt->text = sym_expand_string_value(rootmenu.prompt->text);