Re: [PATCH] kconfig: Add merge_config.sh script

[Darren Hart <dvhart@linux.intel.com>]

Hi Arnaud,

On 11/18/2011 10:51 PM, Arnaud Lacombe wrote:
> Hi,
> 
> On Thu, Nov 17, 2011 at 4:58 PM, john stultz <johnstul@us.ibm.com> wrote:
>> [...]
>>
>> v2:
>>  * Reworked to use alldefconfig instead of the proposed
>> olddefconfig as suggested by Sam Ravnborg.
>>
>> v3:
>>  * Script improvements from Dmitri.
>>  * allnoconfig option from Darren
>>  * pre-make exit option from Darren
>>  * lots of other fixes/cleanups from Darren.
>>  * Fix final check to not compain about config values in comments
>>
> If Dmitri and Darren have direct contribution to the script, shouldn't their
> Signed-off-by tag be present ?

Yes, that would be more correct.

> 
>> Please let me know if you have any comments or thoughts!
>>
>> CC: Sam Ravnborg <sam@ravnborg.org>
>> CC: gthelen@google.com
>> CC: tartler@cs.fau.de
>> CC: Dmitry Fink <Dmitry.Fink@palm.com>
>> CC: Darren Hart <dvhart@linux.intel.com>
>> CC: Eric B Munson <ebmunson@us.ibm.com>
>> CC: Bruce Ashfield <Bruce.Ashfield@windriver.com>
>> CC: Michal Marek <mmarek@suse.cz>
>> CC: linux-kbuild@vger.kernel.org
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
> 
> You'll find below some more nits
> 
> 1) bail out early on error.
> 
> This fixes handling of non-existant file:
> 
> Before:
>     % sh scripts/kconfig/merge_config.sh non existant files
>     Merging non
>     sed: can't read non: No such file or directory
>     cat: non: No such file or directory
>     Merging existant
>     sed: can't read existant: No such file or directory
>     cat: existant: No such file or directory
>     Merging files
>     sed: can't read files: No such file or directory
>     cat: files: No such file or directory
>     scripts/kconfig/conf --alldefconfig Kconfig
>     #
>     # configuration written to .config
>     #
> 
> After:
> 
>     % sh scripts/kconfig/merge_config.sh non existant files
>     Merging non
>     sed: can't read non: No such file or directory

An early test (-f) of each non-option argument would be easy enough to
add, and would provide better error handling/reporting.

> 
> 2) re-implement argument parsing using sh(1) getopts builtin

I was trying to keep it simple - and I was also concerned about
colliding with dash/bash inconsistencies. I have no objection to getopt
if it works consistently in dash and bash. Did you test with dash as
well as bash?

> 
> 3) verify that the script was given enough argument to proceed. There isn't
> much point running the script with less than 2 arguments.

Sure.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel