From 326ef19d942c8922ebba68e96d870d5117f67472 Mon Sep 17 00:00:00 2001
From: Chris Evich <cevich@redhat.com>
Date: Wed, 30 Oct 2019 10:35:20 -0400
Subject: Cirrus: Fix upload_release_archive on branch or tag

Cirrus-CI only sets `$CIRRUS_BASE_SHA` when testing PRs since the
destination and it's state is easy to discover.  However, when
post-merge and/or tag-push testing, the previous state is not
easily discoverable (changes have already merged).  The
`upload_release_archive` script incorrectly assumed this
variable was always set, causing a constant stream of post-merge
testing failures.

Tweak the `is_release()` function to properly handle an empty
`$CIRRUS_BASE_SHA` whether or not `$CIRRUS_TAG` is also set.  Also
update the unit-tests to check for this.  Also account for a
corner case where hack/get_ci_vm.sh is running on a VM w/o git.

Signed-off-by: Chris Evich <cevich@redhat.com>
---
 contrib/cirrus/lib.sh   | 46 +++++++++++++++++++++++++++++-----------------
 contrib/cirrus/lib.sh.t | 17 ++++++++++-------
 2 files changed, 39 insertions(+), 24 deletions(-)

(limited to 'contrib')

diff --git a/contrib/cirrus/lib.sh b/contrib/cirrus/lib.sh
index 051157702..297ed49ce 100644
--- a/contrib/cirrus/lib.sh
+++ b/contrib/cirrus/lib.sh
@@ -238,34 +238,46 @@ ircmsg() {
 # there is at least one release tag not having any '-' characters (return 0)
 # or otherwise (return non-0).
 is_release() {
-    req_env_var CIRRUS_BASE_SHA CIRRUS_CHANGE_IN_REPO
-    local range="${CIRRUS_BASE_SHA}..${CIRRUS_CHANGE_IN_REPO}"
-    # Easy check first, default non-useful values
-    if echo "${range}$CIRRUS_TAG" | grep -iq 'unknown'; then
-        die 11 "is_release() unusable range ${range} or tag $CIRRUS_TAG"
-    fi
-    # Next easy check, is CIRRUS_TAG set
     unset RELVER
+    local ret
+    req_env_var CIRRUS_CHANGE_IN_REPO
     if [[ -n "$CIRRUS_TAG" ]]; then
         RELVER="$CIRRUS_TAG"
-    else # Lastly, look through the range for tags
-        git fetch --all --tags &> /dev/null|| \
-            die 12 "is_release() failed to fetch tags"
-        RELVER=$(git log --pretty='format:%d' $range | \
-                 grep '(tag:' | sed -r -e 's/\s+[(]tag:\s+(v[0-9].*)[)]/\1/' | \
-                 sort -uV | tail -1)
-        [[ "$?" -eq "0" ]] || \
+    elif [[ ! "$CIRRUS_BASE_SHA" =~ "unknown" ]]
+    then
+        # Normally not possible for this to be empty, except when unittesting.
+        req_env_var CIRRUS_BASE_SHA
+        local range="${CIRRUS_BASE_SHA}..${CIRRUS_CHANGE_IN_REPO}"
+        if echo "${range}$CIRRUS_TAG" | grep -iq 'unknown'; then
+            die 11 "is_release() unusable range ${range} or tag $CIRRUS_TAG"
+        fi
+
+        if type -P git &> /dev/null
+        then
+            git fetch --all --tags &> /dev/null|| \
+                die 12 "is_release() failed to fetch tags"
+            RELVER=$(git log --pretty='format:%d' $range | \
+                     grep '(tag:' | sed -r -e 's/\s+[(]tag:\s+(v[0-9].*)[)]/\1/' | \
+                     sort -uV | tail -1)
+            ret=$?
+        else
+            warn -1 "Git command not found while checking for release"
+            ret="-1"
+        fi
+        [[ "$ret" -eq "0" ]] || \
             die 13 "is_release() failed to parse tags"
+    else  # Not testing a PR, but neither CIRRUS_BASE_SHA or CIRRUS_TAG are set
+        return 1
     fi
-    echo "Found \$RELVER $RELVER"
     if [[ -n "$RELVER" ]]; then
+        echo "Found \$RELVER $RELVER"
         if echo "$RELVER" | grep -q '-'; then
-            return 2
+            return 2  # development tag
         else
             return 0
         fi
     else
-        return 1
+        return 1  # not a release
     fi
 }
 
diff --git a/contrib/cirrus/lib.sh.t b/contrib/cirrus/lib.sh.t
index 9915b42a4..8f4080dd5 100755
--- a/contrib/cirrus/lib.sh.t
+++ b/contrib/cirrus/lib.sh.t
@@ -138,16 +138,19 @@ function test_is_release() {
 }
 
 #                FROM    TO    TAG    RET    MSG
-#test_is_release ""      ""    ""     ""     ""
-
-test_is_release  ""      ""    ""     "9"     "FATAL: is_release() requires \$CIRRUS_BASE_SHA to be non-empty"
+test_is_release  ""      ""    ""     "9"     "FATAL: is_release() requires \$CIRRUS_CHANGE_IN_REPO to be non-empty"
 test_is_release  "x"     ""    ""     "9"     "FATAL: is_release() requires \$CIRRUS_CHANGE_IN_REPO to be non-empty"
 
-test_is_release  "unknown" "x" ""     "11"    "is_release() unusable range unknown..x or tag "
-test_is_release  "x" "unknown" ""     "11"    "is_release() unusable range x..unknown or tag "
-test_is_release  "x" "x" "unknown"    "11"    "is_release() unusable range x..x or tag unknown"
+# post-merge / tag-push testing, FROM will be set 'unknown' by (lib.sh default)
+test_is_release  "unknown" "x" ""     "1"    ""
+# post-merge / tag-push testing, oddball tag is set, FROM will be set 'unknown'
+test_is_release "unknown"  "unknown" "test-tag" "2"    "Found \$RELVER test-tag"
+# post-merge / tag-push testing, sane tag is set, FROM will be set 'unknown'
+test_is_release "unknown"  "unknown" "0.0.0" "0"    "Found \$RELVER 0.0.0"
+# hack/get_ci_vm or PR testing, FROM and TO are set, no tag is set
+test_is_release  "x"     "x"   ""     "1"    ""
 
-# Negative-testing git with this function is very difficult, assume it works
+# Negative-testing git with this function is very difficult, assume git works
 # test_is_release ... "is_release() failed to fetch tags"
 # test_is_release ... "is_release() failed to parse tags"
 
-- 
cgit v1.2.3-54-g00ecf