From f528b2b0b8a4a7a7cb48cf401192c6a52f0926d0 Mon Sep 17 00:00:00 2001
From: Tim Johnson <tijohnson@linuxfoundation.org>
Date: Mon, 6 Jan 2020 15:09:05 -0800
Subject: [PATCH] Clean up gerrit push change script

Removed broken code that removed leading/trailing whitespace from
variables. Use lf-acitvate-venv() to instll openstack. Enabled 'set -euf
pipefail' and updated code to handle errors. Updated out of date
comments.

ISSUE: RELENG-2633
Change-Id: I76d2bf9ed9f7f26303f56dd98ebbd4b458d0d54f
Signed-off-by: Tim Johnson <tijohnson@linuxfoundation.org>
---
 ...leanup-gerrit-push-change-f77402b88764c6ff.yaml |  7 ++
 shell/gerrit-push-patch.sh                         | 84 ++++++++++------------
 2 files changed, 46 insertions(+), 45 deletions(-)
 create mode 100644 releasenotes/notes/cleanup-gerrit-push-change-f77402b88764c6ff.yaml

diff --git a/releasenotes/notes/cleanup-gerrit-push-change-f77402b88764c6ff.yaml b/releasenotes/notes/cleanup-gerrit-push-change-f77402b88764c6ff.yaml
new file mode 100644
index 00000000..c2ad64b0
--- /dev/null
+++ b/releasenotes/notes/cleanup-gerrit-push-change-f77402b88764c6ff.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Removed broken code that removed leading/trailing white-space from
+    variables. Use lf-activate-venv() to install openstack. Enabled 'set -euf
+    pipefail' and updated code to handle errors. Updated out of date
+    comments. Some minor cleanup of code for clarity.
diff --git a/shell/gerrit-push-patch.sh b/shell/gerrit-push-patch.sh
index 8bddfa9d..b33f0f52 100644
--- a/shell/gerrit-push-patch.sh
+++ b/shell/gerrit-push-patch.sh
@@ -11,18 +11,20 @@
 echo "---> gerrit-push-patch.sh"
 # Push a change to Gerrit if files modified in repository.
 #
-# The script requires to install the minimum version 1.25 of git-review using
-# virtualenv and pip install which supports `--reviewers` option.
+# Need to use git-review >= 1.28 to be compatible with Gerrit 3
 #
 # The script allows a job to push a patch to Gerrit in an automated fashion.
 # This is meant for tasks that creates the same patch regularly and needs the
 # ability to detect if an unreviewed patch already exists. In which case it
 # will update the existing patch.
 #
-# Note: This patch assumes the $WORKSPACE contains the project repo with
-#       the files changed already "git add" and waiting for a "git commit" call.
+# Note: This script expects $WORKSPACE to point to a project git repo that
+# may contain staged commits. This script will exit with OK status if no
+# staged commits are present, otherwise the staged commits will be commited and
+# a gerrit review will be created.
 #
-# This script requires the following JJB variables to be passed in:
+# This script expects the following environment variables to be set in the
+# JJB configuration
 #
 #   $PROJECT              : Gerrit project-name
 #   $GERRIT_COMMIT_MESSAGE: Commit message to assign to commit
@@ -31,52 +33,44 @@ echo "---> gerrit-push-patch.sh"
 #   $GERRIT_USER          : Gerrit user
 #   $REVIEWERS_EMAIL      : Reviewers email
 
-# TODO: remove the workaround when v1.26 is available on all images
-# Workaround for git-review bug in v1.24
-# https://storyboard.openstack.org/#!/story/2001081
-set +u  # Allow unbound variables for virtualenv
-virtualenv --quiet "/tmp/v/git-review"
-# shellcheck source=/tmp/v/git-review/bin/activate disable=SC1091
-source "/tmp/v/git-review/bin/activate"
-pip install --quiet --upgrade "pip==9.0.3" setuptools
-pip install --quiet --upgrade git-review
-set -u
-# End git-review workaround
-# Remove any leading or trailing quotes surrounding the strings
-# which can cause parse errors when passed as CLI options to commands
-# shellcheck disable=SC2001
-PROJECT="$(echo "$PROJECT" | sed "s/^\([\"']\)\(.*\)\1\$/\2/g")"
-# shellcheck disable=SC2001
-GERRIT_COMMIT_MESSAGE="$(echo "$GERRIT_COMMIT_MESSAGE" | sed "s/^\([\"']\)\(.*\)\1\$/\2/g")"
-# shellcheck disable=SC2001
-GERRIT_HOST="$(echo "$GERRIT_HOST" | sed "s/^\([\"']\)\(.*\)\1\$/\2/g")"
-# shellcheck disable=SC2001
-GERRIT_TOPIC="$(echo "$GERRIT_TOPIC" | sed "s/^\([\"']\)\(.*\)\1\$/\2/g")"
-# shellcheck disable=SC2001
-GERRIT_USER="$(echo "$GERRIT_USER" | sed "s/^\([\"']\)\(.*\)\1\$/\2/g")"
-# shellcheck disable=SC2001
-REVIEWERS_EMAIL="$(echo "$REVIEWERS_EMAIL" | sed "s/^\([\"']\)\(.*\)\1\$/\2/g")"
-job=$JOB_NAME/$BUILD_NUMBER
+set -eufo pipefail
+
+# No reason to continue if there are no staged commits
+staged_commits=$(git diff --cached --name-only)
+if [[ -z $staged_commits ]]; then
+    echo "INFO: Nothing to commit"
+    exit 0
+fi
+
+echo -e "INFO: Staged for commit:\n$staged_commits\n"
 
-CHANGE_ID=$(ssh -p 29418 "$GERRIT_USER@$GERRIT_HOST" gerrit query \
-               limit:1 owner:self is:open project:"$PROJECT" \
-               message: "$GERRIT_COMMIT_MESSAGE" \
-               topic: "$GERRIT_TOPIC" | \
-               grep 'Change-Id:' | \
-               awk '{ print $2 }')
+# shellcheck disable=SC1090
+source ~/lf-env.sh
 
-if [ -z "$CHANGE_ID" ]; then
-   git commit -sm "$GERRIT_COMMIT_MESSAGE" -m "Job: ${job}"
+lf-activate-venv "git-review>=1.28"
+
+# Query for a pre-existing gerrit review
+query_result=$(ssh -p 29418 "$GERRIT_USER@$GERRIT_HOST" gerrit query \
+               limit:1 owner:self is:open project:"$PROJECT"         \
+               message: "$GERRIT_COMMIT_MESSAGE"                     \
+               topic: "$GERRIT_TOPIC")
+
+# Extract the change_id from the query_result
+job=$JOB_NAME/$BUILD_NUMBER
+# If available, add change_id to commit message
+if change_id=$(echo "$query_result" | grep 'Change-Id:' | awk '{print $2}'); then
+    echo "NOTE: Found gerrit review: $change_id"
+    message="Job: $job\nChange-Id: $change_id"
 else
-   git commit -sm "$GERRIT_COMMIT_MESSAGE" -m "Job: ${job}\nChange-Id: $CHANGE_ID"
+    echo "NOTE: No gerrit review found"
+    message="Job: $job"
 fi
+git commit -sm "$GERRIT_COMMIT_MESSAGE" -m "$message"
 
 git status
 git remote add gerrit "ssh://$GERRIT_USER@$GERRIT_HOST:29418/$PROJECT.git"
 
-# if the reviewers email is empty then use a default
-REVIEWERS_EMAIL=${REVIEWERS_EMAIL:-"$GERRIT_USER@$GERRIT_HOST"}
+# If the reviewers email is unset/empty then use a default
+reviewers_email=${REVIEWERS_EMAIL:-"$GERRIT_USER@$GERRIT_HOST"}
 
-# Don't fail the build if this command fails because it's possible that there
-# is no changes since last update.
-git review --yes -t "$GERRIT_TOPIC" --reviewers "$REVIEWERS_EMAIL" || true
+git review --yes -t "$GERRIT_TOPIC" --reviewers "$reviewers_email"
-- 
2.16.6