split up warnings

bin/common.sh

@@ -13,6 +13,27 @@ info() {
   echo "       $*"
 }
 
+warning() {
+  tip=$1
+  url=$2
+  echo "WARNING: $tip" >> $warnings
+  echo "${url:-https://devcenter.heroku.com/articles/nodejs-support}" >> $warnings
+  echo "" >> $warnings
+}
+
+build_failed() {
+  head "Build failed"
+  echo ""
+  cat $warnings | indent
+  info "We're sorry this build is failing! If you can't find the issue in application code,"
+  info "please submit a ticket so we can help: https://help.heroku.com/"
+  info "You can also try reverting to our legacy Node.js buildpack:"
+  info "heroku config:set BUILDPACK_URL=https://github.com/heroku/heroku-buildpack-nodejs#v63"
+  info ""
+  info "Love,"
+  info "Heroku"
+}
+
 file_contents() {
   if test -f $1; then
     echo "$(cat $1)"

bin/compile

@@ -17,23 +17,12 @@ warnings=$(mktemp)
 
 # Load some convenience functions like status(), echo(), and indent()
 source $bp_dir/bin/common.sh
+source $bp_dir/bin/warnings.sh
 
 # Avoid GIT_DIR leak from previous build steps
 unset GIT_DIR
 
 # Provide hook to deal with errors
-build_failed() {
-  head "Build failed"
-  echo ""
-  cat $warnings | indent
-  info "We're sorry this build is failing! If you can't find the issue in application code,"
-  info "please submit a ticket so we can help: https://help.heroku.com/"
-  info "You can also try reverting to our legacy Node.js buildpack:"
-  info "heroku config:set BUILDPACK_URL=https://github.com/heroku/heroku-buildpack-nodejs#v63"
-  info ""
-  info "Love,"
-  info "Heroku"
-}
 trap build_failed ERR
 
 # Load config vars into environment; start with defaults
@@ -82,7 +71,8 @@ echo ""
 printenv | grep ^NPM_CONFIG_ | indent
 info "NODE_MODULES_CACHE=$NODE_MODULES_CACHE"
 
-source $bp_dir/bin/warnings.sh
+warn_node_engine "$node_engine"
+warn_node_modules "$modules_source"
 
 ####### Vendor in binaries
 
@@ -118,6 +108,8 @@ if [ "$npm_engine" != "" ]; then
   fi
 fi
 
+warn_old_npm `npm --version`
+
 # Run subsequent commands from the build directory
 cd $build_dir
 
@@ -180,6 +172,8 @@ elif [ "$start_method" == "server.js" ]; then
   echo "web: node server.js" > $build_dir/Procfile
 fi
 
+warn_start "$start_method"
+
 ####### Create the runtime environment (profile.d)
 
 head "Finalizing build"

bin/test

@@ -37,7 +37,6 @@ testSpecificVersion() {
 testStableVersion() {
   compile "stable-node"
   assertCaptured "Downloading and installing node 0.10"
-  assertNotCaptured "WARNING"
   assertCapturedSuccess
 }
 
@@ -50,7 +49,8 @@ testUnstableVersion() {
 
 testOldNpm() {
   compile "old-npm"
-  assertCaptured "WARNING: This version of npm has several known issues"
+  assertCaptured "WARNING: This version of npm (1.2.8000) has several known issues - consider upgrading to the latest release (2."
+  assertNotCaptured "integer expression expected"
   assertCapturedSuccess
 }
 
@@ -295,6 +295,7 @@ testNpmVersionSpecific() {
   compile "npm-version-specific"
   assertCaptured "installing npm 2.1.11"
   assertNotCaptured "Resolving npm version"
+  assertNotCaptured "WARNING"
   assertCapturedSuccess
 }
 

bin/warnings.sh

-warning() {
-  tip=$1
-  url=$2
-  echo "WARNING: $tip" >> $warnings
-  echo "${url:-https://devcenter.heroku.com/articles/nodejs-support}" >> $warnings
-  echo "" >> $warnings
+warn_node_engine() {
+  local node_engine=$1
+  if [ "$node_engine" == "" ]; then
+    warning "Node version not specified in package.json" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
+  elif [ "$node_engine" == "*" ]; then
+    warning "Avoid semver ranges like '*' in engines.node" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
+  elif [ ${node_engine:0:1} == ">" ]; then
+    warning "Avoid semver ranges starting with '>' in engines.node" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
+  fi
 }
 
-if [ "$node_engine" == "" ]; then
-  warning "Node version not specified in package.json" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
-elif [ "$node_engine" == "*" ]; then
-  warning "Avoid semver ranges like '*' in engines.node" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
-elif [ ${node_engine:0:1} == ">" ]; then
-  warning "Avoid semver ranges starting with '>' in engines.node" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
-fi
-
-if [ "$npm_engine" != "" ]; then
-  if [ "${npm_engine:0:1}" -lt "2" ]; then
-    warning "This version of npm has several known issues - consider upgrading to the latest release"
+warn_node_modules() {
+  local modules_source=$1
+  if [ "$modules_source" == "prebuilt" ]; then
+    warning "Avoid checking node_modules into source control" "https://www.npmjs.org/doc/misc/npm-faq.html#should-i-check-my-node_modules-folder-into-git-"
+  elif [ "$modules_source" == "" ]; then
+    warning "No package.json found"
   fi
-fi
+}
 
-if [ "$modules_source" == "prebuilt" ]; then
-  warning "Avoid checking node_modules into source control" "https://www.npmjs.org/doc/misc/npm-faq.html#should-i-check-my-node_modules-folder-into-git-"
-elif [ "$modules_source" == "" ]; then
-  warning "No package.json found"
-fi
+warn_start() {
+  local start_method=$1
+  if [ "$start_method" == "" ]; then
+    warning "No Procfile, package.json start script, or server.js file found" "https://devcenter.heroku.com/articles/nodejs-support#runtime-behavior"
+  fi
+}
 
-if [ "$start_method" == "" ]; then
-  warning "No Procfile, package.json start script, or server.js file found" "https://devcenter.heroku.com/articles/nodejs-support#runtime-behavior"
-fi
+warn_old_npm() {
+  local npm_version=$1
+  if [ "${npm_version:0:1}" -lt "2" ]; then
+    local latest_npm=$(curl --silent --get https://semver.herokuapp.com/npm/stable)
+    warning "This version of npm ($npm_version) has several known issues - consider upgrading to the latest release ($latest_npm)"
+  fi
+}

test/npm-version-specific/package.json

@@ -6,6 +6,9 @@
     "type" : "git",
     "url" : "http://github.com/example/example.git"
   },
+  "scripts": {
+    "start": "node index.js"
+  },
   "dependencies": {
     "hashish": "*"
   },

test/old-npm/package.json

@@ -11,7 +11,7 @@
   },
   "engines": {
     "node": "0.10.x",
-    "npm": "1.2"
+    "npm": "~1.2"
   },
   "scripts": {
     "start": "node server.js"