merge_config.sh: Check error codes from make

PATCH Superseded Mark Brown 2 participants 3 messages Aug 8, 2019 – Aug 12, 2019

Subsystems (2)

Kbuild Kconfig

lore

Share & tools

lore

Mark Brown

author

2019-08-08 22:27 UTC #0 lore raw

PATCH 1/1 merge_config.sh: Check error codes from make

When we execute make after merging the configurations we ignore any errors it produces causing whatever is running merge_config.sh to be unaware of any failures. This issue was noticed by Guillaume Tucker while looking at problems with testing of clang only builds in KernelCI which caused Kbuild to be unable to find a working host compiler.

Reported-by Guillaume Tucker <[email protected]>

Signed-off-by Mark Brown <[email protected]>

---
 scripts/kconfig/merge_config.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

scripts/kconfig/merge_config.sh +6 -1

index d924c51d28b7..96e960dce968 100755
@@ -14,9 +14,10 @@
 #  Copyright 2011 Linaro
 
 clean_up() {
+	RET=$1
 	rm -f $TMP_FILE
 	rm -f $MERGE_FILE
-	exit
+	exit ${RET}
 }
 trap clean_up HUP INT TERM

@@ -171,6 +172,10 @@ fi
 # 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 $OUTPUT_ARG $ALLTARGET
+RET=$?
+if [ "$RET" != "0" ]; then
+	clean_up $RET
+fi
 
 
 # Check all specified config values took (might have missed-dependency issues)

Signature · 1 line

2.20.1

Masahiro Yamada

2019-08-10 09:11 UTC #1 lore raw

Quoted 10 lines · Mark Brown

When we execute make after merging the configurations we ignore any errors it produces causing whatever is running merge_config.sh to be unaware of any failures. This issue was noticed by Guillaume Tucker while looking at problems with testing of clang only builds in KernelCI which caused Kbuild to be unable to find a working host compiler.
Reported-by Guillaume Tucker <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---

I am not a big fan of this way of fixing.

[1] 'set -e' is useful to catch any error in this script.

[2] I think trapping only EXIT is smarter.
The clean() help will be invoked when this script exits for whatever reason; the temporary files will be cleaned up when the script is interrupted, errors-out, or finishes successfully.

I would change like follows:

scripts/kconfig/merge_config.sh +5 -3

index d924c51d28b7..bec246719aea 100755
@@ -13,12 +13,12 @@
 #  Copyright (c) 2009-2010 Wind River Systems, Inc.
 #  Copyright 2011 Linaro
 
+set -e
+
 clean_up() {
        rm -f $TMP_FILE
        rm -f $MERGE_FILE
-       exit
 }
-trap clean_up HUP INT TERM
 
 usage() {
        echo "Usage: $0 [OPTIONS] [CONFIG [...]]"

@@ -110,6 +110,9 @@ TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
 MERGE_FILE=$(mktemp ./.merge_tmp.config.XXXXXXXXXX)
 
 echo "Using $INITFILE as base"
+
+trap clean_up EXIT
+
 cat $INITFILE > $TMP_FILE
 
 # Merge files, printing warnings on overridden values

@@ -155,7 +158,6 @@ if [ "$RUNMAKE" = "false" ]; then
        echo "#"
        echo "# merged configuration written to $KCONFIG_CONFIG (needs make)"
        echo "#"
-       clean_up
        exit
 fi

@@ -185,5 +187,3 @@ for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e

"$SED_CONFIG_EXP2" $TMP_FILE); do

                echo ""
        fi

done
-
-clean_up

--
Best Regards
Masahiro Yamada

Mark Brown

author Reviewed-by Tested-by

2019-08-12 10:43 UTC #2 lore raw

Quoted 9 lines · Masahiro Yamada

Quoted 3 lines · Mark Brown
When we execute make after merging the configurations we ignore any errors it produces causing whatever is running merge_config.sh to be unaware of any failures. This issue was noticed by Guillaume Tucker
I am not a big fan of this way of fixing.
[1] 'set -e' is useful to catch any error in this script.

Right, that was actually my first thought but since there was a handler and it was already being called directly this must be a deliberate style so I did things this way in order to fit in with the existing style :/

Quoted 7 lines

[2] I think trapping only EXIT is smarter.
The clean() help will be invoked when this script exits for whatever reason; the temporary files will be cleaned up when the script is interrupted, errors-out, or finishes successfully.
I would change like follows:

That works for me,

Reviewed-by Mark Brown <[email protected]>

Tested-by Mark Brown <[email protected]>