Merge remote-tracking branch 'upstream/master'

CHANGELOG.md

@@ -2,6 +2,34 @@
 
 ## master
 
+## v132 (2018-11-12)
+
+- Quietly add new build script behavior behind a flag (#584, #585)
+- Move cache directory layout (#587)
+
+## v131 (2018-10-31)
+
+- Improve bin/detect error messages (#575)
+- Add support for Node 11 (#578, #582, #580)
+- Update default Node version to 10.x
+
+## v130 (2018-10-03)
+
+- Unpin Yarn from 1.9.x (#569)
+
+## v129 (2018-10-02)
+
+- Fix a bug with formatting (#572)
+- internal changes (#567)
+
+## v128 (2018-09-13)
+
+- Default to Yarn 1.9.x to avoid a bug (#568)
+
+## v127 (2018-09-13)
+
+- interal changes (#566)
+
 ## v126 (2018-09-06)
 
 - Icrease Node memory default during builds (#561)

bin/compile

@@ -122,8 +122,8 @@ install_bins() {
     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
+    monitor "install-node-binary" install_nodejs "$node_engine" "$BUILD_DIR/.heroku/node"
+    monitor "install-npm-binary" install_npm "$npm_engine" "$BUILD_DIR/.heroku/node" $NPM_LOCK
     local node_version="$(node --version)"
     mcount "version.node.$node_version"
   fi
@@ -132,7 +132,7 @@ install_bins() {
   # has specified a version of yarn under "engines". We'll still
   # only install using yarn if there is a yarn.lock file
   if $YARN || [ -n "$yarn_engine" ]; then
-    install_yarn "$BUILD_DIR/.heroku/yarn" "$yarn_engine"
+    monitor "install-yarn-binary" install_yarn "$BUILD_DIR/.heroku/yarn" "$yarn_engine"
   fi
 
   if $YARN; then

lib/build-data.sh

+# variable shared by this whole module
+BUILD_DATA_FILE=""
+
+bd_create() {
+  local cache_dir="$1"
+  BUILD_DATA_FILE="$cache_dir/build-data/node"
+  kv_create $BUILD_DATA_FILE
+}
+
+bd_get() {
+  kv_get $BUILD_DATA_FILE "$1"
+}
+
+bd_set() {
+  kv_set $BUILD_DATA_FILE "$1" "$2"
+}
+
+log_build_data() {
+  # print all values on one line in logfmt format
+  # https://brandur.org/logfmt
+  echo $(kv_list $BUILD_DATA_FILE)
+}

lib/cache.sh

 source $BP_DIR/lib/binaries.sh
 
 create_signature() {
-  echo "${STACK}; $(node --version); $(npm --version); $(yarn --version 2>/dev/null || true); ${PREBUILD}"
+  echo "v2; ${STACK}; $(node --version); $(npm --version); $(yarn --version 2>/dev/null || true); ${PREBUILD}"
 }
 
 save_signature() {
@@ -46,16 +46,16 @@ restore_default_cache_directories() {
   # node_modules
   if [[ -e "$build_dir/node_modules" ]]; then
     echo "- node_modules is checked into source control and cannot be cached"
-  elif [[ -e "$cache_dir/node/node_modules" ]]; then
+  elif [[ -e "$cache_dir/node/cache/node_modules" ]]; then
     echo "- node_modules"
     mkdir -p "$(dirname "$build_dir/node_modules")"
-    mv "$cache_dir/node/node_modules" "$build_dir/node_modules"
+    mv "$cache_dir/node/cache/node_modules" "$build_dir/node_modules"
   else
     echo "- node_modules (not cached - skipping)"
   fi
 
   # bower_components, should be silent if it is not in the cache
-  if [[ -e "$cache_dir/node/bower_components" ]]; then
+  if [[ -e "$cache_dir/node/cache/bower_components" ]]; then
     echo "- bower_components"
   fi
 }
@@ -71,10 +71,10 @@ restore_custom_cache_directories() {
     if [ -e "$build_dir/$cachepath" ]; then
       echo "- $cachepath (exists - skipping)"
     else
-      if [ -e "$cache_dir/node/$cachepath" ]; then
+      if [ -e "$cache_dir/node/cache/$cachepath" ]; then
         echo "- $cachepath"
         mkdir -p "$(dirname "$build_dir/$cachepath")"
-        mv "$cache_dir/node/$cachepath" "$build_dir/$cachepath"
+        mv "$cache_dir/node/cache/$cachepath" "$build_dir/$cachepath"
       else
         echo "- $cachepath (not cached - skipping)"
       fi
@@ -85,6 +85,7 @@ restore_custom_cache_directories() {
 clear_cache() {
   rm -rf $CACHE_DIR/node
   mkdir -p $CACHE_DIR/node
+  mkdir -p $CACHE_DIR/node/cache
 }
 
 save_default_cache_directories() {
@@ -94,8 +95,8 @@ save_default_cache_directories() {
   # node_modules
   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")"
+    mkdir -p "$cache_dir/node/cache/node_modules"
+    cp -a "$build_dir/node_modules" "$(dirname "$cache_dir/node/cache/node_modules")"
   else
     # this can happen if there are no dependencies
     mcount "cache.no-node-modules"
@@ -106,8 +107,8 @@ save_default_cache_directories() {
   if [[ -e "$build_dir/bower_components" ]]; then
     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")"
+    mkdir -p "$cache_dir/node/cache/bower_components"
+    cp -a "$build_dir/bower_components" "$(dirname "$cache_dir/node/cache/bower_components")"
   fi
 }
 
@@ -121,8 +122,8 @@ save_custom_cache_directories() {
   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")"
+      mkdir -p "$cache_dir/node/cache/$cachepath"
+      cp -a "$build_dir/$cachepath" "$(dirname "$cache_dir/node/cache/$cachepath")"
     else
       echo "- $cachepath (nothing to cache)"
     fi

lib/failure.sh

@@ -365,8 +365,21 @@ log_other_failures() {
     return 0
   fi
 
-  if grep -i -e "npm ERR! code E404" -e "error An unexpected error occurred: .* Request failed \"404 Not Found\"" "$log_file"; then
+  if grep -qi -e "npm ERR! code E404" -e "error An unexpected error occurred: .* Request failed \"404 Not Found\"" "$log_file"; then
     mcount "failures.module-404"
+
+    if grep -qi "flatmap-stream" "$log_file"; then
+      mcount "flatmap-stream-404"
+      warn "The flatmap-stream module has been removed from the npm registry
+
+       On November 26th, npm was notified of a malicious package that had made its
+       way into event-stream, a popular npm package. After triaging the malware,
+       npm responded by removing flatmap-stream and event-stream@3.3.6 from the Registry
+       and taking ownership of the event-stream package to prevent further abuse.
+      " https://kb.heroku.com/4OM7X18J/why-am-i-seeing-npm-404-errors-for-event-stream-flatmap-stream-in-my-build-logs
+      exit 1
+    fi
+
     return 0
   fi
 

lib/kvstore.sh

@@ -19,6 +19,7 @@ kv_set() {
   fi
 }
 
+# get the value, but don't unwrap quotes
 kv_get() {
   if [[ $# -eq 2 ]]; then
     local f=$1
@@ -28,6 +29,15 @@ kv_get() {
   fi
 }
 
+kv_get_escaped() {
+  local value=$(kv_get $1 $2 $3)
+  if [[ $value =~ [[:space:]]+ ]]; then
+    echo "\"$value\""
+  else
+    echo $value
+  fi
+}
+
 kv_keys() {
   local f=$1
   local keys=()
@@ -47,7 +57,7 @@ kv_list() {
 
   kv_keys $f | tr ' ' '\n' | while read -r key; do
     if [[ -n $key ]]; then
-      echo "$key=$(kv_get $f $key)"
+      echo "$key=$(kv_get_escaped $f $key)"
     fi
   done
 }

lib/monitor.sh

@@ -45,12 +45,13 @@ monitor_memory_usage() {
 monitor() {
   local command_name=$1
   shift
-  local command="${@:-}"
+
+  local command=( "$@" )
   local peak_mem_output=$(mktemp)
   local start=$(nowms)
 
   # execute the subcommand and save the peak memory usage
-  monitor_memory_usage $peak_mem_output $command
+  monitor_memory_usage $peak_mem_output "${command[@]}"
 
   mtime "exec.$command_name.time" "${start}"
   mmeasure "exec.$command_name.memory" "$(cat $peak_mem_output)"

profile/WEB_CONCURRENCY.sh

 #!/usr/bin/env bash
 
 calculate_concurrency() {
-  WEB_CONCURRENCY=${WEB_CONCURRENCY-$((MEMORY_AVAILABLE/WEB_MEMORY))}
-  if (( WEB_CONCURRENCY < 1 )); then
-    WEB_CONCURRENCY=1
-  elif (( WEB_CONCURRENCY > 200 )); then
+  local available=$1
+  local web_memory=$2
+  local concurrency
+
+  concurrency=${WEB_CONCURRENCY-$(($available/$web_memory))}
+  if (( concurrency < 1 )); then
+    concurrency=1
+  elif (( concurrency > 200 )); then
     # Ex: This will happen on Dokku on DO
-    WEB_CONCURRENCY=1
+    concurrency=1
   fi
-  echo $WEB_CONCURRENCY
+  echo "$concurrency"
 }
 
 log_concurrency() {
@@ -26,6 +30,18 @@ detect_memory() {
   fi
 }
 
+bound_memory() {
+  local detected=$1
+  local detected max_detected_memory=14336
+
+  # The hardcoded value is 16GB of memory
+  if (( detected > max_detected_memory )); then
+    echo "$max_detected_memory"
+  else
+    echo "$detected"
+  fi
+}
+
 warn_bad_web_concurrency() {
   local concurrency=$((MEMORY_AVAILABLE/WEB_MEMORY))
   if [ "$concurrency" -gt "200" ]; then
@@ -39,9 +55,10 @@ appropriate for your application."
   fi
 }
 
-export MEMORY_AVAILABLE=${MEMORY_AVAILABLE-$(detect_memory 512)}
+DETECTED=$(detect_memory 512)
+export MEMORY_AVAILABLE=${MEMORY_AVAILABLE-$(bound_memory $DETECTED)}
 export WEB_MEMORY=${WEB_MEMORY-512}
-export WEB_CONCURRENCY=$(calculate_concurrency)
+export WEB_CONCURRENCY=$(calculate_concurrency $MEMORY_AVAILABLE $WEB_MEMORY)
 
 warn_bad_web_concurrency
 

test/fixtures/flatmap-stream/package-lock.json

+{
+  "name": "flatmap-stream",
+  "version": "1.0.0",
+  "lockfileVersion": 1,
+  "dependencies": {
+    "flatmap-stream": {
+      "version": "0.1.1",
+      "resolved": "https://registry.npmjs.org/flatmap-stream/-/flatmap-stream-0.1.1.tgz",
+      "integrity": "sha1-mDxroZk7WOroWZmKpof/6I34TBc="
+    }
+  }
+}
\ No newline at end of file

test/fixtures/flatmap-stream/package.json

+{
+  "name": "flatmap-stream",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "keywords": [],
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "flatmap-stream": "0.1.1"
+  }
+}
\ No newline at end of file

test/mocks/stdlib.sh

+# mock these functions from the stdlib
+
+nowms() {
+  :
+}
+
+mtime() {
+  :
+}
+
+mmeasure() {
+  :
+}

test/run

@@ -8,6 +8,13 @@
 #  assertCapturedError
 #}
 
+testFlatmapStream() {
+  compile "flatmap-stream"
+  assertCaptured "flatmap-stream module has been removed from the npm registry"
+  assertCaptured "why-am-i-seeing-npm-404-errors"
+  assertCapturedError
+}
+
 testBuildScriptBehavior() {
   # opt in to new build script behavior
   cache=$(mktmpdir)
@@ -69,7 +76,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 -c lodash | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/cache/node_modules | grep -c lodash | tr -d ' ')"
   assertCapturedSuccess
 
   compile "node-modules-cache-2" $cache $env_dir
@@ -87,7 +94,7 @@ testNodeModulesCached() {
 
   compile "caching" $cache
   assertCaptured "- node_modules"
-  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep -c express | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/cache/node_modules | grep -c express | tr -d ' ')"
   assertCapturedSuccess
 }
 
@@ -115,12 +122,17 @@ testYarnCacheDirectory() {
   local cache=$(mktmpdir)
   local env_dir=$(mktmpdir)
   local cache_dir=$(mktmpdir)
-  echo "${cache_dir}/yarn"> "$env_dir"/YARN_CACHE_FOLDER
+  echo "${cache_dir}/yarn"> "$env_dir/YARN_CACHE_FOLDER"
   compile "yarn" $cache $env_dir
   # These will be created if yarn is using the directory for its cache
+
   assertDirectoryExists ${cache_dir}/yarn
-  assertDirectoryExists ${cache_dir}/yarn/v3
-  assertFileExists ${cache_dir}/yarn/v3/npm-lodash-4.16.4-01ce306b9bad1319f2a5528674f88297aeb70127
+  # yarn frequently bumps the version number used in its cache
+  # so use a wildcard here to prevent frequent CI failures
+  ls ${cache_dir}/yarn/v*/npm-lodash-4.16.4-01ce306b9bad1319f2a5528674f88297aeb70127
+
+  # assert that the above ls command exited successfully, which only happens if the file exists
+  assertEquals "0" "$?"
   assertCapturedSuccess
 }
 
@@ -141,7 +153,7 @@ testBuildWithCache() {
 
   compile "stable-node" $cache
   assertNotCaptured "Restoring cache"
-  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep -c hashish | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/cache/node_modules | grep -c hashish | tr -d ' ')"
   assertCapturedSuccess
 
   compile "stable-node" $cache
@@ -149,7 +161,7 @@ testBuildWithCache() {
   assertFileContains "${STACK}" "${cache}/node/signature"
   assertCapturedSuccess
 
-  rm -rf "$cache/node/node_modules"
+  rm -rf "$cache/node/cache/node_modules"
   compile "stable-node" $cache
   assertCaptured "- node_modules (not cached - skipping)"
   assertCapturedSuccess
@@ -394,8 +406,8 @@ testBuildWithUserCacheDirectoriesCamel() {
 
   compile "cache-directories-camel" $cache
   assertCaptured "- non/existent (nothing to cache)"
-  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 ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/cache/server | grep -c node_modules | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/cache/client | grep -c node_modules | tr -d ' ')"
   assertCapturedSuccess
 
   compile "cache-directories-camel" $cache
@@ -639,8 +651,8 @@ testBuildWithUserCacheDirectories() {
 
   compile "cache-directories" $cache
   assertCaptured "Saving 2 cacheDirectories"
-  assertEquals "1" "$(ls -1 $cache/node | grep -c bower_components | tr -d ' ')"
-  assertEquals "1" "$(ls -1 $cache/node | grep -c node_modules | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/cache | grep -c bower_components | tr -d ' ')"
+  assertEquals "1" "$(ls -1 $cache/node/cache | grep -c node_modules | tr -d ' ')"
   assertCapturedSuccess
 
   compile "cache-directories" $cache
@@ -996,6 +1008,8 @@ testMemoryMetrics() {
   echo "$metrics_log" > $env_dir/BUILDPACK_LOG_FILE
 
   compile "pre-post-build-scripts" "$(mktmpdir)" $env_dir
+  assertFileContains "measure#buildpack.nodejs.exec.install-node-binary.time=" $metrics_log
+  assertFileContains "measure#buildpack.nodejs.exec.install-npm-binary.time=" $metrics_log
   assertFileContains "measure#buildpack.nodejs.exec.heroku-prebuild.time=" $metrics_log
   assertFileContains "measure#buildpack.nodejs.exec.heroku-prebuild.memory=" $metrics_log
   assertFileContains "measure#buildpack.nodejs.exec.npm-install.time=" $metrics_log
@@ -1006,6 +1020,8 @@ testMemoryMetrics() {
   # erase the metrics log
   echo "" > $metrics_log
   compile "yarn" "$(mktmpdir)" $env_dir
+  assertFileContains "measure#buildpack.nodejs.exec.install-node-binary.time=" $metrics_log
+  assertFileContains "measure#buildpack.nodejs.exec.install-yarn-binary.time=" $metrics_log
   assertFileContains "measure#buildpack.nodejs.exec.yarn-install.memory=" "$metrics_log"
   assertFileContains "measure#buildpack.nodejs.exec.yarn-install.time=" "$metrics_log"
   # this fixture does not have pre or post-build scripts

test/unit

@@ -19,6 +19,10 @@ print_args() {
   done
 }
 
+print_number_args() {
+  echo "$#"
+}
+
 testMonitorMemory() {
   local mem_output=$(mktemp)
   local stdout_capture=$(mktemp)
@@ -36,6 +40,29 @@ testMonitorMemory() {
   assertEquals "second line" "--bar=baz lol hi" "$(tail -n 1 $stdout_capture)"
 }
 
+testMonitor() {
+  local out
+
+  # test that we're forwarding empty arguments correctly
+  out=$(monitor "command-name" print_number_args "" "" "" "")
+  assertEquals "4" "$out"
+
+  # Don't expand *
+  out=$(monitor "command-name" echo "*")
+  assertEquals "*" "$out"
+
+  out=$(monitor "command-name" print_number_args "*")
+  assertEquals "1" "$out"
+
+  # # Don't split arguments with a space
+  out=$(monitor "command-name" echo "1  3")
+  assertEquals "1  3" "$out"
+
+  # # Test everything with an empty arg
+  out=$(monitor "command-name" echo 1 "" 2 "3   4" "*")
+  assertEquals "1  2 3   4 *" "$out"
+}
+
 testOutput() {
   local stdout
 
@@ -81,6 +108,16 @@ testKeyValue() {
   assertEquals "" "$(kv_list $store)"
 }
 
+testKeyValueEscaping() {
+  local store=$(mktemp)
+
+  kv_create $store
+
+  kv_set $store "key" "value with a space"
+  assertEquals "key=\"value with a space\"" "$(kv_list $store)"
+  assertEquals "value with a space" "$(kv_get $store "key")"
+}
+
 # if the file doesn't exist, everything should be a no-op
 testKeyValueNoFile() {
   # empty file argument
@@ -111,10 +148,77 @@ testKeyValueNoFile() {
   assertEquals "$(kv_list $space)" ""
 }
 
+testBuildData() {
+  local cache_dir=$(mktemp -d)
+
+  bd_create $cache_dir
+
+  bd_set "test" "foo"
+  assertEquals "test=foo" "$(log_build_data)"
+
+  bd_set "test" "different-foo"
+  assertEquals "test=different-foo" "$(log_build_data)"
+
+  bd_set "foo" "value with spaces"
+  assertEquals "foo=\"value with spaces\" test=different-foo" "$(log_build_data)"
+
+  # values are printed with the keys sorted alphabetically
+  # this isn't required, and this test serves as documentation
+  bd_set "a" "this should come first"
+  assertEquals "a=\"this should come first\" foo=\"value with spaces\" test=different-foo" "$(log_build_data)"
+}
+
+testWebConcurrencyProfileScript() {
+  # this was set when we sourced the WEB_CONCURRENCY.sh file
+  unset WEB_MEMORY
+  unset MEMORY_AVAILABLE
+  unset WEB_CONCURRENCY
+
+  # memory in MB of a 2X dyno
+  assertEquals "512" "$(bound_memory 512)"
+
+  # memory in MB of a 2X dyno
+  assertEquals "1024" "$(bound_memory 1024)"
+
+  # memory in MB of a Peformance-M dyno
+  assertEquals "2560" "$(bound_memory 2560)"
+
+  # memory in MB of a Peformance-L dyno
+  assertEquals "14336" "$(bound_memory 14336)"
+
+  # one more MB
+  assertEquals "14336" "$(bound_memory 14337)"
+
+  # On non-Heroku systems `detect_memory` can return non-sensically large values
+  # In this case, we should bound
+  assertEquals "14336" "$(bound_memory 1000000)"
+
+  # test calculate_concurrency
+
+  # 1x
+  assertEquals "1" "$(calculate_concurrency 512 512)"
+  # 2x
+  assertEquals "2" "$(calculate_concurrency 1024 512)"
+  # Performance-M
+  assertEquals "5" "$(calculate_concurrency 2560 512)"
+  # Performance-L
+  assertEquals "28" "$(calculate_concurrency 14336 512)"
+
+  # In case some very large memory available value gets passed in
+  assertEquals "1" "$(calculate_concurrency 103401 512)"
+  # of if web memory is set really low
+  assertEquals "1" "$(calculate_concurrency 512 1)"
+}
+
+# mocks
+source "$(pwd)"/test/mocks/stdlib.sh
+
 # the modules to be tested
 source "$(pwd)"/lib/monitor.sh
 source "$(pwd)"/lib/output.sh
 source "$(pwd)"/lib/kvstore.sh
+source "$(pwd)"/lib/build-data.sh
+source "$(pwd)"/profile/WEB_CONCURRENCY.sh
 
 # import the testing framework
 source "$(pwd)"/test/shunit2