* [PATCH] kconfig: Add merge_config.sh script
@ 2011-11-17 21:58 john stultz
2011-11-17 22:44 ` Arnaud Lacombe
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: john stultz @ 2011-11-17 21:58 UTC (permalink / raw)
To: Andrew Morton
Cc: Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart,
Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild
Hey Andrew,
I've tried sending this via Michal a few times, but haven't heard much
back. So I wanted to check if you would consider merging it via your
tree, or if you had any suggestions of who would be better to
review/merge this.
thanks
-john
After noticing almost every distro has their own method of managing
config fragments, I went looking at some best practices, and wanted
to try to consolidate some of the different approaches so this fairly
simple infrastructure can be shared (and new distros/build systems
don't have to implement yet another config fragment merge script).
This script is most influenced by the Windriver tools used in
the Yocto Project, reusing some portions found there.
This script merges multiple config fragments, warning on any
overrided values. It then sets any unspecified values to their
default, then finally checks to make sure no specified value was
dropped due to unsatisfied dependencies.
I'm sure this implementation won't work for everyone, and I expect
it will need to evolve to adapt for various use cases. But I think
its a reasonable starting point.
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
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>
---
scripts/kconfig/merge_config.sh | 117 +++++++++++++++++++++++++++++++++++++++
1 files changed, 117 insertions(+), 0 deletions(-)
create mode 100755 scripts/kconfig/merge_config.sh
diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
new file mode 100755
index 0000000..890276b
--- /dev/null
+++ b/scripts/kconfig/merge_config.sh
@@ -0,0 +1,117 @@
+#!/bin/sh
+# merge_config.sh - Takes a list of config fragment values, and merges
+# them one by one. Provides warnings on overridden values, and specified
+# values that did not make it to the resulting .config file (due to missed
+# dependencies or config symbol removal).
+#
+# Portions reused from kconf_check and generate_cfg:
+# http://git.yoctoproject.org/cgit/cgit.cgi/yocto-kernel-tools/tree/tools/kconf_check
+# http://git.yoctoproject.org/cgit/cgit.cgi/yocto-kernel-tools/tree/tools/generate_cfg
+#
+# Copyright (c) 2009-2010 Wind River Systems, Inc.
+# Copyright 2011 Linaro
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+# See the GNU General Public License for more details.
+
+clean_up() {
+ rm -f $TMP_FILE
+ exit
+}
+trap clean_up SIGHUP SIGINT SIGTERM
+
+usage() {
+ echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
+ echo " -h display this help text"
+ echo " -m only merge the fragments, do not execute the make command"
+ echo " -n use allnoconfig instead of alldefconfig"
+}
+
+MAKE=true
+ALLTARGET=alldefconfig
+
+while true; do
+ case $1 in
+ "-n")
+ ALLTARGET=allnoconfig
+ shift
+ continue
+ ;;
+ "-m")
+ MAKE=false
+ shift
+ continue
+ ;;
+ "-h")
+ usage
+ exit
+ ;;
+ *)
+ break
+ ;;
+ esac
+done
+
+
+
+MERGE_LIST=$*
+SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
+TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
+
+# Merge files, printing warnings on overrided values
+for MERGE_FILE in $MERGE_LIST ; do
+ echo "Merging $MERGE_FILE"
+ CFG_LIST=$(sed -n "$SED_CONFIG_EXP" $MERGE_FILE)
+
+ for CFG in $CFG_LIST ; do
+ grep -q -w $CFG $TMP_FILE
+ if [ $? -eq 0 ] ; then
+ PREV_VAL=$(grep -w $CFG $TMP_FILE)
+ NEW_VAL=$(grep -w $CFG $MERGE_FILE)
+ if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then
+ echo Value of $CFG is redefined by fragment $MERGE_FILE:
+ echo Previous value: $PREV_VAL
+ echo New value: $NEW_VAL
+ echo
+ fi
+ sed -i "/$CFG[ =]/d" $TMP_FILE
+ fi
+ done
+ cat $MERGE_FILE >> $TMP_FILE
+done
+
+if [ "$MAKE" = "false" ]; then
+ cp $TMP_FILE .config
+ echo "#"
+ echo "# merged configuration written to .config (needs make)"
+ echo "#"
+ clean_up
+ exit
+fi
+
+# Use the merged file as the starting point for:
+# alldefconfig: Fills in any missing symbols with Kconfig default
+# allnoconfig: Fills in any missing symbols with # CONFIG_* is not set
+make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET
+
+
+# Check all specified config values took (might have missed-dependency issues)
+for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
+
+ REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG")
+ ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG")
+ if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
+ echo "Value requested for $CFG not in final .config"
+ echo "Requested value: $REQUESTED_VAL"
+ echo "Actual value: $ACTUAL_VAL"
+ echo ""
+ fi
+done
+
+clean_up
--
1.7.3.2.146.gca209
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-17 21:58 [PATCH] kconfig: Add merge_config.sh script john stultz @ 2011-11-17 22:44 ` Arnaud Lacombe 2011-11-17 22:54 ` john stultz 2011-11-19 6:51 ` Arnaud Lacombe ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: Arnaud Lacombe @ 2011-11-17 22:44 UTC (permalink / raw) To: john stultz Cc: Andrew Morton, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild Hi, On Thu, Nov 17, 2011 at 4:58 PM, john stultz <johnstul@us.ibm.com> wrote: > Hey Andrew, > I've tried sending this via Michal a few times, but haven't heard much > back. So I wanted to check if you would consider merging it via your > tree, or if you had any suggestions of who would be better to > review/merge this. > One of the worry I would have is that the script is merging config blindly, ie. there is no dependency checking done. I have some some work-in-progress to help resolving this, but still lots of thought to be implemented. That said, I don't really mind for any gap-filling solution. Regards, - Arnaud > thanks > -john > > > > After noticing almost every distro has their own method of managing > config fragments, I went looking at some best practices, and wanted > to try to consolidate some of the different approaches so this fairly > simple infrastructure can be shared (and new distros/build systems > don't have to implement yet another config fragment merge script). > > This script is most influenced by the Windriver tools used in > the Yocto Project, reusing some portions found there. > > This script merges multiple config fragments, warning on any > overrided values. It then sets any unspecified values to their > default, then finally checks to make sure no specified value was > dropped due to unsatisfied dependencies. > > I'm sure this implementation won't work for everyone, and I expect > it will need to evolve to adapt for various use cases. But I think > its a reasonable starting point. > > 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 > > 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> > --- > scripts/kconfig/merge_config.sh | 117 +++++++++++++++++++++++++++++++++++++++ > 1 files changed, 117 insertions(+), 0 deletions(-) > create mode 100755 scripts/kconfig/merge_config.sh > > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh > new file mode 100755 > index 0000000..890276b > --- /dev/null > +++ b/scripts/kconfig/merge_config.sh > @@ -0,0 +1,117 @@ > +#!/bin/sh > +# merge_config.sh - Takes a list of config fragment values, and merges > +# them one by one. Provides warnings on overridden values, and specified > +# values that did not make it to the resulting .config file (due to missed > +# dependencies or config symbol removal). > +# > +# Portions reused from kconf_check and generate_cfg: > +# http://git.yoctoproject.org/cgit/cgit.cgi/yocto-kernel-tools/tree/tools/kconf_check > +# http://git.yoctoproject.org/cgit/cgit.cgi/yocto-kernel-tools/tree/tools/generate_cfg > +# > +# Copyright (c) 2009-2010 Wind River Systems, Inc. > +# Copyright 2011 Linaro > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License version 2 as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > +# See the GNU General Public License for more details. > + > +clean_up() { > + rm -f $TMP_FILE > + exit > +} > +trap clean_up SIGHUP SIGINT SIGTERM > + > +usage() { > + echo "Usage: $0 [OPTIONS] [CONFIG [...]]" > + echo " -h display this help text" > + echo " -m only merge the fragments, do not execute the make command" > + echo " -n use allnoconfig instead of alldefconfig" > +} > + > +MAKE=true > +ALLTARGET=alldefconfig > + > +while true; do > + case $1 in > + "-n") > + ALLTARGET=allnoconfig > + shift > + continue > + ;; > + "-m") > + MAKE=false > + shift > + continue > + ;; > + "-h") > + usage > + exit > + ;; > + *) > + break > + ;; > + esac > +done > + > + > + > +MERGE_LIST=$* > +SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p" > +TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX) > + > +# Merge files, printing warnings on overrided values > +for MERGE_FILE in $MERGE_LIST ; do > + echo "Merging $MERGE_FILE" > + CFG_LIST=$(sed -n "$SED_CONFIG_EXP" $MERGE_FILE) > + > + for CFG in $CFG_LIST ; do > + grep -q -w $CFG $TMP_FILE > + if [ $? -eq 0 ] ; then > + PREV_VAL=$(grep -w $CFG $TMP_FILE) > + NEW_VAL=$(grep -w $CFG $MERGE_FILE) > + if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then > + echo Value of $CFG is redefined by fragment $MERGE_FILE: > + echo Previous value: $PREV_VAL > + echo New value: $NEW_VAL > + echo > + fi > + sed -i "/$CFG[ =]/d" $TMP_FILE > + fi > + done > + cat $MERGE_FILE >> $TMP_FILE > +done > + > +if [ "$MAKE" = "false" ]; then > + cp $TMP_FILE .config > + echo "#" > + echo "# merged configuration written to .config (needs make)" > + echo "#" > + clean_up > + exit > +fi > + > +# Use the merged file as the starting point for: > +# alldefconfig: Fills in any missing symbols with Kconfig default > +# allnoconfig: Fills in any missing symbols with # CONFIG_* is not set > +make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET > + > + > +# Check all specified config values took (might have missed-dependency issues) > +for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do > + > + REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG") > + ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG") > + if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then > + echo "Value requested for $CFG not in final .config" > + echo "Requested value: $REQUESTED_VAL" > + echo "Actual value: $ACTUAL_VAL" > + echo "" > + fi > +done > + > +clean_up > -- > 1.7.3.2.146.gca209 > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-17 22:44 ` Arnaud Lacombe @ 2011-11-17 22:54 ` john stultz 2011-11-19 6:24 ` Arnaud Lacombe 0 siblings, 1 reply; 20+ messages in thread From: john stultz @ 2011-11-17 22:54 UTC (permalink / raw) To: Arnaud Lacombe Cc: Andrew Morton, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild On Thu, 2011-11-17 at 17:44 -0500, Arnaud Lacombe wrote: > Hi, > > On Thu, Nov 17, 2011 at 4:58 PM, john stultz <johnstul@us.ibm.com> wrote: > > Hey Andrew, > > I've tried sending this via Michal a few times, but haven't heard much > > back. So I wanted to check if you would consider merging it via your > > tree, or if you had any suggestions of who would be better to > > review/merge this. > > > One of the worry I would have is that the script is merging config > blindly, ie. there is no dependency checking done. I have some some > work-in-progress to help resolving this, but still lots of thought to > be implemented. So the script actually does warn you if a specified option is dropped due to missing dependencies or if the option is removed. So, I guess could you clarify your concern a bit more? thanks -john ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-17 22:54 ` john stultz @ 2011-11-19 6:24 ` Arnaud Lacombe 2011-11-21 18:22 ` Darren Hart ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Arnaud Lacombe @ 2011-11-19 6:24 UTC (permalink / raw) To: john stultz Cc: Andrew Morton, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild Hi, On Thu, Nov 17, 2011 at 5:54 PM, john stultz <johnstul@us.ibm.com> wrote: > On Thu, 2011-11-17 at 17:44 -0500, Arnaud Lacombe wrote: >> Hi, >> >> On Thu, Nov 17, 2011 at 4:58 PM, john stultz <johnstul@us.ibm.com> wrote: >> > Hey Andrew, >> > I've tried sending this via Michal a few times, but haven't heard much >> > back. So I wanted to check if you would consider merging it via your >> > tree, or if you had any suggestions of who would be better to >> > review/merge this. >> > >> One of the worry I would have is that the script is merging config >> blindly, ie. there is no dependency checking done. I have some some >> work-in-progress to help resolving this, but still lots of thought to >> be implemented. > > So the script actually does warn you if a specified option is dropped > due to missing dependencies or if the option is removed. So, I guess > could you clarify your concern a bit more? > well, assuming the following Kconfig's snippet: choice bool "choice" config A bool "A" config B bool "B" endchoice and trying to merge: - `config1': CONFIG_A=y - `config2': Result in: % sh scripts/kconfig/merge_config.sh config1 config2 Merging config1 Merging config2 scripts/kconfig/conf --alldefconfig Kconfig ./.tmp.config.uMY8Z97l9T:2:warning: override: B changes choice state # # configuration written to .config # % cat .config # # Automatically generated file; DO NOT EDIT. # Linux Kernel Configuration # # CONFIG_A is not set CONFIG_B=y so we still get the warning from the incantation of `alldefconfig', but the one in the script is defeated. Moreover, there might still be performance optimization to realize, considering a base 'defconfig' amended by disabling CONFIG_DECOMPRESS_GZIP, we get: % time sh scripts/kconfig/merge_config.sh base amend Merging base Merging amend Value of CONFIG_DECOMPRESS_GZIP is redefined by fragment amend: Previous value: CONFIG_DECOMPRESS_GZIP=y New value: # CONFIG_DECOMPRESS_GZIP is not set scripts/kconfig/conf --alldefconfig Kconfig # # configuration written to .config # sh scripts/kconfig/merge_config.sh base amend 158.20s user 30.19s system 100% cpu 3:07.86 total Just by getting rid of the two `| grep ...' in the last step: --- a/scripts/kconfig/merge_config.sh +++ b/scripts/kconfig/merge_config.sh @@ -104,8 +104,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET # Check all specified config values took (might have missed-dependency issues) for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do - REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG") - ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG") + REQUESTED_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" $TMP_FILE) + ACTUAL_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" .config) if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then echo "Value requested for $CFG not in final .config" echo "Requested value: $REQUESTED_VAL" we get: sh scripts/kconfig/merge_config.sh base amend 23.20s user 22.30s system 94% cpu 48.033 total Btw, there is other nits in the scripts I'll comment separately. - Arnaud ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-19 6:24 ` Arnaud Lacombe @ 2011-11-21 18:22 ` Darren Hart 2011-11-21 23:29 ` john stultz 2011-11-21 23:41 ` john stultz 2 siblings, 0 replies; 20+ messages in thread From: Darren Hart @ 2011-11-21 18:22 UTC (permalink / raw) To: Arnaud Lacombe Cc: john stultz, Andrew Morton, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild On 11/18/2011 10:24 PM, Arnaud Lacombe wrote: > Hi, > > On Thu, Nov 17, 2011 at 5:54 PM, john stultz <johnstul@us.ibm.com> wrote: >> On Thu, 2011-11-17 at 17:44 -0500, Arnaud Lacombe wrote: >>> Hi, >>> >>> On Thu, Nov 17, 2011 at 4:58 PM, john stultz <johnstul@us.ibm.com> wrote: >>>> Hey Andrew, >>>> I've tried sending this via Michal a few times, but haven't heard much >>>> back. So I wanted to check if you would consider merging it via your >>>> tree, or if you had any suggestions of who would be better to >>>> review/merge this. >>>> >>> One of the worry I would have is that the script is merging config >>> blindly, ie. there is no dependency checking done. I have some some >>> work-in-progress to help resolving this, but still lots of thought to >>> be implemented. >> >> So the script actually does warn you if a specified option is dropped >> due to missing dependencies or if the option is removed. So, I guess >> could you clarify your concern a bit more? >> > well, assuming the following Kconfig's snippet: > > choice > bool "choice" > config A > bool "A" > config B > bool "B" > endchoice > > and trying to merge: > > - `config1': > > CONFIG_A=y > > - `config2': > > Result in: > > % sh scripts/kconfig/merge_config.sh config1 config2 > Merging config1 > Merging config2 > scripts/kconfig/conf --alldefconfig Kconfig > ./.tmp.config.uMY8Z97l9T:2:warning: override: B changes choice state > # > # configuration written to .config > # > > % cat .config > # > # Automatically generated file; DO NOT EDIT. > # Linux Kernel Configuration > # > # CONFIG_A is not set > CONFIG_B=y > > so we still get the warning from the incantation of `alldefconfig', > but the one in the script is defeated. > > Moreover, there might still be performance optimization to realize, > considering a base 'defconfig' amended by disabling > CONFIG_DECOMPRESS_GZIP, we get: > > % time sh scripts/kconfig/merge_config.sh base amend > Merging base > Merging amend > Value of CONFIG_DECOMPRESS_GZIP is redefined by fragment amend: > Previous value: CONFIG_DECOMPRESS_GZIP=y > New value: # CONFIG_DECOMPRESS_GZIP is not set > > scripts/kconfig/conf --alldefconfig Kconfig > # > # configuration written to .config > # > sh scripts/kconfig/merge_config.sh base amend 158.20s user 30.19s > system 100% cpu 3:07.86 total > > Just by getting rid of the two `| grep ...' in the last step: > > --- a/scripts/kconfig/merge_config.sh > +++ b/scripts/kconfig/merge_config.sh > @@ -104,8 +104,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET > # Check all specified config values took (might have missed-dependency issues) > for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do > > - REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG") > - ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG") > + REQUESTED_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" $TMP_FILE) > + ACTUAL_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" .config) > if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then > echo "Value requested for $CFG not in final .config" > echo "Requested value: $REQUESTED_VAL" These look like good changes to me. We made several optimizations along these lines, but I confess performance was not my top priority while working with John on this script. -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-19 6:24 ` Arnaud Lacombe 2011-11-21 18:22 ` Darren Hart @ 2011-11-21 23:29 ` john stultz 2011-11-21 23:41 ` john stultz 2 siblings, 0 replies; 20+ messages in thread From: john stultz @ 2011-11-21 23:29 UTC (permalink / raw) To: Arnaud Lacombe Cc: Andrew Morton, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild On Sat, 2011-11-19 at 01:24 -0500, Arnaud Lacombe wrote: > Hi, > > On Thu, Nov 17, 2011 at 5:54 PM, john stultz <johnstul@us.ibm.com> wrote: > > On Thu, 2011-11-17 at 17:44 -0500, Arnaud Lacombe wrote: > >> Hi, > >> > >> On Thu, Nov 17, 2011 at 4:58 PM, john stultz <johnstul@us.ibm.com> wrote: > >> > Hey Andrew, > >> > I've tried sending this via Michal a few times, but haven't heard much > >> > back. So I wanted to check if you would consider merging it via your > >> > tree, or if you had any suggestions of who would be better to > >> > review/merge this. > >> > > >> One of the worry I would have is that the script is merging config > >> blindly, ie. there is no dependency checking done. I have some some > >> work-in-progress to help resolving this, but still lots of thought to > >> be implemented. > > > > So the script actually does warn you if a specified option is dropped > > due to missing dependencies or if the option is removed. So, I guess > > could you clarify your concern a bit more? > > > well, assuming the following Kconfig's snippet: > > choice > bool "choice" > config A > bool "A" > config B > bool "B" > endchoice > > and trying to merge: > > - `config1': > > CONFIG_A=y > > - `config2': > > Result in: > > % sh scripts/kconfig/merge_config.sh config1 config2 > Merging config1 > Merging config2 > scripts/kconfig/conf --alldefconfig Kconfig > ./.tmp.config.uMY8Z97l9T:2:warning: override: B changes choice state > # > # configuration written to .config > # > > % cat .config > # > # Automatically generated file; DO NOT EDIT. > # Linux Kernel Configuration > # > # CONFIG_A is not set > CONFIG_B=y > > so we still get the warning from the incantation of `alldefconfig', > but the one in the script is defeated. > > Moreover, there might still be performance optimization to realize, > considering a base 'defconfig' amended by disabling > CONFIG_DECOMPRESS_GZIP, we get: > > % time sh scripts/kconfig/merge_config.sh base amend > Merging base > Merging amend > Value of CONFIG_DECOMPRESS_GZIP is redefined by fragment amend: > Previous value: CONFIG_DECOMPRESS_GZIP=y > New value: # CONFIG_DECOMPRESS_GZIP is not set > > scripts/kconfig/conf --alldefconfig Kconfig > # > # configuration written to .config > # > sh scripts/kconfig/merge_config.sh base amend 158.20s user 30.19s > system 100% cpu 3:07.86 total Yep. So you've caught a bug! Thanks for pointing this out! > Just by getting rid of the two `| grep ...' in the last step: > > --- a/scripts/kconfig/merge_config.sh > +++ b/scripts/kconfig/merge_config.sh > @@ -104,8 +104,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET > # Check all specified config values took (might have missed-dependency issues) > for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do > > - REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG") > - ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG") > + REQUESTED_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" $TMP_FILE) > + ACTUAL_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" .config) > if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then > echo "Value requested for $CFG not in final .config" > echo "Requested value: $REQUESTED_VAL" So, its not the greps in my mind that are the issue, its the SED_CONF_EXP lines. Those shouldn't be applied to the REQUESTED and ACTUAL values, since they would strip out any difference between the two, causing the comparison to be moot. I was trying to fix the script from complaining like: Value requested for CONFIG_EXPERT not in final .config Requested value: # CONFIG_EXPERT=y Actual value: # CONFIG_EXPERT is not set But clearly the change was wrong. Anyway, reverting back to something like the following should fix it: diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh index b146b76..adac068 100755 --- a/scripts/kconfig/merge_config.sh +++ b/scripts/kconfig/merge_config.sh @@ -105,8 +105,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET # Check all specified config values took (might have missed-dependency issues) for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do - REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG") - ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG") + REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE) + ACTUAL_VAL=$(grep -w -e "$CFG" .config) if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then echo "Value requested for $CFG not in final .config" echo "Requested value: $REQUESTED_VAL" Andrew: Would you prefer small fixes ontop of the patch you took, or just a new patch that includes this along with Darren's and Arnaud's fixes? thanks -john ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-19 6:24 ` Arnaud Lacombe 2011-11-21 18:22 ` Darren Hart 2011-11-21 23:29 ` john stultz @ 2011-11-21 23:41 ` john stultz 2 siblings, 0 replies; 20+ messages in thread From: john stultz @ 2011-11-21 23:41 UTC (permalink / raw) To: Arnaud Lacombe Cc: Andrew Morton, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild On Sat, 2011-11-19 at 01:24 -0500, Arnaud Lacombe wrote: > Hi, > > On Thu, Nov 17, 2011 at 5:54 PM, john stultz <johnstul@us.ibm.com> wrote: > > On Thu, 2011-11-17 at 17:44 -0500, Arnaud Lacombe wrote: > >> Hi, > >> > >> On Thu, Nov 17, 2011 at 4:58 PM, john stultz <johnstul@us.ibm.com> wrote: > >> > Hey Andrew, > >> > I've tried sending this via Michal a few times, but haven't heard much > >> > back. So I wanted to check if you would consider merging it via your > >> > tree, or if you had any suggestions of who would be better to > >> > review/merge this. > >> > > >> One of the worry I would have is that the script is merging config > >> blindly, ie. there is no dependency checking done. I have some some > >> work-in-progress to help resolving this, but still lots of thought to > >> be implemented. > > > > So the script actually does warn you if a specified option is dropped > > due to missing dependencies or if the option is removed. So, I guess > > could you clarify your concern a bit more? > > > well, assuming the following Kconfig's snippet: > > choice > bool "choice" > config A > bool "A" > config B > bool "B" > endchoice > > and trying to merge: > > - `config1': > > CONFIG_A=y > > - `config2': > > Result in: > > % sh scripts/kconfig/merge_config.sh config1 config2 > Merging config1 > Merging config2 > scripts/kconfig/conf --alldefconfig Kconfig > ./.tmp.config.uMY8Z97l9T:2:warning: override: B changes choice state > # > # configuration written to .config > # > > % cat .config > # > # Automatically generated file; DO NOT EDIT. > # Linux Kernel Configuration > # > # CONFIG_A is not set > CONFIG_B=y > > so we still get the warning from the incantation of `alldefconfig', > but the one in the script is defeated. You can find the fix for this along with other recent changes from Darren and Arnaud in my git tree here: git://git.linaro.org/people/jstultz/linux.git dev/config-fragments Browse-able here: http://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=shortlog;h=refs/heads/dev/config-fragments Let me know if you have any other thoughts or feedback! thanks -john ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-17 21:58 [PATCH] kconfig: Add merge_config.sh script john stultz 2011-11-17 22:44 ` Arnaud Lacombe @ 2011-11-19 6:51 ` Arnaud Lacombe 2011-11-21 18:32 ` Darren Hart ` (2 more replies) 2011-11-20 12:56 ` Michal Marek 2011-11-21 19:42 ` [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash Darren Hart 3 siblings, 3 replies; 20+ messages in thread From: Arnaud Lacombe @ 2011-11-19 6:51 UTC (permalink / raw) To: John Stultz Cc: Arnaud Lacombe, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild 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 ? > 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 2) re-implement argument parsing using sh(1) getopts builtin 3) verify that the script was given enough argument to proceed. There isn't much point running the script with less than 2 arguments. 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: Arnaud Lacombe <lacombar@gmail.com> --- scripts/kconfig/merge_config.sh | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh index 890276b..abfd8b2 100755 --- a/scripts/kconfig/merge_config.sh +++ b/scripts/kconfig/merge_config.sh @@ -20,6 +20,8 @@ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. # See the GNU General Public License for more details. +set -e + clean_up() { rm -f $TMP_FILE exit @@ -36,29 +38,27 @@ usage() { MAKE=true ALLTARGET=alldefconfig -while true; do - case $1 in - "-n") +while getopts "nmh" opt; do + case ${opt} in + n) ALLTARGET=allnoconfig - shift - continue ;; - "-m") + m) MAKE=false - shift - continue ;; - "-h") + h) usage exit ;; - *) - break - ;; esac done +shift $(expr $OPTIND - 1) +if [ $# -lt 2 ]; then + usage + exit 1 +fi MERGE_LIST=$* SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p" -- 1.7.6.153.g78432 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-19 6:51 ` Arnaud Lacombe @ 2011-11-21 18:32 ` Darren Hart 2011-11-21 22:06 ` John Stultz 2011-11-21 22:48 ` john stultz 2 siblings, 0 replies; 20+ messages in thread From: Darren Hart @ 2011-11-21 18:32 UTC (permalink / raw) To: Arnaud Lacombe Cc: John Stultz, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-19 6:51 ` Arnaud Lacombe 2011-11-21 18:32 ` Darren Hart @ 2011-11-21 22:06 ` John Stultz 2011-11-21 22:48 ` john stultz 2 siblings, 0 replies; 20+ messages in thread From: John Stultz @ 2011-11-21 22:06 UTC (permalink / raw) To: Arnaud Lacombe Cc: Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild On Sat, 2011-11-19 at 01:51 -0500, 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 ? Dmitri didn't provide a Signed-off-by, and I had reworked things sufficiently that his patch didn't apply, so I re-implemented many of the suggested improvements myself. However, Dmitri deserves the credits for pointing the issues out. Similarly, with Darren, there were a number of non-signed-off-by'ed changes, I had to merge in by hand due to my folding of my own work in. But there were also a few proper patches from Darren that I did fold in and should have preserved his Signed-off-by. That was sloppy, so my apologies. Its now fixed in my tree. > 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 > > 2) re-implement argument parsing using sh(1) getopts builtin > > 3) verify that the script was given enough argument to proceed. There isn't > much point running the script with less than 2 arguments. > > 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: Arnaud Lacombe <lacombar@gmail.com> Thanks for these great cleanups and feedback! Acked-by: John Stultz <john.stultz@linaro.org> I've queued these (along with your and Darren's other changes). thanks -john > --- > scripts/kconfig/merge_config.sh | 24 ++++++++++++------------ > 1 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh > index 890276b..abfd8b2 100755 > --- a/scripts/kconfig/merge_config.sh > +++ b/scripts/kconfig/merge_config.sh > @@ -20,6 +20,8 @@ > # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > # See the GNU General Public License for more details. > > +set -e > + > clean_up() { > rm -f $TMP_FILE > exit > @@ -36,29 +38,27 @@ usage() { > MAKE=true > ALLTARGET=alldefconfig > > -while true; do > - case $1 in > - "-n") > +while getopts "nmh" opt; do > + case ${opt} in > + n) > ALLTARGET=allnoconfig > - shift > - continue > ;; > - "-m") > + m) > MAKE=false > - shift > - continue > ;; > - "-h") > + h) > usage > exit > ;; > - *) > - break > - ;; > esac > done > > +shift $(expr $OPTIND - 1) > > +if [ $# -lt 2 ]; then > + usage > + exit 1 > +fi > > MERGE_LIST=$* > SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p" ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-19 6:51 ` Arnaud Lacombe 2011-11-21 18:32 ` Darren Hart 2011-11-21 22:06 ` John Stultz @ 2011-11-21 22:48 ` john stultz 2011-11-21 22:55 ` john stultz 2011-11-22 6:25 ` Arnaud Lacombe 2 siblings, 2 replies; 20+ messages in thread From: john stultz @ 2011-11-21 22:48 UTC (permalink / raw) To: Arnaud Lacombe Cc: Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild On Sat, 2011-11-19 at 01:51 -0500, Arnaud Lacombe wrote: > You'll find below some more nits > > 1) bail out early on error. Actually, I spoke a little too soon. The set -e option causes us to bomb out if grep doesn't find a config option (grep returns 1 if nothing is found). So I'll have to drop that change for now. thanks -john ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-21 22:48 ` john stultz @ 2011-11-21 22:55 ` john stultz 2011-11-22 6:25 ` Arnaud Lacombe 1 sibling, 0 replies; 20+ messages in thread From: john stultz @ 2011-11-21 22:55 UTC (permalink / raw) To: Arnaud Lacombe Cc: Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild On Mon, 2011-11-21 at 14:48 -0800, john stultz wrote: > On Sat, 2011-11-19 at 01:51 -0500, Arnaud Lacombe wrote: > > You'll find below some more nits > > > > 1) bail out early on error. > > Actually, I spoke a little too soon. > > The set -e option causes us to bomb out if grep doesn't find a config > option (grep returns 1 if nothing is found). So I'll have to drop that > change for now. Catching the error explicitly is easy enough, so folded in the following fix (copied here from a terminal, so its whitespace damaged, I know) to your patch: @@ -20,8 +20,6 @@ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. # See the GNU General Public License for more details. -set -e - clean_up() { rm -f $TMP_FILE exit @@ -68,6 +66,9 @@ TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX) for MERGE_FILE in $MERGE_LIST ; do echo "Merging $MERGE_FILE" CFG_LIST=$(sed -n "$SED_CONFIG_EXP" $MERGE_FILE) + if [ $? != 0 ] ; then + exit 1 + fi for CFG in $CFG_LIST ; do grep -q -w $CFG $TMP_FILE thanks -john ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-21 22:48 ` john stultz 2011-11-21 22:55 ` john stultz @ 2011-11-22 6:25 ` Arnaud Lacombe 1 sibling, 0 replies; 20+ messages in thread From: Arnaud Lacombe @ 2011-11-22 6:25 UTC (permalink / raw) To: john stultz Cc: Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild Hi, On Mon, Nov 21, 2011 at 5:48 PM, john stultz <johnstul@us.ibm.com> wrote: > On Sat, 2011-11-19 at 01:51 -0500, Arnaud Lacombe wrote: >> You'll find below some more nits >> >> 1) bail out early on error. > > Actually, I spoke a little too soon. > > The set -e option causes us to bomb out if grep doesn't find a config > option (grep returns 1 if nothing is found). So I'll have to drop that > change for now. > because the way the return value of grep(1) is checked is bogus. It should not be: grep -q -w $CFG $TMP_FILE if [ $? -eq 0 ] ; then ... fi but: if grep -q -w $CFG $TMP_FILE ; then ... fi that way, you can still keep the `set -e'. IMHO, it should be set by default for any shell script, just as a safety net, as `set -u'. - Arnaud ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-17 21:58 [PATCH] kconfig: Add merge_config.sh script john stultz 2011-11-17 22:44 ` Arnaud Lacombe 2011-11-19 6:51 ` Arnaud Lacombe @ 2011-11-20 12:56 ` Michal Marek 2011-11-20 18:05 ` Arnaud Lacombe 2011-11-21 19:42 ` [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash Darren Hart 3 siblings, 1 reply; 20+ messages in thread From: Michal Marek @ 2011-11-20 12:56 UTC (permalink / raw) To: john stultz Cc: Andrew Morton, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, linux-kbuild On 17.11.2011 22:58, john stultz wrote: > Hey Andrew, > I've tried sending this via Michal a few times, but haven't heard much > back. So I wanted to check if you would consider merging it via your > tree, or if you had any suggestions of who would be better to > review/merge this. My concern was that the script works around missing features in kconfig, instead of fixing kconfig. Don't get me wrong, I do believe that the script is useful, but if you are submitting to mainline anyway, why don't you fix/enhance kconfig in mainline and have a much simpler script making use of the enhanced kconfig? Michal ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-20 12:56 ` Michal Marek @ 2011-11-20 18:05 ` Arnaud Lacombe 2011-11-21 18:34 ` Darren Hart 0 siblings, 1 reply; 20+ messages in thread From: Arnaud Lacombe @ 2011-11-20 18:05 UTC (permalink / raw) To: Michal Marek Cc: john stultz, Andrew Morton, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Darren Hart, Eric B Munson, Bruce Ashfield, linux-kbuild Hi, On Sun, Nov 20, 2011 at 7:56 AM, Michal Marek <mmarek@suse.cz> wrote: > On 17.11.2011 22:58, john stultz wrote: >> Hey Andrew, >> I've tried sending this via Michal a few times, but haven't heard much >> back. So I wanted to check if you would consider merging it via your >> tree, or if you had any suggestions of who would be better to >> review/merge this. > > My concern was that the script works around missing features in kconfig, > instead of fixing kconfig. Don't get me wrong, I do believe that the > script is useful, but if you are submitting to mainline anyway, why > don't you fix/enhance kconfig in mainline and have a much simpler script > making use of the enhanced kconfig? > FWIW, as much as I agree with you, kconfig is currently a bit too static, with too much assumption, do any advanced stuff. I have WIP in the area, but I prefer not giving ETA. In the mean time, I think John's script could fill the gap. - Arnaud ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] kconfig: Add merge_config.sh script 2011-11-20 18:05 ` Arnaud Lacombe @ 2011-11-21 18:34 ` Darren Hart 0 siblings, 0 replies; 20+ messages in thread From: Darren Hart @ 2011-11-21 18:34 UTC (permalink / raw) To: Arnaud Lacombe Cc: Michal Marek, john stultz, Andrew Morton, Sam Ravnborg, gthelen, tartler, Dmitry Fink, Eric B Munson, Bruce Ashfield, linux-kbuild On 11/20/2011 10:05 AM, Arnaud Lacombe wrote: > Hi, > > On Sun, Nov 20, 2011 at 7:56 AM, Michal Marek <mmarek@suse.cz> wrote: >> On 17.11.2011 22:58, john stultz wrote: >>> Hey Andrew, >>> I've tried sending this via Michal a few times, but haven't heard much >>> back. So I wanted to check if you would consider merging it via your >>> tree, or if you had any suggestions of who would be better to >>> review/merge this. >> >> My concern was that the script works around missing features in kconfig, >> instead of fixing kconfig. Don't get me wrong, I do believe that the >> script is useful, but if you are submitting to mainline anyway, why >> don't you fix/enhance kconfig in mainline and have a much simpler script >> making use of the enhanced kconfig? >> > FWIW, as much as I agree with you, kconfig is currently a bit too > static, with too much assumption, do any advanced stuff. I have WIP in > the area, but I prefer not giving ETA. In the mean time, I think > John's script could fill the gap. The script is also at least partly usable in older kernel trees without having to modify Kconfig there. It has some value as a standalone script. Not to say that we shouldn't also see about updating Kconfig as well. -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash 2011-11-17 21:58 [PATCH] kconfig: Add merge_config.sh script john stultz ` (2 preceding siblings ...) 2011-11-20 12:56 ` Michal Marek @ 2011-11-21 19:42 ` Darren Hart 2011-11-21 19:42 ` [PATCH 2/2] merge_config.sh: whitespace cleanup Darren Hart 2011-11-21 20:05 ` [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash John Stultz 3 siblings, 2 replies; 20+ messages in thread From: Darren Hart @ 2011-11-21 19:42 UTC (permalink / raw) To: Linux Kernel Mailing List Cc: John Stultz, Andrew Morton, Sam Ravnborg, gthelen, Dmitry Fink, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild, Richard Tartler, Darren Hart The SIGHUP SIGINT and SIGTERM names caused failures when running merge_config.sh with the dash shell. Dropping the "SIG" component makes the script work in both bash and dash. Signed-off-by: Darren Hart <dvhart@linux.intel.com> --- scripts/kconfig/merge_config.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh index 890276b..b91015d 100755 --- a/scripts/kconfig/merge_config.sh +++ b/scripts/kconfig/merge_config.sh @@ -24,7 +24,7 @@ clean_up() { rm -f $TMP_FILE exit } -trap clean_up SIGHUP SIGINT SIGTERM +trap clean_up HUP INT TERM usage() { echo "Usage: $0 [OPTIONS] [CONFIG [...]]" -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] merge_config.sh: whitespace cleanup 2011-11-21 19:42 ` [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash Darren Hart @ 2011-11-21 19:42 ` Darren Hart 2011-11-21 20:14 ` John Stultz 2011-11-21 20:05 ` [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash John Stultz 1 sibling, 1 reply; 20+ messages in thread From: Darren Hart @ 2011-11-21 19:42 UTC (permalink / raw) To: Linux Kernel Mailing List Cc: John Stultz, Andrew Morton, Sam Ravnborg, gthelen, Dmitry Fink, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild, Richard Tartler, Darren Hart Fix whitespace usage in the clean_up routine. Signed-off-by: Darren Hart <dvhart@linux.intel.com> --- scripts/kconfig/merge_config.sh | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh index b91015d..07bb886 100755 --- a/scripts/kconfig/merge_config.sh +++ b/scripts/kconfig/merge_config.sh @@ -21,8 +21,8 @@ # See the GNU General Public License for more details. clean_up() { - rm -f $TMP_FILE - exit + rm -f $TMP_FILE + exit } trap clean_up HUP INT TERM -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] merge_config.sh: whitespace cleanup 2011-11-21 19:42 ` [PATCH 2/2] merge_config.sh: whitespace cleanup Darren Hart @ 2011-11-21 20:14 ` John Stultz 0 siblings, 0 replies; 20+ messages in thread From: John Stultz @ 2011-11-21 20:14 UTC (permalink / raw) To: Darren Hart Cc: Linux Kernel Mailing List, Andrew Morton, Sam Ravnborg, gthelen, Dmitry Fink, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild, Richard Tartler On Mon, 2011-11-21 at 11:42 -0800, Darren Hart wrote: > Fix whitespace usage in the clean_up routine. > > Signed-off-by: Darren Hart <dvhart@linux.intel.com> Acked-by: John Stultz <john.stultz@linaro.org> > --- > scripts/kconfig/merge_config.sh | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh > index b91015d..07bb886 100755 > --- a/scripts/kconfig/merge_config.sh > +++ b/scripts/kconfig/merge_config.sh > @@ -21,8 +21,8 @@ > # See the GNU General Public License for more details. > > clean_up() { > - rm -f $TMP_FILE > - exit > + rm -f $TMP_FILE > + exit > } > trap clean_up HUP INT TERM > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash 2011-11-21 19:42 ` [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash Darren Hart 2011-11-21 19:42 ` [PATCH 2/2] merge_config.sh: whitespace cleanup Darren Hart @ 2011-11-21 20:05 ` John Stultz 1 sibling, 0 replies; 20+ messages in thread From: John Stultz @ 2011-11-21 20:05 UTC (permalink / raw) To: Darren Hart Cc: Linux Kernel Mailing List, Andrew Morton, Sam Ravnborg, gthelen, Dmitry Fink, Eric B Munson, Bruce Ashfield, Michal Marek, linux-kbuild, Richard Tartler On Mon, 2011-11-21 at 11:42 -0800, Darren Hart wrote: > The SIGHUP SIGINT and SIGTERM names caused failures when running > merge_config.sh with the dash shell. Dropping the "SIG" component > makes the script work in both bash and dash. > > Signed-off-by: Darren Hart <dvhart@linux.intel.com> Acked-by: John Stultz <john.stultz@linaro.org> > --- > scripts/kconfig/merge_config.sh | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh > index 890276b..b91015d 100755 > --- a/scripts/kconfig/merge_config.sh > +++ b/scripts/kconfig/merge_config.sh > @@ -24,7 +24,7 @@ clean_up() { > rm -f $TMP_FILE > exit > } > -trap clean_up SIGHUP SIGINT SIGTERM > +trap clean_up HUP INT TERM > > usage() { > echo "Usage: $0 [OPTIONS] [CONFIG [...]]" ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2011-11-22 6:25 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-17 21:58 [PATCH] kconfig: Add merge_config.sh script john stultz 2011-11-17 22:44 ` Arnaud Lacombe 2011-11-17 22:54 ` john stultz 2011-11-19 6:24 ` Arnaud Lacombe 2011-11-21 18:22 ` Darren Hart 2011-11-21 23:29 ` john stultz 2011-11-21 23:41 ` john stultz 2011-11-19 6:51 ` Arnaud Lacombe 2011-11-21 18:32 ` Darren Hart 2011-11-21 22:06 ` John Stultz 2011-11-21 22:48 ` john stultz 2011-11-21 22:55 ` john stultz 2011-11-22 6:25 ` Arnaud Lacombe 2011-11-20 12:56 ` Michal Marek 2011-11-20 18:05 ` Arnaud Lacombe 2011-11-21 18:34 ` Darren Hart 2011-11-21 19:42 ` [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash Darren Hart 2011-11-21 19:42 ` [PATCH 2/2] merge_config.sh: whitespace cleanup Darren Hart 2011-11-21 20:14 ` John Stultz 2011-11-21 20:05 ` [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash John Stultz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).