[PATCH 00/10] merge_config misc reworks and testcases

[PATCH 00/10] merge_config misc reworks and testcases

[Olof Johansson]

Hi,

Somewhat wide distribution list here, since I've added everyone who's
touched the script, with the presumption that those have been the major
users of it. Please make sure none of these changes break your use cases.

I've done some reworks of merge_config.sh. I was quite hesitant to start
this since there are no good ways to see if your changes break others
or not, so the first thing I did was to add some tests. I know this is
highly unorthodox so try not to panic. :-)

As far as what this series does is:

- Adds a way to pass in CONFIG_FOO=<value> on the command line, it gets
  treated as a single-entry fragment file

- The script now prints the warnings on stderr, and returns non-0 when
  something is encountered

- Optionally, it'll also return non-0 when a redundant entry is found. I
  presumed people rely on -r not being a failure so I did this separately

- CONFIG_FOO=n and "# CONFIG_FOO is not set" is now treated the same,
  and using the former doesn't cause an invalid warning when the results
  are checked at the end

- Slightly odd things happened if a fragment contains the same option
  twice: It'd produce a warning that was malformed. Now just ignore that
  and use only the latest value of said option.


-Olof

[PATCH 01/10] merge_config.sh: factor out value parsing

[Olof Johansson]

Since we want to adjust the line later on to deal with CONFIG_FOO=n,
let's make a function for it so we have only one place to change it in.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 scripts/kconfig/merge_config.sh | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 67d1314..d8cd913 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -35,6 +35,10 @@ usage() {
 	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
 }
 
+getval() {
+	grep -w -e "$1" "$2"
+}
+
 RUNMAKE=true
 ALLTARGET=alldefconfig
 WARNREDUN=false
@@ -116,8 +120,8 @@ for MERGE_FILE in $MERGE_LIST ; do
 
 	for CFG in $CFG_LIST ; do
 		grep -q -w $CFG $TMP_FILE || continue
-		PREV_VAL=$(grep -w $CFG $TMP_FILE)
-		NEW_VAL=$(grep -w $CFG $MERGE_FILE)
+		PREV_VAL=$(getval "$CFG" "$TMP_FILE")
+		NEW_VAL=$(getval "$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
@@ -157,8 +161,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $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
 
-	REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
-	ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
+	REQUESTED_VAL=$(getval "$CFG" "$TMP_FILE")
+	ACTUAL_VAL=$(getval "$CFG" "$KCONFIG_CONFIG")
 	if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
 		echo "Value requested for $CFG not in final .config"
 		echo "Requested value:  $REQUESTED_VAL"
-- 
2.1.4

[PATCH 02/10] merge_config.sh: print warnings on stderr

[Olof Johansson]

Makes it easier to grab just errors/warnings in a build log.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 scripts/kconfig/merge_config.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index d8cd913..0e98619 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -131,7 +131,7 @@ for MERGE_FILE in $MERGE_LIST ; do
 			echo Value of $CFG is redundant by fragment $MERGE_FILE:
 		fi
 		sed -i "/$CFG[ =]/d" $TMP_FILE
-	done
+	done >&2
 	cat $MERGE_FILE >> $TMP_FILE
 done
 
@@ -168,7 +168,7 @@ for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
 		echo "Requested value:  $REQUESTED_VAL"
 		echo "Actual value:     $ACTUAL_VAL"
 		echo ""
-	fi
+	fi >&2
 done
 
 clean_up
-- 
2.1.4

[PATCH 03/10] merge_config.sh: minor argument parsing refactoring

[Olof Johansson]

Every case that continues iterating needs a shift, so move it to common location.
Also, the continues are redundant.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 scripts/kconfig/merge_config.sh | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 0e98619..fd0d537 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -48,13 +48,9 @@ while true; do
 	case $1 in
 	"-n")
 		ALLTARGET=allnoconfig
-		shift
-		continue
 		;;
 	"-m")
 		RUNMAKE=false
-		shift
-		continue
 		;;
 	"-h")
 		usage
@@ -62,8 +58,6 @@ while true; do
 		;;
 	"-r")
 		WARNREDUN=true
-		shift
-		continue
 		;;
 	"-O")
 		if [ -d $2 ];then
@@ -72,13 +66,14 @@ while true; do
 			echo "output directory $2 does not exist" 1>&2
 			exit 1
 		fi
-		shift 2
-		continue
+		shift
 		;;
 	*)
 		break
 		;;
 	esac
+
+	shift
 done
 
 if [ "$#" -lt 1 ] ; then
-- 
2.1.4

[PATCH 04/10] merge_config.sh: exit non-0 in case of failures

[Olof Johansson]

Exit with non-0 value in cases where there was a failure to set an option.
Also, add a '-e' during which the conflict warnings are considered failures
(-e -r will result in these being failures, -r will result in them just being
reported).

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 scripts/kconfig/merge_config.sh | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index fd0d537..c244042 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -20,14 +20,17 @@
 #  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 #  See the GNU General Public License for more details.
 
+EXITVAL=0
+
 clean_up() {
 	rm -f $TMP_FILE
-	exit
+	exit $EXITVAL
 }
 trap clean_up HUP INT TERM
 
 usage() {
 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
+	echo "  -e    consider conflicting overrides to be errors"
 	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"
@@ -39,6 +42,7 @@ getval() {
 	grep -w -e "$1" "$2"
 }
 
+CONF_IS_ERR=false
 RUNMAKE=true
 ALLTARGET=alldefconfig
 WARNREDUN=false
@@ -46,6 +50,9 @@ OUTPUT=.
 
 while true; do
 	case $1 in
+	"-e")
+		CONF_IS_ERR=true
+		;;
 	"-n")
 		ALLTARGET=allnoconfig
 		;;
@@ -117,13 +124,19 @@ for MERGE_FILE in $MERGE_LIST ; do
 		grep -q -w $CFG $TMP_FILE || continue
 		PREV_VAL=$(getval "$CFG" "$TMP_FILE")
 		NEW_VAL=$(getval "$CFG" "$MERGE_FILE")
+		WARN=false
 		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
+			WARN=true
 		elif [ "$WARNREDUN" = "true" ]; then
 			echo Value of $CFG is redundant by fragment $MERGE_FILE:
+			WARN=true
+		fi
+		if [ "$CONF_IS_ERR" = "true" -a "$WARN" = "true" ] ; then
+			EXITVAL=1
 		fi
 		sed -i "/$CFG[ =]/d" $TMP_FILE
 	done >&2
@@ -136,7 +149,7 @@ if [ "$RUNMAKE" = "false" ]; then
 	echo "# merged configuration written to $KCONFIG_CONFIG (needs make)"
 	echo "#"
 	clean_up
-	exit
+	exit $EXITVAL
 fi
 
 # If we have an output dir, setup the O= argument, otherwise leave
@@ -152,10 +165,8 @@ fi
 # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set
 make KCONFIG_ALLCONFIG=$TMP_FILE $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
-
 	REQUESTED_VAL=$(getval "$CFG" "$TMP_FILE")
 	ACTUAL_VAL=$(getval "$CFG" "$KCONFIG_CONFIG")
 	if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
@@ -163,6 +174,7 @@ for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
 		echo "Requested value:  $REQUESTED_VAL"
 		echo "Actual value:     $ACTUAL_VAL"
 		echo ""
+		EXITVAL=1
 	fi >&2
 done
 
-- 
2.1.4

[PATCH 05/10] merge_config.sh: Better handling of CONFIG_FOO=n

[Olof Johansson]

Kconfig knows how to handle CONFIG_FOO=n just fine, but it'll always
use "# CONFIG FOO is not set" in the resulting config. Mangle the input
accordingly so we don't report this as a failure when it isn't.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 scripts/kconfig/merge_config.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index c244042..933f2d6 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -39,7 +39,7 @@ usage() {
 }
 
 getval() {
-	grep -w -e "$1" "$2"
+	grep -w -e "$1" "$2" | sed 's/^CONFIG_\(.*\)=n$/# CONFIG_\1 is not set/g'
 }
 
 CONF_IS_ERR=false
-- 
2.1.4

[PATCH 06/10] merge_config.sh: only consider last value of symbols

[Olof Johansson]

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 scripts/kconfig/merge_config.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 933f2d6..1945b2c 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -39,7 +39,7 @@ usage() {
 }
 
 getval() {
-	grep -w -e "$1" "$2" | sed 's/^CONFIG_\(.*\)=n$/# CONFIG_\1 is not set/g'
+	grep -w -e "$1" "$2" | tail -1 | sed 's/^CONFIG_\(.*\)=n/# CONFIG_\1 is not set/g'
 }
 
 CONF_IS_ERR=false
-- 
2.1.4

[PATCH 07/10] merge_config.sh: add tests

[Olof Johansson]

For being a small script, merge_config.sh is fairly scary to change since there's no
real way to know if you did something wrong. So it seems appropriate to add a simple
test suite.

I've started with testcases in the areas I care about, other should of course feel
free to expand on this.

Use is simple, from the kernel tree, run ./scripts/kconfig/merge_config_test/runall.
It'll execute out of a tmpdir under /tmp.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 .../kconfig/merge_config_test/01-no-fragment.sh    | 12 +++++++++++
 .../kconfig/merge_config_test/02-already-set.sh    | 19 ++++++++++++++++++
 .../merge_config_test/03-turnoff-failure.sh        | 19 ++++++++++++++++++
 .../merge_config_test/04-turnoff-failure2.sh       | 19 ++++++++++++++++++
 .../merge_config_test/05-turnoff-success.sh        | 19 ++++++++++++++++++
 .../merge_config_test/06-turnoff-success2.sh       | 19 ++++++++++++++++++
 .../kconfig/merge_config_test/07-turnon-success.sh | 19 ++++++++++++++++++
 .../merge_config_test/08-tristate-success.sh       | 20 +++++++++++++++++++
 .../merge_config_test/09-tristate-failure.sh       | 20 +++++++++++++++++++
 .../merge_config_test/10-turnon-redundant.sh       | 19 ++++++++++++++++++
 .../merge_config_test/11-turnon-redundant-err.sh   | 19 ++++++++++++++++++
 scripts/kconfig/merge_config_test/common.sh        | 23 ++++++++++++++++++++++
 scripts/kconfig/merge_config_test/runall.sh        | 22 +++++++++++++++++++++
 13 files changed, 249 insertions(+)
 create mode 100755 scripts/kconfig/merge_config_test/01-no-fragment.sh
 create mode 100755 scripts/kconfig/merge_config_test/02-already-set.sh
 create mode 100755 scripts/kconfig/merge_config_test/03-turnoff-failure.sh
 create mode 100755 scripts/kconfig/merge_config_test/04-turnoff-failure2.sh
 create mode 100755 scripts/kconfig/merge_config_test/05-turnoff-success.sh
 create mode 100755 scripts/kconfig/merge_config_test/06-turnoff-success2.sh
 create mode 100755 scripts/kconfig/merge_config_test/07-turnon-success.sh
 create mode 100755 scripts/kconfig/merge_config_test/08-tristate-success.sh
 create mode 100755 scripts/kconfig/merge_config_test/09-tristate-failure.sh
 create mode 100755 scripts/kconfig/merge_config_test/10-turnon-redundant.sh
 create mode 100755 scripts/kconfig/merge_config_test/11-turnon-redundant-err.sh
 create mode 100755 scripts/kconfig/merge_config_test/common.sh
 create mode 100755 scripts/kconfig/merge_config_test/runall.sh

diff --git a/scripts/kconfig/merge_config_test/01-no-fragment.sh b/scripts/kconfig/merge_config_test/01-no-fragment.sh
new file mode 100755
index 0000000..c892f9b
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/01-no-fragment.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Simple merge: No fragment specified, just base config
+
+FRAG=$(echo "" | writefrag)
+
+merge ${FRAG}
+M=$?
+
+[ $M -eq 0 ]
diff --git a/scripts/kconfig/merge_config_test/02-already-set.sh b/scripts/kconfig/merge_config_test/02-already-set.sh
new file mode 100755
index 0000000..1d71030
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/02-already-set.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Turn on an option that is already on
+
+FRAG=$(writefrag) << EOF
+CONFIG_MMU=y
+EOF
+
+merge "${FRAG}"
+M=$?
+
+# Return pass if MMU is still set in output
+
+check CONFIG_MMU=y
+G=$?
+
+[ $M -eq 0 -a $G -eq 0 ]
diff --git a/scripts/kconfig/merge_config_test/03-turnoff-failure.sh b/scripts/kconfig/merge_config_test/03-turnoff-failure.sh
new file mode 100755
index 0000000..aa9bf63
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/03-turnoff-failure.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Try to turn off a function that won't turn off.
+
+FRAG=$(writefrag) << EOF
+# CONFIG_MMU is not set
+EOF
+
+merge ${FRAG}
+M=$?
+
+# Return pass if MMU is still set in output
+
+check CONFIG_MMU=y
+G=$?
+
+[ $M -ne 0 -a $G -eq 0 ]
diff --git a/scripts/kconfig/merge_config_test/04-turnoff-failure2.sh b/scripts/kconfig/merge_config_test/04-turnoff-failure2.sh
new file mode 100755
index 0000000..cebcdbc
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/04-turnoff-failure2.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Try to turn off a function that won't turn off.
+
+FRAG=$(writefrag) << EOF
+CONFIG_MMU=n
+EOF
+
+merge "${FRAG}"
+M=$?
+
+# Return pass if MMU is still set in output
+
+check CONFIG_MMU=y
+G=$?
+
+[ $M -ne 0 -a $G -eq 0 ]
diff --git a/scripts/kconfig/merge_config_test/05-turnoff-success.sh b/scripts/kconfig/merge_config_test/05-turnoff-success.sh
new file mode 100755
index 0000000..7a8822e
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/05-turnoff-success.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Try to turn off a function that will turn off.
+
+FRAG=$(writefrag) << EOF
+# CONFIG_64BIT is not set
+EOF
+
+merge "${FRAG}"
+M=$?
+
+# Return fail if 64BIT is still set in output
+
+check CONFIG_64BIT=y
+G=$?
+
+[ $M -eq 0 -a $G -ne 0 ]
diff --git a/scripts/kconfig/merge_config_test/06-turnoff-success2.sh b/scripts/kconfig/merge_config_test/06-turnoff-success2.sh
new file mode 100755
index 0000000..d5e7cc5
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/06-turnoff-success2.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Try to turn off a function that will turn off.
+
+FRAG=$(writefrag) << EOF
+CONFIG_64BIT=n
+EOF
+
+merge "${FRAG}"
+M=$?
+
+# Return fail if 64BIT is still set in output
+
+check CONFIG_64BIT=y
+G=$?
+
+[ $M -eq 0 -a $G -ne 0 ]
diff --git a/scripts/kconfig/merge_config_test/07-turnon-success.sh b/scripts/kconfig/merge_config_test/07-turnon-success.sh
new file mode 100755
index 0000000..eb24ba0
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/07-turnon-success.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Try to turn on a function that will turn on.
+
+FRAG=$(writefrag) << EOF
+CONFIG_EMBEDDED=y
+EOF
+
+merge "${FRAG}"
+M=$?
+
+# Return fail if EMBEDDED is not set in output
+
+check CONFIG_EMBEDDED=y
+G=$?
+
+[ $M -eq 0 -a $G -eq 0 ]
diff --git a/scripts/kconfig/merge_config_test/08-tristate-success.sh b/scripts/kconfig/merge_config_test/08-tristate-success.sh
new file mode 100755
index 0000000..3c454b8
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/08-tristate-success.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Try to turn on a tristate that is allowed
+
+FRAG=$(writefrag) << EOF
+CONFIG_MODULES=y
+CONFIG_PCI_STUB=m
+EOF
+
+merge "${FRAG}"
+M=$?
+
+# Return fail if PCI_STUB=m is not set in output
+
+check CONFIG_PCI_STUB=m
+G=$?
+
+[ $M -eq 0 -a $G -eq 0 ]
diff --git a/scripts/kconfig/merge_config_test/09-tristate-failure.sh b/scripts/kconfig/merge_config_test/09-tristate-failure.sh
new file mode 100755
index 0000000..7f586ea
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/09-tristate-failure.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Try to turn on a tristate that failes
+
+FRAG=$(writefrag) << EOF
+CONFIG_MODULES=n
+CONFIG_PCI_STUB=m
+EOF
+
+merge "${FRAG}"
+M=$?
+
+# Return fail if PCI_STUB=m is set in output
+
+check CONFIG_PCI_STUB=m
+G=$?
+
+[ $M -ne 0 -a $G -ne 0 ]
diff --git a/scripts/kconfig/merge_config_test/10-turnon-redundant.sh b/scripts/kconfig/merge_config_test/10-turnon-redundant.sh
new file mode 100755
index 0000000..65d9ea7
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/10-turnon-redundant.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Turn on something that's already on, should not warn
+
+FRAG=$(writefrag) << EOF
+CONFIG_64BIT=y
+EOF
+
+merge "${FRAG}"
+M=$?
+
+# Return fail if 64BIT=y is not set in output
+
+check CONFIG_64BIT=y
+G=$?
+
+[ $M -eq 0 -a $G -eq 0 ]
diff --git a/scripts/kconfig/merge_config_test/11-turnon-redundant-err.sh b/scripts/kconfig/merge_config_test/11-turnon-redundant-err.sh
new file mode 100755
index 0000000..24c1d9b
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/11-turnon-redundant-err.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Turn on something that's already on, watch it warn/fail
+
+FRAG=$(writefrag) << EOF
+CONFIG_64BIT=y
+EOF
+
+merge_r "${FRAG}" "${FRAG}"
+M=$?
+
+# Return fail if 64BIT=y is not set in output
+
+check CONFIG_64BIT=y
+G=$?
+
+[ $M -ne 0 -a $G -eq 0 ]
diff --git a/scripts/kconfig/merge_config_test/common.sh b/scripts/kconfig/merge_config_test/common.sh
new file mode 100755
index 0000000..710c429
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/common.sh
@@ -0,0 +1,23 @@
+TMPDIR=$(mktemp -d /tmp/mergetest.XXXXX)
+SCRIPT="$(dirname $0)/../merge_config.sh"
+
+writefrag() {
+	FRAG=$(mktemp ${TMPDIR}/frag.XXXX)
+	cat > "${FRAG}"
+	echo $FRAG
+}
+
+merge() {
+	"${SCRIPT}" -e -O "${TMPDIR}" /dev/null $*
+	return $?
+}
+
+merge_r() {
+	"${SCRIPT}" -e -r -O "${TMPDIR}" /dev/null $*
+	return $?
+}
+
+check() {
+	grep -q "$1" "${TMPDIR}/.config"
+	return $?
+}
diff --git a/scripts/kconfig/merge_config_test/runall.sh b/scripts/kconfig/merge_config_test/runall.sh
new file mode 100755
index 0000000..3c73d70
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/runall.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+EXITVAL=0
+TMPDIR=$(mktemp -d /tmp/mergetest.XXXXX) || exit 1
+ARCH=x86
+
+cleanup() {
+	rm -rf "${TMPDIR}"
+	exit $EXITVAL
+}
+
+trap cleanup EXIT
+
+for test in $(dirname $0)/*-*.sh ; do
+	echo -n "test $(basename ${test}):  "
+	if ${test} >/dev/null 2>&1 ; then
+		echo PASSED
+	else
+		echo FAILED
+		EXITVAL=1
+	fi
+done
-- 
2.1.4

[PATCH 08/10] merge_config.sh: use trap for cleanup

[Olof Johansson]

Use the trap to cleanup even on regular exit.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 scripts/kconfig/merge_config.sh | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 1945b2c..b26c0ef 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -23,10 +23,12 @@
 EXITVAL=0
 
 clean_up() {
-	rm -f $TMP_FILE
+	if [ -n "$CLEAN_FILES" ] ; then
+		rm -f $CLEAN_FILES
+	fi
 	exit $EXITVAL
 }
-trap clean_up HUP INT TERM
+trap clean_up HUP INT TERM EXIT
 
 usage() {
 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
@@ -178,4 +180,4 @@ for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
 	fi >&2
 done
 
-clean_up
+# Note: clean_up will run here due to EXIT being trapped
-- 
2.1.4

[PATCH 09/10] merge_config.sh: allow single configs to be passed in on cmdline

[Olof Johansson]

Treat CONFIG_FOO=..  on the command line the same way as a single-entry file would.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 scripts/kconfig/merge_config.sh | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index b26c0ef..69463dd 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -1,8 +1,8 @@
 #!/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).
+#  merge_config.sh - Takes a list of config fragment filenames or configuration
+#  value, 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
@@ -108,7 +108,26 @@ fi
 
 MERGE_LIST=$*
 SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
-TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
+TMP_FILE=$(mktemp $OUTPUT/.tmp.config.XXXXXXXXXX)
+
+CLEAN_FILES=$TMP_FILE
+
+# Process cmdline configs into temporary fragments
+for ENTRY in $MERGE_LIST ; do
+	case $ENTRY in
+	CONFIG*)
+		FF=$(mktemp $OUTPUT/.temp.frag-cmdline.XXXXX)
+		CLEAN_FILES="$CLEAN_FILES $FF"
+		echo $ENTRY > $FF
+		NEW_LIST="$NEW_LIST $FF"
+		;;
+	*)
+		NEW_LIST="$NEW_LIST $ENTRY"
+		;;
+	esac
+done
+
+MERGE_LIST=$NEW_LIST
 
 echo "Using $INITFILE as base"
 cat $INITFILE > $TMP_FILE
-- 
2.1.4

[PATCH 10/10] merge_config.sh: add tests for cmdline configs

[Olof Johansson]

Again, these tests could be more complicated but at least it gets
the very basics covered.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 .../merge_config_test/12-cmdline-success.sh        | 13 ++++++++++
 .../merge_config_test/13-cmdline-failure.sh        | 13 ++++++++++
 .../merge_config_test/14-cmdline-reduntant.sh      | 13 ++++++++++
 .../merge_config_test/15-cmdline-complex.sh        | 30 ++++++++++++++++++++++
 4 files changed, 69 insertions(+)
 create mode 100755 scripts/kconfig/merge_config_test/12-cmdline-success.sh
 create mode 100755 scripts/kconfig/merge_config_test/13-cmdline-failure.sh
 create mode 100755 scripts/kconfig/merge_config_test/14-cmdline-reduntant.sh
 create mode 100755 scripts/kconfig/merge_config_test/15-cmdline-complex.sh

diff --git a/scripts/kconfig/merge_config_test/12-cmdline-success.sh b/scripts/kconfig/merge_config_test/12-cmdline-success.sh
new file mode 100755
index 0000000..f8f2bf1
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/12-cmdline-success.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Turn off an option
+
+merge CONFIG_64BIT=n
+M=$?
+
+check CONFIG_64BIT=y
+G=$?
+
+[ $M -eq 0 -a $G -ne 0 ]
diff --git a/scripts/kconfig/merge_config_test/13-cmdline-failure.sh b/scripts/kconfig/merge_config_test/13-cmdline-failure.sh
new file mode 100755
index 0000000..6504786
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/13-cmdline-failure.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Try to turn off an option that won't turn off.
+
+merge CONFIG_MMU=n
+M=$?
+
+check CONFIG_MMU=y
+G=$?
+
+[ $M -ne 0 -a $G -eq 0 ]
diff --git a/scripts/kconfig/merge_config_test/14-cmdline-reduntant.sh b/scripts/kconfig/merge_config_test/14-cmdline-reduntant.sh
new file mode 100755
index 0000000..c9c3fab
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/14-cmdline-reduntant.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Make sure redundant options are warned about
+
+merge_r CONFIG_64BIT=n CONFIG_64BIT=n
+M=$?
+
+check CONFIG_64BIT=y
+G=$?
+
+[ $M -ne 0 -a $G -ne 0 ]
diff --git a/scripts/kconfig/merge_config_test/15-cmdline-complex.sh b/scripts/kconfig/merge_config_test/15-cmdline-complex.sh
new file mode 100755
index 0000000..b754610
--- /dev/null
+++ b/scripts/kconfig/merge_config_test/15-cmdline-complex.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+. "$(dirname $0)/common.sh"
+
+# Make sure redundant options are warned about
+
+FRAG1=$(writefrag) << EOF
+CONFIG_EMBEDDED=y
+EOF
+
+FRAG2=$(writefrag) << EOF
+CONFIG_MMU=n
+EOF
+
+merge_r ${FRAG1} CONFIG_64BIT=n ${FRAG2}
+M=$?
+
+check CONFIG_64BIT=y
+G1=$?
+
+check CONFIG_EMBEDDED=y
+G2=$?
+
+check CONFIG_MMU=y
+G3=$?
+
+[ $G1 -ne 0 -a $G2 -eq 0 -a $G3 -eq 0 ]
+G=$?
+
+[ $M -ne 0 -a $G -eq 0 ]
-- 
2.1.4

Re: [PATCH 00/10] merge_config misc reworks and testcases

[Olof Johansson]

On Wed, Oct 28, 2015 at 9:42 AM, Olof Johansson <olof@lixom.net> wrote:
> Hi,
>
> Somewhat wide distribution list here, since I've added everyone who's
> touched the script, with the presumption that those have been the major
> users of it. Please make sure none of these changes break your use cases.

Actually, I dropped a handful of cc:s on this repost, so this isn't
necessarily true. Also, since Yann didn't respond on the last posting
I'm sending these to you now, Michal. :)

Hopefully Darren, who seems to be one of the major users of
merge_config, can ack these if they pass in his environment.


-Olof

Re: [PATCH 00/10] merge_config misc reworks and testcases

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:01AM +0900, Olof Johansson wrote:
> Hi,
> 
> Somewhat wide distribution list here, since I've added everyone who's
> touched the script, with the presumption that those have been the major
> users of it. Please make sure none of these changes break your use cases.

+Bruce. I think you were going to test these with the linux-yocto tooling. Did
you get a chance to run that test? I'd like your thoughts on the two comments
below:

> 
> I've done some reworks of merge_config.sh. I was quite hesitant to start
> this since there are no good ways to see if your changes break others
> or not, so the first thing I did was to add some tests. I know this is
> highly unorthodox so try not to panic. :-)
> 
> As far as what this series does is:
> 
> - Adds a way to pass in CONFIG_FOO=<value> on the command line, it gets
>   treated as a single-entry fragment file
> 
> - The script now prints the warnings on stderr, and returns non-0 when
>   something is encountered

This one might impact linux-yocto usage, Bruce? That said, it seems like the
right thing to do. So I'd still like to see it go in, but we may need to plan to
update the dependent tooling to use it.

> 
> - Optionally, it'll also return non-0 when a redundant entry is found. I
>   presumed people rely on -r not being a failure so I did this separately
> 
> - CONFIG_FOO=n and "# CONFIG_FOO is not set" is now treated the same,
>   and using the former doesn't cause an invalid warning when the results
>   are checked at the end
> 
> - Slightly odd things happened if a fragment contains the same option
>   twice: It'd produce a warning that was malformed. Now just ignore that
>   and use only the latest value of said option.

This one will likely impact usage as well. linux-yocto does want to report when
there is an override, not as an error, but for informational purposes - "Where
does my option get clobbered?"



-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 01/10] merge_config.sh: factor out value parsing

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:02AM +0900, Olof Johansson wrote:
> Since we want to adjust the line later on to deal with CONFIG_FOO=n,
> let's make a function for it so we have only one place to change it in.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  scripts/kconfig/merge_config.sh | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> index 67d1314..d8cd913 100755
> --- a/scripts/kconfig/merge_config.sh
> +++ b/scripts/kconfig/merge_config.sh
> @@ -35,6 +35,10 @@ usage() {
>  	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
>  }
>  
> +getval() {
> +	grep -w -e "$1" "$2"

So the inclusion of -e makes sense from a common-denominator perspective... but
is it necessary at all? It's not a big deal either way though. I believe we are
searching only for whole CONFIG_FEATURE options, not a pattern. However, this is
minimal change and is correct.

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

> +}
> +
>  RUNMAKE=true
>  ALLTARGET=alldefconfig
>  WARNREDUN=false
> @@ -116,8 +120,8 @@ for MERGE_FILE in $MERGE_LIST ; do
>  
>  	for CFG in $CFG_LIST ; do
>  		grep -q -w $CFG $TMP_FILE || continue
> -		PREV_VAL=$(grep -w $CFG $TMP_FILE)
> -		NEW_VAL=$(grep -w $CFG $MERGE_FILE)
> +		PREV_VAL=$(getval "$CFG" "$TMP_FILE")
> +		NEW_VAL=$(getval "$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
> @@ -157,8 +161,8 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $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
>  
> -	REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
> -	ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
> +	REQUESTED_VAL=$(getval "$CFG" "$TMP_FILE")
> +	ACTUAL_VAL=$(getval "$CFG" "$KCONFIG_CONFIG")
>  	if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
>  		echo "Value requested for $CFG not in final .config"
>  		echo "Requested value:  $REQUESTED_VAL"
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 00/10] merge_config misc reworks and testcases

[Bruce Ashfield]

On 10/28/2015 01:02 AM, Darren Hart wrote:
> On Wed, Oct 28, 2015 at 09:42:01AM +0900, Olof Johansson wrote:
>> Hi,
>>
>> Somewhat wide distribution list here, since I've added everyone who's
>> touched the script, with the presumption that those have been the major
>> users of it. Please make sure none of these changes break your use cases.
>
> +Bruce. I think you were going to test these with the linux-yocto tooling. Did
> you get a chance to run that test? I'd like your thoughts on the two comments
> below:

I ran some initial tests, but I didn't end up doing a full
update due to other constraints.

But in the next few weeks, I can do that update and full run.

>
>>
>> I've done some reworks of merge_config.sh. I was quite hesitant to start
>> this since there are no good ways to see if your changes break others
>> or not, so the first thing I did was to add some tests. I know this is
>> highly unorthodox so try not to panic. :-)
>>
>> As far as what this series does is:
>>
>> - Adds a way to pass in CONFIG_FOO=<value> on the command line, it gets
>>    treated as a single-entry fragment file
>>
>> - The script now prints the warnings on stderr, and returns non-0 when
>>    something is encountered
>
> This one might impact linux-yocto usage, Bruce? That said, it seems like the
> right thing to do. So I'd still like to see it go in, but we may need to plan to
> update the dependent tooling to use it.

I don't directly let the merge_config output be visible, but capture it
and then do more processing later. So while this may mean that I have
to update some wrappers to capture stderr, it shouldn't be a big deal.

>
>>
>> - Optionally, it'll also return non-0 when a redundant entry is found. I
>>    presumed people rely on -r not being a failure so I did this separately
>>
>> - CONFIG_FOO=n and "# CONFIG_FOO is not set" is now treated the same,
>>    and using the former doesn't cause an invalid warning when the results
>>    are checked at the end
>>
>> - Slightly odd things happened if a fragment contains the same option
>>    twice: It'd produce a warning that was malformed. Now just ignore that
>>    and use only the latest value of said option.
>
> This one will likely impact usage as well. linux-yocto does want to report when
> there is an override, not as an error, but for informational purposes - "Where
> does my option get clobbered?"

I haven't looked at the patches yet (and I will shortly), but if that
is within a single fragment, I can live with it going away, since it is
easy to check that outside of the merge script.

But if this is a redefinition between fragments, that's something different
and something that I capture and report to users, and yes, I
currently take it from the output of the merge_config run. If it goes
away, I'd have to recreate it somehow.

So if this can at least be maintained as enabled via a parameter, that
would be be ideal. Otherwise, I'll have to recreate the output some
other way.

Bruce

>
>
>

Re: [PATCH 00/10] merge_config misc reworks and testcases

[Bruce Ashfield]

On 10/28/2015 01:02 AM, Darren Hart wrote:
> On Wed, Oct 28, 2015 at 09:42:01AM +0900, Olof Johansson wrote:
>> Hi,
>>
>> Somewhat wide distribution list here, since I've added everyone who's
>> touched the script, with the presumption that those have been the major
>> users of it. Please make sure none of these changes break your use cases.
>
> +Bruce. I think you were going to test these with the linux-yocto tooling. Did
> you get a chance to run that test? I'd like your thoughts on the two comments
> below:

I ran some initial tests, but I didn't end up doing a full
update due to other constraints.

But in the next few weeks, I can do that update and full run.

>
>>
>> I've done some reworks of merge_config.sh. I was quite hesitant to start
>> this since there are no good ways to see if your changes break others
>> or not, so the first thing I did was to add some tests. I know this is
>> highly unorthodox so try not to panic. :-)
>>
>> As far as what this series does is:
>>
>> - Adds a way to pass in CONFIG_FOO=<value> on the command line, it gets
>>    treated as a single-entry fragment file
>>
>> - The script now prints the warnings on stderr, and returns non-0 when
>>    something is encountered
>
> This one might impact linux-yocto usage, Bruce? That said, it seems like the
> right thing to do. So I'd still like to see it go in, but we may need to plan to
> update the dependent tooling to use it.

I don't directly let the merge_config output be visible, but capture it
and then do more processing later. So while this may mean that I have
to update some wrappers to capture stderr, it shouldn't be a big deal.

>
>>
>> - Optionally, it'll also return non-0 when a redundant entry is found. I
>>    presumed people rely on -r not being a failure so I did this separately
>>
>> - CONFIG_FOO=n and "# CONFIG_FOO is not set" is now treated the same,
>>    and using the former doesn't cause an invalid warning when the results
>>    are checked at the end
>>
>> - Slightly odd things happened if a fragment contains the same option
>>    twice: It'd produce a warning that was malformed. Now just ignore that
>>    and use only the latest value of said option.
>
> This one will likely impact usage as well. linux-yocto does want to report when
> there is an override, not as an error, but for informational purposes - "Where
> does my option get clobbered?"

I haven't looked at the patches yet (and I will shortly), but if that
is within a single fragment, I can live with it going away, since it is
easy to check that outside of the merge script.

But if this is a redefinition between fragments, that's something different
and something that I capture and report to users, and yes, I
currently take it from the output of the merge_config run. If it goes
away, I'd have to recreate it somehow.

So if this can at least be maintained as enabled via a parameter, that
would be be ideal. Otherwise, I'll have to recreate the output some
other way.

Bruce

>
>
>

Re: [PATCH 02/10] merge_config.sh: print warnings on stderr

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:03AM +0900, Olof Johansson wrote:
> Makes it easier to grab just errors/warnings in a build log.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

I think this is the right change, I just want to make sure Bruce (Cc'd) sees it
for linux-yocto.

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

> ---
>  scripts/kconfig/merge_config.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> index d8cd913..0e98619 100755
> --- a/scripts/kconfig/merge_config.sh
> +++ b/scripts/kconfig/merge_config.sh
> @@ -131,7 +131,7 @@ for MERGE_FILE in $MERGE_LIST ; do
>  			echo Value of $CFG is redundant by fragment $MERGE_FILE:
>  		fi
>  		sed -i "/$CFG[ =]/d" $TMP_FILE
> -	done
> +	done >&2
>  	cat $MERGE_FILE >> $TMP_FILE
>  done
>  
> @@ -168,7 +168,7 @@ for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
>  		echo "Requested value:  $REQUESTED_VAL"
>  		echo "Actual value:     $ACTUAL_VAL"
>  		echo ""
> -	fi
> +	fi >&2
>  done
>  
>  clean_up
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 03/10] merge_config.sh: minor argument parsing refactoring

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:04AM +0900, Olof Johansson wrote:
> Every case that continues iterating needs a shift, so move it to common location.
> Also, the continues are redundant.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

Nice cleanup, thanks Olof.

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 04/10] merge_config.sh: exit non-0 in case of failures

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:05AM +0900, Olof Johansson wrote:
> Exit with non-0 value in cases where there was a failure to set an option.
> Also, add a '-e' during which the conflict warnings are considered failures
> (-e -r will result in these being failures, -r will result in them just being
> reported).
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

It's useful to be able to check the return code for errors. Since override is
often expected, making tat configurable makes sense.

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 05/10] merge_config.sh: Better handling of CONFIG_FOO=n

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:06AM +0900, Olof Johansson wrote:
> Kconfig knows how to handle CONFIG_FOO=n just fine, but it'll always
> use "# CONFIG FOO is not set" in the resulting config. Mangle the input
> accordingly so we don't report this as a failure when it isn't.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

Matching Kconfig behavior is the logical approach, no point in complaining if
Kconfig will accpet the result.

Bruce, I think we'll just need to update the linux-yocto tool documentation not
to mark =n as wrong.

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 05/10] merge_config.sh: Better handling of CONFIG_FOO=n

[Bruce Ashfield]

On 10/28/2015 02:26 AM, Darren Hart wrote:
> On Wed, Oct 28, 2015 at 09:42:06AM +0900, Olof Johansson wrote:
>> Kconfig knows how to handle CONFIG_FOO=n just fine, but it'll always
>> use "# CONFIG FOO is not set" in the resulting config. Mangle the input
>> accordingly so we don't report this as a failure when it isn't.
>>
>> Signed-off-by: Olof Johansson <olof@lixom.net>
>
> Matching Kconfig behavior is the logical approach, no point in complaining if
> Kconfig will accpet the result.
>
> Bruce, I think we'll just need to update the linux-yocto tool documentation not
> to mark =n as wrong.

purists still think it is wrong :) The tools don't report this as
an error anymore, so there's no change here.

Bruce

>
> Reviewed-by: Darren Hart <dvhart@linux.intel.com>
>

Re: [PATCH 05/10] merge_config.sh: Better handling of CONFIG_FOO=n

[Bruce Ashfield]

On 10/28/2015 02:26 AM, Darren Hart wrote:
> On Wed, Oct 28, 2015 at 09:42:06AM +0900, Olof Johansson wrote:
>> Kconfig knows how to handle CONFIG_FOO=n just fine, but it'll always
>> use "# CONFIG FOO is not set" in the resulting config. Mangle the input
>> accordingly so we don't report this as a failure when it isn't.
>>
>> Signed-off-by: Olof Johansson <olof@lixom.net>
>
> Matching Kconfig behavior is the logical approach, no point in complaining if
> Kconfig will accpet the result.
>
> Bruce, I think we'll just need to update the linux-yocto tool documentation not
> to mark =n as wrong.

purists still think it is wrong :) The tools don't report this as
an error anymore, so there's no change here.

Bruce

>
> Reviewed-by: Darren Hart <dvhart@linux.intel.com>
>

Re: [PATCH 06/10] merge_config.sh: only consider last value of symbols

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:07AM +0900, Olof Johansson wrote:

Needs a commit message here that says (I think): Avoid mangled error output by
collecting only the last value. Warnings are still issued when overrides occur.

Agreed on the change though.

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  scripts/kconfig/merge_config.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> index 933f2d6..1945b2c 100755
> --- a/scripts/kconfig/merge_config.sh
> +++ b/scripts/kconfig/merge_config.sh
> @@ -39,7 +39,7 @@ usage() {
>  }
>  
>  getval() {
> -	grep -w -e "$1" "$2" | sed 's/^CONFIG_\(.*\)=n$/# CONFIG_\1 is not set/g'
> +	grep -w -e "$1" "$2" | tail -1 | sed 's/^CONFIG_\(.*\)=n/# CONFIG_\1 is not set/g'
>  }
>  
>  CONF_IS_ERR=false
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 07/10] merge_config.sh: add tests

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:08AM +0900, Olof Johansson wrote:
> For being a small script, merge_config.sh is fairly scary to change since there's no
> real way to know if you did something wrong. So it seems appropriate to add a simple
> test suite.
> 
> I've started with testcases in the areas I care about, other should of course feel
> free to expand on this.
> 
> Use is simple, from the kernel tree, run ./scripts/kconfig/merge_config_test/runall.
> It'll execute out of a tmpdir under /tmp.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

Thanks for writing the test cases. Shell scripting is so easy to bikeshed - so
I'm going to skip that. I didn't catch any bashisms, and the test coverage is
good.

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 00/10] merge_config misc reworks and testcases

[Darren Hart]

On Wed, Oct 28, 2015 at 01:30:59AM -0400, Bruce Ashfield wrote:
> On 10/28/2015 01:02 AM, Darren Hart wrote:
> >On Wed, Oct 28, 2015 at 09:42:01AM +0900, Olof Johansson wrote:
> >>- The script now prints the warnings on stderr, and returns non-0 when
> >>   something is encountered
> >
> >This one might impact linux-yocto usage, Bruce? That said, it seems like the
> >right thing to do. So I'd still like to see it go in, but we may need to plan to
> >update the dependent tooling to use it.
> 
> I don't directly let the merge_config output be visible, but capture it
> and then do more processing later. So while this may mean that I have
> to update some wrappers to capture stderr, it shouldn't be a big deal.
> 
> >
> >>
> >>- Optionally, it'll also return non-0 when a redundant entry is found. I
> >>   presumed people rely on -r not being a failure so I did this separately
> >>
> >>- CONFIG_FOO=n and "# CONFIG_FOO is not set" is now treated the same,
> >>   and using the former doesn't cause an invalid warning when the results
> >>   are checked at the end
> >>
> >>- Slightly odd things happened if a fragment contains the same option
> >>   twice: It'd produce a warning that was malformed. Now just ignore that
> >>   and use only the latest value of said option.
> >
> >This one will likely impact usage as well. linux-yocto does want to report when
> >there is an override, not as an error, but for informational purposes - "Where
> >does my option get clobbered?"
> 
> I haven't looked at the patches yet (and I will shortly), but if that
> is within a single fragment, I can live with it going away, since it is
> easy to check that outside of the merge script.
> 
> But if this is a redefinition between fragments, that's something different
> and something that I capture and report to users, and yes, I
> currently take it from the output of the merge_config run. If it goes
> away, I'd have to recreate it somehow.
> 
> So if this can at least be maintained as enabled via a parameter, that
> would be be ideal. Otherwise, I'll have to recreate the output some
> other way.

It still reports redundancies across different fragments. It just fixes the grep
so it doesn't display two options from the same file.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 07/10] merge_config.sh: add tests

[Olof Johansson]

On Wed, Oct 28, 2015 at 4:00 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Wed, Oct 28, 2015 at 09:42:08AM +0900, Olof Johansson wrote:
>> For being a small script, merge_config.sh is fairly scary to change since there's no
>> real way to know if you did something wrong. So it seems appropriate to add a simple
>> test suite.
>>
>> I've started with testcases in the areas I care about, other should of course feel
>> free to expand on this.
>>
>> Use is simple, from the kernel tree, run ./scripts/kconfig/merge_config_test/runall.
>> It'll execute out of a tmpdir under /tmp.
>>
>> Signed-off-by: Olof Johansson <olof@lixom.net>
>
> Thanks for writing the test cases. Shell scripting is so easy to bikeshed - so
> I'm going to skip that. I didn't catch any bashisms, and the test coverage is
> good.

Thanks!

Anyone who wishes to contribute style comments has to contribute at
least one additional testcase as well. Fair? :)


-Olof

Re: [PATCH 08/10] merge_config.sh: use trap for cleanup

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:09AM +0900, Olof Johansson wrote:
> Use the trap to cleanup even on regular exit.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  scripts/kconfig/merge_config.sh | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> index 1945b2c..b26c0ef 100755
> --- a/scripts/kconfig/merge_config.sh
> +++ b/scripts/kconfig/merge_config.sh
> @@ -23,10 +23,12 @@
>  EXITVAL=0
>  
>  clean_up() {
> -	rm -f $TMP_FILE
> +	if [ -n "$CLEAN_FILES" ] ; then
> +		rm -f $CLEAN_FILES

Hrm... where did CLEAN_FILES come from? I dodn't see it in master and I don't
see it in patches 1-7. Did I miss it?

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 09/10] merge_config.sh: allow single configs to be passed in on cmdline

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:10AM +0900, Olof Johansson wrote:
> Treat CONFIG_FOO=..  on the command line the same way as a single-entry file would.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  scripts/kconfig/merge_config.sh | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> index b26c0ef..69463dd 100755
> --- a/scripts/kconfig/merge_config.sh
> +++ b/scripts/kconfig/merge_config.sh
> @@ -1,8 +1,8 @@
>  #!/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).
> +#  merge_config.sh - Takes a list of config fragment filenames or configuration
> +#  value, 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
> @@ -108,7 +108,26 @@ fi
>  
>  MERGE_LIST=$*
>  SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
> -TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
> +TMP_FILE=$(mktemp $OUTPUT/.tmp.config.XXXXXXXXXX)
> +
> +CLEAN_FILES=$TMP_FILE

Oh, here it is :-) Looks like this should come before 8/10 ?

But, content is good. Multiple files are needed in case CONFIG_* options are
intermixed with file fragments.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 08/10] merge_config.sh: use trap for cleanup

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:09AM +0900, Olof Johansson wrote:
> Use the trap to cleanup even on regular exit.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

Looks like this should trade spots in the series with 9 where CLEAN_FILES is
defined. Otherwise:

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

> ---
>  scripts/kconfig/merge_config.sh | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> index 1945b2c..b26c0ef 100755
> --- a/scripts/kconfig/merge_config.sh
> +++ b/scripts/kconfig/merge_config.sh
> @@ -23,10 +23,12 @@
>  EXITVAL=0
>  
>  clean_up() {
> -	rm -f $TMP_FILE
> +	if [ -n "$CLEAN_FILES" ] ; then
> +		rm -f $CLEAN_FILES
> +	fi
>  	exit $EXITVAL
>  }
> -trap clean_up HUP INT TERM
> +trap clean_up HUP INT TERM EXIT
>  
>  usage() {
>  	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
> @@ -178,4 +180,4 @@ for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
>  	fi >&2
>  done
>  
> -clean_up
> +# Note: clean_up will run here due to EXIT being trapped
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 10/10] merge_config.sh: add tests for cmdline configs

[Darren Hart]

On Wed, Oct 28, 2015 at 09:42:11AM +0900, Olof Johansson wrote:
> Again, these tests could be more complicated but at least it gets
> the very basics covered.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

Good thing you added =n as valid, this would be much harder otherwise :-)

Signed-off-by: Darren Hart <dvhart@linux.intel.com>

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 00/10] merge_config misc reworks and testcases

[Michal Marek]

On 2015-10-28 01:46, Olof Johansson wrote:
> On Wed, Oct 28, 2015 at 9:42 AM, Olof Johansson <olof@lixom.net> wrote:
>> Hi,
>>
>> Somewhat wide distribution list here, since I've added everyone who's
>> touched the script, with the presumption that those have been the major
>> users of it. Please make sure none of these changes break your use cases.
> 
> Actually, I dropped a handful of cc:s on this repost, so this isn't
> necessarily true. Also, since Yann didn't respond on the last posting
> I'm sending these to you now, Michal. :)

Thanks. Can you please resend with the ACKs collected and with 08/10 and
09/10 swapped?

Thanks,
Michal

[PATCH 08/10] merge_config.sh: use trap for cleanup

[Olof Johansson]

Use the trap to cleanup even on regular exit.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 scripts/kconfig/merge_config.sh |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 932fd52..fd27294 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -23,10 +23,12 @@
 EXITVAL=0
 
 clean_up() {
-	rm -f $TMP_FILE
+	if [ -n "$CLEAN_FILES" ] ; then
+		rm -f $CLEAN_FILES
+	fi
 	exit $EXITVAL
 }
-trap clean_up HUP INT TERM
+trap clean_up HUP INT TERM EXIT
 
 usage() {
 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
@@ -166,4 +168,4 @@ for CFG in $(sed -n "$SED_CONFIG_EXP" $TMP_FILE); do
 	fi >&2
 done
 
-clean_up
+# Note: clean_up will run here due to EXIT being trapped
-- 
1.7.10.4