public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/4] kconfig: add *_silentdefconfig feature for config targets
@ 2008-04-26  2:35 Andres Salomon
  2008-04-27 20:47 ` Sam Ravnborg
  0 siblings, 1 reply; 5+ messages in thread
From: Andres Salomon @ 2008-04-26  2:35 UTC (permalink / raw)
  To: sam; +Cc: Andrew Morton, linux-kernel, linux-kbuild


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 <dilinger@debian.org>
---
 Makefile                 |    4 ++++
 scripts/kconfig/Makefile |    3 +++
 scripts/kconfig/conf.c   |   13 +++++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index e77149e..c264f7f 100644
--- a/Makefile
+++ b/Makefile
@@ -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'
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index ce7d754..19ba562 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -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'
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 9a27638..264eee9 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -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:
-- 
1.5.5


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

* Re: [PATCH 4/4] kconfig: add *_silentdefconfig feature for config targets
  2008-04-26  2:35 [PATCH 4/4] kconfig: add *_silentdefconfig feature for config targets Andres Salomon
@ 2008-04-27 20:47 ` Sam Ravnborg
  2008-04-28  1:43   ` Andres Salomon
  0 siblings, 1 reply; 5+ messages in thread
From: Sam Ravnborg @ 2008-04-27 20:47 UTC (permalink / raw)
  To: Andres Salomon; +Cc: Andrew Morton, linux-kernel, linux-kbuild

On Fri, Apr 25, 2008 at 10:35:30PM -0400, Andres Salomon wrote:
> 
> 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.

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


> 
> Signed-off-by: Andres Salomon <dilinger@debian.org>
> ---
>  Makefile                 |    4 ++++
>  scripts/kconfig/Makefile |    3 +++
>  scripts/kconfig/conf.c   |   13 +++++++++++--
>  3 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index e77149e..c264f7f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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.

>  
>  	@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'
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index ce7d754..19ba562 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -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'
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 9a27638..264eee9 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -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.

>  			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);

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

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

* Re: [PATCH 4/4] kconfig: add *_silentdefconfig feature for config targets
  2008-04-27 20:47 ` Sam Ravnborg
@ 2008-04-28  1:43   ` Andres Salomon
  2008-04-28 21:34     ` Sam Ravnborg
  0 siblings, 1 reply; 5+ messages in thread
From: Andres Salomon @ 2008-04-28  1:43 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: Andrew Morton, linux-kernel, linux-kbuild

On Sun, 27 Apr 2008 22:47:44 +0200
Sam Ravnborg <sam@ravnborg.org> wrote:

> On Fri, Apr 25, 2008 at 10:35:30PM -0400, Andres Salomon wrote:
> > 
> > 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.
> 
> 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. :)

> 
> 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.

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. 

> 
> 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?

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?




> 
> See a few comments below.
> 
> 	Sam
> 
> 
> > 
> > Signed-off-by: Andres Salomon <dilinger@debian.org>
> > ---
> >  Makefile                 |    4 ++++
> >  scripts/kconfig/Makefile |    3 +++
> >  scripts/kconfig/conf.c   |   13 +++++++++++--
> >  3 files changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index e77149e..c264f7f 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -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.


> 
> >  
> >  	@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'
> > diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> > index ce7d754..19ba562 100644
> > --- a/scripts/kconfig/Makefile
> > +++ b/scripts/kconfig/Makefile
> > @@ -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'
> > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> > index 9a27638..264eee9 100644
> > --- a/scripts/kconfig/conf.c
> > +++ b/scripts/kconfig/conf.c
> > @@ -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.

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.

> 
> >  			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);
> 
> 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.


> 
> 	Sam

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

* Re: [PATCH 4/4] kconfig: add *_silentdefconfig feature for config targets
  2008-04-28  1:43   ` Andres Salomon
@ 2008-04-28 21:34     ` Sam Ravnborg
  2008-04-28 21:48       ` Andres Salomon
  0 siblings, 1 reply; 5+ messages in thread
From: Sam Ravnborg @ 2008-04-28 21:34 UTC (permalink / raw)
  To: Andres Salomon; +Cc: Andrew Morton, linux-kernel, linux-kbuild

> > 
> > 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.

> 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?

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

> > > 
> > > diff --git a/Makefile b/Makefile
> > > index e77149e..c264f7f 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -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.
I'm blind and you are right.

> > > --- a/scripts/kconfig/conf.c
> > > +++ b/scripts/kconfig/conf.c
> > > @@ -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.
> 
> 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.

> 
> > 
> > >  			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);
> > 
> > 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..
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

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

* Re: [PATCH 4/4] kconfig: add *_silentdefconfig feature for config targets
  2008-04-28 21:34     ` Sam Ravnborg
@ 2008-04-28 21:48       ` Andres Salomon
  0 siblings, 0 replies; 5+ messages in thread
From: Andres Salomon @ 2008-04-28 21:48 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: Andrew Morton, linux-kernel, linux-kbuild

On Mon, 28 Apr 2008 23:34:37 +0200
Sam Ravnborg <sam@ravnborg.org> wrote:

> > > 
> > > 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.
> 
> > 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?
> 
> V= shall not be used for this. I will try to cook up something.

Ok.

> 
> > > > 
> > > > diff --git a/Makefile b/Makefile
> > > > index e77149e..c264f7f 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -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.
> I'm blind and you are right.
> 
> > > > --- a/scripts/kconfig/conf.c
> > > > +++ b/scripts/kconfig/conf.c
> > > > @@ -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.
> > 
> > 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?


> 
> > 
> > > 
> > > >  			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);
> > > 
> > > 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..
> Thought we could simplify the code if defconfig_file is by default NULL.
> Then we can drop the else.

Ok.

> 
> 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..



> 
> 	Sam

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

end of thread, other threads:[~2008-04-28 21:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-26  2:35 [PATCH 4/4] kconfig: add *_silentdefconfig feature for config targets Andres Salomon
2008-04-27 20:47 ` Sam Ravnborg
2008-04-28  1:43   ` Andres Salomon
2008-04-28 21:34     ` Sam Ravnborg
2008-04-28 21:48       ` Andres Salomon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox