From 7142b3317bf74f08c825da8c818bafb8ce06e02b Mon Sep 17 00:00:00 2001
From: James O'Kane <jo2y@google.com>
Date: Sun, 1 Apr 2018 13:01:14 -0700
Subject: [PATCH] Address post-merge comment on PR #71 - Quote bash variable
 when possible. - Fix an undefined $root_dir - Remove a redundant rm and let
 the defined trap handler do it. - echo -e is needed when trying to display
 escape codes. - Remove incorrect {{foo}} double curly braces.

---
 appengine/java/appengine_deploy.sh.template | 27 ++++++++---------
 appengine/java/appengine_runner.sh.template |  8 ++---
 appengine/py/appengine_deploy.sh.template   | 33 ++++++++++-----------
 appengine/py/appengine_runner.sh.template   |  2 +-
 4 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/appengine/java/appengine_deploy.sh.template b/appengine/java/appengine_deploy.sh.template
index a795e20..c873c46 100644
--- a/appengine/java/appengine_deploy.sh.template
+++ b/appengine/java/appengine_deploy.sh.template
@@ -24,24 +24,21 @@ if [[ -z "$JAVA_RUNFILES" ]]; then
   fi
 fi
 
-root_path=$(pwd)
-tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/war.XXXXXXXX)
-trap "{ cd ${root_path}; rm -rf ${tmp_dir}; }" EXIT
-cd ${tmp_dir}
+root_path="$(pwd)"
+tmp_dir=$(mktemp -d "${TMPDIR:-/tmp}/war.XXXXXXXX")
+trap "{ cd "${root_path}"; rm -rf "${tmp_dir}"; }" EXIT
+cd "${tmp_dir}"
 
-${JAVA_RUNFILES}/%{zipper} x ${JAVA_RUNFILES}/%{war}
-cd ${root_dir}
+"${JAVA_RUNFILES}/%{zipper}" x "${JAVA_RUNFILES}/%{war}"
+cd "${root_dir}"
 
-APP_ENGINE_ROOT=${JAVA_RUNFILES}/%{appengine_sdk}
+APP_ENGINE_ROOT="${JAVA_RUNFILES}/%{appengine_sdk}"
 if [ -n "${1-}" ]; then
-  ${APP_ENGINE_ROOT}/bin/appcfg.sh -A "$1" update ${tmp_dir}
-  retCode=$?
+  "${APP_ENGINE_ROOT}/bin/appcfg.sh" -A "$1" update "${tmp_dir}"
+  retCode="$?"
 else
-  ${APP_ENGINE_ROOT}/bin/appcfg.sh update ${tmp_dir}
-  retCode=$?
+  "${APP_ENGINE_ROOT}/bin/appcfg.sh" update "${tmp_dir}"
+  retCode="$?"
 fi
 
-rm -rf ${tmp_dir}
-trap - EXIT
-
-exit $retCode
+exit "$retCode"
diff --git a/appengine/java/appengine_runner.sh.template b/appengine/java/appengine_runner.sh.template
index 787f609..eefc5b5 100644
--- a/appengine/java/appengine_runner.sh.template
+++ b/appengine/java/appengine_runner.sh.template
@@ -25,8 +25,8 @@ if [[ -z "$JAVA_RUNFILES" ]]; then
 fi
 
 jvm_bin="${JAVA_RUNFILES}/%{java}"
-if [[ ! -x ${jvm_bin} ]]; then
-  jvm_bin=$(which java)
+if [[ ! -x "${jvm_bin}" ]]; then
+  jvm_bin="$(which java)"
 fi
 
 APP_ENGINE_ROOT="${JAVA_RUNFILES}/%{appengine_sdk}"
@@ -41,7 +41,7 @@ classpath="%{classpath}"
 cd "%{data_path}"
 
 mkdir -p WEB-INF/lib
-for i in $(echo $classpath | tr ":" "\n"); do
+for i in $(echo "$classpath" | tr ":" "\n"); do
   jar="WEB-INF/lib/$(basename "$i")"
   rm -f "$jar"
   ln -s "$i" "$jar"
@@ -54,7 +54,7 @@ JVM_FLAGS_CMDLINE=()
 function process_wrapper_argument() {
   case "$1" in
     --jvm_flag=*) JVM_FLAGS_CMDLINE+=( "${1#--jvm_flag=}" ) ;;
-    --jvm_flags=*) JVM_FLAGS_CMDLINE+=( ${1#--jvm_flags=} ) ;;
+    --jvm_flags=*) JVM_FLAGS_CMDLINE+=( "${1#--jvm_flags=}" ) ;;
     *)
       return 1 ;;
   esac
diff --git a/appengine/py/appengine_deploy.sh.template b/appengine/py/appengine_deploy.sh.template
index 5cd3ed7..bada232 100644
--- a/appengine/py/appengine_deploy.sh.template
+++ b/appengine/py/appengine_deploy.sh.template
@@ -19,37 +19,34 @@ case "$0" in
 esac
 if [[ -e "$self.runfiles/%{workspace_name}" ]]; then
   RUNFILES="$self.runfiles/%{workspace_name}"
-  cd $RUNFILES
+  cd "$RUNFILES"
 fi
 
-ROOT=$PWD
-tmp_dir=$(mktemp -d ${{TMPDIR:-/tmp}}/war.XXXXXXXX)
-cp -R $ROOT $tmp_dir
-trap "{{ cd ${{root_path}}; rm -rf $tmp_dir; }}" EXIT
-rm -Rf $tmp_dir/%{workspace_name}/external/com_google_appengine_py
-if [ -n "${{1-}}" ]; then
-  has_app_yaml=$(echo ${{@:2}} | grep -E "^([^ ]+ +)*app.yaml")
-  other_mod_cfgs=$(echo ${{@:2}} | xargs printf "%s\n" | grep -v "^app\.yaml$" | xargs echo)
+ROOT="$PWD"
+tmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/war.XXXXXXXX")"
+cp -R "$ROOT" "$tmp_dir"
+trap "{ cd "$ROOT"; rm -rf "$tmp_dir"; }}" EXIT
+rm -Rf "$tmp_dir/%{workspace_name}/external/com_google_appengine_py"
+if [ -n "${1-}" ]; then
+  has_app_yaml="$(echo "${@:2}" | grep -E "^([^ ]+ +)*app.yaml")"
+  other_mod_cfgs="$(echo "${@:2}" | xargs printf "%s\n" | grep -v "^app\.yaml$" | xargs echo)"
   ret_code=0
   # Secondary modules need to be uploaded first because if any of them are
   # referenced in dispatch.yaml, App Engine must have a version of that module
   # prior to the app.yaml upload.
   if [ -n "$other_mod_cfgs" ]; then
-    (cd $tmp_dir/%{workspace_name} && $ROOT/%{appcfg} -A "$1" update $other_mod_cfgs)
-    ret_code=$?
+    (cd "$tmp_dir/%{workspace_name}" && "$ROOT/%{appcfg}" -A "$1" update "$other_mod_cfgs")
+    ret_code="$?"
   fi
-  if [ $ret_code -eq 0 ] && [ -n "$has_app_yaml" ] || [ -z "$other_mod_cfgs" ]; then
-    $ROOT/%{appcfg} -A "$1" update $tmp_dir/%{workspace_name}
-    ret_code=$?
+  if [ "$ret_code" -eq 0 ] && [ -n "$has_app_yaml" ] || [ -z "$other_mod_cfgs" ]; then
+    "$ROOT/%{appcfg}" -A "$1" update "$tmp_dir/%{workspace_name}"
+    ret_code="$?"
   fi
 
 else
-  echo "\033[1;31mERROR:\033[0m Application ID must be provided as first argument
+  echo -e "\033[1;31mERROR:\033[0m Application ID must be provided as first argument
   USAGE: bazel run path/to/my/gae_binary_target.deploy -- my-project-id [module.yaml files ...]"
   ret_code=-1
 fi
 
-rm -Rf $tmp_dir
-trap - EXIT
-
 exit $ret_code
diff --git a/appengine/py/appengine_runner.sh.template b/appengine/py/appengine_runner.sh.template
index 14014f6..c55dd46 100644
--- a/appengine/py/appengine_runner.sh.template
+++ b/appengine/py/appengine_runner.sh.template
@@ -19,7 +19,7 @@ case "$0" in
 esac
 if [[ -e "$self.runfiles/%{workspace_name}" ]]; then
   RUNFILES="$self.runfiles/%{workspace_name}"
-  cd $RUNFILES
+  cd "$RUNFILES"
 fi
 
 %{devappserver} --skip_sdk_update_check 1 app.yaml