From f70292585aabfa29006398e941d2f76512d4d962 Mon Sep 17 00:00:00 2001 From: Maxim Kuvyrkov Date: Fri, 15 Mar 2019 12:18:24 +0000 Subject: round-robin.sh: Fix un-intutive bash behavior Turns out that condition in "if ${undefined_var+false}; then ... fi" evaluated to "true" when $undefined_var is undefined -- i.e., empty condition is a "true" condition, as long as it is not physically empty ("if ; then ... fi" will produce a syntatic error). This perculiarity of bash caused unexpected behavior and we used "empty" branch of base-artifacts for several builds -- which, unfortunately, caused us to loose history regressions in several configurations. We caught this quickly, so no serious damage done. Fix by adding verbose conditionals. Change-Id: Ib6f1a624a9ea41f7223bc3e85e2e4eddc3fad9a9 --- round-robin.sh | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/round-robin.sh b/round-robin.sh index 129e6fd5..6e669e11 100644 --- a/round-robin.sh +++ b/round-robin.sh @@ -95,15 +95,16 @@ reset_artifacts () # don't overwrite it. # base-artifacts repo is big and changes all the time, so we # fetch only the $baseline_branch, instead of all branches. - # - # Since we re-write history of base-artifacts all the time we can't - # rely on SHA1 stored in manifests (aka ${rr[base-artifacts_rev]}) to - # exist. The important case here is reproduction of regressions from - # email instructions: baseline run with reset_baseline set. We could - # have tagged such SHA1s to keep them stored in the repo, but it is easier - # to just ignore base-artifacts_rev when reset_baseline is set. - if ${rr[init_configuration]-false} \ - || ${rr[base-artifacts_rev]+${rr[reset_baseline]}}; then + if ${rr[init_configuration]-false}; then + rr[base-artifacts_rev]=empty + single_branch=empty + elif [ x"${rr[base-artifacts_rev]+set}" = x"set" ] && ${rr[reset_baseline]}; then + # Since we re-write history of base-artifacts all the time we can't + # rely on SHA1 stored in manifests (aka ${rr[base-artifacts_rev]}) to + # exist. The important case here is reproduction of regressions from + # email instructions: baseline run with reset_baseline set. We could + # have tagged such SHA1s to keep them stored in the repo, but it is easier + # to just ignore base-artifacts_rev when reset_baseline is set. rr[base-artifacts_rev]=empty single_branch=empty else -- cgit v1.2.3