fate: Error out more gracefully on configure failure

Message ID 20180219181516.13468-1-diego@biurrun.de
State Superseded
Headers show
Series
  • fate: Error out more gracefully on configure failure
Related show

Commit Message

Diego Biurrun Feb. 19, 2018, 6:15 p.m.
If configure fails before config.fate is generated, the report file misses
some values and gets discarded by the FATE server. In these cases, make an
effort to fill in sensible values and provide "unknown" as fallback value.
---

Made some changes as suggested by Janne on IRC, i.e. try to provide some
info instead of calling all fields "unknown".

 tests/fate.sh | 60 +++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 26 deletions(-)

Comments

Diego Biurrun Feb. 20, 2018, 1:56 p.m. | #1
On Mon, Feb 19, 2018 at 07:15:16PM +0100, Diego Biurrun wrote:
> If configure fails before config.fate is generated, the report file misses
> some values and gets discarded by the FATE server. In these cases, make an
> effort to fill in sensible values and provide "unknown" as fallback value.
> ---
> 
> Made some changes as suggested by Janne on IRC, i.e. try to provide some
> info instead of calling all fields "unknown".

22:11 <@jannau> DonDiego: I don't think the best effort guess for all fields is a good idea.
22:13 <@jannau> those fields are usually somewhat sanitized through configure and compiler
22:13 <@jannau> and we use it to filter the fate results
22:16 <@jannau> those values are also not really unknown. they are missing since configure failed for some reason
22:18 <@jannau> so 'failed' is IMO a better value in this case
22:21 <@jannau> the best effor guesses do not contain any additional information compared to the configure command so it's probably better to just use 'failed' for all runs which fail to write a config.fate
22:22  * jannau probably should have written an email

Yeah, I've thought about the same things. Updated patch according to
your suggestion coming up.

Diego

Patch

diff --git a/tests/fate.sh b/tests/fate.sh
index c93e20a464..0544a82e75 100755
--- a/tests/fate.sh
+++ b/tests/fate.sh
@@ -21,6 +21,33 @@  test -d "$samples" || die "samples location not specified"
 
 : ${branch:=master}
 
+src=${workdir}/src
+: ${build:=${workdir}/build}
+: ${inst:=${workdir}/install}
+
+configuration='
+    --enable-gpl
+    --prefix="${inst}"
+    --samples="${samples}"
+    ${ignore_tests:+--ignore-tests="$ignore_tests"}
+    ${arch:+--arch="$arch"}
+    ${cpu:+--cpu="$cpu"}
+    ${toolchain:+--toolchain="$toolchain"}
+    ${cross_prefix:+--cross-prefix="$cross_prefix"}
+    ${as:+--as="$as"}
+    ${cc:+--cc="$cc"}
+    ${ld:+--ld="$ld"}
+    ${target_os:+--target-os="$target_os"}
+    ${sysroot:+--sysroot="$sysroot"}
+    ${target_exec:+--target-exec="$target_exec"}
+    ${target_path:+--target-path="$target_path"}
+    ${target_samples:+--target-samples="$target_samples"}
+    ${extra_cflags:+--extra-cflags="$extra_cflags"}
+    ${extra_ldflags:+--extra-ldflags="$extra_ldflags"}
+    ${extra_libs:+--extra-libs="$extra_libs"}
+    ${extra_conf}
+'
+
 lock(){
     lock=$1/fate.lock
     (set -C; exec >$lock) 2>/dev/null || return
@@ -43,27 +70,7 @@  update()(
 
 configure()(
     cd ${build} || return
-    ${src}/configure                                                    \
-        --prefix="${inst}"                                              \
-        --samples="${samples}"                                          \
-        --enable-gpl                                                    \
-        ${ignore_tests:+--ignore-tests="$ignore_tests"}                 \
-        ${arch:+--arch=$arch}                                           \
-        ${cpu:+--cpu="$cpu"}                                            \
-        ${toolchain:+--toolchain="$toolchain"}                          \
-        ${cross_prefix:+--cross-prefix="$cross_prefix"}                 \
-        ${as:+--as="$as"}                                               \
-        ${cc:+--cc="$cc"}                                               \
-        ${ld:+--ld="$ld"}                                               \
-        ${target_os:+--target-os="$target_os"}                          \
-        ${sysroot:+--sysroot="$sysroot"}                                \
-        ${target_exec:+--target-exec="$target_exec"}                    \
-        ${target_path:+--target-path="$target_path"}                    \
-        ${target_samples:+--target-samples="$target_samples"}           \
-        ${extra_cflags:+--extra-cflags="$extra_cflags"}                 \
-        ${extra_ldflags:+--extra-ldflags="$extra_ldflags"}              \
-        ${extra_libs:+--extra-libs="$extra_libs"}                       \
-        ${extra_conf}
+    eval ${src}/configure ${configuration}
 )
 
 compile()(
@@ -84,7 +91,12 @@  clean(){
 report(){
     date=$(date -u +%Y%m%d%H%M%S)
     echo "fate:1:${date}:${slot}:${version}:$1:$2:${branch}:${comment}" >report
-    cat ${build}/avbuild/config.fate ${build}/tests/data/fate/*.rep >> report 2> /dev/null
+    if test -e ${build}/avbuild/config.fate; then
+        cat ${build}/avbuild/config.fate >> report 2> /dev/null
+    else
+        eval echo config:${arch:-unknown}:${arch:-unknown}:${cpu:-unknown}:${target_os:-unknown}:${cc:-unknown}:${configuration} >> report 2> /dev/null
+    fi
+    cat ${build}/tests/data/fate/*.rep >> report 2> /dev/null
     test -n "$fate_recv" && $tar report *.log | gzip | $fate_recv
 }
 
@@ -98,10 +110,6 @@  mkdir -p ${workdir} || die "Error creating ${workdir}"
 lock ${workdir}     || die "${workdir} locked"
 cd ${workdir}       || die "cd ${workdir} failed"
 
-src=${workdir}/src
-: ${build:=${workdir}/build}
-: ${inst:=${workdir}/install}
-
 test -d "$src" && update || checkout || die "Error fetching source"
 
 cd ${workdir}