[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Nasser]

We use BR2_* style for configuration variables in buildroot so we should
use this style when extracting configuration options. Otherwise CFG_LIST
will almost always be empty.

The CONFIG_* style has been taken form the Linux kernel and is not
appropriate in this context.

Signed-off-by: Nasser <Afshin.Nasser@gmail.com>
---

If you add 'set -x' in the beginning of the script you can see that $CFG_LIST
is always empty.

When having redundancy in the configuration fragments, you can see that the
$TMP_FILE file has redundant configuration options when calling 'make'. Note
that after applying this patch, calling 'make' in this script, will not produce
any warnings due to redundancy issues anymore. This is because now repeated
options are all properly omitted before calling 'make'. After applying this
patch '-r -m' options work as expected.

Further more, note that without this patch, any inconsistent re-assignment is
not reported by this script. Although there are some warnings if you did not
specify -m option. They are all 'make' reports.

 support/kconfig/merge_config.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/kconfig/merge_config.sh b/support/kconfig/merge_config.sh
index 67d1314..87a8d02 100755
--- a/support/kconfig/merge_config.sh
+++ b/support/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\}\(BR2_[a-zA-Z0-9_]*\)[= ].*/\2/p"
 TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
 
 echo "Using $INITFILE as base"
-- 
2.7.4

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Matthew Weber]

Nasser,

On Fri, Oct 19, 2018 at 11:58 PM Nasser <afshin.nasser@gmail.com> wrote:
>
> We use BR2_* style for configuration variables in buildroot so we should
> use this style when extracting configuration options. Otherwise CFG_LIST
> will almost always be empty.
>
> The CONFIG_* style has been taken form the Linux kernel and is not
> appropriate in this context.
>

The merge_config.sh is used for a couple scenarios
 - Appending kconfigs together (CONFIG_*)
 - Buildroot cfgs for runtime tests (BR2_*)
 - As a tool by users to merge together Buildroot configs

I'm not sure of the cleanest approach to support both
 - you could detect if the file is one or the other and adjust the regex
 - do the inverse and build a list of lines that are not comments

Matt

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Arnout Vandecappelle]

On 20/10/2018 15:56, Matthew Weber wrote:
> Nasser,
> 
> On Fri, Oct 19, 2018 at 11:58 PM Nasser <afshin.nasser@gmail.com> wrote:
>>
>> We use BR2_* style for configuration variables in buildroot so we should
>> use this style when extracting configuration options. Otherwise CFG_LIST
>> will almost always be empty.
>>
>> The CONFIG_* style has been taken form the Linux kernel and is not
>> appropriate in this context.

 A similar patch was actually submitted earlier by Angelo [1]. I commented on
that that it should be simplified like you propose. However...

>>
> 
> The merge_config.sh is used for a couple scenarios
>  - Appending kconfigs together (CONFIG_*)

 ... I forgot about this one. Indeed, the buildroot merge_config.sh script is
used in pkg-kconfig.mk to merge kernel configs. Ideally we should change that to
use the package's merge_config.sh script. However, the location of that script
may vary, or it may even be missing...

>  - Buildroot cfgs for runtime tests (BR2_*)
>  - As a tool by users to merge together Buildroot configs
> 
> I'm not sure of the cleanest approach to support both
>  - you could detect if the file is one or the other and adjust the regex
>  - do the inverse and build a list of lines that are not comments

 I think Angelo's patch is the best approach after all.

 However, there is one comment that I made on Angelo's patch that still applies
here:

 Since the kconfig stuff comes from upstream but is modified, we also maintain
the changes as a stack of patches in support/kconfig/patches. So you should
generate a new patch for this change and add it to the series file. I'm not sure
why we don't use a vendor branch and just merge, but that's the way we do it :-).


 Also, Nasser, we require your Signed-off-by to contain your full name. The
Signed-off-by is a short way for you to assert that you are entitled to
contribute the patch under buildroot's GPL license.  See
http://elinux.org/Developer_Certificate_Of_Origin for more details. Itis an
official statement, so it should be done with your full and real name. And
please make sure that the author information matches the Signed-off-by.


 Regards,
 Arnout

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

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Petr Vorel]

Hi,

> > The merge_config.sh is used for a couple scenarios
> >  - Appending kconfigs together (CONFIG_*)

>  ... I forgot about this one. Indeed, the buildroot merge_config.sh script is
> used in pkg-kconfig.mk to merge kernel configs. Ideally we should change that to
> use the package's merge_config.sh script. However, the location of that script
> may vary, or it may even be missing...

> >  - Buildroot cfgs for runtime tests (BR2_*)
> >  - As a tool by users to merge together Buildroot configs

> > I'm not sure of the cleanest approach to support both
> >  - you could detect if the file is one or the other and adjust the regex
> >  - do the inverse and build a list of lines that are not comments

>  I think Angelo's patch is the best approach after all.
+1. I also think keep using our merge_config.sh with -b switch from Angelo's
patch [1] is the way.

>  However, there is one comment that I made on Angelo's patch that still applies
> here:

>  Since the kconfig stuff comes from upstream but is modified, we also maintain
> the changes as a stack of patches in support/kconfig/patches. So you should
> generate a new patch for this change and add it to the series file. I'm not sure
> why we don't use a vendor branch and just merge, but that's the way we do it :-).
IMHO it's a common practice across distributions packaging to use quilt for own
patches. I guess it's to help show easily distro changes (in our case patches in
support/kconfig/patches/ directory are in tarballs, not just in git).


Kind regards,
Petr

>  Regards,
>  Arnout

> [1] http://patchwork.ozlabs.org/patch/824051/

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Matthew Weber]

Petr,

On Sun, Oct 21, 2018 at 6:27 PM Petr Vorel <petr.vorel@gmail.com> wrote:
>
> Hi,
>
> > > The merge_config.sh is used for a couple scenarios
> > >  - Appending kconfigs together (CONFIG_*)
>
> >  ... I forgot about this one. Indeed, the buildroot merge_config.sh script is
> > used in pkg-kconfig.mk to merge kernel configs. Ideally we should change that to
> > use the package's merge_config.sh script. However, the location of that script
> > may vary, or it may even be missing...
>
> > >  - Buildroot cfgs for runtime tests (BR2_*)
> > >  - As a tool by users to merge together Buildroot configs
>
> > > I'm not sure of the cleanest approach to support both
> > >  - you could detect if the file is one or the other and adjust the regex
> > >  - do the inverse and build a list of lines that are not comments
>
> >  I think Angelo's patch is the best approach after all.
> +1. I also think keep using our merge_config.sh with -b switch from Angelo's
> patch [1] is the way.

Do you think you'll be able to provide an updated patch based on Angelo's work?

>
> >  However, there is one comment that I made on Angelo's patch that still applies
> > here:
>
> >  Since the kconfig stuff comes from upstream but is modified, we also maintain
> > the changes as a stack of patches in support/kconfig/patches. So you should
> > generate a new patch for this change and add it to the series file. I'm not sure
> > why we don't use a vendor branch and just merge, but that's the way we do it :-).
> IMHO it's a common practice across distributions packaging to use quilt for own
> patches. I guess it's to help show easily distro changes (in our case patches in
> support/kconfig/patches/ directory are in tarballs, not just in git).
>
>

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Petr Vorel]

Hi Matthew, Angelo,

> Petr,

> On Sun, Oct 21, 2018 at 6:27 PM Petr Vorel <petr.vorel@gmail.com> wrote:

> > Hi,

> > > > The merge_config.sh is used for a couple scenarios
> > > >  - Appending kconfigs together (CONFIG_*)

> > >  ... I forgot about this one. Indeed, the buildroot merge_config.sh script is
> > > used in pkg-kconfig.mk to merge kernel configs. Ideally we should change that to
> > > use the package's merge_config.sh script. However, the location of that script
> > > may vary, or it may even be missing...

> > > >  - Buildroot cfgs for runtime tests (BR2_*)
> > > >  - As a tool by users to merge together Buildroot configs

> > > > I'm not sure of the cleanest approach to support both
> > > >  - you could detect if the file is one or the other and adjust the regex
> > > >  - do the inverse and build a list of lines that are not comments

> > >  I think Angelo's patch is the best approach after all.
> > +1. I also think keep using our merge_config.sh with -b switch from Angelo's
> > patch [1] is the way.

> Do you think you'll be able to provide an updated patch based on Angelo's work?
Matthew, sure I can have look into it. Unless Angelo is planning to finish it.

Kind regards,
Petr

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Nasser Afshin]

Hi Arnout,

On Sat, Oct 20, 2018 at 05:01:33PM +0100, Arnout Vandecappelle wrote:
> 
> 
> On 20/10/2018 15:56, Matthew Weber wrote:
> > Nasser,
> > 
> > On Fri, Oct 19, 2018 at 11:58 PM Nasser <afshin.nasser@gmail.com> wrote:
> >>
> >> We use BR2_* style for configuration variables in buildroot so we should
> >> use this style when extracting configuration options. Otherwise CFG_LIST
> >> will almost always be empty.
> >>
> >> The CONFIG_* style has been taken form the Linux kernel and is not
> >> appropriate in this context.
> 
>  A similar patch was actually submitted earlier by Angelo [1]. I commented on
> that that it should be simplified like you propose. However...
> 
> >>
> > 
> > The merge_config.sh is used for a couple scenarios
> >  - Appending kconfigs together (CONFIG_*)
> 
>  ... I forgot about this one. Indeed, the buildroot merge_config.sh script is
> used in pkg-kconfig.mk to merge kernel configs. Ideally we should change that to
> use the package's merge_config.sh script. However, the location of that script
> may vary, or it may even be missing...
> 
> >  - Buildroot cfgs for runtime tests (BR2_*)
> >  - As a tool by users to merge together Buildroot configs
> > 
> > I'm not sure of the cleanest approach to support both
> >  - you could detect if the file is one or the other and adjust the regex
> >  - do the inverse and build a list of lines that are not comments
> 
>  I think Angelo's patch is the best approach after all.

I agree with you. The -b switch can be used to have a general solution
for both cases.

> 
>  However, there is one comment that I made on Angelo's patch that still applies
> here:
> 
>  Since the kconfig stuff comes from upstream but is modified, we also maintain
> the changes as a stack of patches in support/kconfig/patches. So you should
> generate a new patch for this change and add it to the series file. I'm not sure
> why we don't use a vendor branch and just merge, but that's the way we do it :-).
> 
> 
>  Also, Nasser, we require your Signed-off-by to contain your full name. The
> Signed-off-by is a short way for you to assert that you are entitled to
> contribute the patch under buildroot's GPL license.  See
> http://elinux.org/Developer_Certificate_Of_Origin for more details. Itis an
> official statement, so it should be done with your full and real name. And
> please make sure that the author information matches the Signed-off-by.

Sorry, this was a miss-configuration on the machine I've used to send the
patch. I'll correct my Signed-off-by clause.

Should I resend the patch applying Angelo -b approach?
> 
> 
>  Regards,
>  Arnout
> 
> [1] http://patchwork.ozlabs.org/patch/824051/
> 

Regards,
Nasser Afshin

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Arnout Vandecappelle]

On 10/23/18 4:19 PM, Nasser Afshin wrote:
> Hi Arnout,
> 
> On Sat, Oct 20, 2018 at 05:01:33PM +0100, Arnout Vandecappelle wrote:
[snip]
>>  I think Angelo's patch is the best approach after all.
> 
> I agree with you. The -b switch can be used to have a general solution
> for both cases.
> 
>>
>>  However, there is one comment that I made on Angelo's patch that still applies
>> here:
>>
>>  Since the kconfig stuff comes from upstream but is modified, we also maintain
>> the changes as a stack of patches in support/kconfig/patches. So you should
>> generate a new patch for this change and add it to the series file. I'm not sure
>> why we don't use a vendor branch and just merge, but that's the way we do it :-).
>>
>>
>>  Also, Nasser, we require your Signed-off-by to contain your full name. The
>> Signed-off-by is a short way for you to assert that you are entitled to
>> contribute the patch under buildroot's GPL license.  See
>> http://elinux.org/Developer_Certificate_Of_Origin for more details. Itis an
>> official statement, so it should be done with your full and real name. And
>> please make sure that the author information matches the Signed-off-by.
> 
> Sorry, this was a miss-configuration on the machine I've used to send the
> patch. I'll correct my Signed-off-by clause.
> 
> Should I resend the patch applying Angelo -b approach?

 Yes, please do. Make sure you keep Angelo's authorship and add your Sob. And
please fix the remaining two comments I made:

- remove the hunk that replaces alldefconfig with olddefconfig;

- a patch should be added to support/kconfig/patches.


 As it happens, I talked today with the guy who submitted the merge_config.sh
script to the kernel (apparently this script comes from yocto, originally). He
thinks it would be a good idea to upstream this -b option to the kernel. I
didn't find the kconfig maintainer (Masahiro) though, but it's worth a try.

 Regards,
 Arnout

>>
>>
>>  Regards,
>>  Arnout
>>
>> [1] http://patchwork.ozlabs.org/patch/824051/
>>
> 
> Regards,
> Nasser Afshin
> 

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Petr Vorel]

Hi Arnout, Nasser,

> > Should I resend the patch applying Angelo -b approach?
>  Yes, please do. Make sure you keep Angelo's authorship and add your Sob.
Nice. So I'm not going to implement it (asked by Arnout).
If you CC me, I'll review your patch.

> And please fix the remaining two comments I made:

> - remove the hunk that replaces alldefconfig with olddefconfig;

> - a patch should be added to support/kconfig/patches.


>  As it happens, I talked today with the guy who submitted the merge_config.sh
> script to the kernel (apparently this script comes from yocto, originally). He
> thinks it would be a good idea to upstream this -b option to the kernel. I
> didn't find the kconfig maintainer (Masahiro) though, but it's worth a try.
I was planning to send a patch to linux-kbuild as well (after we complete this
patch) as I also think it's something which upstream could take, so projects
didn't have to carry patch for it. But feel free to implement send it there.

>  Regards,
>  Arnout

Kind regards,
Petr

[Buildroot] [PATCH] merge_config.sh: Fix finding redundant config mechanism

[Nasser]

Hi Petr
On Wed, Oct 24, 2018 at 09:15:39PM +0200, Petr Vorel wrote:
> Hi Arnout, Nasser,
> 
> > > Should I resend the patch applying Angelo -b approach?
> >  Yes, please do. Make sure you keep Angelo's authorship and add your Sob.
> Nice. So I'm not going to implement it (asked by Arnout).
> If you CC me, I'll review your patch.
> 
Sure, I'm preparing the patch. I'll send it here when it's ready. I
appreciate your review and help.
> > And please fix the remaining two comments I made:
> 
> > - remove the hunk that replaces alldefconfig with olddefconfig;
> 
> > - a patch should be added to support/kconfig/patches.
> 
> 
> >  As it happens, I talked today with the guy who submitted the merge_config.sh
> > script to the kernel (apparently this script comes from yocto, originally). He
> > thinks it would be a good idea to upstream this -b option to the kernel. I
> > didn't find the kconfig maintainer (Masahiro) though, but it's worth a try.
> I was planning to send a patch to linux-kbuild as well (after we complete this
> patch) as I also think it's something which upstream could take, so projects
> didn't have to carry patch for it. But feel free to implement send it there.
> 
OK.
> >  Regards,
> >  Arnout
> 
> Kind regards,
> Petr
Kind regards,
Nasser

[Buildroot] [PATCH v2] merge_config.sh: merge also buildroot config files

[Nasser Afshin]

From: Angelo Compagnucci <angelo.compagnucci@gmail.com>

This patch adds a way to merge buildroot config file programmatically.
It adds an option (-b, buildroot mode) to manage buildroot config files.
The buildroot mode changes the way the script looks for configurations
entries using the BR2_ prefix and modify the call to the make command
to be buildroot friendly.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
Signed-off-by: Nasser Afshin <afshin.nasser@gmail.com>
---
 ...18-fix-finding-redundant-config-mechanism.patch | 41 ++++++++++++++++++++++
 support/kconfig/patches/series                     |  1 +
 2 files changed, 42 insertions(+)
 create mode 100644 support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch

diff --git a/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch b/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch
new file mode 100644
index 0000000000..eec92a5fad
--- /dev/null
+++ b/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch
@@ -0,0 +1,41 @@
+Index: kconfig/merge_config.sh
+===================================================================
+--- kconfig.orig/merge_config.sh
++++ kconfig/merge_config.sh
+@@ -29,6 +29,7 @@ trap clean_up HUP INT TERM
+ usage() {
+ 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
+ 	echo "  -h    display this help text"
++	echo "  -b    buildroot mode (searches for BR2_ and uses a custom make command)"
+ 	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"
+@@ -39,6 +40,7 @@ RUNMAKE=true
+ ALLTARGET=alldefconfig
+ WARNREDUN=false
+ OUTPUT=.
++CONFIG_PREFIX=CONFIG_
+ 
+ while true; do
+ 	case $1 in
+@@ -71,6 +73,11 @@ while true; do
+ 		shift 2
+ 		continue
+ 		;;
++	"-b")
++		CONFIG_PREFIX=BR2_
++		shift
++		continue
++		;;
+ 	*)
+ 		break
+ 		;;
+@@ -99,7 +106,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_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
+ TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
+ 
+ echo "Using $INITFILE as base"
diff --git a/support/kconfig/patches/series b/support/kconfig/patches/series
index 36591e2189..5ebc2e0225 100644
--- a/support/kconfig/patches/series
+++ b/support/kconfig/patches/series
@@ -6,3 +6,4 @@
 14-support-out-of-tree-config.patch
 16-fix-space-to-de-select-options.patch
 17-backport-kecho.patch
+18-fix-finding-redundant-config-mechanism.patch
-- 
2.15.0

[Buildroot] [PATCH v2] merge_config.sh: merge also buildroot config files

[Nasser]

Hi all,

On Thu, Oct 25, 2018 at 04:43:43AM +0330, Nasser Afshin wrote:
> From: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> 
> This patch adds a way to merge buildroot config file programmatically.
> It adds an option (-b, buildroot mode) to manage buildroot config files.
> The buildroot mode changes the way the script looks for configurations
> entries using the BR2_ prefix and modify the call to the make command
> to be buildroot friendly.
> 
> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> Signed-off-by: Nasser Afshin <afshin.nasser@gmail.com>
I'm not sure if this is the preferred way of keeping the authorship.
Please correct me if it's not.
> ---
>  ...18-fix-finding-redundant-config-mechanism.patch | 41 ++++++++++++++++++++++
>  support/kconfig/patches/series                     |  1 +
>  2 files changed, 42 insertions(+)
>  create mode 100644 support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch
> 
> diff --git a/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch b/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch
> new file mode 100644
> index 0000000000..eec92a5fad
> --- /dev/null
> +++ b/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch
> @@ -0,0 +1,41 @@
> +Index: kconfig/merge_config.sh
> +===================================================================
> +--- kconfig.orig/merge_config.sh
> ++++ kconfig/merge_config.sh
> +@@ -29,6 +29,7 @@ trap clean_up HUP INT TERM
> + usage() {
> + 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
> + 	echo "  -h    display this help text"
> ++	echo "  -b    buildroot mode (searches for BR2_ and uses a custom make command)"
> + 	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"
> +@@ -39,6 +40,7 @@ RUNMAKE=true
> + ALLTARGET=alldefconfig
> + WARNREDUN=false
> + OUTPUT=.
> ++CONFIG_PREFIX=CONFIG_
> + 
> + while true; do
> + 	case $1 in
> +@@ -71,6 +73,11 @@ while true; do
> + 		shift 2
> + 		continue
> + 		;;
> ++	"-b")
> ++		CONFIG_PREFIX=BR2_
> ++		shift
> ++		continue
> ++		;;
> + 	*)
> + 		break
> + 		;;
> +@@ -99,7 +106,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_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
> + TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
> + 
> + echo "Using $INITFILE as base"
> diff --git a/support/kconfig/patches/series b/support/kconfig/patches/series
> index 36591e2189..5ebc2e0225 100644
> --- a/support/kconfig/patches/series
> +++ b/support/kconfig/patches/series
> @@ -6,3 +6,4 @@
>  14-support-out-of-tree-config.patch
>  16-fix-space-to-de-select-options.patch
>  17-backport-kecho.patch
> +18-fix-finding-redundant-config-mechanism.patch
> -- 
> 2.15.0
> 
Kind regards,
Nasser

[Buildroot] [PATCH v3] merge_config.sh: merge also buildroot config files

[Nasser Afshin]

From: Angelo Compagnucci <angelo.compagnucci@gmail.com>

This patch adds a way to merge buildroot config file programmatically.
It adds an option (-b, buildroot mode) to manage buildroot config files.
The buildroot mode changes the way the script looks for configurations
entries using the BR2_ prefix and modify the call to the make command
to be buildroot friendly.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
Signed-off-by: Nasser Afshin <afshin.nasser@gmail.com>
---
I forgot to apply the presented patch. Now it's applied.

 support/kconfig/merge_config.sh                    |  9 ++++-
 ...18-fix-finding-redundant-config-mechanism.patch | 41 ++++++++++++++++++++++
 support/kconfig/patches/series                     |  1 +
 3 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch

diff --git a/support/kconfig/merge_config.sh b/support/kconfig/merge_config.sh
index 67d1314476..f22198023c 100755
--- a/support/kconfig/merge_config.sh
+++ b/support/kconfig/merge_config.sh
@@ -29,6 +29,7 @@ trap clean_up HUP INT TERM
 usage() {
 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
 	echo "  -h    display this help text"
+	echo "  -b    buildroot mode (searches for BR2_ and uses a custom make command)"
 	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"
@@ -39,6 +40,7 @@ RUNMAKE=true
 ALLTARGET=alldefconfig
 WARNREDUN=false
 OUTPUT=.
+CONFIG_PREFIX=CONFIG_
 
 while true; do
 	case $1 in
@@ -71,6 +73,11 @@ while true; do
 		shift 2
 		continue
 		;;
+	"-b")
+		CONFIG_PREFIX=BR2_
+		shift
+		continue
+		;;
 	*)
 		break
 		;;
@@ -99,7 +106,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_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
 TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
 
 echo "Using $INITFILE as base"
diff --git a/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch b/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch
new file mode 100644
index 0000000000..eec92a5fad
--- /dev/null
+++ b/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch
@@ -0,0 +1,41 @@
+Index: kconfig/merge_config.sh
+===================================================================
+--- kconfig.orig/merge_config.sh
++++ kconfig/merge_config.sh
+@@ -29,6 +29,7 @@ trap clean_up HUP INT TERM
+ usage() {
+ 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
+ 	echo "  -h    display this help text"
++	echo "  -b    buildroot mode (searches for BR2_ and uses a custom make command)"
+ 	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"
+@@ -39,6 +40,7 @@ RUNMAKE=true
+ ALLTARGET=alldefconfig
+ WARNREDUN=false
+ OUTPUT=.
++CONFIG_PREFIX=CONFIG_
+ 
+ while true; do
+ 	case $1 in
+@@ -71,6 +73,11 @@ while true; do
+ 		shift 2
+ 		continue
+ 		;;
++	"-b")
++		CONFIG_PREFIX=BR2_
++		shift
++		continue
++		;;
+ 	*)
+ 		break
+ 		;;
+@@ -99,7 +106,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_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
+ TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
+ 
+ echo "Using $INITFILE as base"
diff --git a/support/kconfig/patches/series b/support/kconfig/patches/series
index 36591e2189..5ebc2e0225 100644
--- a/support/kconfig/patches/series
+++ b/support/kconfig/patches/series
@@ -6,3 +6,4 @@
 14-support-out-of-tree-config.patch
 16-fix-space-to-de-select-options.patch
 17-backport-kecho.patch
+18-fix-finding-redundant-config-mechanism.patch
-- 
2.15.0

[Buildroot] Antwort: [PATCH v3] merge_config.sh: merge also buildroot config files

[Marcel Patzlaff]

Hi Nasser, Arnout,

after looking into the uclibc config, I think it would be better to
just remove the prefix restriction completely. What purpose does
filtering for "CONFIG_" or "BR2_" serve?

Kind regards,
Marcel

"buildroot" <buildroot-bounces@busybox.net> schrieb am 25.10.2018 
04:20:31:

> Von: Nasser Afshin <afshin.nasser@gmail.com>
> An: Arnout Vandecappelle <arnout@mind.be>
> Kopie: Nasser Afshin <afshin.nasser@gmail.com>, buildroot 
> <buildroot@buildroot.org>
> Datum: 25.10.2018 04:21
> Betreff: [Buildroot] [PATCH v3] merge_config.sh: merge also 
> buildroot config files
> Gesendet von: "buildroot" <buildroot-bounces@busybox.net>
> 
> From: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> 
> This patch adds a way to merge buildroot config file programmatically.
> It adds an option (-b, buildroot mode) to manage buildroot config files.
> The buildroot mode changes the way the script looks for configurations
> entries using the BR2_ prefix and modify the call to the make command
> to be buildroot friendly.
> 
> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> Signed-off-by: Nasser Afshin <afshin.nasser@gmail.com>
> ---
> I forgot to apply the presented patch. Now it's applied.
> 
>  support/kconfig/merge_config.sh                    |  9 ++++-
>  ...18-fix-finding-redundant-config-mechanism.patch | 41 +++++++++++
> +++++++++++
>  support/kconfig/patches/series                     |  1 +
>  3 files changed, 50 insertions(+), 1 deletion(-)
>  create mode 100644 support/kconfig/patches/18-fix-finding-
> redundant-config-mechanism.patch
> 
> diff --git a/support/kconfig/merge_config.sh 
b/support/kconfig/merge_config.sh
> index 67d1314476..f22198023c 100755
> --- a/support/kconfig/merge_config.sh
> +++ b/support/kconfig/merge_config.sh
> @@ -29,6 +29,7 @@ trap clean_up HUP INT TERM
>  usage() {
>     echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
>     echo "  -h    display this help text"
> +   echo "  -b    buildroot mode (searches for BR2_ and uses a 
> custom make command)"
>     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"
> @@ -39,6 +40,7 @@ RUNMAKE=true
>  ALLTARGET=alldefconfig
>  WARNREDUN=false
>  OUTPUT=.
> +CONFIG_PREFIX=CONFIG_
> 
>  while true; do
>     case $1 in
> @@ -71,6 +73,11 @@ while true; do
>        shift 2
>        continue
>        ;;
> +   "-b")
> +      CONFIG_PREFIX=BR2_
> +      shift
> +      continue
> +      ;;
>     *)
>        break
>        ;;
> @@ -99,7 +106,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_PREFIX}[a-zA-Z0-9_]*\)[=].*/\2/p"
>  TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
> 
>  echo "Using $INITFILE as base"
> diff --git a/support/kconfig/patches/18-fix-finding-redundant-
> config-mechanism.patch b/support/kconfig/patches/18-fix-finding-
> redundant-config-mechanism.patch
> new file mode 100644
> index 0000000000..eec92a5fad
> --- /dev/null
> +++ 
b/support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch
> @@ -0,0 +1,41 @@
> +Index: kconfig/merge_config.sh
> +===================================================================
> +--- kconfig.orig/merge_config.sh
> ++++ kconfig/merge_config.sh
> +@@ -29,6 +29,7 @@ trap clean_up HUP INT TERM
> + usage() {
> +    echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
> +    echo "  -h    display this help text"
> ++   echo "  -b    buildroot mode (searches for BR2_ and uses a 
> custom make command)"
> +    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"
> +@@ -39,6 +40,7 @@ RUNMAKE=true
> + ALLTARGET=alldefconfig
> + WARNREDUN=false
> + OUTPUT=.
> ++CONFIG_PREFIX=CONFIG_
> + 
> + while true; do
> +    case $1 in
> +@@ -71,6 +73,11 @@ while true; do
> +       shift 2
> +       continue
> +       ;;
> ++   "-b")
> ++      CONFIG_PREFIX=BR2_
> ++      shift
> ++      continue
> ++      ;;
> +    *)
> +       break
> +       ;;
> +@@ -99,7 +106,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_PREFIX}[a-zA-Z0-9_]*\)
> [= ].*/\2/p"
> + TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
> + 
> + echo "Using $INITFILE as base"
> diff --git a/support/kconfig/patches/series 
b/support/kconfig/patches/series
> index 36591e2189..5ebc2e0225 100644
> --- a/support/kconfig/patches/series
> +++ b/support/kconfig/patches/series
> @@ -6,3 +6,4 @@
>  14-support-out-of-tree-config.patch
>  16-fix-space-to-de-select-options.patch
>  17-backport-kecho.patch
> +18-fix-finding-redundant-config-mechanism.patch
> -- 
> 2.15.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot


Gesch?ftsf?hrung: Susanne Kunschert, Thomas Pilz
Pilz GmbH & Co. KG, Sitz: Ostfildern, HRA 210 893, Amtsgericht Stuttgart
Kompl. Ges. Peter Pilz GmbH, Sitz: Ostfildern, HRB 210 612, Amtsgericht Stuttgart
Umsatzsteuer: ID-Nr. DE 145 355 773, WEEE-Reg.-Nr. DE 71636849
This email is intended solely for the use of the named address(es). Any unauthorised disclosure, copying or distribution of these confidential information contained therein, or the taking of any action based on it, is prohibited. The sender disclaims any liability for the integrity of this email. Legally binding declarations must be in written form.
Umweltschutz liegt uns am Herzen! - Bitte denken Sie an unsere Umwelt, bevor Sie diese E-Mail drucken.
We do care about the environment! - Please consider the environment before printing this e-mail.

[Buildroot] [PATCH v3] merge_config.sh: merge also buildroot config files

[Petr Vorel]

Hi Angelo,

> From: Angelo Compagnucci <angelo.compagnucci@gmail.com>

> This patch adds a way to merge buildroot config file programmatically.
> It adds an option (-b, buildroot mode) to manage buildroot config files.
> The buildroot mode changes the way the script looks for configurations
> entries using the BR2_ prefix and modify the call to the make command
> to be buildroot friendly.

> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> Signed-off-by: Nasser Afshin <afshin.nasser@gmail.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>

LGTM. Could you please add commit where you actually use -b option?
I guess it should be in utils/test-pkg and support/kconfig/Makefile.
Not sure whether all packages using kconfig-package are using prefix CONFIG_,
but probably yes.

I'd suggest to move underscore to $SED_CONFIG_EXP. Or is it less intuitive?

> +CONFIG_PREFIX=CONFIG_
   CONFIG_PREFIX=CONFIG

>  while true; do
>  	case $1 in
> @@ -71,6 +73,11 @@ while true; do
>  		shift 2
>  		continue
>  		;;
> +	"-b")
> +		CONFIG_PREFIX=BR2_
		CONFIG_PREFIX=BR2

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

BTW There is some mixed spaces and tabs (pwclient fixed that but bare that in
mind next time):
Description: [v3] merge_config.sh: merge also buildroot config files
Applying: merge_config.sh: merge also buildroot config files
.git/rebase-apply/patch:63: space before tab in indent.
 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
.git/rebase-apply/patch:64: space before tab in indent.
 	echo "  -h    display this help text"
.git/rebase-apply/patch:66: space before tab in indent.
 	echo "  -m    only merge the fragments, do not execute the make command"
.git/rebase-apply/patch:67: space before tab in indent.
 	echo "  -n    use allnoconfig instead of alldefconfig"
.git/rebase-apply/patch:68: space before tab in indent.
 	echo "  -r    list redundant entries when merging fragments"
warning: squelched 10 whitespace errors


Kind regards,
Petr

[Buildroot] Antwort: Re: [PATCH v3] merge_config.sh: merge also buildroot config files

[Marcel Patzlaff]

Hi Petr,

> Not sure whether all packages using kconfig-package are using prefix 
CONFIG_,
> but probably yes.

at least the uclibc package does not use this prefix.

Gesch?ftsf?hrung: Susanne Kunschert, Thomas Pilz
Pilz GmbH & Co. KG, Sitz: Ostfildern, HRA 210 893, Amtsgericht Stuttgart
Kompl. Ges. Peter Pilz GmbH, Sitz: Ostfildern, HRB 210 612, Amtsgericht Stuttgart
Umsatzsteuer: ID-Nr. DE 145 355 773, WEEE-Reg.-Nr. DE 71636849
This email is intended solely for the use of the named address(es). Any unauthorised disclosure, copying or distribution of these confidential information contained therein, or the taking of any action based on it, is prohibited. The sender disclaims any liability for the integrity of this email. Legally binding declarations must be in written form.
Umweltschutz liegt uns am Herzen! - Bitte denken Sie an unsere Umwelt, bevor Sie diese E-Mail drucken.
We do care about the environment! - Please consider the environment before printing this e-mail.

[Buildroot] Antwort: Re: [PATCH v3] merge_config.sh: merge also buildroot config files

[Petr Vorel]

Hi Thomas,

> Hi Petr,

> > Not sure whether all packages using kconfig-package are using prefix 
> CONFIG_,
> > but probably yes.

> at least the uclibc package does not use this prefix.

Looking in the code uclibc-ng uses several prefixes: CONFIG_, TARGET_, UCLIBC_
and sometimes even configs without prefix: VERSION, WARNINGS, EXTRA_WARNINGS [1].

But their SED_CONFIG_EXP uses also CONFIG_.

BTW their kconfig is from linux-3.11, suppose our recent merge_config.sh would work with it.


Kind regards,
Petr


[1] https://cgit.openadk.org/cgi/cgit/uclibc-ng.git/tree/extra/Configs/Config.in
[2] https://cgit.openadk.org/cgi/cgit/uclibc-ng.git/tree/extra/config/merge_config.sh#n84

[Buildroot] Antwort: Re: Antwort: Re: [PATCH v3] merge_config.sh: merge also buildroot config files

[Marcel Patzlaff]

Hi Petr,

> Looking in the code uclibc-ng uses several prefixes: CONFIG_, TARGET_, 
UCLIBC_
> and sometimes even configs without prefix: VERSION, WARNINGS, 
> EXTRA_WARNINGS [1].
> 
> But their SED_CONFIG_EXP uses also CONFIG_.

As you said, they use several prefixes. IMHO, it does not make sense to
filter out configuration items based on a prefix.
And BTW, uclibc does not use it's own merge_config.sh anywhere and I
assume that it is also not used elsewhere.

I would suggest to just remove the prefix restrictions in buildroot's
merge_config.sh unless someone can tell why they are important to be
there.

Kind regards,
Marcel

PS: Sorry for the second reply. My mailclient dropped the list...

-- 

Gesch?ftsf?hrung: Susanne Kunschert, Thomas Pilz
Pilz GmbH & Co. KG, Sitz: Ostfildern, HRA 210 893, Amtsgericht Stuttgart
Kompl. Ges. Peter Pilz GmbH, Sitz: Ostfildern, HRB 210 612, Amtsgericht Stuttgart
Umsatzsteuer: ID-Nr. DE 145 355 773, WEEE-Reg.-Nr. DE 71636849
This email is intended solely for the use of the named address(es). Any unauthorised disclosure, copying or distribution of these confidential information contained therein, or the taking of any action based on it, is prohibited. The sender disclaims any liability for the integrity of this email. Legally binding declarations must be in written form.
Umweltschutz liegt uns am Herzen! - Bitte denken Sie an unsere Umwelt, bevor Sie diese E-Mail drucken.
We do care about the environment! - Please consider the environment before printing this e-mail.

[Buildroot] Antwort: Re: Antwort: Re: [PATCH v3] merge_config.sh: merge also buildroot config files

[Nasser]

Hi Marcel, Petr,
On Fri, Oct 26, 2018 at 08:03:59AM +0200, Marcel Patzlaff wrote:
> Hi Petr,
> 
> > Looking in the code uclibc-ng uses several prefixes: CONFIG_, TARGET_, 
> UCLIBC_
> > and sometimes even configs without prefix: VERSION, WARNINGS, 
> > EXTRA_WARNINGS [1].
> > 
> > But their SED_CONFIG_EXP uses also CONFIG_.
> 
> As you said, they use several prefixes. IMHO, it does not make sense to
> filter out configuration items based on a prefix.
> And BTW, uclibc does not use it's own merge_config.sh anywhere and I
> assume that it is also not used elsewhere.
Yes uclibc does not use it's own merge_config.sh anywhere.
> 
> I would suggest to just remove the prefix restrictions in buildroot's
> merge_config.sh unless someone can tell why they are important to be
> there.
The problem is that some of our defconfig files include comment lines
(e.g raspberrypi_defconfig, cubieboard2_defconfig,
qemu_arm_versatile_defconfig, ...). If we completely omit the
${CONFIG_PREFIX} we will add first word of comments to $CFG_LIST and
therefor will produce some false warning reports for comment lines (if
the fist word is just repeated anywhere throughout the fragments which
is none sense)

If we consider the controversial part of the script which is either:
sed -n 's/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
defconfig_file_name

for kernel style config files or

sed -n 's/^\(# \)\{0,1\}\(BR2_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
defconfig_file_name

for buildroot style config files or

sed -n 's/^\(# \)\{0,1\}\([a-zA-Z0-9_]*\)[= ].*/\2/p' \
defconfig_file_name

as a general command then for different buildroot/kernel defconfig
files, the last one will extract comments as well which IMHO is not what we
want.
> 
> Kind regards,
> Marcel
> 
> PS: Sorry for the second reply. My mailclient dropped the list...
> 
> -- 
> 
> Gesch?ftsf?hrung: Susanne Kunschert, Thomas Pilz
> Pilz GmbH & Co. KG, Sitz: Ostfildern, HRA 210 893, Amtsgericht Stuttgart
> Kompl. Ges. Peter Pilz GmbH, Sitz: Ostfildern, HRB 210 612, Amtsgericht Stuttgart
> Umsatzsteuer: ID-Nr. DE 145 355 773, WEEE-Reg.-Nr. DE 71636849
> This email is intended solely for the use of the named address(es). Any unauthorised disclosure, copying or distribution of these confidential information contained therein, or the taking of any action based on it, is prohibited. The sender disclaims any liability for the integrity of this email. Legally binding declarations must be in written form.
> Umweltschutz liegt uns am Herzen! - Bitte denken Sie an unsere Umwelt, bevor Sie diese E-Mail drucken.
> We do care about the environment! - Please consider the environment before printing this e-mail.
Kind regards,
Nasser

[Buildroot] Antwort: Re: Antwort: Re: [PATCH v3] merge_config.sh: merge also buildroot config files

[Petr Vorel]

Hi Marcel, Nasser,

> > I would suggest to just remove the prefix restrictions in buildroot's
> > merge_config.sh unless someone can tell why they are important to be
> > there.
> The problem is that some of our defconfig files include comment lines
> (e.g raspberrypi_defconfig, cubieboard2_defconfig,
> qemu_arm_versatile_defconfig, ...). If we completely omit the
> ${CONFIG_PREFIX} we will add first word of comments to $CFG_LIST and
> therefor will produce some false warning reports for comment lines (if
> the fist word is just repeated anywhere throughout the fragments which
> is none sense)

> If we consider the controversial part of the script which is either:
> sed -n 's/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
> defconfig_file_name

> for kernel style config files or

> sed -n 's/^\(# \)\{0,1\}\(BR2_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
> defconfig_file_name

> for buildroot style config files or

> sed -n 's/^\(# \)\{0,1\}\([a-zA-Z0-9_]*\)[= ].*/\2/p' \
> defconfig_file_name

> as a general command then for different buildroot/kernel defconfig
> files, the last one will extract comments as well which IMHO is not what we
> want.

Agree. Maybe we could try to add "any prefix" pattern: '[A-Z0-9_]\+'

sed -n 's/^\(# \)\{0,1\}\([A-Z0-9_]\+_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
defconfig_file_name


Kind regards,
Petr

[Buildroot] Antwort: Re: Antwort: Re: [PATCH v3] merge_config.sh: merge also buildroot config files

[Nasser Afshin]

Hi Petr, Marcel,
On Sat, Oct 27, 2018 at 06:46:36AM +0200, Petr Vorel wrote:
> Hi Marcel, Nasser,
> 
> > > I would suggest to just remove the prefix restrictions in buildroot's
> > > merge_config.sh unless someone can tell why they are important to be
> > > there.
> > The problem is that some of our defconfig files include comment lines
> > (e.g raspberrypi_defconfig, cubieboard2_defconfig,
> > qemu_arm_versatile_defconfig, ...). If we completely omit the
> > ${CONFIG_PREFIX} we will add first word of comments to $CFG_LIST and
> > therefor will produce some false warning reports for comment lines (if
> > the fist word is just repeated anywhere throughout the fragments which
> > is none sense)
> 
> > If we consider the controversial part of the script which is either:
> > sed -n 's/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
> > defconfig_file_name
> 
> > for kernel style config files or
> 
> > sed -n 's/^\(# \)\{0,1\}\(BR2_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
> > defconfig_file_name
> 
> > for buildroot style config files or
> 
> > sed -n 's/^\(# \)\{0,1\}\([a-zA-Z0-9_]*\)[= ].*/\2/p' \
> > defconfig_file_name
> 
> > as a general command then for different buildroot/kernel defconfig
> > files, the last one will extract comments as well which IMHO is not what we
> > want.
> 
> Agree. Maybe we could try to add "any prefix" pattern: '[A-Z0-9_]\+'
> 
> sed -n 's/^\(# \)\{0,1\}\([A-Z0-9_]\+_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
> defconfig_file_name
I agree. It seems to be a better solution.
> 
> 
> Kind regards,
> Petr
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Kind regards,
Nasser

[Buildroot] [PATCH v3] merge_config.sh: merge also buildroot config files

[Petr Vorel]

Hi Nasser,

> > > as a general command then for different buildroot/kernel defconfig
> > > files, the last one will extract comments as well which IMHO is not what we
> > > want.

> > Agree. Maybe we could try to add "any prefix" pattern: '[A-Z0-9_]\+'

> > sed -n 's/^\(# \)\{0,1\}\([A-Z0-9_]\+_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
> > defconfig_file_name
> I agree. It seems to be a better solution.
Upstream solution in kernel has been merged [1] and will be part of kernel 4.21
(kbuild updates for 4.20 has been merged day before).
Would you like to backport it to buildroot + add necessary applications (setting
the variable?) or shell I do it?

I also have plans to upgrade our kconfig to 4.19 + review the patches (some of
them might can be adjusted to be suitable for upstream, so we'd be able to get
rid of them).


> Kind regards,
> Nasser

Kind regards,
Petr

[1] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/commit/?h=for-next&id=f6fe19e9d84400e1c9544c1909952c2c7c6a90a2

[Buildroot] [PATCH v3] merge_config.sh: merge also buildroot config files

[Nasser]

On Wed, Oct 31, 2018 at 08:35:51PM +0100, Petr Vorel wrote:
> Hi Nasser,
> 
> > > > as a general command then for different buildroot/kernel defconfig
> > > > files, the last one will extract comments as well which IMHO is not what we
> > > > want.
> 
> > > Agree. Maybe we could try to add "any prefix" pattern: '[A-Z0-9_]\+'
> 
> > > sed -n 's/^\(# \)\{0,1\}\([A-Z0-9_]\+_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
> > > defconfig_file_name
> > I agree. It seems to be a better solution.
> Upstream solution in kernel has been merged [1] and will be part of kernel 4.21
> (kbuild updates for 4.20 has been merged day before).
I see. Seems great.
> Would you like to backport it to buildroot + add necessary applications (setting
> the variable?) or shell I do it?
Sure, I'll do it.
> 
> I also have plans to upgrade our kconfig to 4.19 + review the patches (some of
> them might can be adjusted to be suitable for upstream, so we'd be able to get
> rid of them).
Yes, I would like to do it also.
I think I should begin with the first task and then upgrade our kconfig
to 4.19.
> 
> 
> > Kind regards,
> > Nasser
> 
> Kind regards,
> Petr
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/commit/?h=for-next&id=f6fe19e9d84400e1c9544c1909952c2c7c6a90a2
Kind regards,
Nasser

[Buildroot] [PATCH v3] merge_config.sh: merge also buildroot config files

[Petr Vorel]

Hi Nasser,

> > Upstream solution in kernel has been merged [1] and will be part of kernel 4.21
> > (kbuild updates for 4.20 has been merged day before).
> I see. Seems great.
> > Would you like to backport it to buildroot + add necessary applications (setting
> > the variable?) or shell I do it?
> Sure, I'll do it.
Good, leave it for you.

> > I also have plans to upgrade our kconfig to 4.19 + review the patches (some of
> > them might can be adjusted to be suitable for upstream, so we'd be able to get
> > rid of them).
> Yes, I would like to do it also.
> I think I should begin with the first task and then upgrade our kconfig
> to 4.19.
Good. I'll leave update kconfig in buildroot for you.
I also leave upstreaming kconfig patches for you, if you want.
Please CC me on these patches.
Thanks a lot.

Kind regards,
Petr

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Nasser Afshin]

This patch allows us to define config prefix with CONFIG_ environment
variable. Now we can define BR2_ prefix when we are going to merge
buildroot fragment configs.

By setting the proper config prefix, we will have proper 'redundant
configuration warnings' when we use '-r -m' options.

This is actually an upstream patch [1] from linux/kbuild project.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/commit/?h=for-next&id=f6fe19e9d84400e1c9544c1909952c2c7c6a90a2

Signed-off-by: Nasser Afshin <afshin.nasser@gmail.com>
---
 support/kconfig/merge_config.sh                    |  8 ++++-
 ...e_config.sh-Allow-to-define-config-prefix.patch | 39 ++++++++++++++++++++++
 support/kconfig/patches/series                     |  1 +
 utils/test-pkg                                     |  2 +-
 4 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100644 support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch

diff --git a/support/kconfig/merge_config.sh b/support/kconfig/merge_config.sh
index 50de5114dc..15441182cf 100755
--- a/support/kconfig/merge_config.sh
+++ b/support/kconfig/merge_config.sh
@@ -34,12 +34,16 @@ usage() {
 	echo "  -r    list redundant entries when merging fragments"
 	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
 	echo "  -e    colon-separated list of br2-external trees to use (optional)"
+	echo
+	echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_
+	environment variable."
 }
 
 RUNMAKE=true
 ALLTARGET=alldefconfig
 WARNREDUN=false
 OUTPUT=.
+CONFIG_PREFIX=${CONFIG_-CONFIG_}
 
 while true; do
 	case $1 in
@@ -105,7 +109,8 @@ 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_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
+
 TMP_FILE=$(mktemp -t .tmp.config.XXXXXXXXXX)
 
 echo "Using $INITFILE as base"
@@ -157,6 +162,7 @@ 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
+unset CONFIG_
 make KCONFIG_ALLCONFIG=$TMP_FILE $EXTERNAL_ARG $OUTPUT_ARG $ALLTARGET
 
 
diff --git a/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch b/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch
new file mode 100644
index 0000000000..7b05736b05
--- /dev/null
+++ b/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch
@@ -0,0 +1,39 @@
+Index: kconfig/merge_config.sh
+===================================================================
+--- kconfig.orig/merge_config.sh
++++ kconfig/merge_config.sh
+@@ -34,12 +34,16 @@ usage() {
+ 	echo "  -r    list redundant entries when merging fragments"
+ 	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
+ 	echo "  -e    colon-separated list of br2-external trees to use (optional)"
++	echo
++	echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_
++	environment variable."
+ }
+ 
+ RUNMAKE=true
+ ALLTARGET=alldefconfig
+ WARNREDUN=false
+ OUTPUT=.
++CONFIG_PREFIX=${CONFIG_-CONFIG_}
+ 
+ while true; do
+ 	case $1 in
+@@ -105,7 +109,8 @@ 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_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
++
+ TMP_FILE=$(mktemp -t .tmp.config.XXXXXXXXXX)
+ 
+ echo "Using $INITFILE as base"
+@@ -157,6 +162,7 @@ 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
++unset CONFIG_
+ make KCONFIG_ALLCONFIG=$TMP_FILE $EXTERNAL_ARG $OUTPUT_ARG $ALLTARGET
+ 
+ 
diff --git a/support/kconfig/patches/series b/support/kconfig/patches/series
index e136de7937..be8627db13 100644
--- a/support/kconfig/patches/series
+++ b/support/kconfig/patches/series
@@ -8,3 +8,4 @@
 17-backport-kecho.patch
 18-merge-config.sh-create-temporary-files-in-tmp.patch
 19-merge_config.sh-add-br2-external-support.patch
+20-merge_config.sh-Allow-to-define-config-prefix.patch
diff --git a/utils/test-pkg b/utils/test-pkg
index aa91ee02cf..f2d519b4f6 100755
--- a/utils/test-pkg
+++ b/utils/test-pkg
@@ -129,7 +129,7 @@ build_one() {
 
     mkdir -p "${dir}"
 
-    support/kconfig/merge_config.sh -O "${dir}" \
+    CONFIG_=BR2_ support/kconfig/merge_config.sh -O "${dir}" \
         "${toolchainconfig}" "support/config-fragments/minimal.config" "${cfg}" \
         >> "${dir}/logfile" 2>&1
     # We want all the options from the snippet to be present as-is (set
-- 
2.15.0

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[yamada.masahiro at socionext.com]

Hi.

> -----Original Message-----
> From: buildroot [mailto:buildroot-bounces at busybox.net] On Behalf Of
> Nasser Afshin
> Sent: Thursday, November 01, 2018 10:10 AM
> To: Petr Vorel <petr.vorel@gmail.com>
> Cc: Nasser Afshin <afshin.nasser@gmail.com>; buildroot
> <buildroot@buildroot.org>
> Subject: [Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config
> files
> 
> This patch allows us to define config prefix with CONFIG_ environment
> variable. Now we can define BR2_ prefix when we are going to merge
> buildroot fragment configs.
> 
> By setting the proper config prefix, we will have proper 'redundant
> configuration warnings' when we use '-r -m' options.
> 
> This is actually an upstream patch [1] from linux/kbuild project.
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild
> .git/commit/?h=for-next&id=f6fe19e9d84400e1c9544c1909952c2c7c6a90a2


Please refer to commit ID only after it gets stable in Linus' tree.

I will end up with rebasing
In order to drop some other commits causing a problem.


Thanks.

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Petr Vorel]

Hi.

> > This is actually an upstream patch [1] from linux/kbuild project.

> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild
> > .git/commit/?h=for-next&id=f6fe19e9d84400e1c9544c1909952c2c7c6a90a2


> Please refer to commit ID only after it gets stable in Linus' tree.

> I will end up with rebasing
> In order to drop some other commits causing a problem.
Make sense, thanks, Yamada!
Nasser, maybe use link to patch in patchwork.
I'd put this link also into the patch itself (IMHO good idea for faster
resolving when doing kconfig update in buildroot).
Once the patch gets into mainline, we can update the link.


Kind regards,
Petr

[1] https://patchwork.kernel.org/patch/10660207/

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Nasser]

On Thu, Nov 01, 2018 at 06:49:42AM +0100, Petr Vorel wrote:
> Hi.
> 
> > > This is actually an upstream patch [1] from linux/kbuild project.
> 
> > > [1]
> > > https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild
> > > .git/commit/?h=for-next&id=f6fe19e9d84400e1c9544c1909952c2c7c6a90a2
> 
> 
> > Please refer to commit ID only after it gets stable in Linus' tree.
> 
> > I will end up with rebasing
> > In order to drop some other commits causing a problem.
> Make sense, thanks, Yamada!
> Nasser, maybe use link to patch in patchwork.
> I'd put this link also into the patch itself (IMHO good idea for faster
> resolving when doing kconfig update in buildroot).
> Once the patch gets into mainline, we can update the link.
Ok.
> 
> 
> Kind regards,
> Petr
> 
> [1] https://patchwork.kernel.org/patch/10660207/
Thank you for adding the link here.

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Petr Vorel]

Hi Nasser,

...
> @@ -157,6 +162,7 @@ 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
> +unset CONFIG_
You added unset, which isn't in upstream patch. This has no effect, so I'd be
for removing it. Even if it has (if it was needed), it'd be better to 1) ask
upstream to add it 2) keep it in separate patch in meanwhile (otherwise it
complicates patch rebasing during update and can be lost).

...
> diff --git a/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch b/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch
...
> + # 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
> ++unset CONFIG_

BTW (I noted that before) your patch contain trailing whitespace and mixing tab
and spaces. git am and pwclient fixes that, only when applying with patch they
get committed, so nothing serious.

$ pwclient git-am -p buildroot 991781
.git/rebase-apply/patch:59: space before tab in indent.
 	echo "  -r    list redundant entries when merging fragments"
.git/rebase-apply/patch:60: space before tab in indent.
 	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
.git/rebase-apply/patch:61: space before tab in indent.
 	echo "  -e    colon-separated list of br2-external trees to use (optional)"
.git/rebase-apply/patch:66: trailing whitespace.
 
.git/rebase-apply/patch:72: trailing whitespace.
 
warning: squelched 6 whitespace errors
warning: 10 lines applied after fixing whitespace errors.
Applying: merge_config.sh: Fix merging buildroot config files
Applying patch #991781 using 'git am'
Description: merge_config.sh: Fix merging buildroot config files

Kind regards,
Petr

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Nasser]

Hi Petr,
On Thu, Nov 01, 2018 at 07:24:49AM +0100, Petr Vorel wrote:
> Hi Nasser,
> 
> ...
> > @@ -157,6 +162,7 @@ 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
> > +unset CONFIG_
> You added unset, which isn't in upstream patch. This has no effect, so I'd be
> for removing it. Even if it has (if it was needed), it'd be better to 1) ask
> upstream to add it 2) keep it in separate patch in meanwhile (otherwise it
> complicates patch rebasing during update and can be lost).
Removing unset will cause the following make command work wrong. Note
that the value of this environment variable is checked [1] and used. The
result of removing the unset line is that we will have double BR2_
prefixes in the final .config file as well as lots of warnings when
checking if all specified config values have been taken.

I think we should follow your steps to include it.
> 
> ...
> > diff --git a/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch b/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch
> ...
> > + # 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
> > ++unset CONFIG_
> 
> BTW (I noted that before) your patch contain trailing whitespace and mixing tab
> and spaces. git am and pwclient fixes that, only when applying with patch they
> get committed, so nothing serious.
> 
> $ pwclient git-am -p buildroot 991781
> .git/rebase-apply/patch:59: space before tab in indent.
>  	echo "  -r    list redundant entries when merging fragments"
> .git/rebase-apply/patch:60: space before tab in indent.
>  	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
> .git/rebase-apply/patch:61: space before tab in indent.
>  	echo "  -e    colon-separated list of br2-external trees to use (optional)"
> .git/rebase-apply/patch:66: trailing whitespace.
>  
> .git/rebase-apply/patch:72: trailing whitespace.
>  
> warning: squelched 6 whitespace errors
> warning: 10 lines applied after fixing whitespace errors.
> Applying: merge_config.sh: Fix merging buildroot config files
> Applying patch #991781 using 'git am'
> Description: merge_config.sh: Fix merging buildroot config files
> 
I think this is because we are including a patch in another patch. As
you can see in your previous commit:
a3366b270562f42723a3b8032f73bb03b541d113, this patch has the same issue
because it includes another patch.
I don't know how to avoid that if we should do so.
> Kind regards,
> Petr
[1] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/tree/scripts/kconfig/lkc.h?h=for-next&id=f6fe19e9d84400e1c9544c1909952c2c7c6a90#n26
Kind regards,
Nasser

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Arnout Vandecappelle]

On 01/11/18 11:55, Nasser wrote:
> Hi Petr,
> On Thu, Nov 01, 2018 at 07:24:49AM +0100, Petr Vorel wrote:
>> Hi Nasser,
>>
>> ...
>>> @@ -157,6 +162,7 @@ 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
>>> +unset CONFIG_
>> You added unset, which isn't in upstream patch. This has no effect, so I'd be
>> for removing it. Even if it has (if it was needed), it'd be better to 1) ask
>> upstream to add it 2) keep it in separate patch in meanwhile (otherwise it
>> complicates patch rebasing during update and can be lost).
> Removing unset will cause the following make command work wrong. Note
> that the value of this environment variable is checked [1] and used. The
> result of removing the unset line is that we will have double BR2_
> prefixes in the final .config file as well as lots of warnings when
> checking if all specified config values have been taken.

 We have no prefix, the BR2_ is mere convention. So when calling merge_config,
we should set CONFIG_ to empty, not to BR2_.

 So indeed, the unset should be removed.

> 
> I think we should follow your steps to include it.
>>
>> ...
>>> diff --git a/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch b/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch
>> ...
>>> + # 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
>>> ++unset CONFIG_
>>
>> BTW (I noted that before) your patch contain trailing whitespace and mixing tab
>> and spaces. git am and pwclient fixes that, only when applying with patch they
>> get committed, so nothing serious.
>>
>> $ pwclient git-am -p buildroot 991781
>> .git/rebase-apply/patch:59: space before tab in indent.
>>  	echo "  -r    list redundant entries when merging fragments"
>> .git/rebase-apply/patch:60: space before tab in indent.
>>  	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
>> .git/rebase-apply/patch:61: space before tab in indent.
>>  	echo "  -e    colon-separated list of br2-external trees to use (optional)"
>> .git/rebase-apply/patch:66: trailing whitespace.
>>  
>> .git/rebase-apply/patch:72: trailing whitespace.
>>  
>> warning: squelched 6 whitespace errors
>> warning: 10 lines applied after fixing whitespace errors.
>> Applying: merge_config.sh: Fix merging buildroot config files
>> Applying patch #991781 using 'git am'
>> Description: merge_config.sh: Fix merging buildroot config files
>>
> I think this is because we are including a patch in another patch. As
> you can see in your previous commit:
> a3366b270562f42723a3b8032f73bb03b541d113, this patch has the same issue
> because it includes another patch.
> I don't know how to avoid that if we should do so.

 You're correct, it's expected for *.patch files to have whitespace errors.

 I'm not entirely sure if we should include the *.patch file in this case, since
it actually does come from upstream. However, it definitely doesn't hurt to
include it, it can be removed again when the kconfig is updated again.

 Regards,
 Arnout

>> Kind regards,
>> Petr
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/tree/scripts/kconfig/lkc.h?h=for-next&id=f6fe19e9d84400e1c9544c1909952c2c7c6a90#n26
> Kind regards,
> Nasser
> 

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Petr Vorel]

Hi Arnout,

> > Removing unset will cause the following make command work wrong. Note
> > that the value of this environment variable is checked [1] and used. The
> > result of removing the unset line is that we will have double BR2_
> > prefixes in the final .config file as well as lots of warnings when
> > checking if all specified config values have been taken.

>  We have no prefix, the BR2_ is mere convention. So when calling merge_config,
> we should set CONFIG_ to empty, not to BR2_.

>  So indeed, the unset should be removed.

+1. I overlooked that regexp works with empty $CONFIG_ as well.
Thanks for explanation.

Kind regards,
Petr

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Nasser]

Hi Petr, Arnout,
On Thu, Nov 01, 2018 at 05:19:19PM +0100, Petr Vorel wrote:
> Hi Arnout,
> 
> > > Removing unset will cause the following make command work wrong. Note
> > > that the value of this environment variable is checked [1] and used. The
> > > result of removing the unset line is that we will have double BR2_
> > > prefixes in the final .config file as well as lots of warnings when
> > > checking if all specified config values have been taken.
> 
> >  We have no prefix, the BR2_ is mere convention. So when calling merge_config,
> > we should set CONFIG_ to empty, not to BR2_.
> 
> >  So indeed, the unset should be removed.
> 
> +1. I overlooked that regexp works with empty $CONFIG_ as well.
> Thanks for explanation.
As we discussed earlier [1], setting CONFIG_ to empty, makes most comments to
be identified as config fragments and generate false warnings.
> 
> Kind regards,
> Petr
Kind regards,
Nasser

[1] https://patchwork.ozlabs.org/comment/2018492/

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[yamada.masahiro at socionext.com]

Hi.

> -----Original Message-----
> From: Nasser [mailto:afshin.nasser at gmail.com]
> Sent: Friday, November 02, 2018 11:13 AM
> To: Petr Vorel <petr.vorel@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>; buildroot at buildroot.org;
> Matthew Weber <matthew.weber@rockwellcollins.com>; Angelo Compagnucci
> <angelo.compagnucci@gmail.com>; Yamada, Masahiro/?? ??
> <yamada.masahiro@socionext.com>
> Subject: Re: [PATCH] merge_config.sh: Fix merging buildroot config files
> 
> Hi Petr, Arnout,
> On Thu, Nov 01, 2018 at 05:19:19PM +0100, Petr Vorel wrote:
> > Hi Arnout,
> >
> > > > Removing unset will cause the following make command work wrong. Note
> > > > that the value of this environment variable is checked [1] and used.
> The
> > > > result of removing the unset line is that we will have double BR2_
> > > > prefixes in the final .config file as well as lots of warnings when
> > > > checking if all specified config values have been taken.
> >
> > >  We have no prefix, the BR2_ is mere convention. So when calling
> merge_config,
> > > we should set CONFIG_ to empty, not to BR2_.
> >
> > >  So indeed, the unset should be removed.
> >
> > +1. I overlooked that regexp works with empty $CONFIG_ as well.
> > Thanks for explanation.
> As we discussed earlier [1], setting CONFIG_ to empty, makes most comments
> to
> be identified as config fragments and generate false warnings.

I was also being worried about this.

We could improve the sed pattern.

How about this?
https://patchwork.kernel.org/patch/10665119/



> >
> > Kind regards,
> > Petr
> Kind regards,
> Nasser
> 
> [1] https://patchwork.ozlabs.org/comment/2018492/

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Arnout Vandecappelle]

On 02/11/18 08:55, yamada.masahiro at socionext.com wrote:
> Hi.
> 
>> -----Original Message-----
>> From: Nasser [mailto:afshin.nasser at gmail.com]
>> Sent: Friday, November 02, 2018 11:13 AM
>> To: Petr Vorel <petr.vorel@gmail.com>
>> Cc: Arnout Vandecappelle <arnout@mind.be>; buildroot at buildroot.org;
>> Matthew Weber <matthew.weber@rockwellcollins.com>; Angelo Compagnucci
>> <angelo.compagnucci@gmail.com>; Yamada, Masahiro/?? ??
>> <yamada.masahiro@socionext.com>
>> Subject: Re: [PATCH] merge_config.sh: Fix merging buildroot config files
>>
>> Hi Petr, Arnout,
>> On Thu, Nov 01, 2018 at 05:19:19PM +0100, Petr Vorel wrote:
>>> Hi Arnout,
>>>
>>>>> Removing unset will cause the following make command work wrong. Note
>>>>> that the value of this environment variable is checked [1] and used.
>> The
>>>>> result of removing the unset line is that we will have double BR2_
>>>>> prefixes in the final .config file as well as lots of warnings when
>>>>> checking if all specified config values have been taken.
>>>
>>>>  We have no prefix, the BR2_ is mere convention. So when calling
>> merge_config,
>>>> we should set CONFIG_ to empty, not to BR2_.
>>>
>>>>  So indeed, the unset should be removed.
>>>
>>> +1. I overlooked that regexp works with empty $CONFIG_ as well.
>>> Thanks for explanation.
>> As we discussed earlier [1], setting CONFIG_ to empty, makes most comments
>> to
>> be identified as config fragments and generate false warnings.
> 
> I was also being worried about this.
> 
> We could improve the sed pattern.
> 
> How about this?
> https://patchwork.kernel.org/patch/10665119/

 Perhaps it's better to make the entire pattern explicit, like:

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

(\1\2 works because only one of the two subexpressions matches, so either \1 or
\2 must be empty).

 Regards,
 Arnout

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Petr Vorel]

Hi Arnout,

> > How about this?
> > https://patchwork.kernel.org/patch/10665119/

>  Perhaps it's better to make the entire pattern explicit, like:

> SED_CONFIG_EXP="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*$\|^#
> \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1\2/p"

> (\1\2 works because only one of the two subexpressions matches, so either \1 or
> \2 must be empty).
I'd prefer original Masahiro's version - it's easier to read and without
duplicity.

>  Regards,
>  Arnout


Kind regards,
Petr

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Nasser]

Hi Petr, Mahasiro,
On Fri, Nov 02, 2018 at 11:50:58PM +0100, Petr Vorel wrote:
> Hi Arnout,
> 
> > > How about this?
> > > https://patchwork.kernel.org/patch/10665119/
> 
> >  Perhaps it's better to make the entire pattern explicit, like:
> 
> > SED_CONFIG_EXP="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*$\|^#
> > \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1\2/p"
> 
> > (\1\2 works because only one of the two subexpressions matches, so either \1 or
> > \2 must be empty).
> I'd prefer original Masahiro's version - it's easier to read and without
> duplicity.
> 
I agree with you. +1 for not having duplicity.
This patch makes merge_config.sh to work well in many more tests. I'm waiting for Masahiro's version to be applied to
the his kbuild tree and then re-prepare the patch here.
> >  Regards,
> >  Arnout
> 
> 
> Kind regards,
> Petr
> 
Kind regards,
Nasser

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[yamada.masahiro at socionext.com]

Hi.

> -----Original Message-----
> From: Nasser [mailto:afshin.nasser at gmail.com]
> Sent: Sunday, November 04, 2018 6:32 AM
> To: Petr Vorel <petr.vorel@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>; Yamada, Masahiro/?? ??
> <yamada.masahiro@socionext.com>; buildroot at buildroot.org;
> matthew.weber at rockwellcollins.com; angelo.compagnucci at gmail.com
> Subject: Re: [PATCH] merge_config.sh: Fix merging buildroot config files
> 
> Hi Petr, Mahasiro,
> On Fri, Nov 02, 2018 at 11:50:58PM +0100, Petr Vorel wrote:
> > Hi Arnout,
> >
> > > > How about this?
> > > > https://patchwork.kernel.org/patch/10665119/
> >
> > >  Perhaps it's better to make the entire pattern explicit, like:
> >
> > > SED_CONFIG_EXP="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*$\|^#
> > > \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1\2/p"
> >
> > > (\1\2 works because only one of the two subexpressions matches, so either
> \1 or
> > > \2 must be empty).
> > I'd prefer original Masahiro's version - it's easier to read and without
> > duplicity.
> >
> I agree with you. +1 for not having duplicity.
> This patch makes merge_config.sh to work well in many more tests. I'm waiting
> for Masahiro's version to be applied to
> the his kbuild tree and then re-prepare the patch here.


OK, I think Arnout's one is unreadable, but
we can make it readable by splitting the complex pattern into two.

I posted this for comparison.
https://patchwork.kernel.org/patch/10667571/

Thanks.

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Petr Vorel]

Hi,

> > > >  Perhaps it's better to make the entire pattern explicit, like:

> > > > SED_CONFIG_EXP="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*$\|^#
> > > > \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1\2/p"

> > > > (\1\2 works because only one of the two subexpressions matches, so either
> > \1 or
> > > > \2 must be empty).
> > > I'd prefer original Masahiro's version - it's easier to read and without
> > > duplicity.

> > I agree with you. +1 for not having duplicity.
> > This patch makes merge_config.sh to work well in many more tests. I'm waiting
> > for Masahiro's version to be applied to
> > the his kbuild tree and then re-prepare the patch here.


> OK, I think Arnout's one is unreadable, but
> we can make it readable by splitting the complex pattern into two.

> I posted this for comparison.
> https://patchwork.kernel.org/patch/10667571/

LGTM. It's quite verbose, but more precise regexp is better and readability is also
good.


Kind regards,
Petr

[Buildroot] [PATCH v3 0/3] Fix merging configuration fragments

[Nasser Afshin]

From: Nasser Afshin <Afshin.Nasser@gmail.com>

We use merge_config.sh as a helper script to merge many types of config
fragments. The script has recently improved to better support buildroot. Now
the report mechanism (-r -m) works as expected for buildroot configuration
fragments.

These are patches to reproduce the state of the Linux Kernel Build
mainline project.


Nasser Afshin (3):
  merge_config.sh: Fix merging buildroot config files
  test-pkg: Use the correct config prefix when merging
  merge_config.sh: Avoid false positive matches from comment lines

 support/kconfig/merge_config.sh                    | 12 ++++++--
 ...e_config.sh-Allow-to-define-config-prefix.patch | 31 +++++++++++++++++++++
 ...false-positive-matches-from-comment-lines.patch | 32 ++++++++++++++++++++++
 support/kconfig/patches/series                     |  2 ++
 utils/test-pkg                                     |  2 +-
 5 files changed, 75 insertions(+), 4 deletions(-)
 create mode 100644 support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch
 create mode 100644 support/kconfig/patches/21-Avoid-false-positive-matches-from-comment-lines.patch

-- 
2.15.0

[Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files

[Nasser Afshin]

This patch allows us to define config prefix with CONFIG_ environment
variable.

By setting the proper config prefix, we will have proper 'redundant
configuration warnings' when we use '-r -m' options.

This is actually an upstream patch [1] from linux/kbuild project.

[1] https://patchwork.kernel.org/patch/10660207/

Signed-off-by: Nasser Afshin <afshin.nasser@gmail.com>
---
 support/kconfig/merge_config.sh                    |  7 ++++-
 ...e_config.sh-Allow-to-define-config-prefix.patch | 31 ++++++++++++++++++++++
 support/kconfig/patches/series                     |  1 +
 3 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch

diff --git a/support/kconfig/merge_config.sh b/support/kconfig/merge_config.sh
index 50de5114dc..404ca3864f 100755
--- a/support/kconfig/merge_config.sh
+++ b/support/kconfig/merge_config.sh
@@ -34,12 +34,16 @@ usage() {
 	echo "  -r    list redundant entries when merging fragments"
 	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
 	echo "  -e    colon-separated list of br2-external trees to use (optional)"
+	echo
+	echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_
+	environment variable."
 }
 
 RUNMAKE=true
 ALLTARGET=alldefconfig
 WARNREDUN=false
 OUTPUT=.
+CONFIG_PREFIX=${CONFIG_-CONFIG_}
 
 while true; do
 	case $1 in
@@ -105,7 +109,8 @@ 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_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
+
 TMP_FILE=$(mktemp -t .tmp.config.XXXXXXXXXX)
 
 echo "Using $INITFILE as base"
diff --git a/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch b/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch
new file mode 100644
index 0000000000..645043b163
--- /dev/null
+++ b/support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch
@@ -0,0 +1,31 @@
+Index: kconfig/merge_config.sh
+===================================================================
+--- kconfig.orig/merge_config.sh
++++ kconfig/merge_config.sh
+@@ -34,12 +34,16 @@ usage() {
+ 	echo "  -r    list redundant entries when merging fragments"
+ 	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
+ 	echo "  -e    colon-separated list of br2-external trees to use (optional)"
++	echo
++	echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_
++	environment variable."
+ }
+ 
+ RUNMAKE=true
+ ALLTARGET=alldefconfig
+ WARNREDUN=false
+ OUTPUT=.
++CONFIG_PREFIX=${CONFIG_-CONFIG_}
+ 
+ while true; do
+ 	case $1 in
+@@ -105,7 +109,8 @@ 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_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
++
+ TMP_FILE=$(mktemp -t .tmp.config.XXXXXXXXXX)
+ 
+ echo "Using $INITFILE as base"
diff --git a/support/kconfig/patches/series b/support/kconfig/patches/series
index e136de7937..be8627db13 100644
--- a/support/kconfig/patches/series
+++ b/support/kconfig/patches/series
@@ -8,3 +8,4 @@
 17-backport-kecho.patch
 18-merge-config.sh-create-temporary-files-in-tmp.patch
 19-merge_config.sh-add-br2-external-support.patch
+20-merge_config.sh-Allow-to-define-config-prefix.patch
-- 
2.15.0

[Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files

[Petr Vorel]

Hi Nasser,

> This patch allows us to define config prefix with CONFIG_ environment
> variable.

> By setting the proper config prefix, we will have proper 'redundant
> configuration warnings' when we use '-r -m' options.

> This is actually an upstream patch [1] from linux/kbuild project.

> [1] https://patchwork.kernel.org/patch/10660207/
This is actually already in mainline for v4.20-rc1:
2cd3faf87d2d8f6123adf34741b9a7b98828a76f
("merge_config.sh: Allow to define config prefix")

> Signed-off-by: Nasser Afshin <afshin.nasser@gmail.com>
Reviewed-by: Petr Vorel <petr.vorel@gmail.com>

Kind regards,
Petr

[Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files

[Nasser Afshin]

Hi Petr
On Tue, Nov 13, 2018 at 07:35:55PM +0100, Petr Vorel wrote:
> Hi Nasser,
> 
> > This patch allows us to define config prefix with CONFIG_ environment
> > variable.
> 
> > By setting the proper config prefix, we will have proper 'redundant
> > configuration warnings' when we use '-r -m' options.
> 
> > This is actually an upstream patch [1] from linux/kbuild project.
> 
> > [1] https://patchwork.kernel.org/patch/10660207/
> This is actually already in mainline for v4.20-rc1:
> 2cd3faf87d2d8f6123adf34741b9a7b98828a76f
> ("merge_config.sh: Allow to define config prefix")
> 
Oh yes, corrected, resent.
> > Signed-off-by: Nasser Afshin <afshin.nasser@gmail.com>
> Reviewed-by: Petr Vorel <petr.vorel@gmail.com>
> 
> Kind regards,
> Petr
Kind regards,
Nasser

[Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging

[Nasser Afshin]

From: Nasser Afshin <Afshin.Nasser@gmail.com>

We should use an empty prefix as we do not have any prefix.
Note that BR2_ is mere a convention.

Signed-off-by: Nasser Afshin <Afshin.Nasser@gmail.com>
---
 utils/test-pkg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/test-pkg b/utils/test-pkg
index aa91ee02cf..7bf1c97fba 100755
--- a/utils/test-pkg
+++ b/utils/test-pkg
@@ -129,7 +129,7 @@ build_one() {
 
     mkdir -p "${dir}"
 
-    support/kconfig/merge_config.sh -O "${dir}" \
+    CONFIG_="" support/kconfig/merge_config.sh -O "${dir}" \
         "${toolchainconfig}" "support/config-fragments/minimal.config" "${cfg}" \
         >> "${dir}/logfile" 2>&1
     # We want all the options from the snippet to be present as-is (set
-- 
2.15.0

[Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging

[Petr Vorel]

Hi Nasser,

> From: Nasser Afshin <Afshin.Nasser@gmail.com>

> We should use an empty prefix as we do not have any prefix.
> Note that BR2_ is mere a convention.

> Signed-off-by: Nasser Afshin <Afshin.Nasser@gmail.com>
Reviewed-by: Petr Vorel <petr.vorel@gmail.com>
> ---
>  utils/test-pkg | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/utils/test-pkg b/utils/test-pkg
> index aa91ee02cf..7bf1c97fba 100755
> --- a/utils/test-pkg
> +++ b/utils/test-pkg
> @@ -129,7 +129,7 @@ build_one() {

>      mkdir -p "${dir}"

> -    support/kconfig/merge_config.sh -O "${dir}" \
> +    CONFIG_="" support/kconfig/merge_config.sh -O "${dir}" \
Please, can you omit "" (not needed):
CONFIG_= support/kconfig/merge_config.sh -O "${dir}" \


Kind regards,
Petr

[Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines

[Nasser Afshin]

From: Nasser Afshin <Afshin.Nasser@gmail.com>

We are using empty CONFIG_PREFIX_. This results in false positive match
for comment lines when merging config fragments.

To avoid false positive reports, we use separate sed expressions and
address comment lines explicitly.

This is actually an upstream patch [1] from linux/kbuild project.

[1] https://patchwork.kernel.org/patch/10667571/

Signed-off-by: Nasser Afshin <Afshin.Nasser@gmail.com>
---
 support/kconfig/merge_config.sh                    |  7 +++--
 ...false-positive-matches-from-comment-lines.patch | 32 ++++++++++++++++++++++
 support/kconfig/patches/series                     |  1 +
 3 files changed, 37 insertions(+), 3 deletions(-)
 create mode 100644 support/kconfig/patches/21-Avoid-false-positive-matches-from-comment-lines.patch

diff --git a/support/kconfig/merge_config.sh b/support/kconfig/merge_config.sh
index 404ca3864f..14917806a3 100755
--- a/support/kconfig/merge_config.sh
+++ b/support/kconfig/merge_config.sh
@@ -109,7 +109,8 @@ if [ ! -r "$INITFILE" ]; then
 fi
 
 MERGE_LIST=$*
-SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
+SED_CONFIG_EXP1="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*/\1/p"
+SED_CONFIG_EXP2="s/^# \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1/p"
 
 TMP_FILE=$(mktemp -t .tmp.config.XXXXXXXXXX)
 
@@ -123,7 +124,7 @@ for MERGE_FILE in $MERGE_LIST ; do
 		echo "The merge file '$MERGE_FILE' does not exist.  Exit." >&2
 		exit 1
 	fi
-	CFG_LIST=$(sed -n "$SED_CONFIG_EXP" $MERGE_FILE)
+	CFG_LIST=$(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $MERGE_FILE)
 
 	for CFG in $CFG_LIST ; do
 		grep -q -w $CFG $TMP_FILE || continue
@@ -166,7 +167,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $EXTERNAL_ARG $OUTPUT_ARG $ALLTARGET
 
 
 # Check all specified config values took (might have missed-dependency issues)
-for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
+for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
 
 	REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
 	ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
diff --git a/support/kconfig/patches/21-Avoid-false-positive-matches-from-comment-lines.patch b/support/kconfig/patches/21-Avoid-false-positive-matches-from-comment-lines.patch
new file mode 100644
index 0000000000..c11144e47e
--- /dev/null
+++ b/support/kconfig/patches/21-Avoid-false-positive-matches-from-comment-lines.patch
@@ -0,0 +1,32 @@
+Index: kconfig/merge_config.sh
+===================================================================
+--- kconfig.orig/merge_config.sh
++++ kconfig/merge_config.sh
+@@ -109,7 +109,8 @@ if [ ! -r "$INITFILE" ]; then
+ fi
+ 
+ MERGE_LIST=$*
+-SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
++SED_CONFIG_EXP1="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*/\1/p"
++SED_CONFIG_EXP2="s/^# \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1/p"
+ 
+ TMP_FILE=$(mktemp -t .tmp.config.XXXXXXXXXX)
+ 
+@@ -123,7 +124,7 @@ for MERGE_FILE in $MERGE_LIST ; do
+ 		echo "The merge file '$MERGE_FILE' does not exist.  Exit." >&2
+ 		exit 1
+ 	fi
+-	CFG_LIST=$(sed -n "$SED_CONFIG_EXP" $MERGE_FILE)
++	CFG_LIST=$(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $MERGE_FILE)
+ 
+ 	for CFG in $CFG_LIST ; do
+ 		grep -q -w $CFG $TMP_FILE || continue
+@@ -166,7 +167,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $EXTERN
+ 
+ 
+ # Check all specified config values took (might have missed-dependency issues)
+-for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
++for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
+ 
+ 	REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
+ 	ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
diff --git a/support/kconfig/patches/series b/support/kconfig/patches/series
index be8627db13..e5a6f69d8f 100644
--- a/support/kconfig/patches/series
+++ b/support/kconfig/patches/series
@@ -9,3 +9,4 @@
 18-merge-config.sh-create-temporary-files-in-tmp.patch
 19-merge_config.sh-add-br2-external-support.patch
 20-merge_config.sh-Allow-to-define-config-prefix.patch
+21-Avoid-false-positive-matches-from-comment-lines.patch
-- 
2.15.0

[Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines

[Petr Vorel]

Hi Nasser,

> From: Nasser Afshin <Afshin.Nasser@gmail.com>

> We are using empty CONFIG_PREFIX_. This results in false positive match
> for comment lines when merging config fragments.

> To avoid false positive reports, we use separate sed expressions and
> address comment lines explicitly.

> This is actually an upstream patch [1] from linux/kbuild project.

> [1] https://patchwork.kernel.org/patch/10667571/
And this one is in mainline as well (v4.20-rc2):
6bbe4385d035c6fac56f840a59861a0310ce137b
("kconfig: merge_config: avoid false positive matches from comment lines")

> Signed-off-by: Nasser Afshin <Afshin.Nasser@gmail.com>
Reviewed-by: Petr Vorel <petr.vorel@gmail.com>


Kind regards,
Petr

[Buildroot] [PATCH v3 0/3] Fix merging configuration fragments

[Nasser Afshin]

Hi all
On Tue, Nov 13, 2018 at 05:14:01PM +0330, Nasser Afshin wrote:
> From: Nasser Afshin <Afshin.Nasser@gmail.com>
> 
> We use merge_config.sh as a helper script to merge many types of config
> fragments. The script has recently improved to better support buildroot. Now
> the report mechanism (-r -m) works as expected for buildroot configuration
> fragments.
> 
> These are patches to reproduce the state of the Linux Kernel Build
> mainline project.
> 
> 
> Nasser Afshin (3):
>   merge_config.sh: Fix merging buildroot config files
>   test-pkg: Use the correct config prefix when merging
>   merge_config.sh: Avoid false positive matches from comment lines
> 
>  support/kconfig/merge_config.sh                    | 12 ++++++--
>  ...e_config.sh-Allow-to-define-config-prefix.patch | 31 +++++++++++++++++++++
>  ...false-positive-matches-from-comment-lines.patch | 32 ++++++++++++++++++++++
>  support/kconfig/patches/series                     |  2 ++
>  utils/test-pkg                                     |  2 +-
>  5 files changed, 75 insertions(+), 4 deletions(-)
>  create mode 100644 support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch
>  create mode 100644 support/kconfig/patches/21-Avoid-false-positive-matches-from-comment-lines.patch
> 
> -- 
> 2.15.0
> 
This patch series is superseded by a new version.

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Petr Vorel]

Hi Nasser,

> > > +unset CONFIG_
> > You added unset, which isn't in upstream patch. This has no effect, so I'd be
> > for removing it. Even if it has (if it was needed), it'd be better to 1) ask
> > upstream to add it 2) keep it in separate patch in meanwhile (otherwise it
> > complicates patch rebasing during update and can be lost).
> Removing unset will cause the following make command work wrong. Note
> that the value of this environment variable is checked [1] and used. The
> result of removing the unset line is that we will have double BR2_
> prefixes in the final .config file as well as lots of warnings when
> checking if all specified config values have been taken.
You're right. The reason is that we don't actually use BR2_ as a real prefix
(as Masahiro noted [1], but I didn't realize that), but here in utils/test-pkg
we pass it as merge_config.sh needs to work with prefixes [2]:

-    support/kconfig/merge_config.sh -O "${dir}" \
+    CONFIG_=BR2_ support/kconfig/merge_config.sh -O "${dir}" \

Delta is smaller, but it's still a patch, which is needed :(.

> > BTW (I noted that before) your patch contain trailing whitespace and mixing tab
> > and spaces. git am and pwclient fixes that, only when applying with patch they
> > get committed, so nothing serious.

> > $ pwclient git-am -p buildroot 991781
> > .git/rebase-apply/patch:59: space before tab in indent.
> >  	echo "  -r    list redundant entries when merging fragments"
> > .git/rebase-apply/patch:60: space before tab in indent.
> >  	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
> > .git/rebase-apply/patch:61: space before tab in indent.
> >  	echo "  -e    colon-separated list of br2-external trees to use (optional)"
> > .git/rebase-apply/patch:66: trailing whitespace.

> > .git/rebase-apply/patch:72: trailing whitespace.

> > warning: squelched 6 whitespace errors
> > warning: 10 lines applied after fixing whitespace errors.
> > Applying: merge_config.sh: Fix merging buildroot config files
> > Applying patch #991781 using 'git am'
> > Description: merge_config.sh: Fix merging buildroot config files

> I think this is because we are including a patch in another patch. As
> you can see in your previous commit:
> a3366b270562f42723a3b8032f73bb03b541d113, this patch has the same issue
> because it includes another patch.
> I don't know how to avoid that if we should do so.
Again, you're right. I'm sorry for bothering you with it.


Kind regards,
Petr

[1] https://patchwork.kernel.org/patch/10658589/#22290097
[2] https://patchwork.ozlabs.org/patch/991781/

[Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files

[Nasser]

Hi Petr,
On Thu, Nov 01, 2018 at 02:23:51PM +0100, Petr Vorel wrote:
> Hi Nasser,
> 
> > > > +unset CONFIG_
> > > You added unset, which isn't in upstream patch. This has no effect, so I'd be
> > > for removing it. Even if it has (if it was needed), it'd be better to 1) ask
> > > upstream to add it 2) keep it in separate patch in meanwhile (otherwise it
> > > complicates patch rebasing during update and can be lost).
> > Removing unset will cause the following make command work wrong. Note
> > that the value of this environment variable is checked [1] and used. The
> > result of removing the unset line is that we will have double BR2_
> > prefixes in the final .config file as well as lots of warnings when
> > checking if all specified config values have been taken.
> You're right. The reason is that we don't actually use BR2_ as a real prefix
> (as Masahiro noted [1], but I didn't realize that), but here in utils/test-pkg
> we pass it as merge_config.sh needs to work with prefixes [2]:
> 
> -    support/kconfig/merge_config.sh -O "${dir}" \
> +    CONFIG_=BR2_ support/kconfig/merge_config.sh -O "${dir}" \
> 
> Delta is smaller, but it's still a patch, which is needed :(.
OK. I'll prepare a separate patch for this chunk.
> 
> > > BTW (I noted that before) your patch contain trailing whitespace and mixing tab
> > > and spaces. git am and pwclient fixes that, only when applying with patch they
> > > get committed, so nothing serious.
> 
> > > $ pwclient git-am -p buildroot 991781
> > > .git/rebase-apply/patch:59: space before tab in indent.
> > >  	echo "  -r    list redundant entries when merging fragments"
> > > .git/rebase-apply/patch:60: space before tab in indent.
> > >  	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
> > > .git/rebase-apply/patch:61: space before tab in indent.
> > >  	echo "  -e    colon-separated list of br2-external trees to use (optional)"
> > > .git/rebase-apply/patch:66: trailing whitespace.
> 
> > > .git/rebase-apply/patch:72: trailing whitespace.
> 
> > > warning: squelched 6 whitespace errors
> > > warning: 10 lines applied after fixing whitespace errors.
> > > Applying: merge_config.sh: Fix merging buildroot config files
> > > Applying patch #991781 using 'git am'
> > > Description: merge_config.sh: Fix merging buildroot config files
> 
> > I think this is because we are including a patch in another patch. As
> > you can see in your previous commit:
> > a3366b270562f42723a3b8032f73bb03b541d113, this patch has the same issue
> > because it includes another patch.
> > I don't know how to avoid that if we should do so.
> Again, you're right. I'm sorry for bothering you with it.
No problem :-).
> 
> 
> Kind regards,
> Petr
> 
> [1] https://patchwork.kernel.org/patch/10658589/#22290097
> [2] https://patchwork.ozlabs.org/patch/991781/
Kind regards,
Nasser

[Buildroot] Antwort: Re: Antwort: Re: [PATCH v3] merge_config.sh: merge also buildroot config files

[Petr Vorel]

Hi,

FYI: Masahiro Yamada (kconfig kernel maintainer) proposed [1] solution using env.
variable CONFIG_ (as it's already used in kconfig binary). I'm ok with it,
I propose to take upstream commit from kernel.


Kind regards,
Petr

[1] https://patchwork.kernel.org/patch/10658589/#22289439