| Message ID | 20190808222705.35973-1-broonie@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | merge_config.sh: Check error codes from make | expand |
On Fri, Aug 9, 2019 at 7:27 AM Mark Brown <broonie@kernel.org> wrote: > > 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 <guillaume.tucker@collabora.com> > Signed-off-by: Mark Brown <broonie@kernel.org> > --- 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: diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh index d924c51d28b7..bec246719aea 100755 --- a/scripts/kconfig/merge_config.sh +++ b/scripts/kconfig/merge_config.sh @@ -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
On Sat, Aug 10, 2019 at 06:11:10PM +0900, Masahiro Yamada wrote: > On Fri, Aug 9, 2019 at 7:27 AM Mark Brown <broonie@kernel.org> wrote: > > 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 :/ > [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 <broonie@kernel.org> Tested-by: Mark Brown <broonie@kernel.org>
diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh index d924c51d28b7..96e960dce968 100755 --- a/scripts/kconfig/merge_config.sh +++ b/scripts/kconfig/merge_config.sh @@ -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)
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 <guillaume.tucker@collabora.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- scripts/kconfig/merge_config.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)