| Message ID | 20181102114119.23859-1-anders.roxell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | scripts/kconfig/merge_config: don't redefine 'y' to 'm' | expand |
Hi Anders, On Fri, Nov 2, 2018 at 8:41 PM Anders Roxell <anders.roxell@linaro.org> wrote: > > In today's merge_config.sh the order of the config fragment files dictates > the output of a config option. With this approach we will get different > .config files depending on the order of the config fragment files. > Adding a switch to add precedence for builtin over modules, this will > make the .config file the same > > Suggested-by: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > --- I think this patch makes sense. Just in case, could you please provide me the context of the discussion? Does the real problem exist in the kernel tree, or for local fragment files? Thanks. > scripts/kconfig/merge_config.sh | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh > index da66e7742282..902a60b45614 100755 > --- a/scripts/kconfig/merge_config.sh > +++ b/scripts/kconfig/merge_config.sh > @@ -32,6 +32,7 @@ usage() { > echo " -m only merge the fragments, do not execute the make command" > echo " -n use allnoconfig instead of alldefconfig" > echo " -r list redundant entries when merging fragments" > + echo " -y make builtin have precedence over modules" > echo " -O dir to put generated output files. Consider setting \$KCONFIG_CONFIG instead." > echo > echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_ environment variable." > @@ -40,6 +41,7 @@ usage() { > RUNMAKE=true > ALLTARGET=alldefconfig > WARNREDUN=false > +BUILTIN=false > OUTPUT=. > CONFIG_PREFIX=${CONFIG_-CONFIG_} > > @@ -64,6 +66,11 @@ while true; do > shift > continue > ;; > + "-y") > + BUILTIN=true > + shift > + continue > + ;; > "-O") > if [ -d $2 ];then > OUTPUT=$(echo $2 | sed 's/\/*$//') > @@ -122,7 +129,13 @@ for MERGE_FILE in $MERGE_LIST ; do > grep -q -w $CFG $TMP_FILE || continue > PREV_VAL=$(grep -w $CFG $TMP_FILE) > NEW_VAL=$(grep -w $CFG $MERGE_FILE) > - if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then > + if test "$BUILTIN" = "true" && echo $PREV_VAL |grep -Eq '^\w+=y' && echo $NEW_VAL |grep -Eq '^\w+=m' ; then > + echo Value of $CFG is \'y\' and we don\'t want to redefine the fragment $MERGE_FILE: > + echo Previous value: $PREV_VAL > + echo New value: $NEW_VAL > + echo Will use previous value. > + echo > + elif [ "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 > -- > 2.11.0 > -- Best Regards Masahiro Yamada
On Mon, 5 Nov 2018 at 09:35, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > Hi Anders, > > On Fri, Nov 2, 2018 at 8:41 PM Anders Roxell <anders.roxell@linaro.org> wrote: > > > > In today's merge_config.sh the order of the config fragment files dictates > > the output of a config option. With this approach we will get different > > .config files depending on the order of the config fragment files. > > Adding a switch to add precedence for builtin over modules, this will > > make the .config file the same > > > > Suggested-by: Arnd Bergmann <arnd@arndb.de> > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > --- > > I think this patch makes sense. > > Just in case, could you please provide me the context of the discussion? For instance we don't want to force X86 from DRM=y to DRM=m, when enabling selftest, that would surely break somebody's setup and you also don't want to force ARM64 from DRM=m to DRM=y, that seems unnecessary for a big subsystem like DRM. > > Does the real problem exist in the kernel tree, Not that I'm aware about. Cheers, Anders > or for local fragment files? > > > Thanks. > > > > > scripts/kconfig/merge_config.sh | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh > > index da66e7742282..902a60b45614 100755 > > --- a/scripts/kconfig/merge_config.sh > > +++ b/scripts/kconfig/merge_config.sh > > @@ -32,6 +32,7 @@ usage() { > > echo " -m only merge the fragments, do not execute the make command" > > echo " -n use allnoconfig instead of alldefconfig" > > echo " -r list redundant entries when merging fragments" > > + echo " -y make builtin have precedence over modules" > > echo " -O dir to put generated output files. Consider setting \$KCONFIG_CONFIG instead." > > echo > > echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_ environment variable." > > @@ -40,6 +41,7 @@ usage() { > > RUNMAKE=true > > ALLTARGET=alldefconfig > > WARNREDUN=false > > +BUILTIN=false > > OUTPUT=. > > CONFIG_PREFIX=${CONFIG_-CONFIG_} > > > > @@ -64,6 +66,11 @@ while true; do > > shift > > continue > > ;; > > + "-y") > > + BUILTIN=true > > + shift > > + continue > > + ;; > > "-O") > > if [ -d $2 ];then > > OUTPUT=$(echo $2 | sed 's/\/*$//') > > @@ -122,7 +129,13 @@ for MERGE_FILE in $MERGE_LIST ; do > > grep -q -w $CFG $TMP_FILE || continue > > PREV_VAL=$(grep -w $CFG $TMP_FILE) > > NEW_VAL=$(grep -w $CFG $MERGE_FILE) > > - if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then > > + if test "$BUILTIN" = "true" && echo $PREV_VAL |grep -Eq '^\w+=y' && echo $NEW_VAL |grep -Eq '^\w+=m' ; then > > + echo Value of $CFG is \'y\' and we don\'t want to redefine the fragment $MERGE_FILE: > > + echo Previous value: $PREV_VAL > > + echo New value: $NEW_VAL > > + echo Will use previous value. > > + echo > > + elif [ "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 > > -- > > 2.11.0 > > > > > -- > Best Regards > Masahiro Yamada
On Tue, Nov 06, 2018 at 02:57:40PM +0100, Anders Roxell wrote: > On Mon, 5 Nov 2018 at 09:35, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > Hi Anders, > > > > On Fri, Nov 2, 2018 at 8:41 PM Anders Roxell <anders.roxell@linaro.org> wrote: > > > > > > In today's merge_config.sh the order of the config fragment files dictates > > > the output of a config option. With this approach we will get different > > > .config files depending on the order of the config fragment files. > > > Adding a switch to add precedence for builtin over modules, this will > > > make the .config file the same > > > > > > Suggested-by: Arnd Bergmann <arnd@arndb.de> > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > > --- > > > > I think this patch makes sense. > > > > Just in case, could you please provide me the context of the discussion? > > For instance we don't want to force X86 from DRM=y to DRM=m, when > enabling selftest, that would surely break somebody's setup and you also > don't want to force ARM64 from DRM=m to DRM=y, that seems > unnecessary for a big subsystem like DRM. > > > > > Does the real problem exist in the kernel tree, > > Not that I'm aware about. > > Cheers, > Anders > > > or for local fragment files? This is really about ordering of fragment files and not "making a symbol 'less'", I think were Arnd's words. So, doing something like: $ make arm64-selftest.config drm.config where arm64-selftest.config defines DRM=y and drm.config defines DRM=m, the result should be "DRM=y". So the first step is to make merge_config.sh support it. A follow up step would be to integrate this into the kernel Makefile system if we determine this is the correct behavior, or to provide a parameter if we don't want to change the default behavior. -- Darren Hart VMware Open Source Technology Center
On Fri, Nov 02, 2018 at 12:41:19PM +0100, Anders Roxell wrote: > In today's merge_config.sh the order of the config fragment files dictates > the output of a config option. With this approach we will get different > .config files depending on the order of the config fragment files. > Adding a switch to add precedence for builtin over modules, this will > make the .config file the same > > Suggested-by: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> Thanks for the patch Anders! > --- > scripts/kconfig/merge_config.sh | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh > index da66e7742282..902a60b45614 100755 > --- a/scripts/kconfig/merge_config.sh > +++ b/scripts/kconfig/merge_config.sh > @@ -32,6 +32,7 @@ usage() { > echo " -m only merge the fragments, do not execute the make command" > echo " -n use allnoconfig instead of alldefconfig" > echo " -r list redundant entries when merging fragments" > + echo " -y make builtin have precedence over modules" > echo " -O dir to put generated output files. Consider setting \$KCONFIG_CONFIG instead." > echo > echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_ environment variable." > @@ -40,6 +41,7 @@ usage() { > RUNMAKE=true > ALLTARGET=alldefconfig > WARNREDUN=false > +BUILTIN=false > OUTPUT=. > CONFIG_PREFIX=${CONFIG_-CONFIG_} > > @@ -64,6 +66,11 @@ while true; do > shift > continue > ;; > + "-y") > + BUILTIN=true > + shift > + continue > + ;; > "-O") > if [ -d $2 ];then > OUTPUT=$(echo $2 | sed 's/\/*$//') > @@ -122,7 +129,13 @@ for MERGE_FILE in $MERGE_LIST ; do > grep -q -w $CFG $TMP_FILE || continue > PREV_VAL=$(grep -w $CFG $TMP_FILE) > NEW_VAL=$(grep -w $CFG $MERGE_FILE) > - if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then > + if test "$BUILTIN" = "true" && echo $PREV_VAL |grep -Eq '^\w+=y' && echo $NEW_VAL |grep -Eq '^\w+=m' ; then I think we can avoid the rather lengthy/forky "echo | grep" mechanism with POSIX substring parameter expansion (should work in bash, dash, etc.). http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_02 We can also be more consistent with the other tests in the script, consider: if [ "$BUILTIN" = "true" ] && [ "${NEW_VAL#CONFIG_*=}" = "m" ] && [ "${PREV_VAL#CONFIG_*=}" = "y" ]; then > + echo Value of $CFG is \'y\' and we don\'t want to redefine the fragment $MERGE_FILE: I think we can drop the above and perhaps update the last line with... > + echo Previous value: $PREV_VAL > + echo New value: $NEW_VAL > + echo Will use previous value. echo "-y passed, will not demote y to m" Or something along those lines. > + echo > + elif [ "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 > -- > 2.11.0 > > -- Darren Hart VMware Open Source Technology Center
On Wed, 7 Nov 2018 at 20:36, Darren Hart <dvhart@infradead.org> wrote: > > On Fri, Nov 02, 2018 at 12:41:19PM +0100, Anders Roxell wrote: > > In today's merge_config.sh the order of the config fragment files dictates > > the output of a config option. With this approach we will get different > > .config files depending on the order of the config fragment files. > > Adding a switch to add precedence for builtin over modules, this will > > make the .config file the same > > > > Suggested-by: Arnd Bergmann <arnd@arndb.de> > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > Thanks for the patch Anders! > > > --- > > scripts/kconfig/merge_config.sh | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh > > index da66e7742282..902a60b45614 100755 > > --- a/scripts/kconfig/merge_config.sh > > +++ b/scripts/kconfig/merge_config.sh > > @@ -32,6 +32,7 @@ usage() { > > echo " -m only merge the fragments, do not execute the make command" > > echo " -n use allnoconfig instead of alldefconfig" > > echo " -r list redundant entries when merging fragments" > > + echo " -y make builtin have precedence over modules" > > echo " -O dir to put generated output files. Consider setting \$KCONFIG_CONFIG instead." > > echo > > echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_ environment variable." > > @@ -40,6 +41,7 @@ usage() { > > RUNMAKE=true > > ALLTARGET=alldefconfig > > WARNREDUN=false > > +BUILTIN=false > > OUTPUT=. > > CONFIG_PREFIX=${CONFIG_-CONFIG_} > > > > @@ -64,6 +66,11 @@ while true; do > > shift > > continue > > ;; > > + "-y") > > + BUILTIN=true > > + shift > > + continue > > + ;; > > "-O") > > if [ -d $2 ];then > > OUTPUT=$(echo $2 | sed 's/\/*$//') > > @@ -122,7 +129,13 @@ for MERGE_FILE in $MERGE_LIST ; do > > grep -q -w $CFG $TMP_FILE || continue > > PREV_VAL=$(grep -w $CFG $TMP_FILE) > > NEW_VAL=$(grep -w $CFG $MERGE_FILE) > > - if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then > > + if test "$BUILTIN" = "true" && echo $PREV_VAL |grep -Eq '^\w+=y' && echo $NEW_VAL |grep -Eq '^\w+=m' ; then > > I think we can avoid the rather lengthy/forky "echo | grep" mechanism > with POSIX substring parameter expansion (should work in bash, dash, > etc.). > > http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_02 > > We can also be more consistent with the other tests in the script, consider: > > if [ "$BUILTIN" = "true" ] && [ "${NEW_VAL#CONFIG_*=}" = "m" ] && [ "${PREV_VAL#CONFIG_*=}" = "y" ]; then > > > > + echo Value of $CFG is \'y\' and we don\'t want to redefine the fragment $MERGE_FILE: > > I think we can drop the above and perhaps update the last line with... > > > + echo Previous value: $PREV_VAL > > + echo New value: $NEW_VAL > > + echo Will use previous value. > > echo "-y passed, will not demote y to m" > > Or something along those lines. Thank you for your input, I'll update and send a v2 shortly. Cheers, Anders > > > + echo > > + elif [ "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 > > -- > > 2.11.0 > > > > > > -- > Darren Hart > VMware Open Source Technology Center
diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh index da66e7742282..902a60b45614 100755 --- a/scripts/kconfig/merge_config.sh +++ b/scripts/kconfig/merge_config.sh @@ -32,6 +32,7 @@ usage() { echo " -m only merge the fragments, do not execute the make command" echo " -n use allnoconfig instead of alldefconfig" echo " -r list redundant entries when merging fragments" + echo " -y make builtin have precedence over modules" echo " -O dir to put generated output files. Consider setting \$KCONFIG_CONFIG instead." echo echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_ environment variable." @@ -40,6 +41,7 @@ usage() { RUNMAKE=true ALLTARGET=alldefconfig WARNREDUN=false +BUILTIN=false OUTPUT=. CONFIG_PREFIX=${CONFIG_-CONFIG_} @@ -64,6 +66,11 @@ while true; do shift continue ;; + "-y") + BUILTIN=true + shift + continue + ;; "-O") if [ -d $2 ];then OUTPUT=$(echo $2 | sed 's/\/*$//') @@ -122,7 +129,13 @@ for MERGE_FILE in $MERGE_LIST ; do grep -q -w $CFG $TMP_FILE || continue PREV_VAL=$(grep -w $CFG $TMP_FILE) NEW_VAL=$(grep -w $CFG $MERGE_FILE) - if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then + if test "$BUILTIN" = "true" && echo $PREV_VAL |grep -Eq '^\w+=y' && echo $NEW_VAL |grep -Eq '^\w+=m' ; then + echo Value of $CFG is \'y\' and we don\'t want to redefine the fragment $MERGE_FILE: + echo Previous value: $PREV_VAL + echo New value: $NEW_VAL + echo Will use previous value. + echo + elif [ "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
In today's merge_config.sh the order of the config fragment files dictates the output of a config option. With this approach we will get different .config files depending on the order of the config fragment files. Adding a switch to add precedence for builtin over modules, this will make the .config file the same Suggested-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> --- scripts/kconfig/merge_config.sh | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) -- 2.11.0