Re: [PATCH 2/2] Enable 'make CONFIG_FOO=y oldconfig'

[David Woodhouse <dwmw2@infradead.org>]

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