kconfig: add *_silentdefconfig feature for config targets

PATCH In review Andres Salomon 2 participants 5 messages Apr 26, 2008 – Apr 28, 2008

Subsystems (2)

Kbuild Kconfig

lore

Share & tools

lore

Andres Salomon

author

2008-04-26 02:33 UTC #0 lore raw

PATCH 4/4 kconfig: add *_silentdefconfig feature for config targets

Being able to run 'silentoldconfig' with an existing .config has been immensely useful, especially for automated builds. If the kernel code changes in an incompatible manner without the associated .config being updated, the build will fail and call attention to the need for an update.

AFAICT, there is nothing similar when using *_defconfig; one must copy a .config manually, and then run silentoldconfig. Simply running the associated _defconfig will quietly update the config (which may silently drop config options). This patch adds a *_silentdefconfig target, with semantics similar to silentoldconfig. It will take the defconfig from arch/$(SRCARCH)/configs/$x_defconfig, check for changes, and if there are none, write out a .config. If there have been changes and stdin is valid, it will prompt for updates. If there have been changes and stdin is not valid, it will bail out with an error.

Signed-off-by Andres Salomon <[email protected]>

---
 Makefile                 |    4 ++++
 scripts/kconfig/Makefile |    3 +++
 scripts/kconfig/conf.c   |   13 +++++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)

Makefile +4 -0

index e77149e..c264f7f 100644
@@ -1225,6 +1225,10 @@ help:
 		$(foreach b, $(boards), \
 		printf "  %-24s - Build for %s\\n" $(b) $(subst _defconfig,,$(b));) \
 		echo '')
+	@$(if $(boards), \
+		$(foreach b, $(boards), \
+		printf "  %-24s - Quiet Build for %s\\n" $(subst _defconfig,_silentdefconfig,$(b)) $(subst _defconfig,,$(b));) \
+		echo '')
 
 	@echo  '  make V=0|1 [targets] 0 => quiet build (default), 1 => verbose build'
 	@echo  '  make V=2   [targets] 2 => give reason for rebuild of target'

scripts/kconfig/Makefile +3 -0

index ce7d754..19ba562 100644
@@ -72,6 +72,9 @@ endif
 %_defconfig: $(obj)/conf
 	$(Q)$< -d -D arch/$(SRCARCH)/configs/$@ $(Kconfig)
 
+%_silentdefconfig: $(obj)/conf
+	$(Q)$< -s -o -D arch/$(SRCARCH)/configs/$(subst _silentdefconfig,_defconfig,$@) $(Kconfig)
+
 # Help text used by make help
 help:
 	@echo  '  config	  - Update current config utilising a line-oriented program'

scripts/kconfig/conf.c +11 -2

index 9a27638..264eee9 100644
@@ -558,7 +558,8 @@ int main(int ac, char **av)
 		}
 		break;
 	case ask_new:
-		if (silent_mode && stat(".config", &tmpstat)) {
+		if (!defconfig_file && silent_mode &&
+				stat(".config", &tmpstat)) {
 			printf(_("***\n"
 				"*** You have not yet configured your kernel!\n"
 				"*** (missing kernel .config file)\n"

@@ -570,7 +571,15 @@ int main(int ac, char **av)
 		}
 		/* fall through */
 	case ask_all:
-		conf_read(NULL);
+		if (defconfig_file) {
+			if (conf_read(defconfig_file)) {
+				printf(_("***\n*** Can't find default "
+					 "configuration \"%s\"!\n***\n"),
+					 defconfig_file);
+				exit(1);
+			}
+		} else
+			conf_read(NULL);
 		break;
 	case set_no:
 	case set_mod:

Signature · 1 line

1.5.5

Sam Ravnborg

2008-04-27 20:47 UTC #1 lore raw

Quoted 6 lines · Andres Salomon

Being able to run 'silentoldconfig' with an existing .config has been immensely useful, especially for automated builds. If the kernel code changes in an incompatible manner without the associated .config being updated, the build will fail and call attention to the need for an update.

Quoted 9 lines

AFAICT, there is nothing similar when using *_defconfig; one must copy a .config manually, and then run silentoldconfig. Simply running the associated _defconfig will quietly update the config (which may silently drop config options). This patch adds a *_silentdefconfig target, with semantics similar to silentoldconfig. It will take the defconfig from arch/$(SRCARCH)/configs/$x_defconfig, check for changes, and if there are none, write out a .config. If there have been changes and stdin is valid, it will prompt for updates. If there have been changes and stdin is not valid, it will bail out with an error.

I like what you achieve by this patchset.
But I do not agree on the naming you chose.

We have today:
oldconfig => very chatty
silentoldconfig => Asks only relevant questions
defconfig => silent

[I plan one day to make oldconfig behave like
silentoldconfig and drop the chatty mode]

And I see why you went for the name *_silentdefconfig But in reality what we want to say is that we want to interactively apply the _defconfig.

So if we could come up with something where we told that we want to interactively use i386_defconfig then the users would hopefully be less confused.

I have considered a few way to do so:

a) make I=1 i386_defconfig
b) make i_i386_defconfig
c) make ii386_defconfig
d) make i386_config

And none of these are actually good.
Any better ideas here?

See a few comments below.

Sam

Quoted 13 lines

Signed-off-by Andres Salomon <[email protected]>

---
 Makefile                 |    4 ++++
 scripts/kconfig/Makefile |    3 +++
 scripts/kconfig/conf.c   |   13 +++++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)

Makefile +0 -0

index e77149e..c264f7f 100644

Makefile help: · line 1231

Quoted 7 lines

Makefile +4 -0

@@ -1225,6 +1225,10 @@ help:
 		$(foreach b, $(boards), \
 		printf "  %-24s - Build for %s\\n" $(b) $(subst _defconfig,,$(b));) \
 		echo '')
+	@$(if $(boards), \
+		$(foreach b, $(boards), \
+		printf "  %-24s - Quiet Build for %s\\n" $(subst _defconfig,_silentdefconfig,$(b)) $(subst _defconfig,,$(b));) \
+		echo '')

This is the first time we use printf in the top-level Makefile. Most likely because I never use printf in my shell scripts so I guess this is not a problem.

Quoted 22 lines

Makefile +0 -0

 	@echo  '  make V=0|1 [targets] 0 => quiet build (default), 1 => verbose build'
 	@echo  '  make V=2   [targets] 2 => give reason for rebuild of target'

scripts/kconfig/Makefile +3 -0

index ce7d754..19ba562 100644
@@ -72,6 +72,9 @@ endif
 %_defconfig: $(obj)/conf
 	$(Q)$< -d -D arch/$(SRCARCH)/configs/$@ $(Kconfig)
 
+%_silentdefconfig: $(obj)/conf
+	$(Q)$< -s -o -D arch/$(SRCARCH)/configs/$(subst _silentdefconfig,_defconfig,$@) $(Kconfig)
+
 # Help text used by make help
 help:
 	@echo  '  config	  - Update current config utilising a line-oriented program'

scripts/kconfig/conf.c +0 -0

index 9a27638..264eee9 100644

scripts/kconfig/conf.c int main(int ac, char **av) · line 562

Quoted 6 lines

scripts/kconfig/conf.c +2 -1

@@ -558,7 +558,8 @@ int main(int ac, char **av)
 		}
 		break;
 	case ask_new:
-		if (silent_mode && stat(".config", &tmpstat)) {
+		if (!defconfig_file && silent_mode &&
+				stat(".config", &tmpstat)) {

This belong in a preparation patch. We should handle this also if we do not do so from the Makefile.

Quoted 7 lines

scripts/kconfig/conf.c +0 -0

 			printf(_("***\n"
 				"*** You have not yet configured your kernel!\n"
 				"*** (missing kernel .config file)\n"

@@ -570,7 +571,15 @@ int main(int ac, char **av)
 		}
 		/* fall through */
 	case ask_all:

scripts/kconfig/conf.c int main(int ac, char **av) · line 582

Quoted 10 lines

scripts/kconfig/conf.c +9 -1

-		conf_read(NULL);
+		if (defconfig_file) {
+			if (conf_read(defconfig_file)) {
+				printf(_("***\n*** Can't find default "
+					 "configuration \"%s\"!\n***\n"),
+					 defconfig_file);
+				exit(1);
+			}
+		} else
+			conf_read(NULL);

Does conf_read() fail if we use the NULL argument?
I assume not so the above code can be simplified and should also be in the same preparational patch as the change above.

Sam

Andres Salomon

author

2008-04-28 01:40 UTC #2 lore raw

Quoted 17 lines · On Sun, 27 Apr 2008 22:47:44 +0200 Sam Ravnborg <[email protected]> wrote:

Quoted 6 lines · Andres Salomon
Being able to run 'silentoldconfig' with an existing .config has been immensely useful, especially for automated builds. If the kernel code changes in an incompatible manner without the associated .config being updated, the build will fail and call attention to the need for an update.
Quoted 9 lines
AFAICT, there is nothing similar when using *_defconfig; one must copy a .config manually, and then run silentoldconfig. Simply running the associated _defconfig will quietly update the config (which may silently drop config options). This patch adds a *_silentdefconfig target, with semantics similar to silentoldconfig. It will take the defconfig from arch/$(SRCARCH)/configs/$x_defconfig, check for changes, and if there are none, write out a .config. If there have been changes and stdin is valid, it will prompt for updates. If there have been changes and stdin is not valid, it will bail out with an error.

Quoted 2 lines

I like what you achieve by this patchset.
But I do not agree on the naming you chose.

Yeah, I don't really like the name either, but I wasn't sure what to call it. I want behavior that is similar to silentoldconfig, but not because I want it to not be chatty, but because I want it to fail if there have been changes and there's no tty (ie, !valid_stdin). So basically, silentdefoldconfig or something ridiculously long like that. :)

Quoted 6 lines

We have today:
oldconfig => very chatty
silentoldconfig => Asks only relevant questions
defconfig => silent

Quoted 6 lines

[I plan one day to make oldconfig behave like
silentoldconfig and drop the chatty mode]
And I see why you went for the name *_silentdefconfig But in reality what we want to say is that we want to interactively apply the _defconfig.

Do I? I'm not sure what you mean by "interactively apply". I want to non-interactively apply the defconfig, and fail if prompting is required (rather than just choosing default values).

Perhaps _silentoldconfig would've been a better name. Actually, I'm pretty sure that it is.

Quoted 7 lines

So if we could come up with something where we told that we want to interactively use i386_defconfig then the users would hopefully be less confused.
I have considered a few way to do so:

Quoted 7 lines

a) make I=1 i386_defconfig
b) make i_i386_defconfig
c) make ii386_defconfig
d) make i386_config
And none of these are actually good.
Any better ideas here?

Sounds like you're saying that you want:

make oldconfig V=1 (chatty, prompt if possible or fail)
make oldconfig V=0 (silentoldconfig, prompt if possible or fail)

make defconfig V=1 (chatty, use defaults)
make defconfig V=0 (silent, use defaults)

make i386_oldconfig V=1 (chatty, prompt if possible or fail) make i386_oldconfig V=0 (silent, prompt if possible or fail)

make i386_defconfig V=1 (chatty, use defaults)
make i386_defconfig V=0 (silent, use defaults)

Does that sound right? Would using the build system's verbose variable work? If so, what should the default be?

Quoted 14 lines

See a few comments below.
Sam
Quoted 8 lines
Signed-off-by Andres Salomon <[email protected]>
---
 Makefile                 |    4 ++++
 scripts/kconfig/Makefile |    3 +++
 scripts/kconfig/conf.c   |   13 +++++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)

Quoted 15 lines

Makefile help: · line 1231
Quoted 12 lines
Makefile +4 -0
index e77149e..c264f7f 100644
@@ -1225,6 +1225,10 @@ help:
 		$(foreach b, $(boards), \
 		printf "  %-24s - Build for %s\\n" $(b) $(subst _defconfig,,$(b));) \
 		echo '')
+	@$(if $(boards), \
+		$(foreach b, $(boards), \
+		printf "  %-24s - Quiet Build for %s\\n" $(subst _defconfig,_silentdefconfig,$(b)) $(subst _defconfig,,$(b));) \
+		echo '')
This is the first time we use printf in the top-level Makefile. Most likely because I never use printf in my shell scripts so I guess this is not a problem.

Eh? There's already a printf, this just adds an additional printf.

Quoted 24 lines

Makefile +0 -0

 	@echo  '  make V=0|1 [targets] 0 => quiet build (default), 1 => verbose build'
 	@echo  '  make V=2   [targets] 2 => give reason for rebuild of target'

scripts/kconfig/Makefile +3 -0

index ce7d754..19ba562 100644
@@ -72,6 +72,9 @@ endif
 %_defconfig: $(obj)/conf
 	$(Q)$< -d -D arch/$(SRCARCH)/configs/$@ $(Kconfig)
 
+%_silentdefconfig: $(obj)/conf
+	$(Q)$< -s -o -D arch/$(SRCARCH)/configs/$(subst _silentdefconfig,_defconfig,$@) $(Kconfig)
+
 # Help text used by make help
 help:
 	@echo  '  config	  - Update current config utilising a line-oriented program'

scripts/kconfig/conf.c +0 -0

index 9a27638..264eee9 100644
@@ -558,7 +558,8 @@ int main(int ac, char **av)
 		}

Quoted 8 lines

scripts/kconfig/conf.c int main(int ac, char **av) · line 562
Quoted 5 lines
scripts/kconfig/conf.c +2 -1
 		break;
 	case ask_new:
-		if (silent_mode && stat(".config", &tmpstat)) {
+		if (!defconfig_file && silent_mode &&
+				stat(".config", &tmpstat)) {
This belong in a preparation patch. We should handle this also if we do not do so from the Makefile.

I'm not sure what you mean. This isn't really preparation for this patch; it's just ensuring that we can use '-o' and '-D' together without running a check for .config. Basically, if '-o' is specified but '-D' is not, check for .config (and fail if it doesn't exist. If '-o' and '-D' are both specified, we don't care about .config.

Quoted 14 lines

scripts/kconfig/conf.c +5 -1

 			printf(_("***\n"
 				"*** You have not yet configured your kernel!\n"
 				"*** (missing kernel .config file)\n"

@@ -570,7 +571,15 @@ int main(int ac, char **av)
 		}
 		/* fall through */
 	case ask_all:
-		conf_read(NULL);
+		if (defconfig_file) {
+			if (conf_read(defconfig_file)) {
+				printf(_("***\n*** Can't find default "
+					 "configuration \"%s\"!\n***\n"),
+					 defconfig_file);

Quoted 8 lines

scripts/kconfig/conf.c int main(int ac, char **av) · line 582
Quoted 4 lines
scripts/kconfig/conf.c +4 -0
+				exit(1);
+			}
+		} else
+			conf_read(NULL);
Does conf_read() fail if we use the NULL argument?
I assume not so the above code can be simplified and should also be in the same preparational patch as the change above.

I don't believe it fails, it uses a default config name. I'm not sure if it fails if _that_ file isn't found, though. I can't make much sense of the symbol stuff..

We definitely _do_ want to fail if conf_read(defconfig_file) can't find the file, and we definitely don't want to fail if conf_read(NULL) can't open the file.

Quoted 2 lines

Sam

Sam Ravnborg

2008-04-28 21:34 UTC #3 lore raw

Quoted 8 lines

Quoted 4 lines
And I see why you went for the name *_silentdefconfig But in reality what we want to say is that we want to interactively apply the _defconfig.
Do I? I'm not sure what you mean by "interactively apply". I want to non-interactively apply the defconfig, and fail if prompting is required (rather than just choosing default values).

I mean exactly the behaviour you ask for.

Quoted 8 lines

Sounds like you're saying that you want:
make oldconfig V=1 (chatty, prompt if possible or fail)
make oldconfig V=0 (silentoldconfig, prompt if possible or fail)
make defconfig V=1 (chatty, use defaults)
make defconfig V=0 (silent, use defaults)

Quoted 8 lines

make i386_oldconfig V=1 (chatty, prompt if possible or fail) make i386_oldconfig V=0 (silent, prompt if possible or fail)
make i386_defconfig V=1 (chatty, use defaults)
make i386_defconfig V=0 (silent, use defaults)
Does that sound right? Would using the build system's verbose variable work? If so, what should the default be?

V= shall not be used for this. I will try to cook up something.

Quoted 18 lines

Quoted 16 lines
Makefile help: · line 1231
Quoted 13 lines
Makefile +4 -0
index e77149e..c264f7f 100644
@@ -1225,6 +1225,10 @@ help:
 		$(foreach b, $(boards), \
 		printf "  %-24s - Build for %s\\n" $(b) $(subst _defconfig,,$(b));) \
 		echo '')
+	@$(if $(boards), \
+		$(foreach b, $(boards), \
+		printf "  %-24s - Quiet Build for %s\\n" $(subst _defconfig,_silentdefconfig,$(b)) $(subst _defconfig,,$(b));) \
+		echo '')
This is the first time we use printf in the top-level Makefile. Most likely because I never use printf in my shell scripts so I guess this is not a problem.

Quoted 1 line

Eh? There's already a printf, this just adds an additional printf.

I'm blind and you are right.

Quoted 13 lines

Quoted 4 lines
Quoted 4 lines
scripts/kconfig/conf.c +0 -0
@@ -558,7 +558,8 @@ int main(int ac, char **av)
 		}
Quoted 8 lines
scripts/kconfig/conf.c int main(int ac, char **av) · line 562
Quoted 5 lines
scripts/kconfig/conf.c +2 -1
 		break;
 	case ask_new:
-		if (silent_mode && stat(".config", &tmpstat)) {
+		if (!defconfig_file && silent_mode &&
+				stat(".config", &tmpstat)) {
This belong in a preparation patch. We should handle this also if we do not do so from the Makefile.

Quoted 5 lines

I'm not sure what you mean. This isn't really preparation for this patch; it's just ensuring that we can use '-o' and '-D' together without running a check for .config. Basically, if '-o' is specified but '-D' is not, check for .config (and fail if it doesn't exist. If '-o' and '-D' are both specified, we don't care about .config.

OK - but then it really does not belong in this patch.

Quoted 24 lines

Quoted 15 lines
Quoted 15 lines
scripts/kconfig/conf.c +5 -1
 			printf(_("***\n"
 				"*** You have not yet configured your kernel!\n"
 				"*** (missing kernel .config file)\n"
@@ -570,7 +571,15 @@ int main(int ac, char **av)
 		}
 		/* fall through */
 	case ask_all:
-		conf_read(NULL);
+		if (defconfig_file) {
+			if (conf_read(defconfig_file)) {
+				printf(_("***\n*** Can't find default "
+					 "configuration \"%s\"!\n***\n"),
+					 defconfig_file);
Quoted 8 lines
scripts/kconfig/conf.c int main(int ac, char **av) · line 582
Quoted 4 lines
scripts/kconfig/conf.c +4 -0
+				exit(1);
+			}
+		} else
+			conf_read(NULL);
Does conf_read() fail if we use the NULL argument?
I assume not so the above code can be simplified and should also be in the same preparational patch as the change above.

Quoted 3 lines

I don't believe it fails, it uses a default config name. I'm not sure if it fails if _that_ file isn't found, though. I can't make much sense of the symbol stuff..

Thought we could simplify the code if defconfig_file is by default NULL. Then we can drop the else.

I will try to get back to you on this later. The patches are anyway too late for this merge window.

Sam

Andres Salomon

author

2008-04-28 21:44 UTC #4 lore raw

Quoted 21 lines · On Mon, 28 Apr 2008 23:34:37 +0200 Sam Ravnborg <[email protected]> wrote:

Quoted 8 lines
Quoted 4 lines
And I see why you went for the name *_silentdefconfig But in reality what we want to say is that we want to interactively apply the _defconfig.
Do I? I'm not sure what you mean by "interactively apply". I want to non-interactively apply the defconfig, and fail if prompting is required (rather than just choosing default values).
I mean exactly the behaviour you ask for.
Quoted 11 lines
Sounds like you're saying that you want:
make oldconfig V=1 (chatty, prompt if possible or fail)
make oldconfig V=0 (silentoldconfig, prompt if possible or fail)
make defconfig V=1 (chatty, use defaults)
make defconfig V=0 (silent, use defaults)
make i386_oldconfig V=1 (chatty, prompt if possible or fail) make i386_oldconfig V=0 (silent, prompt if possible or fail)

Quoted 7 lines

Quoted 5 lines
make i386_defconfig V=1 (chatty, use defaults)
make i386_defconfig V=0 (silent, use defaults)
Does that sound right? Would using the build system's verbose variable work? If so, what should the default be?
V= shall not be used for this. I will try to cook up something.

Ok.

Quoted 35 lines

Quoted 19 lines
Quoted 17 lines
Quoted 7 lines
Makefile +0 -0
index e77149e..c264f7f 100644
Makefile help: · line 1231
Quoted 7 lines
Makefile +4 -0
@@ -1225,6 +1225,10 @@ help:
 		$(foreach b, $(boards), \
 		printf "  %-24s - Build for %s\\n" $(b) $(subst _defconfig,,$(b));) \
 		echo '')
+	@$(if $(boards), \
+		$(foreach b, $(boards), \
+		printf "  %-24s - Quiet Build for %s\\n" $(subst _defconfig,_silentdefconfig,$(b)) $(subst _defconfig,,$(b));) \
+		echo '')
This is the first time we use printf in the top-level Makefile. Most likely because I never use printf in my shell scripts so I guess this is not a problem.
Quoted 1 line
Eh? There's already a printf, this just adds an additional printf.
I'm blind and you are right.
Quoted 13 lines
Quoted 12 lines
scripts/kconfig/conf.c int main(int ac, char **av) · line 562
Quoted 9 lines
scripts/kconfig/conf.c +2 -1
@@ -558,7 +558,8 @@ int main(int ac, char **av)
 		}
 		break;
 	case ask_new:
-		if (silent_mode && stat(".config", &tmpstat)) {
+		if (!defconfig_file && silent_mode &&
+				stat(".config", &tmpstat)) {
This belong in a preparation patch. We should handle this also if we do not do so from the Makefile.

Quoted 7 lines

Quoted 5 lines
I'm not sure what you mean. This isn't really preparation for this patch; it's just ensuring that we can use '-o' and '-D' together without running a check for .config. Basically, if '-o' is specified but '-D' is not, check for .config (and fail if it doesn't exist. If '-o' and '-D' are both specified, we don't care about .config.
OK - but then it really does not belong in this patch.

I can break it into a separate patch, then. Should I do so now, or wait?

Quoted 25 lines

Quoted 25 lines
Quoted 24 lines
Quoted 10 lines
scripts/kconfig/conf.c +0 -0
 			printf(_("***\n"
 				"*** You have not yet configured your kernel!\n"
 				"*** (missing kernel .config file)\n"
@@ -570,7 +571,15 @@ int main(int ac, char **av)
 		}
 		/* fall through */
 	case ask_all:
scripts/kconfig/conf.c int main(int ac, char **av) · line 582
Quoted 10 lines
scripts/kconfig/conf.c +9 -1
-		conf_read(NULL);
+		if (defconfig_file) {
+			if (conf_read(defconfig_file)) {
+				printf(_("***\n*** Can't find default "
+					 "configuration \"%s\"!\n***\n"),
+					 defconfig_file);
+				exit(1);
+			}
+		} else
+			conf_read(NULL);
Does conf_read() fail if we use the NULL argument?
I assume not so the above code can be simplified and should also be in the same preparational patch as the change above.

Quoted 5 lines

Quoted 3 lines
I don't believe it fails, it uses a default config name. I'm not sure if it fails if _that_ file isn't found, though. I can't make much sense of the symbol stuff..
Thought we could simplify the code if defconfig_file is by default NULL. Then we can drop the else.

Ok.

Quoted 3 lines

I will try to get back to you on this later. The patches are anyway too late for this merge window.

Ok, sure. I guess just let me know when you come up w/ an idea for what it should be named, and I can update patches accordingly..

Quoted 2 lines

Sam