Skip to contentSkip to conversation

kconfig: Add merge_config.sh script

PATCH In review john stultz 5 participants 20 messages Nov 17, 2011 – Nov 22, 2011

Subsystems (2)
Kbuild Kconfig
lore
Share & tools
lore

john stultz

author
#0 lore raw

PATCH 1/1 kconfig: Add merge_config.sh script

Hey Andrew,
I've tried sending this via Michal a few times, but haven't heard much
back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

thanks
-john

After noticing almost every distro has their own method of managing config fragments, I went looking at some best practices, and wanted to try to consolidate some of the different approaches so this fairly simple infrastructure can be shared (and new distros/build systems don't have to implement yet another config fragment merge script).

This script is most influenced by the Windriver tools used in the Yocto Project, reusing some portions found there.

This script merges multiple config fragments, warning on any overrided values. It then sets any unspecified values to their default, then finally checks to make sure no specified value was dropped due to unsatisfied dependencies.

I'm sure this implementation won't work for everyone, and I expect it will need to evolve to adapt for various use cases. But I think its a reasonable starting point.

v2:
* Reworked to use alldefconfig instead of the proposed
olddefconfig as suggested by Sam Ravnborg.

v3:
* Script improvements from Dmitri.
* allnoconfig option from Darren
* pre-make exit option from Darren
* lots of other fixes/cleanups from Darren.
* Fix final check to not compain about config values in comments

Please let me know if you have any comments or thoughts!

CC Sam Ravnborg <[email protected]>
CC [email protected]
CC [email protected]
CC Dmitry Fink <[email protected]>
CC Darren Hart <[email protected]>
CC Eric B Munson <[email protected]>
CC Bruce Ashfield <[email protected]>
CC Michal Marek <[email protected]>
CC [email protected]
Signed-off-by John Stultz <[email protected]>
---
 scripts/kconfig/merge_config.sh |  117 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 117 insertions(+), 0 deletions(-)
 create mode 100755 scripts/kconfig/merge_config.sh
scripts/kconfig/merge_config.sh new file +117 -0
new file mode 100755
index 0000000..890276b
@@ -0,0 +1,117 @@
+#!/bin/sh
+#  merge_config.sh - Takes a list of config fragment values, and merges
+#  them one by one. Provides warnings on overridden values, and specified
+#  values that did not make it to the resulting .config file (due to missed
+#  dependencies or config symbol removal).
+#
+#  Portions reused from kconf_check and generate_cfg:
+#  http://git.yoctoproject.org/cgit/cgit.cgi/yocto-kernel-tools/tree/tools/kconf_check
+#  http://git.yoctoproject.org/cgit/cgit.cgi/yocto-kernel-tools/tree/tools/generate_cfg
+#
+#  Copyright (c) 2009-2010 Wind River Systems, Inc.
+#  Copyright 2011 Linaro
+#
+#  This program is free software; you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License version 2 as
+#  published by the Free Software Foundation.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+#  See the GNU General Public License for more details.
+
+clean_up() {
+       rm -f $TMP_FILE
+       exit
+}
+trap clean_up SIGHUP SIGINT SIGTERM
+
+usage() {
+	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
+	echo "  -h    display this help text"
+	echo "  -m    only merge the fragments, do not execute the make command"
+	echo "  -n    use allnoconfig instead of alldefconfig"
+}
+
+MAKE=true
+ALLTARGET=alldefconfig
+
+while true; do
+	case $1 in
+	"-n")
+		ALLTARGET=allnoconfig
+		shift
+		continue
+		;;
+	"-m")
+		MAKE=false
+		shift
+		continue
+		;;
+	"-h")
+		usage
+		exit
+		;;
+	*)
+		break
+		;;
+	esac
+done
+
+
+
+MERGE_LIST=$*
+SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
+TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
+
+# Merge files, printing warnings on overrided values
+for MERGE_FILE in $MERGE_LIST ; do
+	echo "Merging $MERGE_FILE"
+	CFG_LIST=$(sed -n "$SED_CONFIG_EXP" $MERGE_FILE)
+
+	for CFG in $CFG_LIST ; do
+		grep -q -w $CFG $TMP_FILE
+		if [ $? -eq 0 ] ; then
+			PREV_VAL=$(grep -w $CFG $TMP_FILE)
+			NEW_VAL=$(grep -w $CFG $MERGE_FILE)
+			if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then
+			echo Value of $CFG is redefined by fragment $MERGE_FILE:
+			echo Previous  value: $PREV_VAL
+			echo New value:       $NEW_VAL
+			echo
+			fi
+			sed -i "/$CFG[ =]/d" $TMP_FILE
+		fi
+	done
+	cat $MERGE_FILE >> $TMP_FILE
+done
+
+if [ "$MAKE" = "false" ]; then
+	cp $TMP_FILE .config
+	echo "#"
+	echo "# merged configuration written to .config (needs make)"
+	echo "#"
+	clean_up
+	exit
+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
+make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET
+
+
+# Check all specified config values took (might have missed-dependency issues)
+for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
+
+	REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG")
+	ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG")
+	if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
+		echo "Value requested for $CFG not in final .config"
+		echo "Requested value:  $REQUESTED_VAL"
+		echo "Actual value:     $ACTUAL_VAL"
+		echo ""
+	fi
+done
+
+clean_up
Signature · 1 line
1.7.3.2.146.gca209
#1 lore raw

Hi,

Quoted 6 lines · john stultz

Hey Andrew,
       I've tried sending this via Michal a few times, but haven't heard much back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

One of the worry I would have is that the script is merging config blindly, ie. there is no dependency checking done. I have some some work-in-progress to help resolving this, but still lots of thought to be implemented.

That said, I don't really mind for any gap-filling solution.

Regards,
- Arnaud

Quoted 184 lines

thanks
-john

After noticing almost every distro has their own method of managing config fragments, I went looking at some best practices, and wanted to try to consolidate some of the different approaches so this fairly simple infrastructure can be shared (and new distros/build systems don't have to implement yet another config fragment merge script).

This script is most influenced by the Windriver tools used in the Yocto Project, reusing some portions found there.

This script merges multiple config fragments, warning on any overrided values. It then sets any unspecified values to their default, then finally checks to make sure no specified value was dropped due to unsatisfied dependencies.

I'm sure this implementation won't work for everyone, and I expect it will need to evolve to adapt for various use cases. But I think its a reasonable starting point.

v2:
 * Reworked to use alldefconfig instead of the proposed olddefconfig as suggested by Sam Ravnborg.

v3:
 * Script improvements from Dmitri.
 * allnoconfig option from Darren
 * pre-make exit option from Darren
 * lots of other fixes/cleanups from Darren.
 * Fix final check to not compain about config values in comments

Please let me know if you have any comments or thoughts!

CC Sam Ravnborg <[email protected]>
CC [email protected]
CC [email protected]
CC Dmitry Fink <[email protected]>
CC Darren Hart <[email protected]>
CC Eric B Munson <[email protected]>
CC Bruce Ashfield <[email protected]>
CC Michal Marek <[email protected]>
CC [email protected]
Signed-off-by John Stultz <[email protected]>

---
 scripts/kconfig/merge_config.sh |  117 +++++++++++++++++++++++++++++++++++++++

 1 files changed, 117 insertions(+), 0 deletions(-)

 create mode 100755 scripts/kconfig/merge_config.sh

scripts/kconfig/merge_config.sh new file +117 -1
new file mode 100755
index 0000000..890276b
@@ -0,0 +1,117 @@
+#!/bin/sh
+#  merge_config.sh - Takes a list of config fragment values, and merges
+#  them one by one. Provides warnings on overridden values, and specified
+#  values that did not make it to the resulting .config file (due to missed
+#  dependencies or config symbol removal).
+#
+#  Portions reused from kconf_check and generate_cfg:
+#  http://git.yoctoproject.org/cgit/cgit.cgi/yocto-kernel-tools/tree/tools/kconf_check
+#  http://git.yoctoproject.org/cgit/cgit.cgi/yocto-kernel-tools/tree/tools/generate_cfg
+#
+#  Copyright (c) 2009-2010 Wind River Systems, Inc.
+#  Copyright 2011 Linaro
+#
+#  This program is free software; you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License version 2 as
+#  published by the Free Software Foundation.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+#  See the GNU General Public License for more details.
+
+clean_up() {
+       rm -f $TMP_FILE
+       exit
+}
+trap clean_up SIGHUP SIGINT SIGTERM
+
+usage() {
+       echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
+       echo "  -h    display this help text"
+       echo "  -m    only merge the fragments, do not execute the make command"
+       echo "  -n    use allnoconfig instead of alldefconfig"
+}
+
+MAKE=true
+ALLTARGET=alldefconfig
+
+while true; do
+       case $1 in
+       "-n")
+               ALLTARGET=allnoconfig
+               shift
+               continue
+               ;;
+       "-m")
+               MAKE=false
+               shift
+               continue
+               ;;
+       "-h")
+               usage
+               exit
+               ;;
+       *)
+               break
+               ;;
+       esac
+done
+
+
+
+MERGE_LIST=$*
+SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
+TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
+
+# Merge files, printing warnings on overrided values
+for MERGE_FILE in $MERGE_LIST ; do
+       echo "Merging $MERGE_FILE"
+       CFG_LIST=$(sed -n "$SED_CONFIG_EXP" $MERGE_FILE)
+
+       for CFG in $CFG_LIST ; do
+               grep -q -w $CFG $TMP_FILE
+               if [ $? -eq 0 ] ; then
+                       PREV_VAL=$(grep -w $CFG $TMP_FILE)
+                       NEW_VAL=$(grep -w $CFG $MERGE_FILE)
+                       if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then
+                       echo Value of $CFG is redefined by fragment $MERGE_FILE:
+                       echo Previous  value: $PREV_VAL
+                       echo New value:       $NEW_VAL
+                       echo
+                       fi
+                       sed -i "/$CFG[ =]/d" $TMP_FILE
+               fi
+       done
+       cat $MERGE_FILE >> $TMP_FILE
+done
+
+if [ "$MAKE" = "false" ]; then
+       cp $TMP_FILE .config
+       echo "#"
+       echo "# merged configuration written to .config (needs make)"
+       echo "#"
+       clean_up
+       exit
+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
+make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET
+
+
+# Check all specified config values took (might have missed-dependency issues)
+for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
+
+       REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG")
+       ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG")
+       if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
+               echo "Value requested for $CFG not in final .config"
+               echo "Requested value:  $REQUESTED_VAL"
+               echo "Actual value:     $ACTUAL_VAL"
+               echo ""
+       fi
+done
+
+clean_up
--

1.7.3.2.146.gca209

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

author
#2 lore raw
Quoted 9 lines · Arnaud Lacombe

Hi,

Quoted 6 lines · john stultz

Hey Andrew,
I've tried sending this via Michal a few times, but haven't heard much
back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

Quoted 4 lines

One of the worry I would have is that the script is merging config blindly, ie. there is no dependency checking done. I have some some work-in-progress to help resolving this, but still lots of thought to be implemented.

So the script actually does warn you if a specified option is dropped due to missing dependencies or if the option is removed. So, I guess could you clarify your concern a bit more?

thanks
-john

#3 lore raw

Hi,

Quoted 10 lines · john stultz
Quoted 9 lines · Arnaud Lacombe

Hi,

Quoted 6 lines · john stultz

Hey Andrew,
       I've tried sending this via Michal a few times, but haven't heard much back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

Quoted 9 lines
Quoted 4 lines

One of the worry I would have is that the script is merging config blindly, ie. there is no dependency checking done. I have some some work-in-progress to help resolving this, but still lots of thought to be implemented.

So the script actually does warn you if a specified option is dropped due to missing dependencies or if the option is removed.  So, I guess could you clarify your concern a bit more?

well, assuming the following Kconfig's snippet:

choice
bool "choice"
config A
bool "A"
config B
bool "B"
endchoice

and trying to merge:

- `config1':

CONFIG_A=y

- `config2':

Result in:

% sh scripts/kconfig/merge_config.sh config1 config2
Merging config1
Merging config2
scripts/kconfig/conf --alldefconfig Kconfig ./.tmp.config.uMY8Z97l9T:2:warning: override: B changes choice state #

# configuration written to .config
#

% cat .config
#
# Automatically generated file; DO NOT EDIT.
# Linux Kernel Configuration
#
# CONFIG_A is not set
CONFIG_B=y

so we still get the warning from the incantation of `alldefconfig', but the one in the script is defeated.

Moreover, there might still be performance optimization to realize, considering a base 'defconfig' amended by disabling CONFIG_DECOMPRESS_GZIP, we get:

% time sh scripts/kconfig/merge_config.sh base amend Merging base
Merging amend
Value of CONFIG_DECOMPRESS_GZIP is redefined by fragment amend:
Previous value: CONFIG_DECOMPRESS_GZIP=y
New value: # CONFIG_DECOMPRESS_GZIP is not set

scripts/kconfig/conf --alldefconfig Kconfig
#

# configuration written to .config
#
sh scripts/kconfig/merge_config.sh base amend  158.20s user 30.19s
system 100% cpu 3:07.86 total

Just by getting rid of the two `| grep ...' in the last step:

scripts/kconfig/merge_config.sh +2 -2
@@ -104,8 +104,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET
 # Check all specified config values took (might have missed-dependency issues)
 for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
 
-       REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG")
-       ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG")
+       REQUESTED_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" $TMP_FILE)
+       ACTUAL_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" .config)
        if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
                echo "Value requested for $CFG not in final .config"
                echo "Requested value:  $REQUESTED_VAL"

we get:

sh scripts/kconfig/merge_config.sh base amend 23.20s user 22.30s system 94% cpu 48.033 total

Btw, there is other nits in the scripts I'll comment separately.

- Arnaud

#4 lore raw
Quoted 85 lines · Arnaud Lacombe

Hi,

Quoted 10 lines · john stultz
Quoted 9 lines · Arnaud Lacombe

Hi,

Quoted 6 lines · john stultz

Hey Andrew,
I've tried sending this via Michal a few times, but haven't heard much
back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

Quoted 9 lines
Quoted 4 lines

One of the worry I would have is that the script is merging config blindly, ie. there is no dependency checking done. I have some some work-in-progress to help resolving this, but still lots of thought to be implemented.

So the script actually does warn you if a specified option is dropped due to missing dependencies or if the option is removed. So, I guess could you clarify your concern a bit more?

well, assuming the following Kconfig's snippet:

choice
bool "choice"
config A
bool "A"
config B
bool "B"
endchoice

and trying to merge:

- `config1':

CONFIG_A=y

- `config2':

Result in:

% sh scripts/kconfig/merge_config.sh config1 config2
Merging config1
Merging config2
scripts/kconfig/conf --alldefconfig Kconfig ./.tmp.config.uMY8Z97l9T:2:warning: override: B changes choice state #

# configuration written to .config
#

% cat .config
#
# Automatically generated file; DO NOT EDIT.
# Linux Kernel Configuration
#
# CONFIG_A is not set
CONFIG_B=y

so we still get the warning from the incantation of `alldefconfig', but the one in the script is defeated.

Moreover, there might still be performance optimization to realize, considering a base 'defconfig' amended by disabling CONFIG_DECOMPRESS_GZIP, we get:

% time sh scripts/kconfig/merge_config.sh base amend Merging base
Merging amend
Value of CONFIG_DECOMPRESS_GZIP is redefined by fragment amend:
Previous value: CONFIG_DECOMPRESS_GZIP=y
New value: # CONFIG_DECOMPRESS_GZIP is not set

scripts/kconfig/conf --alldefconfig Kconfig
#

# configuration written to .config
#
sh scripts/kconfig/merge_config.sh base amend  158.20s user 30.19s
system 100% cpu 3:07.86 total

Just by getting rid of the two `| grep ...' in the last step:

scripts/kconfig/merge_config.sh make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET · line 111

Quoted 10 lines
scripts/kconfig/merge_config.sh +2 -2
@@ -104,8 +104,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET
 # Check all specified config values took (might have missed-dependency issues)
 for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
 
-       REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG")
-       ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG")
+       REQUESTED_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" $TMP_FILE)
+       ACTUAL_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" .config)
        if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
                echo "Value requested for $CFG not in final .config"
                echo "Requested value:  $REQUESTED_VAL"

These look like good changes to me. We made several optimizations along these lines, but I confess performance was not my top priority while working with John on this script.

Signature · 3 lines
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
author
#5 lore raw
In reply to Arnaud Lacombe
Quoted 73 lines · Arnaud Lacombe

Hi,

Quoted 10 lines · john stultz
Quoted 9 lines · Arnaud Lacombe

Hi,

Quoted 6 lines · john stultz

Hey Andrew,
I've tried sending this via Michal a few times, but haven't heard much
back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

Quoted 9 lines
Quoted 4 lines

One of the worry I would have is that the script is merging config blindly, ie. there is no dependency checking done. I have some some work-in-progress to help resolving this, but still lots of thought to be implemented.

So the script actually does warn you if a specified option is dropped due to missing dependencies or if the option is removed. So, I guess could you clarify your concern a bit more?

well, assuming the following Kconfig's snippet:

choice
bool "choice"
config A
bool "A"
config B
bool "B"
endchoice

and trying to merge:

- `config1':

CONFIG_A=y

- `config2':

Result in:

% sh scripts/kconfig/merge_config.sh config1 config2
Merging config1
Merging config2
scripts/kconfig/conf --alldefconfig Kconfig ./.tmp.config.uMY8Z97l9T:2:warning: override: B changes choice state #

# configuration written to .config
#

% cat .config
#
# Automatically generated file; DO NOT EDIT.
# Linux Kernel Configuration
#
# CONFIG_A is not set
CONFIG_B=y

so we still get the warning from the incantation of `alldefconfig', but the one in the script is defeated.

Moreover, there might still be performance optimization to realize, considering a base 'defconfig' amended by disabling CONFIG_DECOMPRESS_GZIP, we get:

% time sh scripts/kconfig/merge_config.sh base amend Merging base
Merging amend
Value of CONFIG_DECOMPRESS_GZIP is redefined by fragment amend:
Previous value: CONFIG_DECOMPRESS_GZIP=y
New value: # CONFIG_DECOMPRESS_GZIP is not set

Quoted 6 lines

scripts/kconfig/conf --alldefconfig Kconfig
#

# configuration written to .config
#
sh scripts/kconfig/merge_config.sh base amend  158.20s user 30.19s
system 100% cpu 3:07.86 total

Yep. So you've caught a bug! Thanks for pointing this out!

Quoted 5 lines

Just by getting rid of the two `| grep ...' in the last step:

scripts/kconfig/merge_config.sh make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET · line 111

Quoted 10 lines
scripts/kconfig/merge_config.sh +2 -2
@@ -104,8 +104,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET
 # Check all specified config values took (might have missed-dependency issues)
 for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
 
-       REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG")
-       ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG")
+       REQUESTED_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" $TMP_FILE)
+       ACTUAL_VAL=$(sed -n "/\<$CFG\>/!d; $SED_CONFIG_EXP" .config)
        if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
                echo "Value requested for $CFG not in final .config"
                echo "Requested value:  $REQUESTED_VAL"

So, its not the greps in my mind that are the issue, its the SED_CONF_EXP lines. Those shouldn't be applied to the REQUESTED and ACTUAL values, since they would strip out any difference between the two, causing the comparison to be moot.

I was trying to fix the script from complaining like:
Value requested for CONFIG_EXPERT not in final .config Requested value: # CONFIG_EXPERT=y
Actual value: # CONFIG_EXPERT is not set

But clearly the change was wrong.

Anyway, reverting back to something like the following should fix it:

scripts/kconfig/merge_config.sh +2 -2
index b146b76..adac068 100755
@@ -105,8 +105,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $ALLTARGET
 # Check all specified config values took (might have missed-dependency issues)
 for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
 
-	REQUESTED_VAL=$(sed -n "$SED_CONFIG_EXP" $TMP_FILE | grep -w -e "$CFG")
-	ACTUAL_VAL=$(sed -n "$SED_CONFIG_EXP" .config | grep -w -e "$CFG")
+	REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
+	ACTUAL_VAL=$(grep -w -e "$CFG" .config)
 	if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
 		echo "Value requested for $CFG not in final .config"
 		echo "Requested value:  $REQUESTED_VAL"

Andrew: Would you prefer small fixes ontop of the patch you took, or just a new patch that includes this along with Darren's and Arnaud's fixes?

thanks
-john

author
#6 lore raw
In reply to Arnaud Lacombe
Quoted 59 lines · Arnaud Lacombe

Hi,

Quoted 10 lines · john stultz
Quoted 9 lines · Arnaud Lacombe

Hi,

Quoted 6 lines · john stultz

Hey Andrew,
I've tried sending this via Michal a few times, but haven't heard much
back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

Quoted 9 lines
Quoted 4 lines

One of the worry I would have is that the script is merging config blindly, ie. there is no dependency checking done. I have some some work-in-progress to help resolving this, but still lots of thought to be implemented.

So the script actually does warn you if a specified option is dropped due to missing dependencies or if the option is removed. So, I guess could you clarify your concern a bit more?

well, assuming the following Kconfig's snippet:

choice
bool "choice"
config A
bool "A"
config B
bool "B"
endchoice

and trying to merge:

- `config1':

CONFIG_A=y

- `config2':

Result in:

% sh scripts/kconfig/merge_config.sh config1 config2
Merging config1
Merging config2
scripts/kconfig/conf --alldefconfig Kconfig ./.tmp.config.uMY8Z97l9T:2:warning: override: B changes choice state #

# configuration written to .config
#

% cat .config
#
# Automatically generated file; DO NOT EDIT.
# Linux Kernel Configuration
#
# CONFIG_A is not set
CONFIG_B=y

Quoted 2 lines

so we still get the warning from the incantation of `alldefconfig', but the one in the script is defeated.

You can find the fix for this along with other recent changes from Darren and Arnaud in my git tree here:
git://git.linaro.org/people/jstultz/linux.git dev/config-fragments

Browse-able here:
http://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=shortlog;h=refs/heads/dev/config-fragments

Let me know if you have any other thoughts or feedback!

thanks
-john

#7 lore raw
In reply to john stultz

Hi,

Quoted 6 lines · john stultz

[...]

v2:
* Reworked to use alldefconfig instead of the proposed
olddefconfig as suggested by Sam Ravnborg.

Quoted 7 lines

v3:
* Script improvements from Dmitri.
* allnoconfig option from Darren
* pre-make exit option from Darren
* lots of other fixes/cleanups from Darren.
* Fix final check to not compain about config values in comments

If Dmitri and Darren have direct contribution to the script, shouldn't their Signed-off-by tag be present ?

Quoted 12 lines

Please let me know if you have any comments or thoughts!

CC Sam Ravnborg <[email protected]>
CC [email protected]
CC [email protected]
CC Dmitry Fink <[email protected]>
CC Darren Hart <[email protected]>
CC Eric B Munson <[email protected]>
CC Bruce Ashfield <[email protected]>
CC Michal Marek <[email protected]>
CC [email protected]
Signed-off-by John Stultz <[email protected]>

You'll find below some more nits

1) bail out early on error.

This fixes handling of non-existant file:

Before:
% sh scripts/kconfig/merge_config.sh non existant files Merging non
sed: can't read non: No such file or directory
cat: non: No such file or directory
Merging existant
sed: can't read existant: No such file or directory
cat: existant: No such file or directory Merging files
sed: can't read files: No such file or directory
cat: files: No such file or directory scripts/kconfig/conf --alldefconfig Kconfig
#
# configuration written to .config
#

After:

% sh scripts/kconfig/merge_config.sh non existant files Merging non
sed: can't read non: No such file or directory

2) re-implement argument parsing using sh(1) getopts builtin

3) verify that the script was given enough argument to proceed. There isn't much point running the script with less than 2 arguments.

CC Sam Ravnborg <[email protected]>
CC [email protected]
CC [email protected]
CC Dmitry Fink <[email protected]>
CC Darren Hart <[email protected]>
CC Eric B Munson <[email protected]>
CC Bruce Ashfield <[email protected]>
CC Michal Marek <[email protected]>
CC [email protected]
Signed-off-by Arnaud Lacombe <[email protected]>
---
 scripts/kconfig/merge_config.sh |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)
scripts/kconfig/merge_config.sh +12 -12
index 890276b..abfd8b2 100755
@@ -20,6 +20,8 @@
 #  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 #  See the GNU General Public License for more details.
 
+set -e
+
 clean_up() {
        rm -f $TMP_FILE
        exit

@@ -36,29 +38,27 @@ usage() {
 MAKE=true
 ALLTARGET=alldefconfig
 
-while true; do
-	case $1 in
-	"-n")
+while getopts "nmh" opt; do
+	case ${opt} in
+	n)
 		ALLTARGET=allnoconfig
-		shift
-		continue
 		;;
-	"-m")
+	m)
 		MAKE=false
-		shift
-		continue
 		;;
-	"-h")
+	h)
 		usage
 		exit
 		;;
-	*)
-		break
-		;;
 	esac
 done
 
+shift $(expr $OPTIND - 1)
 
+if [ $# -lt 2 ]; then
+	usage
+	exit 1
+fi
 
 MERGE_LIST=$*
 SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
Signature · 1 line
1.7.6.153.g78432
#8 lore raw

Hi Arnaud,

Quoted 16 lines · Arnaud Lacombe

Hi,

Quoted 6 lines · john stultz

[...]

v2:
* Reworked to use alldefconfig instead of the proposed
olddefconfig as suggested by Sam Ravnborg.

Quoted 7 lines

v3:
* Script improvements from Dmitri.
* allnoconfig option from Darren
* pre-make exit option from Darren
* lots of other fixes/cleanups from Darren.
* Fix final check to not compain about config values in comments

Quoted 2 lines

If Dmitri and Darren have direct contribution to the script, shouldn't their Signed-off-by tag be present ?

Yes, that would be more correct.

Quoted 36 lines
Quoted 13 lines

Please let me know if you have any comments or thoughts!

CC Sam Ravnborg <[email protected]>
CC [email protected]
CC [email protected]
CC Dmitry Fink <[email protected]>
CC Darren Hart <[email protected]>
CC Eric B Munson <[email protected]>
CC Bruce Ashfield <[email protected]>
CC Michal Marek <[email protected]>
CC [email protected]
Signed-off-by John Stultz <[email protected]>

You'll find below some more nits

1) bail out early on error.

This fixes handling of non-existant file:

Before:
% sh scripts/kconfig/merge_config.sh non existant files Merging non
sed: can't read non: No such file or directory
cat: non: No such file or directory
Merging existant
sed: can't read existant: No such file or directory
cat: existant: No such file or directory Merging files
sed: can't read files: No such file or directory
cat: files: No such file or directory scripts/kconfig/conf --alldefconfig Kconfig
#
# configuration written to .config
#

Quoted 5 lines

After:

% sh scripts/kconfig/merge_config.sh non existant files Merging non
sed: can't read non: No such file or directory

An early test (-f) of each non-option argument would be easy enough to add, and would provide better error handling/reporting.

Quoted 2 lines

2) re-implement argument parsing using sh(1) getopts builtin

I was trying to keep it simple - and I was also concerned about colliding with dash/bash inconsistencies. I have no objection to getopt if it works consistently in dash and bash. Did you test with dash as well as bash?

Quoted 3 lines

3) verify that the script was given enough argument to proceed. There isn't much point running the script with less than 2 arguments.

Sure.

Signature · 3 lines
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
Acked-by
#9 lore raw
In reply to Arnaud Lacombe
Quoted 16 lines · Arnaud Lacombe

Hi,

Quoted 6 lines · john stultz

[...]

v2:
* Reworked to use alldefconfig instead of the proposed
olddefconfig as suggested by Sam Ravnborg.

Quoted 7 lines

v3:
* Script improvements from Dmitri.
* allnoconfig option from Darren
* pre-make exit option from Darren
* lots of other fixes/cleanups from Darren.
* Fix final check to not compain about config values in comments

Quoted 2 lines

If Dmitri and Darren have direct contribution to the script, shouldn't their Signed-off-by tag be present ?

Dmitri didn't provide a Signed-off-by, and I had reworked things sufficiently that his patch didn't apply, so I re-implemented many of the suggested improvements myself. However, Dmitri deserves the credits for pointing the issues out.

Similarly, with Darren, there were a number of non-signed-off-by'ed changes, I had to merge in by hand due to my folding of my own work in. But there were also a few proper patches from Darren that I did fold in and should have preserved his Signed-off-by. That was sloppy, so my apologies. Its now fixed in my tree.

Quoted 33 lines

You'll find below some more nits

1) bail out early on error.

This fixes handling of non-existant file:

Before:
% sh scripts/kconfig/merge_config.sh non existant files Merging non
sed: can't read non: No such file or directory
cat: non: No such file or directory
Merging existant
sed: can't read existant: No such file or directory
cat: existant: No such file or directory Merging files
sed: can't read files: No such file or directory
cat: files: No such file or directory scripts/kconfig/conf --alldefconfig Kconfig
#
# configuration written to .config
#

After:

% sh scripts/kconfig/merge_config.sh non existant files Merging non
sed: can't read non: No such file or directory

2) re-implement argument parsing using sh(1) getopts builtin

3) verify that the script was given enough argument to proceed. There isn't much point running the script with less than 2 arguments.

Quoted 10 lines
CC Sam Ravnborg <[email protected]>
CC [email protected]
CC [email protected]
CC Dmitry Fink <[email protected]>
CC Darren Hart <[email protected]>
CC Eric B Munson <[email protected]>
CC Bruce Ashfield <[email protected]>
CC Michal Marek <[email protected]>
CC [email protected]
Signed-off-by Arnaud Lacombe <[email protected]>

Thanks for these great cleanups and feedback!

Acked-by John Stultz <[email protected]>

I've queued these (along with your and Darren's other changes).

thanks
-john

Quoted 57 lines
---
 scripts/kconfig/merge_config.sh |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)
scripts/kconfig/merge_config.sh +12 -12
index 890276b..abfd8b2 100755
@@ -20,6 +20,8 @@
 #  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 #  See the GNU General Public License for more details.
 
+set -e
+
 clean_up() {
        rm -f $TMP_FILE
        exit

@@ -36,29 +38,27 @@ usage() {
 MAKE=true
 ALLTARGET=alldefconfig
 
-while true; do
-	case $1 in
-	"-n")
+while getopts "nmh" opt; do
+	case ${opt} in
+	n)
 		ALLTARGET=allnoconfig
-		shift
-		continue
 		;;
-	"-m")
+	m)
 		MAKE=false
-		shift
-		continue
 		;;
-	"-h")
+	h)
 		usage
 		exit
 		;;
-	*)
-		break
-		;;
 	esac
 done
 
+shift $(expr $OPTIND - 1)
 
+if [ $# -lt 2 ]; then
+	usage
+	exit 1
+fi
 
 MERGE_LIST=$*
 SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
author
#10 lore raw
In reply to Arnaud Lacombe
Quoted 3 lines · Arnaud Lacombe

You'll find below some more nits

1) bail out early on error.

Actually, I spoke a little too soon.

The set -e option causes us to bomb out if grep doesn't find a config option (grep returns 1 if nothing is found). So I'll have to drop that change for now.

thanks
-john

author
#11 lore raw
Quoted 10 lines · john stultz
Quoted 3 lines · Arnaud Lacombe

You'll find below some more nits

1) bail out early on error.

Actually, I spoke a little too soon.

The set -e option causes us to bomb out if grep doesn't find a config option (grep returns 1 if nothing is found). So I'll have to drop that change for now.

Catching the error explicitly is easy enough, so folded in the following fix (copied here from a terminal, so its whitespace damaged, I know) to your patch:

@@ -20,8 +20,6 @@
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. # See the GNU General Public License for more details.

-set -e
-
clean_up() {
rm -f $TMP_FILE
exit
@@ -68,6 +66,9 @@ TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
for MERGE_FILE in $MERGE_LIST ; do
echo "Merging $MERGE_FILE"
CFG_LIST=$(sed -n "$SED_CONFIG_EXP" $MERGE_FILE)
+ if [ $? != 0 ] ; then
+ exit 1
+ fi

for CFG in $CFG_LIST ; do
grep -q -w $CFG $TMP_FILE

thanks
-john

#12 lore raw
In reply to john stultz

Hi,

Quoted 11 lines · john stultz
Quoted 3 lines · Arnaud Lacombe

You'll find below some more nits

1) bail out early on error.

Actually, I spoke a little too soon.

The set -e option causes us to bomb out if grep doesn't find a config option (grep returns 1 if nothing is found). So I'll have to drop that change for now.

because the way the return value of grep(1) is checked is bogus. It should not be:

    grep -q -w $CFG $TMP_FILE
    if [ $? -eq 0 ] ; then
        ...
    fi

but:

if grep -q -w $CFG $TMP_FILE ; then
...
fi

that way, you can still keep the set -e'. IMHO, it should be set by default for any shell script, just as a safety net, as set -u'.

- Arnaud

#13 lore raw
In reply to john stultz
Quoted 5 lines · john stultz

Hey Andrew,
I've tried sending this via Michal a few times, but haven't heard much
back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

My concern was that the script works around missing features in kconfig, instead of fixing kconfig. Don't get me wrong, I do believe that the script is useful, but if you are submitting to mainline anyway, why don't you fix/enhance kconfig in mainline and have a much simpler script making use of the enhanced kconfig?

Michal

#14 lore raw

Hi,

Quoted 7 lines · Michal Marek
Quoted 5 lines · john stultz

Hey Andrew,
      I've tried sending this via Michal a few times, but haven't heard much back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

Quoted 6 lines

My concern was that the script works around missing features in kconfig, instead of fixing kconfig. Don't get me wrong, I do believe that the script is useful, but if you are submitting to mainline anyway, why don't you fix/enhance kconfig in mainline and have a much simpler script making use of the enhanced kconfig?

FWIW, as much as I agree with you, kconfig is currently a bit too static, with too much assumption, do any advanced stuff. I have WIP in the area, but I prefer not giving ETA. In the mean time, I think John's script could fill the gap.

- Arnaud

#15 lore raw
Quoted 16 lines · Arnaud Lacombe

Hi,

Quoted 7 lines · Michal Marek
Quoted 5 lines · john stultz

Hey Andrew,
I've tried sending this via Michal a few times, but haven't heard much
back. So I wanted to check if you would consider merging it via your tree, or if you had any suggestions of who would be better to review/merge this.

Quoted 6 lines

My concern was that the script works around missing features in kconfig, instead of fixing kconfig. Don't get me wrong, I do believe that the script is useful, but if you are submitting to mainline anyway, why don't you fix/enhance kconfig in mainline and have a much simpler script making use of the enhanced kconfig?

Quoted 4 lines

FWIW, as much as I agree with you, kconfig is currently a bit too static, with too much assumption, do any advanced stuff. I have WIP in the area, but I prefer not giving ETA. In the mean time, I think John's script could fill the gap.

The script is also at least partly usable in older kernel trees without having to modify Kconfig there. It has some value as a standalone script. Not to say that we shouldn't also see about updating Kconfig as well.

Signature · 3 lines
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
#16 lore raw

PATCH 1/2 merge_config.sh: use signal names compatible with dash and bash

In reply to john stultz

The SIGHUP SIGINT and SIGTERM names caused failures when running merge_config.sh with the dash shell. Dropping the "SIG" component makes the script work in both bash and dash.

Signed-off-by Darren Hart <[email protected]>
---
 scripts/kconfig/merge_config.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
scripts/kconfig/merge_config.sh +1 -1
index 890276b..b91015d 100755
@@ -24,7 +24,7 @@ clean_up() {
        rm -f $TMP_FILE
        exit
 }
-trap clean_up SIGHUP SIGINT SIGTERM
+trap clean_up HUP INT TERM
 
 usage() {
 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
Signature · 1 line
1.7.6.4
Acked-by on v1 1/2
#17 lore raw

Re: [PATCH 1/2] merge_config.sh: use signal names compatible with dash and bash

Quoted 5 lines · Darren Hart

The SIGHUP SIGINT and SIGTERM names caused failures when running merge_config.sh with the dash shell. Dropping the "SIG" component makes the script work in both bash and dash.

Signed-off-by Darren Hart <[email protected]>
Acked-by John Stultz <[email protected]>
Quoted 17 lines
---
 scripts/kconfig/merge_config.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
scripts/kconfig/merge_config.sh +1 -1
index 890276b..b91015d 100755
@@ -24,7 +24,7 @@ clean_up() {
        rm -f $TMP_FILE
        exit
 }
-trap clean_up SIGHUP SIGINT SIGTERM
+trap clean_up HUP INT TERM
 
 usage() {
 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
#18 lore raw

PATCH 2/2 merge_config.sh: whitespace cleanup

In reply to john stultz

Fix whitespace usage in the clean_up routine.

Signed-off-by Darren Hart <[email protected]>
---
 scripts/kconfig/merge_config.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
scripts/kconfig/merge_config.sh +2 -2
index b91015d..07bb886 100755
@@ -21,8 +21,8 @@
 #  See the GNU General Public License for more details.
 
 clean_up() {
-       rm -f $TMP_FILE
-       exit
+	rm -f $TMP_FILE
+	exit
 }
 trap clean_up HUP INT TERM
Signature · 1 line
1.7.6.4
Acked-by on v1 2/2
#19 lore raw

Re: [PATCH 2/2] merge_config.sh: whitespace cleanup

Quoted 3 lines · Darren Hart

Fix whitespace usage in the clean_up routine.

Signed-off-by Darren Hart <[email protected]>
Acked-by John Stultz <[email protected]>
Quoted 19 lines
---
 scripts/kconfig/merge_config.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
scripts/kconfig/merge_config.sh +2 -2
index b91015d..07bb886 100755
@@ -21,8 +21,8 @@
 #  See the GNU General Public License for more details.
 
 clean_up() {
-       rm -f $TMP_FILE
-       exit
+	rm -f $TMP_FILE
+	exit
 }
 trap clean_up HUP INT TERM