linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] kconfig: Factor out conf_validate_choice_val() function from conf_read_simple()
       [not found] <1312067499.22074.59.camel@i7.infradead.org>
@ 2011-07-30 23:13 ` David Woodhouse
  2011-07-31  2:17   ` Arnaud Lacombe
  2011-07-30 23:14 ` [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig' David Woodhouse
  1 sibling, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-07-30 23:13 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: H. Peter Anvin, linux-kernel, linux-kbuild

We're going to want to do this from elsewhere, shortly...

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 scripts/kconfig/confdata.c |   42 +++++++++++++++++++++++-------------------
 1 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 2bafd9a..a518ab3 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -179,6 +179,27 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
 	return 0;
 }
 
+static void conf_validate_choice_val(struct symbol *sym, int def, int def_flags)
+{
+	struct symbol *cs = prop_get_symbol(sym_get_choice_prop(sym));
+	switch (sym->def[def].tri) {
+	case no:
+		break;
+	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;
+		break;
+	}
+	cs->def[def].tri = EXPR_OR(cs->def[def].tri, sym->def[def].tri);
+}
+
 int conf_read_simple(const char *name, int def)
 {
 	FILE *in = NULL;
@@ -311,25 +332,8 @@ load:
 			continue;
 		}
 setsym:
-		if (sym && sym_is_choice_value(sym)) {
-			struct symbol *cs = prop_get_symbol(sym_get_choice_prop(sym));
-			switch (sym->def[def].tri) {
-			case no:
-				break;
-			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;
-				break;
-			}
-			cs->def[def].tri = EXPR_OR(cs->def[def].tri, sym->def[def].tri);
-		}
+		if (sym && sym_is_choice_value(sym))
+			conf_validate_choice_val(sym, def, def_flags);
 	}
 	fclose(in);
 
-- 
1.7.6




^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
       [not found] <1312067499.22074.59.camel@i7.infradead.org>
  2011-07-30 23:13 ` [PATCH 1/2] kconfig: Factor out conf_validate_choice_val() function from conf_read_simple() David Woodhouse
@ 2011-07-30 23:14 ` David Woodhouse
  2011-07-30 23:44   ` Arnaud Lacombe
  1 sibling, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-07-30 23:14 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: H. Peter Anvin, linux-kernel, linux-kbuild

This allows you to set (and clear) config options on the make command
line, for all config targets. For example:

   make CONFIG_64BIT=n randconfig
   make CONFIG_64BIT=n allmodconfig
   make CONFIG_64BIT=y CONFIG_SATA_MV=y oldconfig

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 scripts/kconfig/conf.c     |    7 ++++++-
 scripts/kconfig/confdata.c |   26 ++++++++++++++++++++++++++
 scripts/kconfig/lkc.h      |    1 +
 3 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 006ad81..2b91e3b 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -456,7 +456,7 @@ static struct option long_opts[] = {
 	{NULL, 0, NULL, 0}
 };
 
-int main(int ac, char **av)
+int main(int ac, char **av, char **ep)
 {
 	int opt;
 	const char *name;
@@ -563,6 +563,11 @@ int main(int ac, char **av)
 		break;
 	}
 
+	for ( ; *ep;  ep++) {
+		if (!strncmp(*ep, CONFIG_, strlen(CONFIG_)))
+			conf_set_symbol_from_env(*ep);
+	}
+
 	if (sync_kconfig) {
 		if (conf_get_changed()) {
 			name = getenv("KCONFIG_NOSILENTUPDATE");
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index a518ab3..c64eb33 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -342,6 +342,32 @@ setsym:
 	return 0;
 }
 
+void conf_set_symbol_from_env(char *str)
+{
+	char *p = strchr(str, '=');
+	struct symbol *sym;
+	int def = S_DEF_USER;
+	int def_flags = SYMBOL_DEF << def;
+
+	if (!p)
+		return;
+
+	*p = 0;
+	sym = sym_find(str + strlen(CONFIG_));
+	*p++ = '=';
+
+	if (!sym)
+		return;
+
+	sym_calc_value(sym);
+	if (!sym_set_string_value(sym, p))
+		return;
+
+	conf_message(CONFIG_ "%s set to %s from environment", sym->name, p);
+	if (sym_is_choice_value(sym))
+		conf_validate_choice_val(sym, def, def_flags);
+}
+
 int conf_read(const char *name)
 {
 	struct symbol *sym, *choice_sym;
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index f34a0a9..fc2f3ad 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -89,6 +89,7 @@ char *conf_get_default_confname(void);
 void sym_set_change_count(int count);
 void sym_add_change_count(int count);
 void conf_set_all_new_symbols(enum conf_def_mode mode);
+void conf_set_symbol_from_env(char *);
 
 /* confdata.c and expr.c */
 static inline void xfwrite(const void *str, size_t len, size_t count, FILE *out)
-- 
1.7.6




^ permalink raw reply related	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-30 23:14 ` [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig' David Woodhouse
@ 2011-07-30 23:44   ` Arnaud Lacombe
  2011-07-30 23:53     ` H. Peter Anvin
  0 siblings, 1 reply; 50+ messages in thread
From: Arnaud Lacombe @ 2011-07-30 23:44 UTC (permalink / raw)
  To: David Woodhouse; +Cc: H. Peter Anvin, linux-kernel, linux-kbuild

Hi,

On Sat, Jul 30, 2011 at 7:14 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> This allows you to set (and clear) config options on the make command
> line, for all config targets. For example:
>
>   make CONFIG_64BIT=n randconfig
>   make CONFIG_64BIT=n allmodconfig
>   make CONFIG_64BIT=y CONFIG_SATA_MV=y oldconfig
>
technically, no, this will not work as you intend. The following:

 make CONFIG_SATA_MV=y allnoconfig

will fail to set CONFIG_SATA_MV=y. This is not a flaw in your patch,
but a known issue with kconfig as even if you are setting
CONFIG_SATA_MV=y, it's direct dependency are still unmet and thus the
symbol in never considered to be output. The same flaw is present with
the "allno.config" logic.

Beside that, the one thing I dislike with your patch is that it is
unconditional and global to all symbols, and you have no way to forbid
the environment to override a value.

What about:
 - a "select"-like approach, which would guarantee symbol selection
and warn you when unmet-dependency are present
 - an opt-in approach to the symbol override from the environment


 - Arnaud


> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> ---
>  scripts/kconfig/conf.c     |    7 ++++++-
>  scripts/kconfig/confdata.c |   26 ++++++++++++++++++++++++++
>  scripts/kconfig/lkc.h      |    1 +
>  3 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 006ad81..2b91e3b 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -456,7 +456,7 @@ static struct option long_opts[] = {
>        {NULL, 0, NULL, 0}
>  };
>
> -int main(int ac, char **av)
> +int main(int ac, char **av, char **ep)
>  {
>        int opt;
>        const char *name;
> @@ -563,6 +563,11 @@ int main(int ac, char **av)
>                break;
>        }
>
> +       for ( ; *ep;  ep++) {
> +               if (!strncmp(*ep, CONFIG_, strlen(CONFIG_)))
> +                       conf_set_symbol_from_env(*ep);
> +       }
> +
>        if (sync_kconfig) {
>                if (conf_get_changed()) {
>                        name = getenv("KCONFIG_NOSILENTUPDATE");
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index a518ab3..c64eb33 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -342,6 +342,32 @@ setsym:
>        return 0;
>  }
>
> +void conf_set_symbol_from_env(char *str)
> +{
> +       char *p = strchr(str, '=');
> +       struct symbol *sym;
> +       int def = S_DEF_USER;
> +       int def_flags = SYMBOL_DEF << def;
> +
> +       if (!p)
> +               return;
> +
> +       *p = 0;
> +       sym = sym_find(str + strlen(CONFIG_));
> +       *p++ = '=';
> +
> +       if (!sym)
> +               return;
> +
> +       sym_calc_value(sym);
> +       if (!sym_set_string_value(sym, p))
> +               return;
> +
> +       conf_message(CONFIG_ "%s set to %s from environment", sym->name, p);
> +       if (sym_is_choice_value(sym))
> +               conf_validate_choice_val(sym, def, def_flags);
> +}
> +
>  int conf_read(const char *name)
>  {
>        struct symbol *sym, *choice_sym;
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index f34a0a9..fc2f3ad 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -89,6 +89,7 @@ char *conf_get_default_confname(void);
>  void sym_set_change_count(int count);
>  void sym_add_change_count(int count);
>  void conf_set_all_new_symbols(enum conf_def_mode mode);
> +void conf_set_symbol_from_env(char *);
>
>  /* confdata.c and expr.c */
>  static inline void xfwrite(const void *str, size_t len, size_t count, FILE *out)
> --
> 1.7.6
>
>
>
>

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-30 23:44   ` Arnaud Lacombe
@ 2011-07-30 23:53     ` H. Peter Anvin
  2011-07-31  0:05       ` Arnaud Lacombe
  0 siblings, 1 reply; 50+ messages in thread
From: H. Peter Anvin @ 2011-07-30 23:53 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: David Woodhouse, linux-kernel, linux-kbuild

On 07/30/2011 04:44 PM, Arnaud Lacombe wrote:
> 
> Beside that, the one thing I dislike with your patch is that it is
> unconditional and global to all symbols, and you have no way to forbid
> the environment to override a value.
> 

Why????

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-30 23:53     ` H. Peter Anvin
@ 2011-07-31  0:05       ` Arnaud Lacombe
  2011-07-31  0:29         ` H. Peter Anvin
  2011-08-09 14:14         ` Michal Marek
  0 siblings, 2 replies; 50+ messages in thread
From: Arnaud Lacombe @ 2011-07-31  0:05 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: David Woodhouse, linux-kernel, linux-kbuild

Hi,

On Sat, Jul 30, 2011 at 7:53 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/30/2011 04:44 PM, Arnaud Lacombe wrote:
>>
>> Beside that, the one thing I dislike with your patch is that it is
>> unconditional and global to all symbols, and you have no way to forbid
>> the environment to override a value.
>>
>
> Why????
>
Because kconfig might not be ran exclusively from a fully controlled
and restricted environment ? Not to mention that it is used by other
people than the linux kernel folks.

 - Arnaud

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31  0:05       ` Arnaud Lacombe
@ 2011-07-31  0:29         ` H. Peter Anvin
  2011-07-31  1:06           ` Arnaud Lacombe
  2011-08-09 14:14         ` Michal Marek
  1 sibling, 1 reply; 50+ messages in thread
From: H. Peter Anvin @ 2011-07-31  0:29 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: David Woodhouse, linux-kernel, linux-kbuild

On 07/30/2011 05:05 PM, Arnaud Lacombe wrote:
>>
>> Why????
>>
> Because kconfig might not be ran exclusively from a fully controlled
> and restricted environment ? Not to mention that it is used by other
> people than the linux kernel folks.
> 

I'm sorry, but whitelisting specific options is an absolutely idiotic
way to deal with that.  The options use a specific namespace (CONFIG_*),
and only allowing some options to be set on the command line, but not
others, is a serious violation of the principle of least surprise.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31  0:29         ` H. Peter Anvin
@ 2011-07-31  1:06           ` Arnaud Lacombe
  2011-07-31  1:28             ` H. Peter Anvin
  2011-07-31  7:33             ` David Woodhouse
  0 siblings, 2 replies; 50+ messages in thread
From: Arnaud Lacombe @ 2011-07-31  1:06 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: David Woodhouse, linux-kernel, linux-kbuild, Roman Zippel

Hi,

[Added Roman Zippel to the Cc: list.]

On Sat, Jul 30, 2011 at 8:29 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/30/2011 05:05 PM, Arnaud Lacombe wrote:
>>>
>>> Why????
>>>
>> Because kconfig might not be ran exclusively from a fully controlled
>> and restricted environment ? Not to mention that it is used by other
>> people than the linux kernel folks.
>>
>
> I'm sorry, but whitelisting specific options is an absolutely idiotic
> way to deal with that.
I'm sure the author of `option env=""' will appreciate that. I'd be
interested to know if there was a reason to do it that way rather than
allow the environment to override all symbols.

> The options use a specific namespace (CONFIG_*),
CONFIG_ is sure very specific namespace...

> and only allowing some options to be set on the command line, but not
> others, is a serious violation of the principle of least surprise.
>
The principle of least surprise is broken anyway as the proposed patch
has absolutely no dependency checking and verification. You can `make
CONFIG_SATA_MV=y allnoconfig', you will _not_ get it set.

 - Arnaud

>        -hpa
>
> --
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel.  I don't speak on their behalf.
>
>

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31  1:06           ` Arnaud Lacombe
@ 2011-07-31  1:28             ` H. Peter Anvin
  2011-07-31  2:09               ` Arnaud Lacombe
  2011-07-31  7:33             ` David Woodhouse
  1 sibling, 1 reply; 50+ messages in thread
From: H. Peter Anvin @ 2011-07-31  1:28 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: David Woodhouse, linux-kernel, linux-kbuild, Roman Zippel

On 07/30/2011 06:06 PM, Arnaud Lacombe wrote:
>>
> The principle of least surprise is broken anyway as the proposed patch
> has absolutely no dependency checking and verification. You can `make
> CONFIG_SATA_MV=y allnoconfig', you will _not_ get it set.
> 
This gives you exactly the same way as a .config file or a ALLCONFIG
file with the same options, which is what one realistically should expect.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31  1:28             ` H. Peter Anvin
@ 2011-07-31  2:09               ` Arnaud Lacombe
  2011-07-31  5:21                 ` Arnaud Lacombe
  2011-07-31 22:18                 ` David Woodhouse
  0 siblings, 2 replies; 50+ messages in thread
From: Arnaud Lacombe @ 2011-07-31  2:09 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: David Woodhouse, linux-kernel, linux-kbuild, Roman Zippel

Hi,

On Sat, Jul 30, 2011 at 9:28 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/30/2011 06:06 PM, Arnaud Lacombe wrote:
>>>
>> The principle of least surprise is broken anyway as the proposed patch
>> has absolutely no dependency checking and verification. You can `make
>> CONFIG_SATA_MV=y allnoconfig', you will _not_ get it set.
>>
> This gives you exactly the same way as a .config file or a ALLCONFIG
> file with the same options, which is what one realistically should expect.
>
no, I would expect it to have the same effect as a `select
CONFIG_SATA_MV', have it forcibly set to the given value, and be
warned if there was any problem.

Btw, `make CONFIG_GENERIC_BUG=n oldconfig' or `make CONFIG_64BIT=n
oldconfig'[0] does not even work, and I'm getting no error. So either
you make it work for all possible uses, or you warn the user he tried
something illegal, but you don't just fail silently.

>From my point of view, this patch, as-is, open a delicate Pandora's box.

 - Arnaud

[0]: which is still funny to do with CONFIG_X86_64=y :)

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 1/2] kconfig: Factor out conf_validate_choice_val() function from conf_read_simple()
  2011-07-30 23:13 ` [PATCH 1/2] kconfig: Factor out conf_validate_choice_val() function from conf_read_simple() David Woodhouse
@ 2011-07-31  2:17   ` Arnaud Lacombe
  2011-07-31 22:21     ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Arnaud Lacombe @ 2011-07-31  2:17 UTC (permalink / raw)
  To: David Woodhouse
  Cc: H. Peter Anvin, linux-kernel, linux-kbuild, Arnaud Lacombe

Hi,
On Sat, Jul 30, 2011 at 7:13 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> We're going to want to do this from elsewhere, shortly...
>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>
I would go a little further and apply the attached patch too, The two jump
site to `setsym' do not meet the conditionnal and can be avoided.

 - Arnaud

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 0341254..74dcfb1 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -284,7 +284,7 @@ load:
 				sym = sym_find(line + 2 + strlen(CONFIG_));
 				if (!sym) {
 					sym_add_change_count(1);
-					goto setsym;
+					continue;
 				}
 			} else {
 				sym = sym_lookup(line + 2 + strlen(CONFIG_), 0);
@@ -318,7 +318,7 @@ load:
 				sym = sym_find(line + strlen(CONFIG_));
 				if (!sym) {
 					sym_add_change_count(1);
-					goto setsym;
+					continue;
 				}
 			} else {
 				sym = sym_lookup(line + strlen(CONFIG_), 0);
@@ -335,7 +335,7 @@ load:
 				conf_warning("unexpected data");
 			continue;
 		}
-setsym:
 		if (sym && sym_is_choice_value(sym))
 			conf_validate_choice_val(sym, def, def_flags);
 	}

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31  2:09               ` Arnaud Lacombe
@ 2011-07-31  5:21                 ` Arnaud Lacombe
  2011-07-31 22:18                 ` David Woodhouse
  1 sibling, 0 replies; 50+ messages in thread
From: Arnaud Lacombe @ 2011-07-31  5:21 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: David Woodhouse, linux-kernel, linux-kbuild, Roman Zippel

Hi,

On Sat, Jul 30, 2011 at 10:09 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Sat, Jul 30, 2011 at 9:28 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 07/30/2011 06:06 PM, Arnaud Lacombe wrote:
>>>>
>>> The principle of least surprise is broken anyway as the proposed patch
>>> has absolutely no dependency checking and verification. You can `make
>>> CONFIG_SATA_MV=y allnoconfig', you will _not_ get it set.
>>>
>> This gives you exactly the same way as a .config file or a ALLCONFIG
>> file with the same options, which is what one realistically should expect.
>>
> no, I would expect it to have the same effect as a `select
> CONFIG_SATA_MV', have it forcibly set to the given value, and be
> warned if there was any problem.
>
> Btw, `make CONFIG_GENERIC_BUG=n oldconfig' or `make CONFIG_64BIT=n
> oldconfig'[0] does not even work, and I'm getting no error. So either
> you make it work for all possible uses, or you warn the user he tried
> something illegal, but you don't just fail silently.
>
ok, the issue is that you will only be allowed to change visible
symbols. CONFIG_64BIT is conditionally visible (when ARCH=x86), so
right now, you can not do on x86-64:

% make ARCH=i386 defconfig
% make CONFIG_64BIT=n oldconfig # [0]

and expect it to work.

Instead of allowing explicitly a symbol to be overridden, the behavior
is to implicitly, silently and un-deterministicaly[1] deny an
override. Strange ...

 - Arnaud

[0]: this match David's use-case described in
<1311986969.20983.52.camel@i7.infradead.org>

[1]: ie. it depends on the state of the tree when ran

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31  1:06           ` Arnaud Lacombe
  2011-07-31  1:28             ` H. Peter Anvin
@ 2011-07-31  7:33             ` David Woodhouse
  2011-07-31 16:37               ` Randy Dunlap
  1 sibling, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-07-31  7:33 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: H. Peter Anvin, linux-kernel, linux-kbuild, Roman Zippel

On Sat, 2011-07-30 at 21:06 -0400, Arnaud Lacombe wrote:
> The principle of least surprise is broken anyway as the proposed patch
> has absolutely no dependency checking and verification. You can `make
> CONFIG_SATA_MV=y allnoconfig', you will _not_ get it set.

That's always true in kconfig *anyway*. We've *never* really had an
option for "do whatever you need to enable this option". We've even
hard-coded this failure in our language, by introducing this horrible
'select' thing to work around it.

I'd no more expect that, than I would for it to write the code for me if
I type 'make CONFIG_BTRFSv2=y oldconfig'.

So no, it doesn't violate the principle of least surprise.

> ok, the issue is that you will only be allowed to change visible
> symbols. CONFIG_64BIT is conditionally visible (when ARCH=x86), so
> right now, you can not do on x86-64:
> 
> % make ARCH=i386 defconfig
> % make CONFIG_64BIT=n oldconfig # [0]

That works fine here. What was ARCH set to in your second test? If it's
ARCH=x86_64 then that's expected. That's the whole point of my *other*
patch to make 'ARCH=x86' be the default, so that the value of
CONFIG_64BIT in your .config is *not* forcibly overridden to match the
build host. That's a *separate* bug, which I also have a patch for.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31  7:33             ` David Woodhouse
@ 2011-07-31 16:37               ` Randy Dunlap
  2011-07-31 16:57                 ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Randy Dunlap @ 2011-07-31 16:37 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Arnaud Lacombe, H. Peter Anvin, linux-kernel, linux-kbuild, Roman Zippel

On Sun, 31 Jul 2011 08:33:39 +0100 David Woodhouse wrote:

> On Sat, 2011-07-30 at 21:06 -0400, Arnaud Lacombe wrote:
> > The principle of least surprise is broken anyway as the proposed patch
> > has absolutely no dependency checking and verification. You can `make
> > CONFIG_SATA_MV=y allnoconfig', you will _not_ get it set.
> 
> That's always true in kconfig *anyway*. We've *never* really had an
> option for "do whatever you need to enable this option". We've even
> hard-coded this failure in our language, by introducing this horrible
> 'select' thing to work around it.
> 
> I'd no more expect that, than I would for it to write the code for me if
> I type 'make CONFIG_BTRFSv2=y oldconfig'.
> 
> So no, it doesn't violate the principle of least surprise.
> 
> > ok, the issue is that you will only be allowed to change visible
> > symbols. CONFIG_64BIT is conditionally visible (when ARCH=x86), so
> > right now, you can not do on x86-64:
> > 
> > % make ARCH=i386 defconfig
> > % make CONFIG_64BIT=n oldconfig # [0]
> 
> That works fine here. What was ARCH set to in your second test? If it's
> ARCH=x86_64 then that's expected. That's the whole point of my *other*
> patch to make 'ARCH=x86' be the default, so that the value of
> CONFIG_64BIT in your .config is *not* forcibly overridden to match the
> build host. That's a *separate* bug, which I also have a patch for.

Simple question:  what does "ARCH=x86" mean?

It doesn't mean anything to me without SUBARCH or nnBIT specified.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31 16:37               ` Randy Dunlap
@ 2011-07-31 16:57                 ` David Woodhouse
  2011-07-31 17:08                   ` Randy Dunlap
  0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-07-31 16:57 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Arnaud Lacombe, H. Peter Anvin, linux-kernel, linux-kbuild, Roman Zippel

On Sun, 2011-07-31 at 09:37 -0700, Randy Dunlap wrote:
> Simple question:  what does "ARCH=x86" mean?
> 
> It doesn't mean anything to me without SUBARCH or nnBIT specified.

SUBARCH is meaningless for a native build; it's only for ARCH=um. So I
don't know why that would make anything more meaningful to you.

And why would CONFIG_64BIT make a difference either? Or conversely: why
do CONFIG_PAE, CONFIG_LITTLE_ENDIAN, etc. *not* make a difference to
your understanding?

ARCH=x86 means exactly what it says: "build a kernel for the x86
architecture".

Just like ARCH=mips means "build a kernel for MIPS" and ARCH=sparc means
"build a kernel for SPARC", and ARCH=parisc means "build a kernel for
PARISC", and ARCH=powerpc means "build a kernel for PowerPC". and
ARCH=s390 means "build a kernel for S390".

In *all* of those cases, CONFIG_64BIT is just one more configuration
option; one of *many* that define what actual hardware the kernel
supports.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31 16:57                 ` David Woodhouse
@ 2011-07-31 17:08                   ` Randy Dunlap
  2011-07-31 17:40                     ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Randy Dunlap @ 2011-07-31 17:08 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Arnaud Lacombe, H. Peter Anvin, linux-kernel, linux-kbuild, Roman Zippel

On Sun, 31 Jul 2011 17:57:46 +0100 David Woodhouse wrote:

> On Sun, 2011-07-31 at 09:37 -0700, Randy Dunlap wrote:
> > Simple question:  what does "ARCH=x86" mean?
> > 
> > It doesn't mean anything to me without SUBARCH or nnBIT specified.
> 
> SUBARCH is meaningless for a native build; it's only for ARCH=um. So I
> don't know why that would make anything more meaningful to you.
> 
> And why would CONFIG_64BIT make a difference either? Or conversely: why
> do CONFIG_PAE, CONFIG_LITTLE_ENDIAN, etc. *not* make a difference to
> your understanding?
> 
> ARCH=x86 means exactly what it says: "build a kernel for the x86
> architecture".
> 
> Just like ARCH=mips means "build a kernel for MIPS" and ARCH=sparc means
> "build a kernel for SPARC", and ARCH=parisc means "build a kernel for
> PARISC", and ARCH=powerpc means "build a kernel for PowerPC". and
> ARCH=s390 means "build a kernel for S390".
> 
> In *all* of those cases, CONFIG_64BIT is just one more configuration
> option; one of *many* that define what actual hardware the kernel
> supports.

OK, it seems that we agree that ARCH=x86 is an incomplete specification.
Thanks.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31 17:08                   ` Randy Dunlap
@ 2011-07-31 17:40                     ` David Woodhouse
  0 siblings, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2011-07-31 17:40 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Arnaud Lacombe, H. Peter Anvin, linux-kernel, linux-kbuild, Roman Zippel

On Sun, 2011-07-31 at 10:08 -0700, Randy Dunlap wrote:
> 
> OK, it seems that we agree that ARCH=x86 is an incomplete
> specification.
> Thanks. 

Of course it's incomplete, just as the legacy ARCH=i386 was incomplete.
Even an allnoconfig still had over a hundred lines more configuration
before it was a complete description of what gets built.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31  2:09               ` Arnaud Lacombe
  2011-07-31  5:21                 ` Arnaud Lacombe
@ 2011-07-31 22:18                 ` David Woodhouse
  2011-08-09 15:22                   ` Arnaud Lacombe
  1 sibling, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-07-31 22:18 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: H. Peter Anvin, linux-kernel, linux-kbuild, Roman Zippel

On Sat, 2011-07-30 at 22:09 -0400, Arnaud Lacombe wrote:
> Btw, `make CONFIG_GENERIC_BUG=n oldconfig' or `make CONFIG_64BIT=n
> oldconfig'[0] does not even work, and I'm getting no error. So either
> you make it work for all possible uses, or you warn the user he tried
> something illegal, but you don't just fail silently. 

You're absolutely right that we should report it. I'm dubious about
trying to report a *reason*... it would be nice if we could do it
*reliably*, but we don't actually pass the reasons back up so the code
has to guess. I'm torn between ripping it out because it's guessing, and
trying to improve it. What do you think?

I'd *love* to be able to say 'You need to enable CONFIG_SATA in order to
enable CONFIG_SATA_MV'. But if I were to do *that* then I'd probably end
up ripping out the whole of this 'select' atrocity (which would now have
no excuse for its existence) and I'd be even further from the simple
task I started off with :)

[dwmw2@i7 linux-2.6]$ make CONFIG_GENERIC_BUG=n oldconfig
scripts/kconfig/conf --oldconfig Kconfig
#
# Could not set CONFIG_GENERIC_BUG=n; perhaps it is selected by another option?
#
#
# configuration written to .config
#
[dwmw2@i7 linux-2.6]$ make CONFIG_SATA_MV=y oldconfig
scripts/kconfig/conf --oldconfig Kconfig
#
# Could not set CONFIG_SATA_MV=y; perhaps it has unmet dependencies?
#
#
# configuration written to .config
#


Incremental patch: 

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 0341254..ff25669 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -364,8 +364,22 @@ void conf_set_symbol_from_env(char *str)
 		return;
 
 	sym_calc_value(sym);
-	if (!sym_set_string_value(sym, p))
+	if (!sym_set_string_value(sym, p)) {
+		char *reason;
+
+		if ((sym->type == S_BOOLEAN || sym->type == S_TRISTATE) &&
+			 !p[1] && toupper(p[0]) == 'N')
+			/* We were turning it *off* and failed... selected? */
+			reason = "perhaps it is selected by another option?";
+		else if (!sym->visible)
+			reason = "perhaps it has unmet dependencies?";
+		else
+			reason = "perhaps your setting was invalid?";
+
+		conf_message("Could not set " CONFIG_ "%s=%s; %s",
+			     sym->name, p, reason);
 		return;
+	}
 
 	conf_message(CONFIG_ "%s set to %s from environment", sym->name, p);
 	if (sym_is_choice_value(sym))

-- 
dwmw2


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* Re: [PATCH 1/2] kconfig: Factor out conf_validate_choice_val() function from conf_read_simple()
  2011-07-31  2:17   ` Arnaud Lacombe
@ 2011-07-31 22:21     ` David Woodhouse
  0 siblings, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2011-07-31 22:21 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: H. Peter Anvin, linux-kernel, linux-kbuild

On Sat, 2011-07-30 at 22:17 -0400, Arnaud Lacombe wrote:
> 
> I would go a little further and apply the attached patch too, The two jump
> site to `setsym' do not meet the conditionnal and can be avoided. 

Makes sense. It also makes sense to do that as a separate patch; the one
moving code around should *just* move code around. Want to let me have a
Signed-off-by: for it, and I'll add it to
 git://git.infradead.org/users/dwmw2/x86merge.git

Thanks.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31  0:05       ` Arnaud Lacombe
  2011-07-31  0:29         ` H. Peter Anvin
@ 2011-08-09 14:14         ` Michal Marek
  2011-08-09 15:26           ` Arnaud Lacombe
  1 sibling, 1 reply; 50+ messages in thread
From: Michal Marek @ 2011-08-09 14:14 UTC (permalink / raw)
  To: Arnaud Lacombe
  Cc: H. Peter Anvin, David Woodhouse, linux-kernel, linux-kbuild

On 31.7.2011 02:05, Arnaud Lacombe wrote:
> Hi,
>
> On Sat, Jul 30, 2011 at 7:53 PM, H. Peter Anvin<hpa@zytor.com>  wrote:
>> On 07/30/2011 04:44 PM, Arnaud Lacombe wrote:
>>>
>>> Beside that, the one thing I dislike with your patch is that it is
>>> unconditional and global to all symbols, and you have no way to forbid
>>> the environment to override a value.
>>>
>>
>> Why????
>>
> Because kconfig might not be ran exclusively from a fully controlled
> and restricted environment ? Not to mention that it is used by other
> people than the linux kernel folks.

Well, it has always been possible to trick kbuild (not kconfig) into 
accepting CONFIG_* options from environment, because unset kconfig 
options in auto.conf are not seen by make. Of course this is completely 
fragile, because there is no dependency checking and such variables are 
only seen by make and do not appear in autoconf.h. So a patch that 
teaches kconfig to read options from the environment would actually make 
some (albeit currently "illegal") use cases work correctly :).

Michal

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-07-31 22:18                 ` David Woodhouse
@ 2011-08-09 15:22                   ` Arnaud Lacombe
  0 siblings, 0 replies; 50+ messages in thread
From: Arnaud Lacombe @ 2011-08-09 15:22 UTC (permalink / raw)
  To: David Woodhouse
  Cc: H. Peter Anvin, linux-kernel, linux-kbuild, Roman Zippel,
	Michal Marek, Sam Ravnborg

Hi,

On Sun, Jul 31, 2011 at 6:18 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Sat, 2011-07-30 at 22:09 -0400, Arnaud Lacombe wrote:
>> Btw, `make CONFIG_GENERIC_BUG=n oldconfig' or `make CONFIG_64BIT=n
>> oldconfig'[0] does not even work, and I'm getting no error. So either
>> you make it work for all possible uses, or you warn the user he tried
>> something illegal, but you don't just fail silently.
>
> You're absolutely right that we should report it. I'm dubious about
> trying to report a *reason*... it would be nice if we could do it
> *reliably*, but we don't actually pass the reasons back up so the code
> has to guess. I'm torn between ripping it out because it's guessing, and
> trying to improve it. What do you think?
>
> I'd *love* to be able to say 'You need to enable CONFIG_SATA in order to
> enable CONFIG_SATA_MV'. But if I were to do *that* then I'd probably end
> up ripping out the whole of this 'select' atrocity (which would now have
> no excuse for its existence) and I'd be even further from the simple
> task I started off with :)
>
> [dwmw2@i7 linux-2.6]$ make CONFIG_GENERIC_BUG=n oldconfig
> scripts/kconfig/conf --oldconfig Kconfig
> #
> # Could not set CONFIG_GENERIC_BUG=n; perhaps it is selected by another option?
> #
> #
> # configuration written to .config
> #
> [dwmw2@i7 linux-2.6]$ make CONFIG_SATA_MV=y oldconfig
> scripts/kconfig/conf --oldconfig Kconfig
> #
> # Could not set CONFIG_SATA_MV=y; perhaps it has unmet dependencies?
> #
> #
> # configuration written to .config
> #
>
>
> Incremental patch:
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 0341254..ff25669 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -364,8 +364,22 @@ void conf_set_symbol_from_env(char *str)
>                return;
>
>        sym_calc_value(sym);
> -       if (!sym_set_string_value(sym, p))
> +       if (!sym_set_string_value(sym, p)) {
> +               char *reason;
> +
> +               if ((sym->type == S_BOOLEAN || sym->type == S_TRISTATE) &&
> +                        !p[1] && toupper(p[0]) == 'N')
> +                       /* We were turning it *off* and failed... selected? */
> +                       reason = "perhaps it is selected by another option?";
> +               else if (!sym->visible)
> +                       reason = "perhaps it has unmet dependencies?";
> +               else
> +                       reason = "perhaps your setting was invalid?";
> +
> +               conf_message("Could not set " CONFIG_ "%s=%s; %s",
> +                            sym->name, p, reason);
>                return;
> +       }
>
>        conf_message(CONFIG_ "%s set to %s from environment", sym->name, p);
>        if (sym_is_choice_value(sym))
>
You are still breaking dependencies. Currently every environment
variable creates a dependency in auto.conf.cmd in order to force
`auto.conf' regeneration if the environment change. Your patch lacks
such a safety. cf

commit 93449082e905ce73d0346d617dd67c4b668b58af
Author: Roman Zippel <zippel@linux-m68k.org>
Date:   Mon Jan 14 04:50:54 2008 +0100

    kconfig: environment symbol support

for implementation details about how it is dealt with currently.

Btw, Michal (or Sam), is it voluntary not to make `auto.conf' just
depends on `.config' ?

Thanks,
 - Arnaud

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-09 14:14         ` Michal Marek
@ 2011-08-09 15:26           ` Arnaud Lacombe
  2011-08-10 12:59             ` Michal Marek
  0 siblings, 1 reply; 50+ messages in thread
From: Arnaud Lacombe @ 2011-08-09 15:26 UTC (permalink / raw)
  To: Michal Marek; +Cc: H. Peter Anvin, David Woodhouse, linux-kernel, linux-kbuild

Hi,

On Tue, Aug 9, 2011 at 10:14 AM, Michal Marek <mmarek@suse.cz> wrote:
> On 31.7.2011 02:05, Arnaud Lacombe wrote:
>>
>> Hi,
>>
>> On Sat, Jul 30, 2011 at 7:53 PM, H. Peter Anvin<hpa@zytor.com>  wrote:
>>>
>>> On 07/30/2011 04:44 PM, Arnaud Lacombe wrote:
>>>>
>>>> Beside that, the one thing I dislike with your patch is that it is
>>>> unconditional and global to all symbols, and you have no way to forbid
>>>> the environment to override a value.
>>>>
>>>
>>> Why????
>>>
>> Because kconfig might not be ran exclusively from a fully controlled
>> and restricted environment ? Not to mention that it is used by other
>> people than the linux kernel folks.
>
> Well, it has always been possible to trick kbuild (not kconfig) into
> accepting CONFIG_* options from environment, because unset kconfig options
> in auto.conf are not seen by make. Of course this is completely fragile,
> because there is no dependency checking and such variables are only seen by
> make and do not appear in autoconf.h. So a patch that teaches kconfig to
> read options from the environment would actually make some (albeit currently
> "illegal") use cases work correctly :).
>
kconfig can already set symbol value from the environment. The only
limitation I can see is that it is not optional and require an
explicit environment variable name.

 - Arnaud

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-09 15:26           ` Arnaud Lacombe
@ 2011-08-10 12:59             ` Michal Marek
  2011-08-10 13:07               ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Michal Marek @ 2011-08-10 12:59 UTC (permalink / raw)
  To: Arnaud Lacombe
  Cc: H. Peter Anvin, David Woodhouse, linux-kernel, linux-kbuild

On 9.8.2011 17:26, Arnaud Lacombe wrote:
> On Tue, Aug 9, 2011 at 10:14 AM, Michal Marek <mmarek@suse.cz> wrote:
>> On 31.7.2011 02:05, Arnaud Lacombe wrote:
>>> Because kconfig might not be ran exclusively from a fully controlled
>>> and restricted environment ? Not to mention that it is used by other
>>> people than the linux kernel folks.
>>
>> Well, it has always been possible to trick kbuild (not kconfig) into
>> accepting CONFIG_* options from environment, because unset kconfig options
>> in auto.conf are not seen by make. Of course this is completely fragile,
>> because there is no dependency checking and such variables are only seen by
>> make and do not appear in autoconf.h. So a patch that teaches kconfig to
>> read options from the environment would actually make some (albeit currently
>> "illegal") use cases work correctly :).
>>
> kconfig can already set symbol value from the environment. The only
> limitation I can see is that it is not optional and require an
> explicit environment variable name.

I wasn't talking about the env= syntax, but about

make CONFIG_EXT2_FS=m all
which makes kbuild visit fs/ext2 even if CONFIG_EXT2_FS is disabled in
.config. With no update of the configuration or checking the dependencies.

Hm, actually this would be a problem even if kconfig does read the
CONFIG_* variables from the environment, because it could result in a
mismatch if kconfig determines that the variable cannot be set, but make
still sees it in the environment. So we would have to use 'undefine
CONFIG_FOO' instead of '# CONFIG_FOO is not set' in
include/config/auto.conf, to be able to properly support make CONFIG_FOO=y.

Michal

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 12:59             ` Michal Marek
@ 2011-08-10 13:07               ` David Woodhouse
  2011-08-10 14:15                 ` Arnaud Lacombe
  0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-08-10 13:07 UTC (permalink / raw)
  To: Michal Marek; +Cc: Arnaud Lacombe, H. Peter Anvin, linux-kernel, linux-kbuild

On Wed, 2011-08-10 at 14:59 +0200, Michal Marek wrote:
> I wasn't talking about the env= syntax, but about
> 
> make CONFIG_EXT2_FS=m all
> which makes kbuild visit fs/ext2 even if CONFIG_EXT2_FS is disabled in
> .config. With no update of the configuration or checking the dependencies.
> 
> Hm, actually this would be a problem even if kconfig does read the
> CONFIG_* variables from the environment, because it could result in a
> mismatch if kconfig determines that the variable cannot be set, but make
> still sees it in the environment. So we would have to use 'undefine
> CONFIG_FOO' instead of '# CONFIG_FOO is not set' in
> include/config/auto.conf, to be able to properly support make CONFIG_FOO=y. 

That's always been true; it's *always* been broken like that.

But now we're actually *encouraging* people to start setting
CONFIG_FOO=y in the environment or the make command line, so we should
certainly make it more robust.

It should be simple enough to force a reconfig if CONFIG_* is specified
in the environment.

Actually, while we're at it let's stop just picking it up from the
environment and pick it up *only* if it's overridden in the make flags.

Something like this will list the variables which are overridden on the
command line:

CONFIG_OVERRIDES := $(patsubst line:%,%,$(filter line:%,$(foreach var, $(filter CONFIG_%,$(.VARIABLES)), $(origin $(var)):$(var))))

Then we can make auto.conf.cmd trigger a reconfig if it's non-empty, and
make the kconfig tool allow overrides *only* for those variables
specified in $CONFIG_OVERRIDES... which avoids any worries about "stray"
environment variables too.

I'll see if I can clean the above expression up and do that.
 
-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 13:07               ` David Woodhouse
@ 2011-08-10 14:15                 ` Arnaud Lacombe
  2011-08-10 14:17                   ` David Woodhouse
  2011-08-10 17:01                   ` Randy Dunlap
  0 siblings, 2 replies; 50+ messages in thread
From: Arnaud Lacombe @ 2011-08-10 14:15 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

Hi,

On Wed, Aug 10, 2011 at 9:07 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2011-08-10 at 14:59 +0200, Michal Marek wrote:
>> I wasn't talking about the env= syntax, but about
>>
>> make CONFIG_EXT2_FS=m all
>> which makes kbuild visit fs/ext2 even if CONFIG_EXT2_FS is disabled in
>> .config. With no update of the configuration or checking the dependencies.
>>
>> Hm, actually this would be a problem even if kconfig does read the
>> CONFIG_* variables from the environment, because it could result in a
>> mismatch if kconfig determines that the variable cannot be set, but make
>> still sees it in the environment. So we would have to use 'undefine
>> CONFIG_FOO' instead of '# CONFIG_FOO is not set' in
>> include/config/auto.conf, to be able to properly support make CONFIG_FOO=y.
>
> That's always been true; it's *always* been broken like that.
>
> But now we're actually *encouraging* people to start setting
> CONFIG_FOO=y in the environment or the make command line, so we should
> certainly make it more robust.
>
who is "we" ?

> It should be simple enough to force a reconfig if CONFIG_* is specified
> in the environment.
>
which is broken, the complete kernel Kconfig tree and all
inter-dependency cannot be fully understood, especially by
non-developer. So what you ask for is either not doing what the user
wants, but respecting dependency, or doing what the user want, but not
respecting dependency, and by the same, creating illegal
configuration.

> Actually, while we're at it let's stop just picking it up from the
> environment and pick it up *only* if it's overridden in the make flags.
>
again, who's "we" ?

> Something like this will list the variables which are overridden on the
> command line:
>
> CONFIG_OVERRIDES := $(patsubst line:%,%,$(filter line:%,$(foreach var, $(filter CONFIG_%,$(.VARIABLES)), $(origin $(var)):$(var))))
>
> Then we can make auto.conf.cmd trigger a reconfig if it's non-empty, and
> make the kconfig tool allow overrides *only* for those variables
> specified in $CONFIG_OVERRIDES... which avoids any worries about "stray"
> environment variables too.
>
> I'll see if I can clean the above expression up and do that.
>
hum, let's take a real life example: user foo wants his config to
enable CONFIG_WIRELESS_EXT, so with what you propose, he would do:

# make CONFIG_WIRELESS_EXT=y allmodconfig

currently, this would _never_ work, unless one of the symbol selected
by `allmodconfig' selects it, as WIRELESS_EXT is defined the
following:

config WIRELESS_EXT
    bool

I suspect there also an implicit dependency to WIRELESS.

Now, you cannot just force WIRELESS_EXT to 'y', as there would be room
for illegal configuration, or it might just never be built. Moreover,
even if you did that, you would not just have to toggle the value, but
to act as the 'select' do, ie. create a reverse dependency. If that
was working, I would expect that you could do the opposite, that is:

# make CONFIG_WIRELESS_EXT=n allyesconfig

but again, behind implementation details, that might create an illegal
configuration. There is a reason wireless chip select that symbol,
that reason is only known by the wireless folks, so I do not see why
the user should be allowed to go against. That said, if you want to
hack around that, just edit net/wireless/Kconfig, that will be faster
than trying to understand the whole thing.

Btw, warning about potential missing dependencies are useless[0],
user, even kernel hacker do _not_ read them. We spent a few day
tracking down a build failure on powerpc triggered because SND_ISA was
selected without ISA_DMA_API, a warning was present, but nobody cared
about it.

 - Arnaud

> --
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@intel.com                              Intel Corporation
>
>

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 14:15                 ` Arnaud Lacombe
@ 2011-08-10 14:17                   ` David Woodhouse
  2011-08-10 14:34                     ` Arnaud Lacombe
  2011-08-10 17:01                   ` Randy Dunlap
  1 sibling, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-08-10 14:17 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

On Wed, 2011-08-10 at 10:15 -0400, Arnaud Lacombe wrote:
> 
> hum, let's take a real life example: user foo wants his config to
> enable CONFIG_WIRELESS_EXT, so with what you propose, he would do:
> 
> # make CONFIG_WIRELESS_EXT=y allmodconfig
> 
> currently, this would _never_ work, unless one of the symbol selected
> by `allmodconfig' selects it, as WIRELESS_EXT is defined the
> following:
> 
> config WIRELESS_EXT
>     bool
> 
> I suspect there also an implicit dependency to WIRELESS. 

This is a complete red herring. It's *always* been like that, and works
*just* like this for the 'all.config' file etc.

If you have nothing relevant to say, just don't say anything.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 14:17                   ` David Woodhouse
@ 2011-08-10 14:34                     ` Arnaud Lacombe
  2011-08-10 16:33                       ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Arnaud Lacombe @ 2011-08-10 14:34 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

On Wed, Aug 10, 2011 at 10:17 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2011-08-10 at 10:15 -0400, Arnaud Lacombe wrote:
>>
>> hum, let's take a real life example: user foo wants his config to
>> enable CONFIG_WIRELESS_EXT, so with what you propose, he would do:
>>
>> # make CONFIG_WIRELESS_EXT=y allmodconfig
>>
>> currently, this would _never_ work, unless one of the symbol selected
>> by `allmodconfig' selects it, as WIRELESS_EXT is defined the
>> following:
>>
>> config WIRELESS_EXT
>>     bool
>>
>> I suspect there also an implicit dependency to WIRELESS.
>
> This is a complete red herring. It's *always* been like that, and works
> *just* like this for the 'all.config' file etc.
>
your point being ? I might as well tell you that I find the current
behavior of 'all*.config' just as broken wrt. to dependency
management.

> If you have nothing relevant to say, just don't say anything.
>
maybe you can come with a detailed description of your proposal's
behavior, including how to manage case like this, instead of just
throwing patch around ?

If I do:

# make CONFIG_WIRELESS_EXT=y allnoconfig

I expect either a success or an error, not a silent discard. And
*yes*, the problem already exists with "all*.config".

Thanks,
 - Arnaud

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 14:34                     ` Arnaud Lacombe
@ 2011-08-10 16:33                       ` David Woodhouse
  2011-08-10 17:00                         ` Emmanuel Deloget
  2011-08-10 17:44                         ` Arnaud Lacombe
  0 siblings, 2 replies; 50+ messages in thread
From: David Woodhouse @ 2011-08-10 16:33 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

On Wed, 2011-08-10 at 10:34 -0400, Arnaud Lacombe wrote:
> your point being ? I might as well tell you that I find the current
> behavior of 'all*.config' just as broken wrt. to dependency
> management.

You might indeed. And I would find that commentary just as irrelevant
and unhelpful.

> > If you have nothing relevant to say, just don't say anything.
> >
> maybe you can come with a detailed description of your proposal's
> behavior, including how to manage case like this, instead of just
> throwing patch around ?

How's this for a definition:

"The behaviour for unsettable (or unclearable) options shall be exactly
like it already is if you put them in all*.config, or if you manually
edit the .config file and run 'make oldconfig', as people have been
doing for years. There is nothing new to see here."
 
> If I do:
> 
> # make CONFIG_WIRELESS_EXT=y allnoconfig
> 
> I expect either a success or an error, not a silent discard. And
> *yes*, the problem already exists with "all*.config".

[dwmw2@i7 linux-2.6]$ make CONFIG_WIRELESS_EXT=y allnoconfig
scripts/kconfig/conf --allnoconfig Kconfig
#
# Could not set CONFIG_WIRELESS_EXT=y; perhaps it has unmet dependencies?
#
#
# configuration written to .config
#

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 16:33                       ` David Woodhouse
@ 2011-08-10 17:00                         ` Emmanuel Deloget
  2011-08-10 17:52                           ` H. Peter Anvin
  2011-08-10 17:44                         ` Arnaud Lacombe
  1 sibling, 1 reply; 50+ messages in thread
From: Emmanuel Deloget @ 2011-08-10 17:00 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Arnaud Lacombe, Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

On 08/10/2011 06:33 PM, David Woodhouse wrote:
> On Wed, 2011-08-10 at 10:34 -0400, Arnaud Lacombe wrote:
>> your point being ? I might as well tell you that I find the current
>> behavior of 'all*.config' just as broken wrt. to dependency
>> management.
> You might indeed. And I would find that commentary just as irrelevant
> and unhelpful.
>
>>> If you have nothing relevant to say, just don't say anything.
>>>
>> maybe you can come with a detailed description of your proposal's
>> behavior, including how to manage case like this, instead of just
>> throwing patch around ?
> How's this for a definition:
>
> "The behaviour for unsettable (or unclearable) options shall be exactly
> like it already is if you put them in all*.config, or if you manually
> edit the .config file and run 'make oldconfig', as people have been
> doing for years. There is nothing new to see here."
>
>> If I do:
>>
>> # make CONFIG_WIRELESS_EXT=y allnoconfig
>>
>> I expect either a success or an error, not a silent discard. And
>> *yes*, the problem already exists with "all*.config".
> [dwmw2@i7 linux-2.6]$ make CONFIG_WIRELESS_EXT=y allnoconfig
> scripts/kconfig/conf --allnoconfig Kconfig
> #
> # Could not set CONFIG_WIRELESS_EXT=y; perhaps it has unmet dependencies?
> #
> #
> # configuration written to .config
> #
>

I understand that my question is indeed neither wanted nor clever, but 
what's the point of trying to support "make CONFIG_FOO=y"?

Will we be expected to type a 42 meters long command line to compile the 
kernel instead of doing a menuconfig in the foreseable future? (and 
between typos, unmet dependencies and the myriad of other possible 
errors, I'm not sure I'll get more free time).

I don't get it. If the goal is to help the kernel hackers and if it 
really helps them it might be a thing to do - it might prove useful for 
very simple CONFIG_ options but I'm not sure this will stay true for the 
general case.

Best regards,

-- Emmanuel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 14:15                 ` Arnaud Lacombe
  2011-08-10 14:17                   ` David Woodhouse
@ 2011-08-10 17:01                   ` Randy Dunlap
  2011-08-10 17:25                     ` David Woodhouse
  1 sibling, 1 reply; 50+ messages in thread
From: Randy Dunlap @ 2011-08-10 17:01 UTC (permalink / raw)
  To: Arnaud Lacombe
  Cc: David Woodhouse, Michal Marek, H. Peter Anvin, linux-kernel,
	linux-kbuild

On Wed, 10 Aug 2011 10:15:55 -0400 Arnaud Lacombe wrote:

> Btw, warning about potential missing dependencies are useless[0],
> user, even kernel hacker do _not_ read them. We spent a few day
> tracking down a build failure on powerpc triggered because SND_ISA was
> selected without ISA_DMA_API, a warning was present, but nobody cared
> about it.

That one was an anomaly.  I usually see them and often send patches
for them.  But granted, most developers just pass over them.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 17:01                   ` Randy Dunlap
@ 2011-08-10 17:25                     ` David Woodhouse
  0 siblings, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2011-08-10 17:25 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Arnaud Lacombe, Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

On Wed, 2011-08-10 at 10:01 -0700, Randy Dunlap wrote:
> On Wed, 10 Aug 2011 10:15:55 -0400 Arnaud Lacombe wrote:
> 
> > Btw, warning about potential missing dependencies are useless[0],
> > user, even kernel hacker do _not_ read them. We spent a few day
> > tracking down a build failure on powerpc triggered because SND_ISA was
> > selected without ISA_DMA_API, a warning was present, but nobody cared
> > about it.
> 
> That one was an anomaly.  I usually see them and often send patches
> for them.  But granted, most developers just pass over them. 

I see that kind of thing as a natural consequence of the whole 'select'
abomination. But that's even *further* off-topic from the original topic
of this thread than Arnaud has already dragged us.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 16:33                       ` David Woodhouse
  2011-08-10 17:00                         ` Emmanuel Deloget
@ 2011-08-10 17:44                         ` Arnaud Lacombe
  2011-08-10 17:54                           ` H. Peter Anvin
  2011-08-10 17:59                           ` David Woodhouse
  1 sibling, 2 replies; 50+ messages in thread
From: Arnaud Lacombe @ 2011-08-10 17:44 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

Hi,

On Wed, Aug 10, 2011 at 12:33 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2011-08-10 at 10:34 -0400, Arnaud Lacombe wrote:
>> your point being ? I might as well tell you that I find the current
>> behavior of 'all*.config' just as broken wrt. to dependency
>> management.
>
> You might indeed. And I would find that commentary just as irrelevant
> and unhelpful.
>
you are free to do so.

>> > If you have nothing relevant to say, just don't say anything.
>> >
>> maybe you can come with a detailed description of your proposal's
>> behavior, including how to manage case like this, instead of just
>> throwing patch around ?
>
> How's this for a definition:
>
> "The behaviour for unsettable (or unclearable) options shall be exactly
> like it already is if you put them in all*.config, or if you manually
> edit the .config file and run 'make oldconfig', as people have been
> doing for years. There is nothing new to see here."
>
Then I would say it is plain broken, especially if widespread.

>> If I do:
>>
>> # make CONFIG_WIRELESS_EXT=y allnoconfig
>>
>> I expect either a success or an error, not a silent discard. And
>> *yes*, the problem already exists with "all*.config".
>
> [dwmw2@i7 linux-2.6]$ make CONFIG_WIRELESS_EXT=y allnoconfig
> scripts/kconfig/conf --allnoconfig Kconfig
> #
> # Could not set CONFIG_WIRELESS_EXT=y; perhaps it has unmet dependencies?
> #
> #
> # configuration written to .config
> #
Exactly my point, you just successfully created an half-backed config
which is different than what Aunt Tillie wanted you to generate. This
should be an hard error, same for "all*.config", not to mention that
the error message is far from being helpful.

 - Arnaud

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 17:00                         ` Emmanuel Deloget
@ 2011-08-10 17:52                           ` H. Peter Anvin
  0 siblings, 0 replies; 50+ messages in thread
From: H. Peter Anvin @ 2011-08-10 17:52 UTC (permalink / raw)
  To: emmanuel.deloget
  Cc: David Woodhouse, Arnaud Lacombe, Michal Marek, linux-kernel,
	linux-kbuild

On 08/10/2011 12:00 PM, Emmanuel Deloget wrote:
> 
> I understand that my question is indeed neither wanted nor clever, but 
> what's the point of trying to support "make CONFIG_FOO=y"?
> 
> Will we be expected to type a 42 meters long command line to compile the 
> kernel instead of doing a menuconfig in the foreseable future? (and 
> between typos, unmet dependencies and the myriad of other possible 
> errors, I'm not sure I'll get more free time).
> 
> I don't get it. If the goal is to help the kernel hackers and if it 
> really helps them it might be a thing to do - it might prove useful for 
> very simple CONFIG_ options but I'm not sure this will stay true for the 
> general case.
> 

That's useful for some cases, though, instead of having to create a
file; the driving cases here are architecture, platform or 32/64-bit
selection.  For longer ones you'll create a file.

And no, menuconfig and .config files will not go away.

	-hpa

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 17:44                         ` Arnaud Lacombe
@ 2011-08-10 17:54                           ` H. Peter Anvin
  2011-08-10 17:59                           ` David Woodhouse
  1 sibling, 0 replies; 50+ messages in thread
From: H. Peter Anvin @ 2011-08-10 17:54 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: David Woodhouse, Michal Marek, linux-kernel, linux-kbuild

On 08/10/2011 12:44 PM, Arnaud Lacombe wrote:
> Exactly my point, you just successfully created an half-backed config
> which is different than what Aunt Tillie wanted you to generate. This
> should be an hard error, same for "all*.config", not to mention that
> the error message is far from being helpful.

Arnaud,

This is arguably a bug in how config fragments are injected via any
mechanism, but that is completely orthogonal to us wanting another
injection mechanism.

	-hpa


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 17:44                         ` Arnaud Lacombe
  2011-08-10 17:54                           ` H. Peter Anvin
@ 2011-08-10 17:59                           ` David Woodhouse
  2011-08-10 18:40                             ` Arnaud Lacombe
  1 sibling, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-08-10 17:59 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

On Wed, 2011-08-10 at 13:44 -0400, Arnaud Lacombe wrote:
> Exactly my point, you just successfully created an half-backed config
> which is different than what Aunt Tillie wanted you to generate. This
> should be an hard error, same for "all*.config", not to mention that
> the error message is far from being helpful.

You are whining about something that has been true of the kernel config
system for at least the last 16 years that I've been working on it,

You're *right*, of course, but you're getting on my tits by whining
about it only *now*, in this context. At least I have offered *an* error
message reporting that the request was not honoured, which is a whole
lot better that we've been used to.

Please, if this offends you then by all means go and fix it. A sane way
of handling dependencies would give a way to say "do what you need to do
in order to enable CONFIG_SATA_MV", and should remove the abomination of
'select', which was introduced purely to work around that lack.

But none of that is directly relevant in *this* thread.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 17:59                           ` David Woodhouse
@ 2011-08-10 18:40                             ` Arnaud Lacombe
  2011-08-10 18:52                               ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Arnaud Lacombe @ 2011-08-10 18:40 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

Hi,

On Wed, Aug 10, 2011 at 1:59 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2011-08-10 at 13:44 -0400, Arnaud Lacombe wrote:
>> Exactly my point, you just successfully created an half-backed config
>> which is different than what Aunt Tillie wanted you to generate. This
>> should be an hard error, same for "all*.config", not to mention that
>> the error message is far from being helpful.
>
> You are whining about something that has been true of the kernel config
> system for at least the last 16 years that I've been working on it,
>
s/16/6/; the all.config logic has been added in:

commit 90389160efc2864501ced6e662f9419eb7a3e6c8
Author: Roman Zippel <zippel@linux-m68k.org>
Date:   Tue Nov 8 21:34:49 2005 -0800

    [PATCH] kconfig: preset config during all*config

so, at best, this buggy behavior is ~ 6 years old. Before that, I'd
assume that the internal namespace was not accessible by any other
mean than the front-ends.

> You're *right*, of course, but you're getting on my tits by whining
> about it only *now*, in this context.
well... sorry for your tits, 'hope you're enjoying it ;-)

> At least I have offered *an* error
> message reporting that the request was not honoured, which is a whole
> lot better that we've been used to.
>
s/error/warning/, but indeed, that's a step forward.

> Please, if this offends you then by all means go and fix it. A sane way
> of handling dependencies would give a way to say "do what you need to do
> in order to enable CONFIG_SATA_MV", and should remove the abomination of
> 'select', which was introduced purely to work around that lack.
>
> But none of that is directly relevant in *this* thread.
>
to paraphrase you, I'd say, this might looks "cute but might give
behavior that people will come to depend on in their scripts and then
we take it away again", "that's why I'd kind of like to see it done
*once*, *properly*".

 - Arnaud

> --
> dwmw2
>
>

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 18:40                             ` Arnaud Lacombe
@ 2011-08-10 18:52                               ` David Woodhouse
  2011-08-10 22:33                                 ` Arnaud Lacombe
  0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-08-10 18:52 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

On Wed, 2011-08-10 at 14:40 -0400, Arnaud Lacombe wrote:
> so, at best, this buggy behavior is ~ 6 years old. Before that, I'd
> assume that the internal namespace was not accessible by any other
> mean than the front-ends.

I've been editing .config or doing 's/.*CONFIG_FOO[= ].*/CONFIG_FOO=y/'
on it for a decade before that. With all the same limitations as
all.config, and my new CONFIG_FOO=y command line support.

How do *you* quickly, from the command line, enable or disable a single
option in an existing config?

> > Please, if this offends you then by all means go and fix it. A sane way
> > of handling dependencies would give a way to say "do what you need to do
> > in order to enable CONFIG_SATA_MV", and should remove the abomination of
> > 'select', which was introduced purely to work around that lack.
> >
> > But none of that is directly relevant in *this* thread.
> >
> to paraphrase you, I'd say, this might looks "cute but might give
> behavior that people will come to depend on in their scripts and then
> we take it away again", "that's why I'd kind of like to see it done
> *once*, *properly*".

That's a reasonable concern, but I think it's misplaced in this case.
We're not enabling anything that we're later going to break. I can't see
many people *depending* on the fact that 'make CONFIG_SATA_MV=y
oldconfig' actually does *nothing* in some cases.

When we later, hopefully, get proper dependency resolution, that will
take something that *wasn't* working and make it work. For all.config
and for the command line overrides (and in other places) at the same
time.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 18:52                               ` David Woodhouse
@ 2011-08-10 22:33                                 ` Arnaud Lacombe
  2011-08-10 23:16                                   ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Arnaud Lacombe @ 2011-08-10 22:33 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

Hi,

On Wed, Aug 10, 2011 at 2:52 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2011-08-10 at 14:40 -0400, Arnaud Lacombe wrote:
>> so, at best, this buggy behavior is ~ 6 years old. Before that, I'd
>> assume that the internal namespace was not accessible by any other
>> mean than the front-ends.
>
> I've been editing .config or doing 's/.*CONFIG_FOO[= ].*/CONFIG_FOO=y/'
> on it for a decade before that. With all the same limitations as
> all.config, and my new CONFIG_FOO=y command line support.
>
> How do *you* quickly, from the command line, enable or disable a single
> option in an existing config?
>
>> > Please, if this offends you then by all means go and fix it. A sane way
>> > of handling dependencies would give a way to say "do what you need to do
>> > in order to enable CONFIG_SATA_MV", and should remove the abomination of
>> > 'select', which was introduced purely to work around that lack.
>> >
>> > But none of that is directly relevant in *this* thread.
>> >
>> to paraphrase you, I'd say, this might looks "cute but might give
>> behavior that people will come to depend on in their scripts and then
>> we take it away again", "that's why I'd kind of like to see it done
>> *once*, *properly*".
>
> That's a reasonable concern, but I think it's misplaced in this case.
> We're not enabling anything that we're later going to break. I can't see
> many people *depending* on the fact that 'make CONFIG_SATA_MV=y
> oldconfig' actually does *nothing* in some cases.
>
you are wrong, you ends up with half-baked compile-time dependency,
which break the build:

% make allnoconfig drivers/ata/

do nothing, but works.

% make CONFIG_SATA_MV=y allnoconfig drivers/ata/

tries to build `drivers/ata/sata_mv.o' and miserably fails.

[tested on top of your patches]

 - Arnaud

> When we later, hopefully, get proper dependency resolution, that will
> take something that *wasn't* working and make it work. For all.config
> and for the command line overrides (and in other places) at the same
> time.
>
> --
> dwmw2
>
>

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 22:33                                 ` Arnaud Lacombe
@ 2011-08-10 23:16                                   ` David Woodhouse
  2011-08-11  3:29                                     ` Arnaud Lacombe
  0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-08-10 23:16 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

On Wed, 2011-08-10 at 18:33 -0400, Arnaud Lacombe wrote:
> > We're not enabling anything that we're later going to break. I can't see
> > many people *depending* on the fact that 'make CONFIG_SATA_MV=y
> > oldconfig' actually does *nothing* in some cases.
> >
> you are wrong, you ends up with half-baked compile-time dependency,
> which break the build:

s/*nothing*/nothing useful/, for crying out loud.

The point remains that we're not enabling anything which we're later
going to break. Nobody is going to come to *depend* on this behaviour.

And anyway, this behaviour exists even *before* my patches, as Michal
pointed out in a far more helpful and constructive fashion in 
<CACqU3MU4wJb3ij6skod-ZiM+Q0OMTXNdbJ+qWjJW8VZNEP+x1g@mail.gmail.com>
earlier today.

It's simple enough to fix, too:

diff --git a/Makefile b/Makefile
index 1fc5172..6cc7f7b 100644
--- a/Makefile
+++ b/Makefile
@@ -474,6 +474,9 @@ ifeq ($(KBUILD_EXTMOD),)
         endif
 endif
 
+# This could probably be simpler?
+CONFIG_OVERRIDES := $(patsubst line:%,%,$(filter line:%,$(foreach var, $(filter CONFIG_%,$(.VARIABLES)), $(origin $(var)):$(var))))
+
 ifeq ($(mixed-targets),1)
 # ===========================================================================
 # We're called with mixed targets (*config and build targets).
@@ -507,6 +510,10 @@ else
 # Build targets only - this includes vmlinux, arch specific targets, clean
 # targets and others. In general all targets except *config targets.
 
+ifneq ($(CONFIG_OVERRIDES),)
+$(error Cannot perform build targets with CONFIG symbols overridden)
+endif
+
 ifeq ($(KBUILD_EXTMOD),)
 # Additional helpers built in scripts/
 # Carefully list dependencies so we do not try to build scripts twice


-- 
dwmw2


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-10 23:16                                   ` David Woodhouse
@ 2011-08-11  3:29                                     ` Arnaud Lacombe
  2011-08-11  8:42                                       ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Arnaud Lacombe @ 2011-08-11  3:29 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

Hi,

On Wed, Aug 10, 2011 at 7:16 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2011-08-10 at 18:33 -0400, Arnaud Lacombe wrote:
>> > We're not enabling anything that we're later going to break. I can't see
>> > many people *depending* on the fact that 'make CONFIG_SATA_MV=y
>> > oldconfig' actually does *nothing* in some cases.
>> >
>> you are wrong, you ends up with half-baked compile-time dependency,
>> which break the build:
>
> s/*nothing*/nothing useful/, for crying out loud.
>
> The point remains that we're not enabling anything which we're later
> going to break. Nobody is going to come to *depend* on this behaviour.
>
> And anyway, this behaviour exists even *before* my patches, as Michal
> pointed out in a far more helpful and constructive fashion in
> <CACqU3MU4wJb3ij6skod-ZiM+Q0OMTXNdbJ+qWjJW8VZNEP+x1g@mail.gmail.com>
> earlier today.
>
> It's simple enough to fix, too:
>
> diff --git a/Makefile b/Makefile
> index 1fc5172..6cc7f7b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -474,6 +474,9 @@ ifeq ($(KBUILD_EXTMOD),)
>         endif
>  endif
>
> +# This could probably be simpler?
> +CONFIG_OVERRIDES := $(patsubst line:%,%,$(filter line:%,$(foreach var, $(filter CONFIG_%,$(.VARIABLES)), $(origin $(var)):$(var))))
> +
>  ifeq ($(mixed-targets),1)
>  # ===========================================================================
>  # We're called with mixed targets (*config and build targets).
> @@ -507,6 +510,10 @@ else
>  # Build targets only - this includes vmlinux, arch specific targets, clean
>  # targets and others. In general all targets except *config targets.
>
> +ifneq ($(CONFIG_OVERRIDES),)
> +$(error Cannot perform build targets with CONFIG symbols overridden)
> +endif
> +
>  ifeq ($(KBUILD_EXTMOD),)
>  # Additional helpers built in scripts/
>  # Carefully list dependencies so we do not try to build scripts twice
>
how is that supposed to work with your other patches ?

% vi mail.txt
[strip all lines until 'diff --git a/Makefile b/Makefile']
% git apply mail.txt
% make CONFIG_NET=y allyesconfig drivers/ata/
[...]
scripts/kconfig/conf --allyesconfig Kconfig
#
# CONFIG_NET set to y from environment
#
#
# configuration written to .config
#
Makefile:504: *** Cannot perform build targets with CONFIG symbols
overridden.  Stop.
make: *** [prepare] Error 2

 - Arnaud

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11  3:29                                     ` Arnaud Lacombe
@ 2011-08-11  8:42                                       ` David Woodhouse
  2011-08-11  8:58                                         ` Michal Marek
  0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-08-11  8:42 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: Michal Marek, H. Peter Anvin, linux-kernel, linux-kbuild

On Wed, 2011-08-10 at 23:29 -0400, Arnaud Lacombe wrote:
> how is that supposed to work with your other patches ?

You cannot perform build targets with CONFIG symbols overridden.

> % make CONFIG_NET=y allyesconfig drivers/ata/ 
        ^^^^^^^ override           ^^^^^^^^^^ build target

> Makefile:504: *** Cannot perform build targets with CONFIG symbols
> overridden.  Stop.

 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error message which
told you that, if you'd been paying the slightest bit of attention
rather than just making pointless noise.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11  8:42                                       ` David Woodhouse
@ 2011-08-11  8:58                                         ` Michal Marek
  2011-08-11 11:10                                           ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Michal Marek @ 2011-08-11  8:58 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Arnaud Lacombe, H. Peter Anvin, linux-kernel, linux-kbuild

On 11.8.2011 10:42, David Woodhouse wrote:
> On Wed, 2011-08-10 at 23:29 -0400, Arnaud Lacombe wrote:
>> how is that supposed to work with your other patches ?
> 
> You cannot perform build targets with CONFIG symbols overridden.
> 
>> % make CONFIG_NET=y allyesconfig drivers/ata/ 
>         ^^^^^^^ override           ^^^^^^^^^^ build target
> 
>> Makefile:504: *** Cannot perform build targets with CONFIG symbols
>> overridden.  Stop.
> 
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error message which
> told you that, if you'd been paying the slightest bit of attention
> rather than just making pointless noise.

I would much rather see include/config/auto.conf reset all configuration
variables to match what kconfig computed. I.e. use 'undefine CONFIG_FOO'
instead of '# CONFIG_FOO is not set'. Additionally print this for any
CONFIG_* variable found in the environment, even if the symbol is not
visible in the current configuration. The include/config/auto.conf file
is only used internally, so it should be safe to change the format.

Michal

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11  8:58                                         ` Michal Marek
@ 2011-08-11 11:10                                           ` David Woodhouse
  2011-08-11 11:15                                             ` Andreas Schwab
  0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-08-11 11:10 UTC (permalink / raw)
  To: Michal Marek; +Cc: Arnaud Lacombe, H. Peter Anvin, linux-kernel, linux-kbuild

On Thu, 2011-08-11 at 10:58 +0200, Michal Marek wrote:
> I would much rather see include/config/auto.conf reset all configuration
> variables to match what kconfig computed. I.e. use 'undefine CONFIG_FOO'
> instead of '# CONFIG_FOO is not set'. 

That would be cute, but I'm not sure how to undefine something set on
the command line:

$ cat > Makefile <<EOF
undefine BAR

foo:
	echo $(BAR)
EOF
$ make foo BAR=hh
echo hh
hh

Arguably, if someone *does* try something like Arnaud's 
 "make CONFIG_FOO_BAR=y oldconfig bzImage"
.. and it *wasn't* able to set CONFIG_FOO_BAR, then the nicest behaviour
would be to fail, rather than to attempt to build it.

So perhaps we should clean up only those settings inherited from the
environment, and still (as in the patch I sent earlier) refuse to allow
build targets in conjunction with CONFIG overrides on the command line?

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11 11:10                                           ` David Woodhouse
@ 2011-08-11 11:15                                             ` Andreas Schwab
  2011-08-11 11:40                                               ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Andreas Schwab @ 2011-08-11 11:15 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Michal Marek, Arnaud Lacombe, H. Peter Anvin, linux-kernel, linux-kbuild

David Woodhouse <dwmw2@infradead.org> writes:

> That would be cute, but I'm not sure how to undefine something set on
> the command line:
>
> $ cat > Makefile <<EOF
> undefine BAR

override undefine BAR

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11 11:15                                             ` Andreas Schwab
@ 2011-08-11 11:40                                               ` David Woodhouse
  2011-08-11 11:56                                                 ` Michal Marek
  0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-08-11 11:40 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Michal Marek, Arnaud Lacombe, H. Peter Anvin, linux-kernel, linux-kbuild

On Thu, 2011-08-11 at 13:15 +0200, Andreas Schwab wrote:
> > That would be cute, but I'm not sure how to undefine something set on
> > the command line:
> >
> > $ cat > Makefile <<EOF
> > undefine BAR
> 
> override undefine BAR

Thanks. So we don't really need to change the auto.conf syntax; we could
just do:

$(foreach var, $(filter CONFIG_%,$(.VARIABLES)), $(eval override undefine $(var)))

But still I suspect we might *not* want that for options set on the
command line. We *don't* want 'make CONFIG_FOO=y oldconfig bzImage' to
say 'you can't enable CONFIG_FOO' and then build the bzImage anyway.

I'll look at making it work only if what's on the command line matches
the output of kconfig, but I'm also relatively content with saying "no
config overrides on the command line for build targets"

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11 11:40                                               ` David Woodhouse
@ 2011-08-11 11:56                                                 ` Michal Marek
  2011-08-11 13:20                                                   ` David Woodhouse
  2011-08-11 14:57                                                   ` Arnaud Lacombe
  0 siblings, 2 replies; 50+ messages in thread
From: Michal Marek @ 2011-08-11 11:56 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Andreas Schwab, Arnaud Lacombe, H. Peter Anvin, linux-kernel,
	linux-kbuild

On 11.8.2011 13:40, David Woodhouse wrote:
> On Thu, 2011-08-11 at 13:15 +0200, Andreas Schwab wrote:
>>> That would be cute, but I'm not sure how to undefine something set on
>>> the command line:
>>>
>>> $ cat > Makefile <<EOF
>>> undefine BAR
>>
>> override undefine BAR
> 
> Thanks. So we don't really need to change the auto.conf syntax; we could
> just do:
> 
> $(foreach var, $(filter CONFIG_%,$(.VARIABLES)), $(eval override undefine $(var)))
> 
> But still I suspect we might *not* want that for options set on the
> command line. We *don't* want 'make CONFIG_FOO=y oldconfig bzImage' to
> say 'you can't enable CONFIG_FOO' and then build the bzImage anyway.

If you do
$ echo 'CONFIG_FOO=y' >all.config
$ make allnoconfig bzImage
then it will also build bzImage even if kconfig wasn't able to set
CONFIG_FOO=y. IMO printing a warning that CONFIG_FOO could not be set is
sufficient, as long as CONFIG_FOO is consistently unset.

Michal

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11 11:56                                                 ` Michal Marek
@ 2011-08-11 13:20                                                   ` David Woodhouse
  2011-08-11 14:57                                                   ` Arnaud Lacombe
  1 sibling, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2011-08-11 13:20 UTC (permalink / raw)
  To: Michal Marek
  Cc: Andreas Schwab, Arnaud Lacombe, H. Peter Anvin, linux-kernel,
	linux-kbuild

On Thu, 2011-08-11 at 13:56 +0200, Michal Marek wrote:
> 
> If you do
> $ echo 'CONFIG_FOO=y' >all.config
> $ make allnoconfig bzImage
> then it will also build bzImage even if kconfig wasn't able to set
> CONFIG_FOO=y. IMO printing a warning that CONFIG_FOO could not be set is
> sufficient, as long as CONFIG_FOO is consistently unset. 

The thing is, such a warning is likely to be lost in the scrollback.

There's definitely something to be said for:

[dwmw2@i7 linux-2.6]$ make CONFIG_SATA_MV=y bzImage
Makefile:572: *** CONFIG_SATA_MV could not be set to "y" as requested.  Stop.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11 11:56                                                 ` Michal Marek
  2011-08-11 13:20                                                   ` David Woodhouse
@ 2011-08-11 14:57                                                   ` Arnaud Lacombe
  2011-08-11 15:07                                                     ` David Woodhouse
  1 sibling, 1 reply; 50+ messages in thread
From: Arnaud Lacombe @ 2011-08-11 14:57 UTC (permalink / raw)
  To: Michal Marek
  Cc: David Woodhouse, Andreas Schwab, H. Peter Anvin, linux-kernel,
	linux-kbuild

Hi,

On Thu, Aug 11, 2011 at 7:56 AM, Michal Marek <mmarek@suse.cz> wrote:
> On 11.8.2011 13:40, David Woodhouse wrote:
>> On Thu, 2011-08-11 at 13:15 +0200, Andreas Schwab wrote:
>>>> That would be cute, but I'm not sure how to undefine something set on
>>>> the command line:
>>>>
>>>> $ cat > Makefile <<EOF
>>>> undefine BAR
>>>
>>> override undefine BAR
>>
>> Thanks. So we don't really need to change the auto.conf syntax; we could
>> just do:
>>
>> $(foreach var, $(filter CONFIG_%,$(.VARIABLES)), $(eval override undefine $(var)))
>>
>> But still I suspect we might *not* want that for options set on the
>> command line. We *don't* want 'make CONFIG_FOO=y oldconfig bzImage' to
>> say 'you can't enable CONFIG_FOO' and then build the bzImage anyway.
>
> If you do
> $ echo 'CONFIG_FOO=y' >all.config
> $ make allnoconfig bzImage
> then it will also build bzImage even if kconfig wasn't able to set
> CONFIG_FOO=y. IMO printing a warning that CONFIG_FOO could not be set is
> sufficient, as long as CONFIG_FOO is consistently unset.
>
FWIW, this is the broken behavior I have been pointing all along...

If kconfig hard failed on such case, we would not need such Kbuild black-magic.

>From my point of view, if environment override there should be, it
should behave the same as the all.config logic and hard fail when an
override has not been met. Code wise, this would translate as backend
code path being the same.

 - Arnaud

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11 14:57                                                   ` Arnaud Lacombe
@ 2011-08-11 15:07                                                     ` David Woodhouse
  2011-08-11 15:24                                                       ` Michal Marek
  0 siblings, 1 reply; 50+ messages in thread
From: David Woodhouse @ 2011-08-11 15:07 UTC (permalink / raw)
  To: Arnaud Lacombe
  Cc: Michal Marek, Andreas Schwab, H. Peter Anvin, linux-kernel, linux-kbuild

On Thu, 2011-08-11 at 10:57 -0400, Arnaud Lacombe wrote:
> 
> FWIW, this is the broken behavior I have been pointing all along...
> 
> If kconfig hard failed on such case, we would not need such Kbuild
> black-magic.
> 
> From my point of view, if environment override there should be, it
> should behave the same as the all.config logic and hard fail when an
> override has not been met. 
> Code wise, this would translate as backend code path being the same. 
 

The patches I have so far *do* behave the same as the all.config logic,
because the back end code *is* fairly much the same. I was looking at
the all.config logic when I wrote the patch to kconfig.

It *doesn't* currently hard fail. But I'm more than happy to make it do
so. I think you're right; that makes most sense.

I've just been looking at ways to allow real build targets to proceed
*only* if any config options specified on the command line *did* get
honoured by kconfig. But that gets a bit messy since you also want to
automatically trigger an 'oldconfig' run if anything was specified on
the command line. So you end up with one automatic oldconfig run in a
sub-make, then the *second* time around it when the supposedly identical
submake is invoked for the real build target, it would have to behave
differently.

I'm much happier with automatically triggering a reconfig if options are
given on the command line, and a hard fail if they can't be honoured.

That means that 'make CONFIG_FOO=y bzImage' will work 'properly', which
IMO is either to do as it was asked, or fail.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11 15:07                                                     ` David Woodhouse
@ 2011-08-11 15:24                                                       ` Michal Marek
  2011-08-11 15:50                                                         ` David Woodhouse
  0 siblings, 1 reply; 50+ messages in thread
From: Michal Marek @ 2011-08-11 15:24 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Arnaud Lacombe, Andreas Schwab, H. Peter Anvin, linux-kernel,
	linux-kbuild

On 11.8.2011 17:07, David Woodhouse wrote:
> On Thu, 2011-08-11 at 10:57 -0400, Arnaud Lacombe wrote:
>>
>> FWIW, this is the broken behavior I have been pointing all along...
>>
>> If kconfig hard failed on such case, we would not need such Kbuild
>> black-magic.
>>
>> From my point of view, if environment override there should be, it
>> should behave the same as the all.config logic and hard fail when an
>> override has not been met. 
>> Code wise, this would translate as backend code path being the same. 
>  
> 
> The patches I have so far *do* behave the same as the all.config logic,
> because the back end code *is* fairly much the same. I was looking at
> the all.config logic when I wrote the patch to kconfig.
> 
> It *doesn't* currently hard fail. But I'm more than happy to make it do
> so. I think you're right; that makes most sense.

One of my use cases for all.config is

$ cp .../older-working-config all.config
$ make KCONFIG_ALLCONFIG=1 allmodconfig

i.e. reuse what is possible from an older config and enable all new
options. Surely it sometimes results in suboptimal configuration with
unwanted debug options enabled, but most of the time it works. I won't
be happy if you make it hard fail because of renamed or removed config
options.

Michal

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'
  2011-08-11 15:24                                                       ` Michal Marek
@ 2011-08-11 15:50                                                         ` David Woodhouse
  0 siblings, 0 replies; 50+ messages in thread
From: David Woodhouse @ 2011-08-11 15:50 UTC (permalink / raw)
  To: Michal Marek
  Cc: Arnaud Lacombe, Andreas Schwab, H. Peter Anvin, linux-kernel,
	linux-kbuild

On Thu, 2011-08-11 at 17:24 +0200, Michal Marek wrote:
> 
> One of my use cases for all.config is
> 
> $ cp .../older-working-config all.config
> $ make KCONFIG_ALLCONFIG=1 allmodconfig
> 
> i.e. reuse what is possible from an older config and enable all new
> options. Surely it sometimes results in suboptimal configuration with
> unwanted debug options enabled, but most of the time it works. I won't
> be happy if you make it hard fail because of renamed or removed config
> options. 

On the other hand, 'make CONFIG_FOO=y oldconfig' probably *should* fail.
Perhaps it should set what it *can*, if you try to set multiple options
at once — but if it can't set all of them, it should exit with failure
status... which will mean that no subsequent build target will happen.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 50+ messages in thread

end of thread, other threads:[~2011-08-11 15:51 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1312067499.22074.59.camel@i7.infradead.org>
2011-07-30 23:13 ` [PATCH 1/2] kconfig: Factor out conf_validate_choice_val() function from conf_read_simple() David Woodhouse
2011-07-31  2:17   ` Arnaud Lacombe
2011-07-31 22:21     ` David Woodhouse
2011-07-30 23:14 ` [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig' David Woodhouse
2011-07-30 23:44   ` Arnaud Lacombe
2011-07-30 23:53     ` H. Peter Anvin
2011-07-31  0:05       ` Arnaud Lacombe
2011-07-31  0:29         ` H. Peter Anvin
2011-07-31  1:06           ` Arnaud Lacombe
2011-07-31  1:28             ` H. Peter Anvin
2011-07-31  2:09               ` Arnaud Lacombe
2011-07-31  5:21                 ` Arnaud Lacombe
2011-07-31 22:18                 ` David Woodhouse
2011-08-09 15:22                   ` Arnaud Lacombe
2011-07-31  7:33             ` David Woodhouse
2011-07-31 16:37               ` Randy Dunlap
2011-07-31 16:57                 ` David Woodhouse
2011-07-31 17:08                   ` Randy Dunlap
2011-07-31 17:40                     ` David Woodhouse
2011-08-09 14:14         ` Michal Marek
2011-08-09 15:26           ` Arnaud Lacombe
2011-08-10 12:59             ` Michal Marek
2011-08-10 13:07               ` David Woodhouse
2011-08-10 14:15                 ` Arnaud Lacombe
2011-08-10 14:17                   ` David Woodhouse
2011-08-10 14:34                     ` Arnaud Lacombe
2011-08-10 16:33                       ` David Woodhouse
2011-08-10 17:00                         ` Emmanuel Deloget
2011-08-10 17:52                           ` H. Peter Anvin
2011-08-10 17:44                         ` Arnaud Lacombe
2011-08-10 17:54                           ` H. Peter Anvin
2011-08-10 17:59                           ` David Woodhouse
2011-08-10 18:40                             ` Arnaud Lacombe
2011-08-10 18:52                               ` David Woodhouse
2011-08-10 22:33                                 ` Arnaud Lacombe
2011-08-10 23:16                                   ` David Woodhouse
2011-08-11  3:29                                     ` Arnaud Lacombe
2011-08-11  8:42                                       ` David Woodhouse
2011-08-11  8:58                                         ` Michal Marek
2011-08-11 11:10                                           ` David Woodhouse
2011-08-11 11:15                                             ` Andreas Schwab
2011-08-11 11:40                                               ` David Woodhouse
2011-08-11 11:56                                                 ` Michal Marek
2011-08-11 13:20                                                   ` David Woodhouse
2011-08-11 14:57                                                   ` Arnaud Lacombe
2011-08-11 15:07                                                     ` David Woodhouse
2011-08-11 15:24                                                       ` Michal Marek
2011-08-11 15:50                                                         ` David Woodhouse
2011-08-10 17:01                   ` Randy Dunlap
2011-08-10 17:25                     ` David Woodhouse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).