linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] scripts/kconfig/merge_config: don't redefine 'y' to 'm'
@ 2018-11-02 11:41 Anders Roxell
  2018-11-05  8:34 ` Masahiro Yamada
  2018-11-07 19:36 ` Darren Hart
  0 siblings, 2 replies; 6+ messages in thread
From: Anders Roxell @ 2018-11-02 11:41 UTC (permalink / raw)
  To: yamada.masahiro; +Cc: linux-kbuild, linux-kernel, dvhart, arnd, Anders Roxell

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

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts/kconfig/merge_config: don't redefine 'y' to 'm'
  2018-11-02 11:41 [PATCH] scripts/kconfig/merge_config: don't redefine 'y' to 'm' Anders Roxell
@ 2018-11-05  8:34 ` Masahiro Yamada
  2018-11-06 13:57   ` Anders Roxell
  2018-11-07 19:36 ` Darren Hart
  1 sibling, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2018-11-05  8:34 UTC (permalink / raw)
  To: Anders Roxell
  Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, dvhart,
	Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts/kconfig/merge_config: don't redefine 'y' to 'm'
  2018-11-05  8:34 ` Masahiro Yamada
@ 2018-11-06 13:57   ` Anders Roxell
  2018-11-07 18:47     ` Darren Hart
  0 siblings, 1 reply; 6+ messages in thread
From: Anders Roxell @ 2018-11-06 13:57 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Linux Kernel Mailing List, dvhart, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts/kconfig/merge_config: don't redefine 'y' to 'm'
  2018-11-06 13:57   ` Anders Roxell
@ 2018-11-07 18:47     ` Darren Hart
  0 siblings, 0 replies; 6+ messages in thread
From: Darren Hart @ 2018-11-07 18:47 UTC (permalink / raw)
  To: Anders Roxell
  Cc: Masahiro Yamada, linux-kbuild, Linux Kernel Mailing List, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts/kconfig/merge_config: don't redefine 'y' to 'm'
  2018-11-02 11:41 [PATCH] scripts/kconfig/merge_config: don't redefine 'y' to 'm' Anders Roxell
  2018-11-05  8:34 ` Masahiro Yamada
@ 2018-11-07 19:36 ` Darren Hart
  2018-11-08 19:43   ` Anders Roxell
  1 sibling, 1 reply; 6+ messages in thread
From: Darren Hart @ 2018-11-07 19:36 UTC (permalink / raw)
  To: Anders Roxell; +Cc: yamada.masahiro, linux-kbuild, linux-kernel, arnd

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts/kconfig/merge_config: don't redefine 'y' to 'm'
  2018-11-07 19:36 ` Darren Hart
@ 2018-11-08 19:43   ` Anders Roxell
  0 siblings, 0 replies; 6+ messages in thread
From: Anders Roxell @ 2018-11-08 19:43 UTC (permalink / raw)
  To: dvhart
  Cc: Masahiro Yamada, linux-kbuild, Linux Kernel Mailing List, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-11-08 19:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-02 11:41 [PATCH] scripts/kconfig/merge_config: don't redefine 'y' to 'm' Anders Roxell
2018-11-05  8:34 ` Masahiro Yamada
2018-11-06 13:57   ` Anders Roxell
2018-11-07 18:47     ` Darren Hart
2018-11-07 19:36 ` Darren Hart
2018-11-08 19:43   ` Anders Roxell

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