Merge pull request #167 from heroku/warn-old-npm

Warn old npm

bin/common.sh

@@ -13,18 +13,6 @@ info() {
   echo "       $*"
 }
 
-build_failed() {
-  head "Build failed"
-}
-
-protip() {
-  tip=$1
-  url=$2
-  echo
-  echo "PRO TIP: $tip" | indent
-  echo "See ${url:-https://devcenter.heroku.com/articles/nodejs-support}" | indent
-}
-
 file_contents() {
   if test -f $1; then
     echo "$(cat $1)"

bin/compile

@@ -13,6 +13,7 @@ env_dir=$3
 bp_dir=$(cd $(dirname $0); cd ..; pwd)
 heroku_dir=$build_dir/.heroku
 mkdir -p $heroku_dir/node
+warnings=$(mktemp)
 
 # Load some convenience functions like status(), echo(), and indent()
 source $bp_dir/bin/common.sh
@@ -21,6 +22,10 @@ source $bp_dir/bin/common.sh
 unset GIT_DIR
 
 # Provide hook to deal with errors
+build_failed() {
+  head "Build failed"
+  cat $warnings | indent
+}
 trap build_failed ERR
 
 # Load config vars into environment; start with defaults
@@ -205,3 +210,6 @@ fi
 # Show the final dependency tree
 info "Build successful!"
 (npm ls --depth=0 || true) 2>/dev/null | indent
+
+# Show any relevant warnings
+cat $warnings | indent

bin/test

@@ -21,7 +21,7 @@ testDetectWithoutPackageJson() {
 testNoVersion() {
   compile "no-version"
   assertCaptured "Node engine:         unspecified"
-  assertCaptured "PRO TIP: Specify a node version in package.json"
+  assertCaptured "WARNING: Node version not specified in package.json"
   assertCaptured "Resolving node version (latest stable) via semver.io"
   assertCaptured "Downloading and installing node 0.10"
   assertCapturedSuccess
@@ -37,7 +37,7 @@ testSpecificVersion() {
 testStableVersion() {
   compile "stable-node"
   assertCaptured "Downloading and installing node 0.10"
-  assertNotCaptured "PRO TIP"
+  assertNotCaptured "WARNING"
   assertCapturedSuccess
 }
 
@@ -48,6 +48,12 @@ testUnstableVersion() {
   assertCapturedSuccess
 }
 
+testOldNpm() {
+  compile "old-npm"
+  assertCaptured "WARNING: This version of npm has several known issues"
+  assertCapturedSuccess
+}
+
 testInfoEmpty() {
   compile "info-empty"
   assertCaptured "Node engine:         unspecified"
@@ -60,7 +66,7 @@ testInfoEmpty() {
 
 testDangerousRangeStar() {
   compile "dangerous-range-star"
-  assertCaptured "PRO TIP: Avoid semver ranges like '*'"
+  assertCaptured "WARNING: Avoid semver ranges like '*'"
   assertCaptured "Node engine:         *"
   assertCaptured "Resolving node version * via semver.io"
   assertCaptured "Downloading and installing node 0.10"
@@ -69,7 +75,7 @@ testDangerousRangeStar() {
 
 testDangerousRangeGreaterThan() {
   compile "dangerous-range-greater-than"
-  assertCaptured "PRO TIP: Avoid semver ranges starting with '>'"
+  assertCaptured "WARNING: Avoid semver ranges starting with '>'"
   assertCaptured "Resolving node version >0.4 via semver.io"
   assertCaptured "Downloading and installing node 0.10"
   assertCapturedSuccess
@@ -167,7 +173,7 @@ testProcfileAbsentServerPresent() {
 
 testServerPresentOnly() {
   compile "server-present-only"
-  assertCaptured "PRO TIP: Use 'npm init'"
+  assertCaptured "WARNING: No package.json found"
   assertCaptured "Skipping dependencies"
   assertCaptured "'web: node server.js' to new Procfile"
   assertFile "web: node server.js" "Procfile"

bin/warnings.sh

+warning() {
+  tip=$1
+  url=$2
+  echo "WARNING: $tip" >> $warnings
+  echo "${url:-https://devcenter.heroku.com/articles/nodejs-support}" >> $warnings
+  echo "" >> $warnings
+}
+
 if [ "$node_engine" == "" ]; then
-  protip "Specify a node version in package.json" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
+  warning "Node version not specified in package.json" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
 elif [ "$node_engine" == "*" ]; then
-  protip "Avoid semver ranges like '*' in engines.node" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
+  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
-  protip "Avoid semver ranges starting with '>' in engines.node" "https://devcenter.heroku.com/articles/nodejs-support#specifying-a-node-js-version"
+  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"
+  fi
 fi
 
 if [ "$modules_source" == "prebuilt" ]; then
-  protip "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-"
+  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
-  protip "Use 'npm init' and 'npm install --save' to define dependencies"
+  warning "No package.json found"
 fi
 
 if [ "$start_method" == "" ]; then
-  protip "Include a Procfile, package.json start script, or server.js file to start your app" "https://devcenter.heroku.com/articles/nodejs-support#runtime-behavior"
+  warning "No Procfile, package.json start script, or server.js file found" "https://devcenter.heroku.com/articles/nodejs-support#runtime-behavior"
 fi

test/old-npm/package.json

+{
+  "name": "node-buildpack-test-app",
+  "version": "0.0.1",
+  "description": "node buildpack integration test app",
+  "repository" : {
+    "type" : "git",
+    "url" : "http://github.com/example/example.git"
+  },
+  "dependencies": {
+    "hashish": "*"
+  },
+  "engines": {
+    "node": "0.10.x",
+    "npm": "1.2"
+  },
+  "scripts": {
+    "start": "node server.js"
+  }
+}