More shell cleaning (#554)

bin/compile

@@ -16,7 +16,7 @@ BPLOG_PREFIX="buildpack.nodejs"
 BUILD_DIR=${1:-}
 CACHE_DIR=${2:-}
 ENV_DIR=${3:-}
-BP_DIR=$(cd $(dirname ${0:-}); cd ..; pwd)
+BP_DIR=$(cd "$(dirname ${0:-})"; cd ..; pwd)
 STDLIB_FILE=$(mktemp -t stdlib.XXXXX)
 
 ### Load dependencies
@@ -111,13 +111,16 @@ install_bins() {
   if [ -n "$iojs_engine" ]; then
     warn_node_engine "$iojs_engine"
     install_iojs "$iojs_engine" "$BUILD_DIR/.heroku/node"
-    echo "Using bundled npm version for iojs compatibility: `npm --version`"
-    mcount "version.iojs.$(node --version)"
+    local npm_version="$(npm --version)"
+    local node_version="$(node --version)"
+    echo "Using bundled npm version for iojs compatibility: $npm_version"
+    mcount "version.iojs.$node_version"
   else
     warn_node_engine "$node_engine"
     install_nodejs "$node_engine" "$BUILD_DIR/.heroku/node"
     install_npm "$npm_engine" "$BUILD_DIR/.heroku/node" $NPM_LOCK
-    mcount "version.node.$(node --version)"
+    local node_version="$(node --version)"
+    mcount "version.node.$node_version"
   fi
 
   # Download yarn if there is a yarn.lock file or if the user
@@ -209,7 +212,7 @@ header "Building dependencies" | output "$LOG_FILE"
 build_dependencies | output "$LOG_FILE"
 
 cache_build() {
-  local cache_directories=$(get_cache_directories)
+  local cache_directories="$(get_cache_directories)"
 
   clear_cache
   if ! ${NODE_MODULES_CACHE:-true}; then

bin/test-compile

 #!/usr/bin/env bash
 
-BP_DIR=$(cd $(dirname ${0:-}); cd ..; pwd)
+BP_DIR=$(cd "$(dirname ${0:-})"; cd ..; pwd)
 
 source $BP_DIR/lib/environment.sh
 

lib/binaries.sh

@@ -79,14 +79,14 @@ install_npm() {
   fi
 
   if [ "$version" == "" ]; then
-    echo "Using default npm version: `npm --version`"
-  elif [[ `npm --version` == "$version" ]]; then
-    echo "npm `npm --version` already installed with node"
+    echo "Using default npm version: $npm_version"
+  elif [[ "$npm_version" == "$version" ]]; then
+    echo "npm $npm_version already installed with node"
   else
-    echo "Bootstrapping npm $version (replacing `npm --version`)..."
+    echo "Bootstrapping npm $version (replacing $npm_version)..."
     if ! npm install --unsafe-perm --quiet -g "npm@$version" 2>@1>/dev/null; then
       echo "Unable to install npm $version; does it exist?" && false
     fi
-    echo "npm `npm --version` installed"
+    echo "npm $version installed"
   fi
 }

lib/cache.sh

@@ -5,7 +5,7 @@ create_signature() {
 }
 
 save_signature() {
-  echo "$(create_signature)" > $CACHE_DIR/node/signature
+  create_signature > $CACHE_DIR/node/signature
 }
 
 load_signature() {
@@ -48,7 +48,7 @@ restore_default_cache_directories() {
     echo "- node_modules is checked into source control and cannot be cached"
   elif [[ -e "$cache_dir/node/node_modules" ]]; then
     echo "- node_modules"
-    mkdir -p $(dirname "$build_dir/node_modules")
+    mkdir -p "$(dirname "$build_dir/node_modules")"
     mv "$cache_dir/node/node_modules" "$build_dir/node_modules"
   else
     echo "- node_modules (not cached - skipping)"
@@ -63,17 +63,17 @@ restore_default_cache_directories() {
 restore_custom_cache_directories() {
   local build_dir=${1:-}
   local cache_dir=${2:-}
-  local cache_directories="${@:3}"
+  local cache_directories=("${@:3}")
 
-  echo "Loading $(echo $cache_directories | wc -w | xargs) from cacheDirectories (package.json):"
+  echo "Loading ${#cache_directories[@]} from cacheDirectories (package.json):"
 
-  for cachepath in ${@:3}; do
+  for cachepath in "${cache_directories[@]}"; do
     if [ -e "$build_dir/$cachepath" ]; then
       echo "- $cachepath (exists - skipping)"
     else
       if [ -e "$cache_dir/node/$cachepath" ]; then
         echo "- $cachepath"
-        mkdir -p $(dirname "$build_dir/$cachepath")
+        mkdir -p "$(dirname "$build_dir/$cachepath")"
         mv "$cache_dir/node/$cachepath" "$build_dir/$cachepath"
       else
         echo "- $cachepath (not cached - skipping)"
@@ -95,7 +95,7 @@ save_default_cache_directories() {
   if [[ -e "$build_dir/node_modules" ]]; then
     echo "- node_modules"
     mkdir -p "$cache_dir/node/node_modules"
-    cp -a "$build_dir/node_modules" $(dirname "$cache_dir/node/node_modules")
+    cp -a "$build_dir/node_modules" "$(dirname "$cache_dir/node/node_modules")"
   else
     # this can happen if there are no dependencies
     mcount "cache.no-node-modules"
@@ -107,22 +107,22 @@ save_default_cache_directories() {
     mcount "cache.saved-bower-components"
     echo "- bower_components"
     mkdir -p "$cache_dir/node/bower_components"
-    cp -a "$build_dir/bower_components" $(dirname "$cache_dir/node/bower_components")
+    cp -a "$build_dir/bower_components" "$(dirname "$cache_dir/node/bower_components")"
   fi
 }
 
 save_custom_cache_directories() {
   local build_dir=${1:-}
   local cache_dir=${2:-}
-  local cache_directories="${@:3}"
+  local cache_directories=("${@:3}")
 
-  echo "Saving $(echo $cache_directories | wc -w | xargs) cacheDirectories (package.json):"
+  echo "Saving ${#cache_directories[@]} cacheDirectories (package.json):"
 
-  for cachepath in ${@:3}; do
+  for cachepath in "${cache_directories[@]}"; do
     if [ -e "$build_dir/$cachepath" ]; then
       echo "- $cachepath"
       mkdir -p "$cache_dir/node/$cachepath"
-      cp -a "$build_dir/$cachepath" $(dirname "$cache_dir/node/$cachepath")
+      cp -a "$build_dir/$cachepath" "$(dirname "$cache_dir/node/$cachepath")"
     else
       echo "- $cachepath (nothing to cache)"
     fi

lib/dependencies.sh

 measure_size() {
-  echo "$((du -s node_modules 2>/dev/null || echo 0) | awk '{print $1}')"
+  (du -s node_modules 2>/dev/null || echo 0) | awk '{print $1}'
 }
 
 list_dependencies() {

lib/environment.sh

@@ -54,13 +54,14 @@ export_env_dir() {
   if [ -d "$env_dir" ]; then
     local whitelist_regex=${2:-''}
     local blacklist_regex=${3:-'^(PATH|GIT_DIR|CPATH|CPPATH|LD_PRELOAD|LIBRARY_PATH|LANG|BUILD_DIR)$'}
-    if [ -d "$env_dir" ]; then
-      for e in $(ls $env_dir); do
-        echo "$e" | grep -E "$whitelist_regex" | grep -qvE "$blacklist_regex" &&
-        export "$e=$(cat $env_dir/$e)"
-        :
-      done
-    fi
+    pushd "$env_dir" >/dev/null
+    for e in *; do
+      [ -e "$e" ] || continue
+      echo "$e" | grep -E "$whitelist_regex" | grep -qvE "$blacklist_regex" &&
+      export "$e=$(cat $e)"
+      :
+    done
+    popd >/dev/null
   fi
 }
 

lib/failure.sh

@@ -450,9 +450,11 @@ log_other_failures() {
 warning() {
   local tip=${1:-}
   local url=${2:-https://devcenter.heroku.com/articles/nodejs-support}
-  echo "- $tip" >> $warnings
-  echo "  $url" >> $warnings
-  echo "" >> $warnings
+  {
+  echo "- $tip"
+  echo "  $url"
+  echo ""
+  } >> $warnings
 }
 
 warn() {

profile/WEB_CONCURRENCY.sh

@@ -20,7 +20,7 @@ detect_memory() {
   local default=$1
 
   if [ -e /sys/fs/cgroup/memory/memory.limit_in_bytes ]; then
-    expr "$(cat /sys/fs/cgroup/memory/memory.limit_in_bytes)" / 1048576
+    echo $(($(cat /sys/fs/cgroup/memory/memory.limit_in_bytes) / 1048576))
   else
     echo "$default"
   fi

test/quick

 #!/usr/bin/env bash
 
-source $(pwd)/lib/environment.sh
+source "$(pwd)"/lib/environment.sh
 
 mktmpdir() {
   local dir=$(mktemp -t testXXXXX)
@@ -18,7 +18,7 @@ compile() {
   echo "Compiling $fixture"
   echo "in $build_dir"
   echo "(caching in $cache_dir)"
-  cp -a $(pwd)/* ${bp_dir}
+  cp -a "$(pwd)"/* ${bp_dir}
   cp -a ${bp_dir}/test/fixtures/$fixture/. ${build_dir}
   "$bp_dir/bin/compile" "$build_dir" "$cache_dir"
 }
@@ -32,7 +32,7 @@ compileTest() {
   echo "Compiling $fixture"
   echo "in $build_dir"
   echo "(caching in $cache_dir)"
-  cp -a $(pwd)/* ${bp_dir}
+  cp -a "$(pwd)"/* ${bp_dir}
   cp -a ${bp_dir}/test/fixtures/$fixture/. ${build_dir}
   "$bp_dir/bin/test-compile" "$build_dir" "$cache_dir"
 

test/run

@@ -23,7 +23,7 @@ testDisableCache() {
   echo "true" > $env_dir/NODE_VERBOSE
   compile "node-modules-cache-1" $cache $env_dir
   assertCaptured "lodash@1.0.0"
-  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep lodash | wc -l | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep -c lodash | tr -d ' ')"
   assertCapturedSuccess
 
   compile "node-modules-cache-2" $cache $env_dir
@@ -41,7 +41,7 @@ testNodeModulesCached() {
 
   compile "caching" $cache
   assertCaptured "- node_modules"
-  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep express | wc -l | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep -c express | tr -d ' ')"
   assertCapturedSuccess
 }
 
@@ -95,7 +95,7 @@ testBuildWithCache() {
 
   compile "stable-node" $cache
   assertNotCaptured "Restoring cache"
-  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep hashish | wc -l | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep -c hashish | tr -d ' ')"
   assertCapturedSuccess
 
   compile "stable-node" $cache
@@ -369,8 +369,8 @@ testBuildWithUserCacheDirectoriesCamel() {
 
   compile "cache-directories-camel" $cache
   assertCaptured "- non/existent (nothing to cache)"
-  assertEquals "1" "$(ls -1 $cache/node/server | grep node_modules | wc -l | tr -d ' ')"
-  assertEquals "1" "$(ls -1 $cache/node/client | grep node_modules | wc -l | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/server | grep -c node_modules | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/client | grep -c node_modules | tr -d ' ')"
   assertCapturedSuccess
 
   compile "cache-directories-camel" $cache
@@ -382,35 +382,35 @@ testBuildWithUserCacheDirectoriesCamel() {
 }
 
 testConcurrency1X() {
-  LOG_CONCURRENCY=true MEMORY_AVAILABLE=512 capture $(pwd)/profile/WEB_CONCURRENCY.sh
+  LOG_CONCURRENCY=true MEMORY_AVAILABLE=512 capture "$(pwd)"/profile/WEB_CONCURRENCY.sh
   assertCaptured "Detected 512 MB available memory, 512 MB limit per process (WEB_MEMORY)"
   assertCaptured "Recommending WEB_CONCURRENCY=1"
   assertCapturedSuccess
 }
 
 testConcurrency2X() {
-  LOG_CONCURRENCY=true MEMORY_AVAILABLE=1024 capture $(pwd)/profile/WEB_CONCURRENCY.sh
+  LOG_CONCURRENCY=true MEMORY_AVAILABLE=1024 capture "$(pwd)"/profile/WEB_CONCURRENCY.sh
   assertCaptured "Detected 1024 MB available memory, 512 MB limit per process (WEB_MEMORY)"
   assertCaptured "Recommending WEB_CONCURRENCY=2"
   assertCapturedSuccess
 }
 
 testConcurrencyPerformanceM() {
-  LOG_CONCURRENCY=true MEMORY_AVAILABLE=2560 capture $(pwd)/profile/WEB_CONCURRENCY.sh
+  LOG_CONCURRENCY=true MEMORY_AVAILABLE=2560 capture "$(pwd)"/profile/WEB_CONCURRENCY.sh
   assertCaptured "Detected 2560 MB available memory, 512 MB limit per process (WEB_MEMORY)"
   assertCaptured "Recommending WEB_CONCURRENCY=5"
   assertCapturedSuccess
 }
 
 testConcurrencyPerformanceL() {
-   LOG_CONCURRENCY=true MEMORY_AVAILABLE=14336 capture $(pwd)/profile/WEB_CONCURRENCY.sh
+   LOG_CONCURRENCY=true MEMORY_AVAILABLE=14336 capture "$(pwd)"/profile/WEB_CONCURRENCY.sh
    assertCaptured "Detected 14336 MB available memory, 512 MB limit per process (WEB_MEMORY)"
    assertCaptured "Recommending WEB_CONCURRENCY=28"
    assertCapturedSuccess
 }
 
 testConcurrencyCustomLimit() {
-  LOG_CONCURRENCY=true MEMORY_AVAILABLE=1024 WEB_MEMORY=256 capture $(pwd)/profile/WEB_CONCURRENCY.sh
+  LOG_CONCURRENCY=true MEMORY_AVAILABLE=1024 WEB_MEMORY=256 capture "$(pwd)"/profile/WEB_CONCURRENCY.sh
   assertCaptured "Detected 1024 MB available memory, 256 MB limit per process (WEB_MEMORY)"
   assertCaptured "Recommending WEB_CONCURRENCY=4"
   assertCapturedSuccess
@@ -419,7 +419,7 @@ testConcurrencyCustomLimit() {
 # When /sys/fs/cgroup/memory/memory.limit_in_bytes lies and gives a ridiculous value
 # This happens on Dokku for example
 testConcurrencyTooHigh() {
-  LOG_CONCURRENCY=true MEMORY_AVAILABLE=10000000000 capture $(pwd)/profile/WEB_CONCURRENCY.sh
+  LOG_CONCURRENCY=true MEMORY_AVAILABLE=10000000000 capture "$(pwd)"/profile/WEB_CONCURRENCY.sh
   assertCaptured "Could not determine a reasonable value for WEB_CONCURRENCY"
   assertCaptured "Recommending WEB_CONCURRENCY=1"
   assertCapturedSuccess
@@ -614,8 +614,8 @@ testBuildWithUserCacheDirectories() {
 
   compile "cache-directories" $cache
   assertCaptured "Saving 2 cacheDirectories"
-  assertEquals "1" "$(ls -1 $cache/node | grep bower_components | wc -l | tr -d ' ')"
-  assertEquals "1" "$(ls -1 $cache/node | grep node_modules | wc -l | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node | grep -c bower_components | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node | grep -c node_modules | tr -d ' ')"
   assertCapturedSuccess
 
   compile "cache-directories" $cache
@@ -967,11 +967,11 @@ testPluginInstallationUnsupportedNodeRunTime() {
 
 # Utils
 
-pushd $(dirname 0) >/dev/null
+pushd "$(dirname 0)" >/dev/null
 popd >/dev/null
 
-source $(pwd)/test/utils
-source $(pwd)/lib/environment.sh
+source "$(pwd)"/test/utils
+source "$(pwd)"/lib/environment.sh
 
 mktmpdir() {
   dir=$(mktemp -t testXXXXX)
@@ -981,7 +981,7 @@ mktmpdir() {
 }
 
 detect() {
-  capture $(pwd)/bin/detect $(pwd)/test/fixtures/$1
+  capture "$(pwd)"/bin/detect "$(pwd)"/test/fixtures/$1
 }
 
 compile_dir=""
@@ -997,7 +997,7 @@ compile() {
   default_process_types_cleanup
   bp_dir=$(mktmpdir)
   compile_dir=$(mktmpdir)
-  cp -a $(pwd)/* ${bp_dir}
+  cp -a "$(pwd)"/* ${bp_dir}
   cp -a ${bp_dir}/test/fixtures/$1/. ${compile_dir}
   capture ${bp_dir}/bin/compile ${compile_dir} ${2:-$(mktmpdir)} $3
 }
@@ -1036,7 +1036,7 @@ compileTest() {
   local cache_dir=${2:-$(mktmpdir)}
   local env_dir=$3
 
-  cp -a $(pwd)/* ${bp_dir}
+  cp -a "$(pwd)"/* ${bp_dir}
   cp -a ${bp_dir}/test/fixtures/$1/. ${compile_dir}
   capture ${bp_dir}/bin/test-compile ${compile_dir} ${2:-$(mktmpdir)} $3
 
@@ -1065,13 +1065,13 @@ compileDir() {
   local cache_dir=${2:-$(mktmpdir)}
   local env_dir=$3
 
-  cp -a $(pwd)/* ${bp_dir}
+  cp -a "$(pwd)"/* ${bp_dir}
   capture ${bp_dir}/bin/compile ${compile_dir} ${cache_dir} ${env_dir}
 }
 
 release() {
   bp_dir=$(mktmpdir)
-  cp -a $(pwd)/* ${bp_dir}
+  cp -a "$(pwd)"/* ${bp_dir}
   capture ${bp_dir}/bin/release ${bp_dir}/test/fixtures/$1
 }
 
@@ -1079,4 +1079,4 @@ assertFile() {
   assertEquals "$1" "$(cat ${compile_dir}/$2)"
 }
 
-source $(pwd)/test/shunit2
+source "$(pwd)"/test/shunit2

test/utils

@@ -186,7 +186,7 @@ assertFileMD5()
     fail "no suitable MD5 hashing command found on this system"
   fi
 
-  assertEquals "${expected_md5_cmd_output}" "`${md5_cmd}`"
+  assertEquals "${expected_md5_cmd_output}" "$(${md5_cmd})"
 }
 
 assertDirectoryExists() {