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@xxxxxxxxxx> 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@xxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
