All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging
  2018-11-13 13:44 ` [Buildroot] [PATCH v3 0/3] Fix merging configuration fragments Nasser Afshin
@ 2018-11-13 13:44   ` Nasser Afshin
  2018-11-13 18:26     ` Petr Vorel
  0 siblings, 1 reply; 16+ messages in thread
From: Nasser Afshin @ 2018-11-13 13:44 UTC (permalink / raw
  To: buildroot

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

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

* [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging
  2018-11-13 13:44   ` [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging Nasser Afshin
@ 2018-11-13 18:26     ` Petr Vorel
  0 siblings, 0 replies; 16+ messages in thread
From: Petr Vorel @ 2018-11-13 18:26 UTC (permalink / raw
  To: buildroot

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

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

* [Buildroot] [PATCH v3 0/3] Fix merging configuration fragments
@ 2018-11-14  7:11 Nasser Afshin
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files Nasser Afshin
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Nasser Afshin @ 2018-11-14  7:11 UTC (permalink / raw
  To: buildroot

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.

When using this feature in buildroot with empty config prefixes, we have some
false positive reports from comment lines. The last patch tries to avoid such
false positive matches.

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

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

* [Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files
  2018-11-14  7:11 [Buildroot] [PATCH v3 0/3] Fix merging configuration fragments Nasser Afshin
@ 2018-11-14  7:11 ` Nasser Afshin
  2018-11-23 21:14   ` Yann E. MORIN
  2018-11-24  9:12   ` Thomas Petazzoni
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging Nasser Afshin
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines Nasser Afshin
  2 siblings, 2 replies; 16+ messages in thread
From: Nasser Afshin @ 2018-11-14  7:11 UTC (permalink / raw
  To: buildroot

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

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

* [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging
  2018-11-14  7:11 [Buildroot] [PATCH v3 0/3] Fix merging configuration fragments Nasser Afshin
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files Nasser Afshin
@ 2018-11-14  7:11 ` Nasser Afshin
  2018-11-14  7:17   ` Petr Vorel
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines Nasser Afshin
  2 siblings, 1 reply; 16+ messages in thread
From: Nasser Afshin @ 2018-11-14  7:11 UTC (permalink / raw
  To: buildroot

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

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

* [Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines
  2018-11-14  7:11 [Buildroot] [PATCH v3 0/3] Fix merging configuration fragments Nasser Afshin
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files Nasser Afshin
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging Nasser Afshin
@ 2018-11-14  7:11 ` Nasser Afshin
  2018-11-23 21:16   ` Yann E. MORIN
  2018-11-24  9:12   ` Thomas Petazzoni
  2 siblings, 2 replies; 16+ messages in thread
From: Nasser Afshin @ 2018-11-14  7:11 UTC (permalink / raw
  To: buildroot

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 is in the Linux kernel mainline (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>
---
 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

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

* [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging Nasser Afshin
@ 2018-11-14  7:17   ` Petr Vorel
  2018-11-14  7:49     ` Nasser Afshin
  0 siblings, 1 reply; 16+ messages in thread
From: Petr Vorel @ 2018-11-14  7:17 UTC (permalink / raw
  To: buildroot

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..e4f68ed061 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}" \
Thanks for a change.


Kind regards,
Petr

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

* [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging
  2018-11-14  7:17   ` Petr Vorel
@ 2018-11-14  7:49     ` Nasser Afshin
  2018-11-14  8:24       ` Arnout Vandecappelle
  0 siblings, 1 reply; 16+ messages in thread
From: Nasser Afshin @ 2018-11-14  7:49 UTC (permalink / raw
  To: buildroot

Hi Petr
On Wed, Nov 14, 2018 at 08:17:33AM +0100, Petr Vorel wrote:
> 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>
Sorry I forgot to mention that. Resent.
> > ---
> >  utils/test-pkg | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> > diff --git a/utils/test-pkg b/utils/test-pkg
> > index aa91ee02cf..e4f68ed061 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}" \
> Thanks for a change.
> 
> 
> Kind regards,
> Petr
Kind regards,
Nasser

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

* [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging
  2018-11-14  7:49     ` Nasser Afshin
@ 2018-11-14  8:24       ` Arnout Vandecappelle
  2018-11-14  8:29         ` Petr Vorel
  2018-11-14  9:04         ` Nasser Afshin
  0 siblings, 2 replies; 16+ messages in thread
From: Arnout Vandecappelle @ 2018-11-14  8:24 UTC (permalink / raw
  To: buildroot

 Hi Nasser,

 Two remarks for your future contributions.

On 14/11/2018 08:49, Nasser Afshin wrote:
> Hi Petr
> On Wed, Nov 14, 2018 at 08:17:33AM +0100, Petr Vorel wrote:
>> 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>

 When you send a v2 or later, please include a patch changelog, like so:

---
v2: remove quotes from the empty CONFIG_ assignment (suggested by Peter Vorel)


 The --- splits the commit message from additional notes that won't be included
when the patch is applied.

>> Reviewed-by: Petr Vorel <petr.vorel@gmail.com>
> Sorry I forgot to mention that. Resent.

 There is no need to resend for that: patchwork will include it automatically.


 Regards,
 Arnout

>>> ---
>>>  utils/test-pkg | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>> diff --git a/utils/test-pkg b/utils/test-pkg
>>> index aa91ee02cf..e4f68ed061 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}" \
>> Thanks for a change.
>>
>>
>> Kind regards,
>> Petr
> Kind regards,
> Nasser
> 

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

* [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging
  2018-11-14  8:24       ` Arnout Vandecappelle
@ 2018-11-14  8:29         ` Petr Vorel
  2018-11-14  8:55           ` Nasser Afshin
  2018-11-14  9:04         ` Nasser Afshin
  1 sibling, 1 reply; 16+ messages in thread
From: Petr Vorel @ 2018-11-14  8:29 UTC (permalink / raw
  To: buildroot

Hi Nasser,

agree with Arnout. + one more tip: increase version each time you send it,
even it's just a tiny change.

Kind regards,
Petr

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

* [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging
  2018-11-14  8:29         ` Petr Vorel
@ 2018-11-14  8:55           ` Nasser Afshin
  0 siblings, 0 replies; 16+ messages in thread
From: Nasser Afshin @ 2018-11-14  8:55 UTC (permalink / raw
  To: buildroot

Dear Petr,
On Wed, Nov 14, 2018 at 09:29:52AM +0100, Petr Vorel wrote:
> Hi Nasser,
> 
> agree with Arnout. + one more tip: increase version each time you send it,
> even it's just a tiny change.
> 
> Kind regards,
> Petr
Thank you for your tip. Yes, you are right, I forgot to do so.
Kind regards,
Nasser

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

* [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging
  2018-11-14  8:24       ` Arnout Vandecappelle
  2018-11-14  8:29         ` Petr Vorel
@ 2018-11-14  9:04         ` Nasser Afshin
  1 sibling, 0 replies; 16+ messages in thread
From: Nasser Afshin @ 2018-11-14  9:04 UTC (permalink / raw
  To: buildroot

Hi Arnout,
On Wed, Nov 14, 2018 at 09:24:32AM +0100, Arnout Vandecappelle wrote:
>  Hi Nasser,
> 
>  Two remarks for your future contributions.
> 
> On 14/11/2018 08:49, Nasser Afshin wrote:
> > Hi Petr
> > On Wed, Nov 14, 2018 at 08:17:33AM +0100, Petr Vorel wrote:
> >> 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>
> 
>  When you send a v2 or later, please include a patch changelog, like so:
> 
> ---
> v2: remove quotes from the empty CONFIG_ assignment (suggested by Peter Vorel)
> 
> 
>  The --- splits the commit message from additional notes that won't be included
> when the patch is applied.
> 
I thought it's a tiny change. You're right I should have included the
changelog anyway.
> >> Reviewed-by: Petr Vorel <petr.vorel@gmail.com>
> > Sorry I forgot to mention that. Resent.
> 
>  There is no need to resend for that: patchwork will include it automatically.
Ok. Thank you for reviewing my patch and sending your tips.
Sorry for being a novice.
> 
> 
>  Regards,
>  Arnout
> 
> >>> ---
> >>>  utils/test-pkg | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>> diff --git a/utils/test-pkg b/utils/test-pkg
> >>> index aa91ee02cf..e4f68ed061 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}" \
> >> Thanks for a change.
> >>
> >>
> >> Kind regards,
> >> Petr
> > Kind regards,
> > Nasser
> > 
Kind regards,
Nasser

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

* [Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files Nasser Afshin
@ 2018-11-23 21:14   ` Yann E. MORIN
  2018-11-24  9:12   ` Thomas Petazzoni
  1 sibling, 0 replies; 16+ messages in thread
From: Yann E. MORIN @ 2018-11-23 21:14 UTC (permalink / raw
  To: buildroot

Nasser, All,

On 2018-11-14 10:41 +0330, Nasser Afshin spake thusly:
> 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 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>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  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 mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines Nasser Afshin
@ 2018-11-23 21:16   ` Yann E. MORIN
  2018-11-24  9:12   ` Thomas Petazzoni
  1 sibling, 0 replies; 16+ messages in thread
From: Yann E. MORIN @ 2018-11-23 21:16 UTC (permalink / raw
  To: buildroot

Nasser, All,

On 2018-11-14 10:41 +0330, Nasser Afshin spake thusly:
> 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 is in the Linux kernel mainline (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>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  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 mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files Nasser Afshin
  2018-11-23 21:14   ` Yann E. MORIN
@ 2018-11-24  9:12   ` Thomas Petazzoni
  1 sibling, 0 replies; 16+ messages in thread
From: Thomas Petazzoni @ 2018-11-24  9:12 UTC (permalink / raw
  To: buildroot

Hello,

On Wed, 14 Nov 2018 10:41:54 +0330, Nasser Afshin wrote:
> 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 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>
> ---
>  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

Applied to next, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

* [Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines
  2018-11-14  7:11 ` [Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines Nasser Afshin
  2018-11-23 21:16   ` Yann E. MORIN
@ 2018-11-24  9:12   ` Thomas Petazzoni
  1 sibling, 0 replies; 16+ messages in thread
From: Thomas Petazzoni @ 2018-11-24  9:12 UTC (permalink / raw
  To: buildroot

Hello,

On Wed, 14 Nov 2018 10:41:56 +0330, Nasser Afshin wrote:
> 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 is in the Linux kernel mainline (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>
> ---
>  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

Applied to next, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

end of thread, other threads:[~2018-11-24  9:12 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-14  7:11 [Buildroot] [PATCH v3 0/3] Fix merging configuration fragments Nasser Afshin
2018-11-14  7:11 ` [Buildroot] [PATCH v3 1/3] merge_config.sh: Fix merging buildroot config files Nasser Afshin
2018-11-23 21:14   ` Yann E. MORIN
2018-11-24  9:12   ` Thomas Petazzoni
2018-11-14  7:11 ` [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging Nasser Afshin
2018-11-14  7:17   ` Petr Vorel
2018-11-14  7:49     ` Nasser Afshin
2018-11-14  8:24       ` Arnout Vandecappelle
2018-11-14  8:29         ` Petr Vorel
2018-11-14  8:55           ` Nasser Afshin
2018-11-14  9:04         ` Nasser Afshin
2018-11-14  7:11 ` [Buildroot] [PATCH v3 3/3] merge_config.sh: Avoid false positive matches from comment lines Nasser Afshin
2018-11-23 21:16   ` Yann E. MORIN
2018-11-24  9:12   ` Thomas Petazzoni
  -- strict thread matches above, loose matches on Subject: below --
2018-11-05  8:35 [Buildroot] [PATCH] merge_config.sh: Fix merging buildroot config files Petr Vorel
2018-11-13 13:44 ` [Buildroot] [PATCH v3 0/3] Fix merging configuration fragments Nasser Afshin
2018-11-13 13:44   ` [Buildroot] [PATCH v3 2/3] test-pkg: Use the correct config prefix when merging Nasser Afshin
2018-11-13 18:26     ` Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.