[PATCH 1/1] merge_config.sh: Replace prefix CONFIG_ with any prefix

[PATCH 1/1] merge_config.sh: Replace prefix CONFIG_ with any prefix

[Petr Vorel]

merge_config.sh expect prefix from kernel: CONFIG_.
There some other projects using kconfig with different prefixes
(e.g. buildroot: BR2_ prefix). While there could be some option
to specify config prefix [1] [2], IMHO it's better to not specify
prefix (expect just any prefix).

[1] http://patchwork.ozlabs.org/patch/824051/
[2] https://patchwork.ozlabs.org/patch/988890/

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
Suggested-by: Marcel Patzlaff <m.patzlaff@pilz.de>
---
NOTE: I deliberately left prefix CONFIG_ in comment:
# allnoconfig: Fills in any missing symbols with # CONFIG_* is not set

Kind regards,
Petr
---
 scripts/kconfig/merge_config.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 67d131447631..538222848bcf 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -99,7 +99,7 @@ if [ ! -r "$INITFILE" ]; then
 fi
 
 MERGE_LIST=$*
-SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
+SED_CONFIG_EXP="s/^\(# \)\{0,1\}\([A-Z0-9_]\+_[a-zA-Z0-9_]*\)[= ].*/\2/p"
 TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
 
 echo "Using $INITFILE as base"
-- 
2.19.1

Re: [PATCH 1/1] merge_config.sh: Replace prefix CONFIG_ with any prefix

[Masahiro Yamada]

On Sun, Oct 28, 2018 at 9:55 PM Petr Vorel <petr.vorel@gmail.com> wrote:
>
> merge_config.sh expect prefix from kernel: CONFIG_.
> There some other projects using kconfig with different prefixes
> (e.g. buildroot: BR2_ prefix). While there could be some option
> to specify config prefix [1] [2], IMHO it's better to not specify
> prefix (expect just any prefix).
>
> [1] http://patchwork.ozlabs.org/patch/824051/
> [2] https://patchwork.ozlabs.org/patch/988890/
>
> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> Suggested-by: Marcel Patzlaff <m.patzlaff@pilz.de>
> ---
> NOTE: I deliberately left prefix CONFIG_ in comment:
> # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set


I'd like to filter "CONFIG_", "BR2_", or whatever specified prefix.

Some fragment files (e.g. arch/x86/configs/xen.config)
have comment lines.

Matching '# ' + arbitrary string
might hit comment lines accidentally.



The kconfig binary checks the environment variable 'CONFIG_'
to override the default prefix:
https://github.com/torvalds/linux/blob/v4.19/scripts/kconfig/lkc.h#L28


For similarity, how about doing in the same way in merge_config.sh ?


Does the following work for you ?
(you can pass CONFIG_=BR2_ to merge_config.sh)




diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 67d1314..391ee6c 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -90,6 +90,10 @@ if [ -z "$KCONFIG_CONFIG" ]; then
        fi
 fi

+# Check the environment variable "CONFIG_" for the config option prefix.
+# If unset, the default is "CONFIG_".
+: ${CONFIG_=CONFIG_}
+
 INITFILE=$1
 shift;

@@ -99,7 +103,7 @@ if [ ! -r "$INITFILE" ]; then
 fi

 MERGE_LIST=$*
-SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
+SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(${CONFIG_}[a-zA-Z0-9_]*\)[= ].*/\2/p"
 TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)

 echo "Using $INITFILE as base"






--
Best Regards
Masahiro Yamada

Re: [PATCH 1/1] merge_config.sh: Replace prefix CONFIG_ with any prefix

[Petr Vorel]

Hi Masahiro,

thanks for a quick review and patch.

> I'd like to filter "CONFIG_", "BR2_", or whatever specified prefix.

> Some fragment files (e.g. arch/x86/configs/xen.config)
> have comment lines.

> Matching '# ' + arbitrary string
> might hit comment lines accidentally.
Make sense.

> The kconfig binary checks the environment variable 'CONFIG_'
> to override the default prefix:
> https://github.com/torvalds/linux/blob/v4.19/scripts/kconfig/lkc.h#L28
What a crazy variable name. But we're not going to change it.

> For similarity, how about doing in the same way in merge_config.sh ?
Make sense to keep the same approach.

> Does the following work for you ?
> (you can pass CONFIG_=BR2_ to merge_config.sh)
Patch is good (with one note bellow).

Reviewed-by: Petr Vorel <petr.vorel@gmail.com>

Could you push your patch into your tree?
Or going to post a patch? If not here is mine:


> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> index 67d1314..391ee6c 100755
> --- a/scripts/kconfig/merge_config.sh
> +++ b/scripts/kconfig/merge_config.sh
> @@ -90,6 +90,10 @@ if [ -z "$KCONFIG_CONFIG" ]; then
>         fi
>  fi

> +# Check the environment variable "CONFIG_" for the config option prefix.
> +# If unset, the default is "CONFIG_".
> +: ${CONFIG_=CONFIG_}
Although I guess this could be less cryptic:
CONFIG_="${CONFIG_:-CONFIG_}"
but too verbose, so I'm for your variant.

...
>  MERGE_LIST=$*
> -SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
> +SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(${CONFIG_}[a-zA-Z0-9_]*\)[= ].*/\2/p"
>  TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)

Kind regards,
Petr

Re: [PATCH 1/1] merge_config.sh: Replace prefix CONFIG_ with any prefix

[Masahiro Yamada]

Hi Petr,

On Tue, Oct 30, 2018 at 12:40 AM Petr Vorel <petr.vorel@gmail.com> wrote:
>
> Hi Masahiro,
>
> thanks for a quick review and patch.
>
> > I'd like to filter "CONFIG_", "BR2_", or whatever specified prefix.
>
> > Some fragment files (e.g. arch/x86/configs/xen.config)
> > have comment lines.
>
> > Matching '# ' + arbitrary string
> > might hit comment lines accidentally.
> Make sense.
>
> > The kconfig binary checks the environment variable 'CONFIG_'
> > to override the default prefix:
> > https://github.com/torvalds/linux/blob/v4.19/scripts/kconfig/lkc.h#L28
> What a crazy variable name. But we're not going to change it.
>
> > For similarity, how about doing in the same way in merge_config.sh ?
> Make sense to keep the same approach.
>
> > Does the following work for you ?
> > (you can pass CONFIG_=BR2_ to merge_config.sh)
> Patch is good (with one note bellow).
>
> Reviewed-by: Petr Vorel <petr.vorel@gmail.com>
>
> Could you push your patch into your tree?
> Or going to post a patch? If not here is mine:


Please send v2 with the commit log properly filled.

I will take my credit as Suggested-by.



> > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> > index 67d1314..391ee6c 100755
> > --- a/scripts/kconfig/merge_config.sh
> > +++ b/scripts/kconfig/merge_config.sh
> > @@ -90,6 +90,10 @@ if [ -z "$KCONFIG_CONFIG" ]; then
> >         fi
> >  fi
>
> > +# Check the environment variable "CONFIG_" for the config option prefix.
> > +# If unset, the default is "CONFIG_".
> > +: ${CONFIG_=CONFIG_}
> Although I guess this could be less cryptic:
> CONFIG_="${CONFIG_:-CONFIG_}"

You should not use the colon
because we want to allow the environment variable
'CONFIG_' being set as empty.

In fact, Buildroot uses empty prefix instead of "BR2_".
In my understanding, Buildroot just has a convention where
symbols in Config.in files start with "BR2_".


So, in order to make your idea even less cryptic,
how about this?


CONFIG_PREFIX=${CONFIG_-CONFIG_}

Then,

SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"








-- 
Best Regards
Masahiro Yamada

Re: [PATCH 1/1] merge_config.sh: Replace prefix CONFIG_ with any prefix

[Petr Vorel]

Hi Masahiro,

> Please send v2 with the commit log properly filled.
OK I'll do.

> I will take my credit as Suggested-by.
Sure :).

> > > +# Check the environment variable "CONFIG_" for the config option prefix.
> > > +# If unset, the default is "CONFIG_".
> > > +: ${CONFIG_=CONFIG_}
> > Although I guess this could be less cryptic:
> > CONFIG_="${CONFIG_:-CONFIG_}"

> You should not use the colon
> because we want to allow the environment variable
> 'CONFIG_' being set as empty.
How about having default 'CONFIG_' and if it's empty change it to
'[A-Z0-9_]\+' ? I know, it's different from C definition in lkc.h, but IMHO it'd
make sense. But I guess you prefer your suggestion CONFIG_PREFIX=${CONFIG_-CONFIG_}

> In fact, Buildroot uses empty prefix instead of "BR2_".
> In my understanding, Buildroot just has a convention where
> symbols in Config.in files start with "BR2_".
IMHO buildroot uses 'BR2_' prefix. That's what we want to use with
merge_config.sh.


Kind regards,
Petr

Re: [PATCH 1/1] merge_config.sh: Replace prefix CONFIG_ with any prefix

[Masahiro Yamada]

On Tue, Oct 30, 2018 at 3:27 AM Petr Vorel <petr.vorel@gmail.com> wrote:
>
> Hi Masahiro,
>
> > Please send v2 with the commit log properly filled.
> OK I'll do.
>
> > I will take my credit as Suggested-by.
> Sure :).
>
> > > > +# Check the environment variable "CONFIG_" for the config option prefix.
> > > > +# If unset, the default is "CONFIG_".
> > > > +: ${CONFIG_=CONFIG_}
> > > Although I guess this could be less cryptic:
> > > CONFIG_="${CONFIG_:-CONFIG_}"
>
> > You should not use the colon
> > because we want to allow the environment variable
> > 'CONFIG_' being set as empty.
> How about having default 'CONFIG_' and if it's empty change it to
> '[A-Z0-9_]\+' ?

No, I'd like to have the same rule for prefix.



> I know, it's different from C definition in lkc.h, but IMHO it'd
> make sense. But I guess you prefer your suggestion CONFIG_PREFIX=${CONFIG_-CONFIG_}

Yeah, I prefer this.


> > In fact, Buildroot uses empty prefix instead of "BR2_".
> > In my understanding, Buildroot just has a convention where
> > symbols in Config.in files start with "BR2_".
> IMHO buildroot uses 'BR2_' prefix. That's what we want to use with
> merge_config.sh.

Yes, you can use 'BR2_' prefix for merge_config.

I just wanted to point out that
Buildroot's own patch specifies the prefix as empty
wrt the kconfig binary.

https://github.com/buildroot/buildroot/blob/2018.08.2/support/kconfig/patches/10-br-build-system.patch#L32



>
> Kind regards,
> Petr



-- 
Best Regards
Masahiro Yamada