JS engines supporting node should prefer local installed npm

Instead of using webjars npm

build.sbt

@@ -23,7 +23,7 @@ libraryDependencies ++= Seq(
   "io.apigee.trireme" % "trireme-node10src" % "0.9.4",
 
   // NPM
-  "org.webjars" % "npm" % "5.0.0-2",
+  "org.webjars" % "npm" % "5.0.0-2", // we are currently stuck: https://github.com/webjars/webjars/issues/1926
   "org.webjars" % "webjars-locator-core" % "0.52",
 
   // Test deps

src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala

@@ -34,6 +34,7 @@ object JsEngineImport {
     @deprecated("No longer used", "1.3.0")
     val npmTimeout = SettingKey[FiniteDuration]("jse-npm-timeout", "The maximum number amount of time for npm to do its thing.")
     val npmNodeModules = TaskKey[Seq[File]]("jse-npm-node-modules", "Node module files generated by NPM.")
+    val npmUseIntegrated = SettingKey[Boolean]("jse-npm-use-integrated")
   }
 
 }
@@ -96,6 +97,7 @@ object SbtJsEngine extends AutoPlugin {
       val nodeModulesDirectory = (Plugin / webJarsNodeModulesDirectory).value
       val logger = streams.value.log
       val baseDir = baseDirectory.value
+      val useIntegrated = npmUseIntegrated.value
 
       val runUpdate = FileFunction.cached(cacheDirectory, FilesInfo.hash) { _ =>
         if (npmPackageJson.exists) {
@@ -125,7 +127,8 @@ object SbtJsEngine extends AutoPlugin {
         println(s"Unknown engine type $engineTypeStr for sbt.jse.engineType. Resorting back to the default of $defaultEngineType.")
         defaultEngineType
       }),
-    command := sys.props.get("sbt.jse.command").map(file)
+    command := sys.props.get("sbt.jse.command").map(file),
+    npmUseIntegrated := false,
 
   ) ++ inConfig(Assets)(jsEngineUnscopedSettings) ++ inConfig(TestAssets)(jsEngineUnscopedSettings)
 

src/main/scala/com/typesafe/sbt/jse/engines/LocalEngine.scala

@@ -6,27 +6,7 @@ import scala.collection.JavaConverters._
 import scala.collection.immutable
 import scala.sys.process.{Process, ProcessLogger}
 
-class LocalEngine(stdArgs: immutable.Seq[String], stdEnvironment: Map[String, String], override val isNode: Boolean) extends Engine {
-
-  // This quoting functionality is as recommended per http://bugs.java.com/view_bug.do?bug_id=6511002
-  // The JDK can't change due to its backward compatibility requirements, but we have no such constraint
-  // here. Args should be able to be expressed consistently by the user of our API no matter whether
-  // execution is on Windows or not.
-  private def needsQuoting(s: String): Boolean =
-    if (s.isEmpty) true else s.exists(c => c == ' ' || c == '\t' || c == '\\' || c == '"')
-
-  private def winQuote(s: String): String = {
-    if (!needsQuoting(s)) {
-      s
-    } else {
-      "\"" + s.replaceAll("([\\\\]*)\"", "$1$1\\\\\"").replaceAll("([\\\\]*)\\z", "$1$1") + "\""
-    }
-  }
-
-  private val isWindows: Boolean = System.getProperty("os.name").toLowerCase.contains("win")
-
-  private def prepareArgs(args: immutable.Seq[String]): immutable.Seq[String] =
-    if (isWindows) args.map(winQuote) else args
+class LocalEngine(val stdArgs: immutable.Seq[String], val stdEnvironment: Map[String, String], override val isNode: Boolean) extends Engine {
 
   override def executeJs(source: File, args: immutable.Seq[String], environment: Map[String, String],
     stdOutSink: String => Unit, stdErrSink: String => Unit): JsExecutionResult = {
@@ -35,7 +15,7 @@ class LocalEngine(stdArgs: immutable.Seq[String], stdEnvironment: Map[String, St
     val allEnvironment = stdEnvironment ++ environment
 
 
-    val pb = new ProcessBuilder(prepareArgs(allArgs).asJava)
+    val pb = new ProcessBuilder(LocalEngine.prepareArgs(allArgs).asJava)
     pb.environment().putAll(allEnvironment.asJava)
     JsExecutionResult(Process(pb).!(ProcessLogger(stdOutSink, stdErrSink)))
   }
@@ -56,6 +36,26 @@ object LocalEngine {
     val newNodePath = Option(System.getenv("NODE_PATH")).fold(nodePath)(_ + nodePathDelim + nodePath)
     if (newNodePath.isEmpty) Map.empty[String, String] else Map("NODE_PATH" -> newNodePath)
   }
+
+  // This quoting functionality is as recommended per http://bugs.java.com/view_bug.do?bug_id=6511002
+  // The JDK can't change due to its backward compatibility requirements, but we have no such constraint
+  // here. Args should be able to be expressed consistently by the user of our API no matter whether
+  // execution is on Windows or not.
+  private def needsQuoting(s: String): Boolean =
+    if (s.isEmpty) true else s.exists(c => c == ' ' || c == '\t' || c == '\\' || c == '"')
+
+  private def winQuote(s: String): String = {
+    if (!needsQuoting(s)) {
+      s
+    } else {
+      "\"" + s.replaceAll("([\\\\]*)\"", "$1$1\\\\\"").replaceAll("([\\\\]*)\\z", "$1$1") + "\""
+    }
+  }
+
+  private val isWindows: Boolean = System.getProperty("os.name").toLowerCase.contains("win")
+
+  private[jse] def prepareArgs(args: immutable.Seq[String]): immutable.Seq[String] =
+    if (isWindows) args.map(winQuote) else args
 }
 
 /**

src/main/scala/com/typesafe/sbt/jse/npm/Npm.scala

@@ -3,32 +3,87 @@ package com.typesafe.sbt.jse.npm
 
 import java.io.File
 
-import com.typesafe.sbt.jse.engines.{Engine, JsExecutionResult}
+import com.typesafe.sbt.jse.engines.{Engine, JsExecutionResult, LocalEngine}
 
 import scala.collection.immutable
+import scala.collection.JavaConverters._
 import scala.collection.mutable.ListBuffer
+import scala.sys.process.{Process, ProcessLogger}
+import scala.util.Try
 
 /**
   * A JVM class for performing NPM commands. Requires a JS engine to use.
   */
-class Npm(engine: Engine, npmFile: File, verbose: Boolean = false) {
+class Npm(engine: Engine, npmFile: File, verbose: Boolean = false, preferSystemNpm: Boolean = true) {
 
+  // TODO deprecate all constructors, file not always necessary (when using local installed)
   def this(engine: Engine, npmFile: File) = this(engine, npmFile, false)
 
+  def install(global: Boolean = false, names: Seq[String] = Nil, outSink: String => Unit, errSink: String => Unit): JsExecutionResult = {
+    val args = ListBuffer[String]()
+    args += "install"
+    if (global) args += "-g"
+    if (verbose) args += "--verbose"
+    //args += "--no-audit" // TODO: audit as optional
+    args ++= names
+    invokeNpm(args, outSink, errSink)
+  }
+
   def update(global: Boolean = false, names: Seq[String] = Nil, outSink: String => Unit, errSink: String => Unit): JsExecutionResult = {
     val args = ListBuffer[String]()
     args += "update"
     if (global) args += "-g"
     if (verbose) args += "--verbose"
+    //args += "--no-audit" // TODO: audit as optional
     args ++= names
     invokeNpm(args, outSink, errSink)
   }
 
+  // TODO: Also provide ci subcommand, see https://github.com/typesafehub/npm/issues/28
+
+  private def detectNpm(command: String): Option[String] = {
+    val npmExists = Try(Process(s"$command --version").!!).isSuccess
+    if (!npmExists) {
+      println(s"Warning: npm detection failed. Tried the command: $command")
+      None
+    } else {
+      Some(command)
+    }
+  }
+
   private def invokeNpm(args: ListBuffer[String], outSink: String => Unit, errSink: String => Unit): JsExecutionResult = {
     if (!engine.isNode) {
       throw new IllegalStateException("node not found: a Node.js installation is required to run npm.")
     }
-    engine.executeJs(npmFile, args.to[immutable.Seq], Map.empty, outSink, errSink)
+
+    def executeJsNpm(): JsExecutionResult = engine.executeJs(npmFile, args.to[immutable.Seq], Map.empty, outSink, errSink)
+
+    engine match {
+      case localEngine: LocalEngine if preferSystemNpm => {
+        // The first argument always is the command of the js engine, e.g. either just "node", "phantomjs,.. or a path like "/usr/bin/node"
+        // So we first try to detect a npm command in the same folder in case the user provided an explicit command that is a path
+        val localEngineCmd = new File(localEngine.stdArgs.head)
+        val localNpmCmd = if (localEngineCmd.getParent() == null) {
+            // Pretty sure the command was not a path but just something like "node"
+            // Therefore we assume the npm command is on the path, just like the js engine command
+            detectNpm("npm")
+          } else {
+            val cmdPath = new File(localEngineCmd.getParentFile, "npm").getCanonicalPath
+            detectNpm(cmdPath).orElse(detectNpm("npm"))
+          }
+        localNpmCmd match {
+          case Some(cmd) => {
+            val allArgs = immutable.Seq(cmd) ++ args
+            val pb = new ProcessBuilder(LocalEngine.prepareArgs(allArgs).asJava)
+            pb.environment().putAll(localEngine.stdEnvironment.asJava)
+            JsExecutionResult(Process(pb).!(ProcessLogger(outSink, errSink)))
+          }
+          case None => executeJsNpm() // TODO log that fallback is happening
+        }
+      }
+      case _ => // e.g. Trireme provides node, but is not a local install and does not provide npm, therefore fallback using the webjar npm
+        executeJsNpm()
+    }
   }
 
 }

src/test/scala/com/typesafe/sbt/jse/npm/NpmSpec.scala

@@ -32,7 +32,8 @@ class NpmSpec extends Specification {
       val stdOut = out.toString()
 
       result.exitValue must_== 0
-      stdErr must contain("npm http request GET https://registry.npmjs.org/amdefine")
+      stdErr must contain("npm http request GET https://registry.npmjs.org/amdefine") // when using webjar npm
+        .or(contain("npm http fetch GET 200 https://registry.npmjs.org/amdefine")) // when using local installed npm
     }
   }
 }