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

[Sam Ravnborg <sam@ravnborg.org>]

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