merge_config.sh: Fix finding redundant config mechanism
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
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.
---
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(-)
support/kconfig/merge_config.sh +1 -1
@@ -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"
Signature · 1 line
2.7.4
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
Nasser,
Quoted 8 lines · Nasser
We use BR2_* style for configuration variables in buildroot so we should use this style when extracting configuration options. Otherwise
CFG_LISTwill 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
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
Quoted 10 lines · Matthew Weber
Nasser,
Quoted 7 lines · Nasser
We use BR2_* style for configuration variables in buildroot so we should use this style when extracting configuration options. Otherwise
CFG_LISTwill 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...
Quoted 4 lines
Quoted 1 line
The
merge_config.shis 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...
Quoted 6 lines
- Buildroot cfgs for runtime tests (BR2_*)
- As a tool by users to merge together Buildroot configsI'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
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
Hi,
Quoted 8 lines
Quoted 2 lines
The
merge_config.shis used for a couple scenarios
- Appending kconfigs together (CONFIG_*)... I forgot about this one. Indeed, the buildroot
merge_config.shscript is
used in pkg-kconfig.mk to merge kernel configs. Ideally we should change that to use the package'smerge_config.shscript. However, the location of that script may vary, or it may even be missing...
Quoted 8 lines
Quoted 6 lines
- Buildroot cfgs for runtime tests (BR2_*)
- As a tool by users to merge together Buildroot configsI'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 commentsI 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.
Quoted 7 lines
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
Quoted 4 lines
Regards,
Arnout
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
Petr,
Quoted 14 lines · Petr Vorel
Hi,
Quoted 10 lines
Quoted 2 lines
The
merge_config.shis used for a couple scenarios
- Appending kconfigs together (CONFIG_*)... I forgot about this one. Indeed, the buildroot
merge_config.shscript is
used in pkg-kconfig.mk to merge kernel configs. Ideally we should change that to use the package'smerge_config.shscript. However, the location of that script may vary, or it may even be missing...Quoted 2 lines
- Buildroot cfgs for runtime tests (BR2_*)
- As a tool by users to merge together Buildroot configs
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
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 commentsI think Angelo's patch is the best approach after all.
+1. I also think keep using our
merge_config.shwith -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?
Quoted 13 lines
Quoted 8 lines
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).
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
Hi Matthew, Angelo,
Quoted 21 lines
Petr,
Quoted 16 lines · Petr Vorel
Hi,
Quoted 14 lines
Quoted 2 lines
The
merge_config.shis used for a couple scenarios
- Appending kconfigs together (CONFIG_*)... I forgot about this one. Indeed, the buildroot
merge_config.shscript is
used in pkg-kconfig.mk to merge kernel configs. Ideally we should change that to use the package'smerge_config.shscript. However, the location of that script may vary, or it may even be missing...Quoted 6 lines
- Buildroot cfgs for runtime tests (BR2_*)
- As a tool by users to merge together Buildroot configsI'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
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
I think Angelo's patch is the best approach after all.
+1. I also think keep using our
merge_config.shwith -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
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
In reply to Arnout VandecappelleHi Arnout,
Quoted 27 lines · Arnout Vandecappelle
Quoted 10 lines · Matthew Weber
Nasser,
Quoted 7 lines · Nasser
We use BR2_* style for configuration variables in buildroot so we should use this style when extracting configuration options. Otherwise
CFG_LISTwill 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...Quoted 4 lines
Quoted 1 line
The
merge_config.shis used for a couple scenarios
- Appending kconfigs together (CONFIG_*)... I forgot about this one. Indeed, the buildroot
merge_config.shscript is
used in pkg-kconfig.mk to merge kernel configs. Ideally we should change that to use the package'smerge_config.shscript. However, the location of that script may vary, or it may even be missing...
Quoted 8 lines
Quoted 6 lines
- Buildroot cfgs for runtime tests (BR2_*)
- As a tool by users to merge together Buildroot configsI'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 commentsI 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.
Quoted 10 lines
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 :-).
Quoted 6 lines
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?
Quoted 7 lines
Regards,
Arnout
Regards,
Nasser Afshin
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
Quoted 3 lines · Nasser Afshin
Hi Arnout,
On Sat, Oct 20, 2018 at 05:01:33PM +0100, Arnout Vandecappelle wrote:
[snip]
Quoted 22 lines
Quoted 1 line
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.
Quoted 10 lines
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 :-).Quoted 6 lines
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.
Quoted 4 lines
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
Quoted 11 lines
Quoted 7 lines
Regards,
ArnoutRegards,
Nasser Afshin
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
Hi Arnout, Nasser,
Quoted 2 lines
Quoted 1 line
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.
Quoted 11 lines
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.
Quoted 2 lines
Regards,
Arnout
Kind regards,
Petr
PATCH 1/1 merge_config.sh: Fix finding redundant config mechanism
Hi Petr
Quoted 7 lines · Petr Vorel
Hi Arnout, Nasser,
Quoted 2 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 5 lines
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.
Quoted 8 lines
Quoted 4 lines
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.
Quoted 5 lines
Quoted 2 lines
Regards,
ArnoutKind regards,
Petr
Kind regards,
Nasser
PATCH v2 1/1 merge_config.sh: merge also buildroot config files
In reply to Arnout VandecappelleFrom: Angelo Compagnucci <[email protected]>
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.
--- ...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
support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch new file +41 -0
@@ -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"
support/kconfig/patches/series +1 -0
@@ -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
Signature · 1 line
2.15.0
PATCH v2 1/1 merge_config.sh: merge also buildroot config files
Hi all,
Quoted 10 lines · Nasser Afshin
From: Angelo Compagnucci <[email protected]>
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 <[email protected]>Signed-off-by Nasser Afshin <[email protected]>
I'm not sure if this is the preferred way of keeping the authorship. Please correct me if it's not.
Quoted 65 lines
--- ...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.patchsupport/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch new file +41
-0@@ -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"support/kconfig/patches/series +1
-0@@ -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.patchSignature · 1 line
2.15.0
Kind regards,
Nasser
PATCH v3 1/1 merge_config.sh: merge also buildroot config files
In reply to Nasser AfshinFrom: Angelo Compagnucci <[email protected]>
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.
---
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
support/kconfig/merge_config.sh +8 -1
@@ -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"
support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch new file +41 -0
41 lines not on this page · raw message
support/kconfig/patches/series +1 -0
@@ -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
Signature · 1 line
2.15.0
[Buildroot] Antwort: [PATCH v3] merge_config.sh: merge also buildroot config files
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" <[email protected]> schrieb am 25.10.2018 04:20:31:
Quoted 28 lines
Von: Nasser Afshin <[email protected]>
An: Arnout Vandecappelle <[email protected]>
Kopie: Nasser Afshin <[email protected]>, buildroot <[email protected]>
Datum: 25.10.2018 04:21
Betreff: [Buildroot] [PATCH v3]merge_config.sh: merge also buildroot config files
Gesendet von: "buildroot" <[email protected]>From: Angelo Compagnucci <[email protected]>
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 <[email protected]>Signed-off-by Nasser Afshin <[email protected]>---
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-
Review
Quoted 3 lines
redundant-config-mechanism.patch
(quoted diff) +0
-0
b/support/kconfig/merge_config.sh
Quoted 10 lines
index
67d1314476..f22198023c100755support/kconfig/merge_config.sh +1
-0@@ -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 acustom make command)"
echo " -m only merge the fragments, do not execute the make
command"
Quoted 23 lines
echo " -n use allnoconfig instead of alldefconfig" echo " -r list redundant entries when merging fragments"support/kconfig/merge_config.sh +6
-0@@ -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 ;;
support/kconfig/merge_config.sh if [ ! -r "$INITFILE" ]; then · line 109
Quoted 5 lines
support/kconfig/merge_config.sh +1
-1@@ -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"
Quoted 10 lines
support/kconfig/merge_config.sh +0
-0TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX) echo "Using $INITFILE as base"(quoted diff) +0
-0config-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
Quoted 10 lines
support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch new file +9
-0@@ -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
Quoted 2 lines
custom make command)"
+ echo " -m only merge the fragments, do not execute the make
command"
Quoted 32 lines
support/kconfig/patches/18-fix-finding-redundant-config-mechanism.patch new file +28
-0+ 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"
Review
Quoted 1 line
(quoted diff) +0
-0
b/support/kconfig/patches/series
Quoted 15 lines
index
36591e2189..5ebc2e0225100644support/kconfig/patches/series +1
-0@@ -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.patchSignature · 6 lines
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.
PATCH v3 1/1 merge_config.sh: merge also buildroot config files
In reply to Nasser AfshinHi Angelo,
Quoted 10 lines
From: Angelo Compagnucci <[email protected]>
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 <[email protected]>Signed-off-by Nasser Afshin <[email protected]>
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?
Quoted 1 line
+CONFIG_PREFIX=CONFIG_
CONFIG_PREFIX=CONFIG
support/kconfig/merge_config.sh while true; do · line 77
Quoted 8 lines
while true; do
support/kconfig/merge_config.sh +2
-0case $1 in@@ -71,6 +73,11 @@ while true; do shift 2 continue ;; + "-b") + CONFIG_PREFIX=BR2_
CONFIG_PREFIX=BR2
...
support/kconfig/merge_config.sh if [ ! -r "$INITFILE" ]; then · line 109
Quoted 3 lines
support/kconfig/merge_config.sh +1
-1MERGE_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
Hi Petr,
Quoted 1 line
Not sure whether all packages using kconfig-package are using prefix
CONFIG_,
Quoted 1 line
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
Hi Thomas,
Quoted 7 lines
Hi Petr,
Quoted 1 line
Not sure whether all packages using kconfig-package are using prefix
CONFIG_,Quoted 1 line
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
Hi Petr,
Quoted 1 line
Looking in the code uclibc-ng uses several prefixes:
CONFIG_,TARGET_,
UCLIBC_
Quoted 4 lines
and sometimes even configs without prefix: VERSION, WARNINGS,
EXTRA_WARNINGS[1].But their
SED_CONFIG_EXPuses alsoCONFIG_.
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...
Signature · 7 lines
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
Hi Marcel, Petr,
Quoted 7 lines · Marcel Patzlaff
Hi Petr,
Quoted 1 line
Looking in the code uclibc-ng uses several prefixes:
CONFIG_,TARGET_,
UCLIBC_Quoted 3 lines
and sometimes even configs without prefix: VERSION, WARNINGS,
EXTRA_WARNINGS[1].
Quoted 6 lines
Quoted 1 line
But their
SED_CONFIG_EXPuses alsoCONFIG_.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 ownmerge_config.shanywhere and I assume that it is also not used elsewhere.
Yes uclibc does not use it's own merge_config.sh anywhere.
Quoted 4 lines
I would suggest to just remove the prefix restrictions in buildroot's
merge_config.shunless 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.
Quoted 15 lines
Kind regards,
MarcelPS: Sorry for the second reply. My mailclient dropped the list...
Signature · 7 lines
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
Hi Marcel, Nasser,
Quoted 20 lines
Quoted 3 lines
I would suggest to just remove the prefix restrictions in buildroot's
merge_config.shunless 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_namefor kernel style config files or
sed -n 's/^\(# \)\{0,1\}\(BR2_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
defconfig_file_name
Quoted 8 lines
for buildroot style config files or
sed -n 's/^\(# \)\{0,1\}\([a-zA-Z0-9_]*\)[= ].*/\2/p' \
defconfig_file_nameas 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
Hi Petr, Marcel,
Quoted 27 lines · Petr Vorel
Hi Marcel, Nasser,
Quoted 24 lines
Quoted 3 lines
I would suggest to just remove the prefix restrictions in buildroot's
merge_config.shunless 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_namefor kernel style config files or
sed -n 's/^\(# \)\{0,1\}\(BR2_[a-zA-Z0-9_]*\)[= ].*/\2/p' \
defconfig_file_namefor buildroot style config files or
sed -n 's/^\(# \)\{0,1\}\([a-zA-Z0-9_]*\)[= ].*/\2/p' \
defconfig_file_name
Quoted 8 lines
Quoted 3 lines
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.
Quoted 8 lines
Kind regards,
Petr
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Kind regards,
Nasser
PATCH v3 1/1 merge_config.sh: merge also buildroot config files
Hi Nasser,
Quoted 9 lines
Quoted 8 lines
Quoted 3 lines
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_nameI 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).
Quoted 2 lines
Kind regards,
Nasser
Kind regards,
Petr
PATCH v3 1/1 merge_config.sh: merge also buildroot config files
Quoted 6 lines · Petr Vorel
Hi Nasser,
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
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.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
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_nameI 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.
Quoted 2 lines
Would you like to backport it to buildroot + add necessary applications (setting the variable?) or shell I do it?
Sure, I'll do it.
Quoted 4 lines
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.
Quoted 9 lines
Quoted 4 lines
Kind regards,
NasserKind regards,
Petr
Kind regards,
Nasser
PATCH v3 1/1 merge_config.sh: merge also buildroot config files
Hi Nasser,
Quoted 6 lines
Quoted 2 lines
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.
Quoted 2 lines
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.
Quoted 6 lines
Quoted 3 lines
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
PATCH 1/1 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.
--- 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
support/kconfig/merge_config.sh +7 -1
@@ -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
support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch new file +39 -0
@@ -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 + +
support/kconfig/patches/series +1 -0
@@ -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
utils/test-pkg +1 -1
@@ -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
Signature · 1 line
2.15.0
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi.
Quoted 14 lines
-----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 <[email protected]>
Cc: Nasser Afshin <[email protected]>; buildroot <[email protected]>
Subject: [Buildroot] [PATCH]merge_config.sh: Fix merging buildroot config filesThis patch allows us to define config prefix with
CONFIG_environment variable. Now we can defineBR2_prefix when we are going to merge buildroot fragment configs.
Quoted 8 lines
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.
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi.
Quoted 11 lines
Quoted 5 lines
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=f6fe19e9d84400e1c9544c1909952c2c7c6a90a2Please 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
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Quoted 11 lines · Petr Vorel
Hi.
Quoted 8 lines
Quoted 5 lines
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=f6fe19e9d84400e1c9544c1909952c2c7c6a90a2Please refer to commit ID only after it gets stable in Linus' tree.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 6 lines
Kind regards,
Petr
Thank you for adding the link here.
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
In reply to Nasser AfshinHi Nasser,
...
support/kconfig/merge_config.sh fi · line 165
Quoted 5 lines
support/kconfig/merge_config.sh +1
-0@@ -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).
...
Quoted 1 line
support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch +0
-0
...
Quoted 4 lines
+ # Use the merged file as the starting point for:
+ # alldefconfig: Fills in any missing symbols withKconfigdefault
+ # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set ++unsetCONFIG_
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
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi Petr,
Quoted 12 lines · Petr Vorel
Hi Nasser,
...
support/kconfig/merge_config.sh fi · line 165
Quoted 5 lines
support/kconfig/merge_config.sh +1
-0@@ -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.
Quoted 22 lines
...
Quoted 1 line
support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch +0
-0...
Quoted 4 lines
+ # Use the merged file as the starting point for:
+ # alldefconfig: Fills in any missing symbols withKconfigdefault
+ # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set ++unsetCONFIG_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.
Quoted 8 lines
.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.
Quoted 2 lines
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
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Quoted 14 lines · Nasser
Hi Petr,
Quoted 12 lines · Petr Vorel
Hi Nasser,
...
support/kconfig/merge_config.sh fi · line 165
Quoted 5 lines
support/kconfig/merge_config.sh +1
-0@@ -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).
Quoted 5 lines
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.
Quoted 32 lines
I think we should follow your steps to include it.
Quoted 22 lines
...
Quoted 1 line
support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch +0
-0...
Quoted 4 lines
+ # Use the merged file as the starting point for:
+ # alldefconfig: Fills in any missing symbols withKconfigdefault
+ # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set ++unsetCONFIG_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.Quoted 8 lines
.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
Quoted 5 lines
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
Quoted 6 lines
Quoted 2 lines
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
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi Arnout,
Quoted 10 lines
Quoted 5 lines
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 callingmerge_config,
we should setCONFIG_to empty, not toBR2_.So indeed, the unset should be removed.
+1. I overlooked that regexp works with empty $CONFIG_ as well. Thanks for explanation.
Kind regards,
Petr
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi Petr, Arnout,
Quoted 8 lines · Petr Vorel
Hi Arnout,
Quoted 5 lines
Quoted 5 lines
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.
Quoted 7 lines
Quoted 4 lines
We have no prefix, the
BR2_is mere convention. So when callingmerge_config,
we should setCONFIG_to empty, not toBR2_.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.
Quoted 3 lines
Kind regards,
Petr
Kind regards,
Nasser
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi.
Quoted 25 lines
-----Original Message-----
From: Nasser [mailto:afshin.nasser at gmail.com]
Sent: Friday, November 02, 2018 11:13 AM
To: Petr Vorel <[email protected]>
Cc: Arnout Vandecappelle <[email protected]>; buildroot at buildroot.org; Matthew Weber <[email protected]>; Angelo Compagnucci <[email protected]>; Yamada, Masahiro/?? ?? <[email protected]>
Subject: Re: [PATCH]merge_config.sh: Fix merging buildroot config filesHi Petr, Arnout,
Quoted 4 lines · Petr Vorel
Hi Arnout,
Quoted 2 lines
Quoted 2 lines
Removing unset will cause the following make command work wrong. Note that the value of this environment variable is checked [1] and used.
The
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
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,Quoted 2 lines
Quoted 1 line
we should set
CONFIG_to empty, not toBR2_.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
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/
Quoted 7 lines
Quoted 3 lines
Kind regards,
PetrKind regards,
Nasser
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Quoted 35 lines · yamada.masahiro at socionext.com
Hi.
Quoted 25 lines
-----Original Message-----
From: Nasser [mailto:afshin.nasser at gmail.com]
Sent: Friday, November 02, 2018 11:13 AM
To: Petr Vorel <[email protected]>
Cc: Arnout Vandecappelle <[email protected]>; buildroot at buildroot.org; Matthew Weber <[email protected]>; Angelo Compagnucci <[email protected]>; Yamada, Masahiro/?? ?? <[email protected]>
Subject: Re: [PATCH]merge_config.sh: Fix merging buildroot config filesHi Petr, Arnout,
Quoted 4 lines · Petr Vorel
Hi Arnout,
Quoted 2 lines
Quoted 2 lines
Removing unset will cause the following make command work wrong. Note that the value of this environment variable is checked [1] and used.
The
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
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,Quoted 2 lines
Quoted 1 line
we should set
CONFIG_to empty, not toBR2_.Quoted 7 lines
Quoted 4 lines
Quoted 1 line
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.
Quoted 6 lines
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
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi Arnout,
Quoted 10 lines
Quoted 2 lines
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.
Quoted 2 lines
Regards,
Arnout
Kind regards,
Petr
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi Petr, Mahasiro,
Quoted 7 lines · Petr Vorel
Hi Arnout,
Quoted 4 lines
Quoted 2 lines
How about this?
https://patchwork.kernel.org/patch/10665119/Perhaps it's better to make the entire pattern explicit, like:
Quoted 8 lines
Quoted 5 lines
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.
Quoted 7 lines
Quoted 2 lines
Regards,
ArnoutKind regards,
Petr
Kind regards,
Nasser
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi.
Quoted 23 lines
-----Original Message-----
From: Nasser [mailto:afshin.nasser at gmail.com]
Sent: Sunday, November 04, 2018 6:32 AM
To: Petr Vorel <[email protected]>
Cc: Arnout Vandecappelle <[email protected]>; Yamada, Masahiro/?? ?? <[email protected]>; buildroot at buildroot.org; matthew.weber at rockwellcollins.com; angelo.compagnucci at gmail.com
Subject: Re: [PATCH]merge_config.sh: Fix merging buildroot config filesHi Petr, Mahasiro,
Quoted 11 lines · Petr Vorel
Hi Arnout,
Quoted 9 lines
Quoted 2 lines
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
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
\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 makesmerge_config.shto 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.
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi,
Quoted 17 lines
Quoted 7 lines
Quoted 6 lines
Quoted 6 lines
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
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
\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 makesmerge_config.shto 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.
Quoted 5 lines
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
v3 cover Fix merging configuration fragments
From: Nasser Afshin <[email protected]>
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
Signature · 1 line
2.15.0
PATCH v3 1/3 merge_config.sh: Fix merging buildroot config files
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/
--- 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
support/kconfig/merge_config.sh +6 -1
@@ -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"
support/kconfig/patches/20-merge_config.sh-Allow-to-define-config-prefix.patch new file +31 -0
@@ -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"
support/kconfig/patches/series +1 -0
@@ -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
Signature · 1 line
2.15.0
PATCH v3 1/3 merge_config.sh: Fix merging buildroot config files
Hi Nasser,
Quoted 9 lines
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.
This is actually already in mainline for v4.20-rc1:2cd3faf87d2d8f6123adf34741b9a7b98828a76f ("merge_config.sh: Allow to define config prefix")
Quoted 1 line
Signed-off-by Nasser Afshin <[email protected]>
Kind regards,
Petr
PATCH v3 1/3 merge_config.sh: Fix merging buildroot config files
Hi Petr
Quoted 8 lines · Petr Vorel
Hi Nasser,
Quoted 5 lines
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.
Quoted 7 lines
Quoted 3 lines
This is actually an upstream patch [1] from linux/kbuild project.
This is actually already in mainline for v4.20-rc1:
2cd3faf87d2d8f6123adf34741b9a7b98828a76f("merge_config.sh: Allow to define config prefix")
Oh yes, corrected, resent.
Quoted 5 lines
Quoted 1 line
Signed-off-by Nasser Afshin <[email protected]>Reviewed-by Petr Vorel <[email protected]>Kind regards,
Petr
Kind regards,
Nasser
PATCH v3 2/3 test-pkg: Use the correct config prefix when merging
In reply to Nasser AfshinFrom: Nasser Afshin <[email protected]>
We should use an empty prefix as we do not have any prefix. Note that BR2_ is mere a convention.
--- utils/test-pkg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
utils/test-pkg +1 -1
@@ -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
Signature · 1 line
2.15.0
PATCH v3 2/3 test-pkg: Use the correct config prefix when merging
Hi Nasser,
Quoted 6 lines
From: Nasser Afshin <[email protected]>
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 <[email protected]>
Quoted 9 lines
--- utils/test-pkg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)utils/test-pkg +0
-0
utils/test-pkg build_one() { · line 132
Quoted 5 lines
utils/test-pkg +1
-1@@ -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
PATCH v3 3/3 merge_config.sh: Avoid false positive matches from comment lines
In reply to Nasser AfshinFrom: Nasser Afshin <[email protected]>
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/
--- 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
support/kconfig/merge_config.sh +4 -3
@@ -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")
support/kconfig/patches/21-Avoid-false-positive-matches-from-comment-lines.patch new file +32 -0
@@ -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")
support/kconfig/patches/series +1 -0
@@ -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
Signature · 1 line
2.15.0
PATCH v3 3/3 merge_config.sh: Avoid false positive matches from comment lines
Hi Nasser,
Quoted 11 lines
From: Nasser Afshin <[email protected]>
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.
And this one is in mainline as well (v4.20-rc2):6bbe4385d035c6fac56f840a59861a0310ce137b
("kconfig: merge_config: avoid false positive matches from comment lines")
Quoted 1 line
Signed-off-by Nasser Afshin <[email protected]>
Kind regards,
Petr
v3 cover Fix merging configuration fragments
In reply to Nasser AfshinHi all
Quoted 16 lines · Nasser Afshin
From: Nasser Afshin <[email protected]>
We use
merge_config.shas 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 mergingmerge_config.sh: Avoid false positive matches from comment lines
Quoted 12 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.patchSignature · 1 line
2.15.0
This patch series is superseded by a new version.
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
In reply to NasserHi Nasser,
Quoted 10 lines
Quoted 5 lines
Quoted 1 line
+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 :(.
Quoted 21 lines
Quoted 13 lines
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.Quoted 7 lines
.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
Quoted 5 lines
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/
PATCH 1/1 merge_config.sh: Fix merging buildroot config files
Hi Petr,
Quoted 19 lines · Petr Vorel
Hi Nasser,
Quoted 10 lines
Quoted 5 lines
Quoted 1 line
+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 asmerge_config.shneeds to work with prefixes [2]:utils/test-pkg +1
-1- support/kconfig/merge_config.sh -O "${dir}" \ + CONFIG_=BR2_ support/kconfig/merge_config.sh -O "${dir}" \
Quoted 1 line
Delta is smaller, but it's still a patch, which is needed :(.
OK. I'll prepare a separate patch for this chunk.
Quoted 22 lines
Quoted 21 lines
Quoted 21 lines
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
Quoted 6 lines
Quoted 5 lines
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 :-).
Quoted 7 lines
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
In reply to Marcel PatzlaffHi,
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