Skip to content

Commit 82e5423

Browse files
committed
JS engines supporting node should prefer local installed npm
Instead of using webjars npm
1 parent 732bef3 commit 82e5423

File tree

5 files changed

+87
-28
lines changed

5 files changed

+87
-28
lines changed

build.sbt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ libraryDependencies ++= Seq(
2323
"io.apigee.trireme" % "trireme-node10src" % "0.9.4",
2424

2525
// NPM
26-
"org.webjars" % "npm" % "5.0.0-2",
26+
"org.webjars" % "npm" % "5.0.0-2", // we are currently stuck: https://github.com/webjars/webjars/issues/1926
2727
"org.webjars" % "webjars-locator-core" % "0.52",
2828

2929
// Test deps

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ object JsEngineImport {
3434
@deprecated("No longer used", "1.3.0")
3535
val npmTimeout = SettingKey[FiniteDuration]("jse-npm-timeout", "The maximum number amount of time for npm to do its thing.")
3636
val npmNodeModules = TaskKey[Seq[File]]("jse-npm-node-modules", "Node module files generated by NPM.")
37+
val npmUseIntegrated = SettingKey[Boolean]("jse-npm-use-integrated")
3738
}
3839

3940
}
@@ -96,6 +97,7 @@ object SbtJsEngine extends AutoPlugin {
9697
val nodeModulesDirectory = (Plugin / webJarsNodeModulesDirectory).value
9798
val logger = streams.value.log
9899
val baseDir = baseDirectory.value
100+
val useIntegrated = npmUseIntegrated.value
99101

100102
val runUpdate = FileFunction.cached(cacheDirectory, FilesInfo.hash) { _ =>
101103
if (npmPackageJson.exists) {
@@ -125,7 +127,8 @@ object SbtJsEngine extends AutoPlugin {
125127
println(s"Unknown engine type $engineTypeStr for sbt.jse.engineType. Resorting back to the default of $defaultEngineType.")
126128
defaultEngineType
127129
}),
128-
command := sys.props.get("sbt.jse.command").map(file)
130+
command := sys.props.get("sbt.jse.command").map(file),
131+
npmUseIntegrated := false,
129132

130133
) ++ inConfig(Assets)(jsEngineUnscopedSettings) ++ inConfig(TestAssets)(jsEngineUnscopedSettings)
131134

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

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,7 @@ import scala.collection.JavaConverters._
66
import scala.collection.immutable
77
import scala.sys.process.{Process, ProcessLogger}
88

9-
class LocalEngine(stdArgs: immutable.Seq[String], stdEnvironment: Map[String, String], override val isNode: Boolean) extends Engine {
10-
11-
// This quoting functionality is as recommended per http://bugs.java.com/view_bug.do?bug_id=6511002
12-
// The JDK can't change due to its backward compatibility requirements, but we have no such constraint
13-
// here. Args should be able to be expressed consistently by the user of our API no matter whether
14-
// execution is on Windows or not.
15-
private def needsQuoting(s: String): Boolean =
16-
if (s.isEmpty) true else s.exists(c => c == ' ' || c == '\t' || c == '\\' || c == '"')
17-
18-
private def winQuote(s: String): String = {
19-
if (!needsQuoting(s)) {
20-
s
21-
} else {
22-
"\"" + s.replaceAll("([\\\\]*)\"", "$1$1\\\\\"").replaceAll("([\\\\]*)\\z", "$1$1") + "\""
23-
}
24-
}
25-
26-
private val isWindows: Boolean = System.getProperty("os.name").toLowerCase.contains("win")
27-
28-
private def prepareArgs(args: immutable.Seq[String]): immutable.Seq[String] =
29-
if (isWindows) args.map(winQuote) else args
9+
class LocalEngine(val stdArgs: immutable.Seq[String], val stdEnvironment: Map[String, String], override val isNode: Boolean) extends Engine {
3010

3111
override def executeJs(source: File, args: immutable.Seq[String], environment: Map[String, String],
3212
stdOutSink: String => Unit, stdErrSink: String => Unit): JsExecutionResult = {
@@ -35,7 +15,7 @@ class LocalEngine(stdArgs: immutable.Seq[String], stdEnvironment: Map[String, St
3515
val allEnvironment = stdEnvironment ++ environment
3616

3717

38-
val pb = new ProcessBuilder(prepareArgs(allArgs).asJava)
18+
val pb = new ProcessBuilder(LocalEngine.prepareArgs(allArgs).asJava)
3919
pb.environment().putAll(allEnvironment.asJava)
4020
JsExecutionResult(Process(pb).!(ProcessLogger(stdOutSink, stdErrSink)))
4121
}
@@ -56,6 +36,26 @@ object LocalEngine {
5636
val newNodePath = Option(System.getenv("NODE_PATH")).fold(nodePath)(_ + nodePathDelim + nodePath)
5737
if (newNodePath.isEmpty) Map.empty[String, String] else Map("NODE_PATH" -> newNodePath)
5838
}
39+
40+
// This quoting functionality is as recommended per http://bugs.java.com/view_bug.do?bug_id=6511002
41+
// The JDK can't change due to its backward compatibility requirements, but we have no such constraint
42+
// here. Args should be able to be expressed consistently by the user of our API no matter whether
43+
// execution is on Windows or not.
44+
private def needsQuoting(s: String): Boolean =
45+
if (s.isEmpty) true else s.exists(c => c == ' ' || c == '\t' || c == '\\' || c == '"')
46+
47+
private def winQuote(s: String): String = {
48+
if (!needsQuoting(s)) {
49+
s
50+
} else {
51+
"\"" + s.replaceAll("([\\\\]*)\"", "$1$1\\\\\"").replaceAll("([\\\\]*)\\z", "$1$1") + "\""
52+
}
53+
}
54+
55+
private val isWindows: Boolean = System.getProperty("os.name").toLowerCase.contains("win")
56+
57+
private[jse] def prepareArgs(args: immutable.Seq[String]): immutable.Seq[String] =
58+
if (isWindows) args.map(winQuote) else args
5959
}
6060

6161
/**

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

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,87 @@ package com.typesafe.sbt.jse.npm
33

44
import java.io.File
55

6-
import com.typesafe.sbt.jse.engines.{Engine, JsExecutionResult}
6+
import com.typesafe.sbt.jse.engines.{Engine, JsExecutionResult, LocalEngine}
77

88
import scala.collection.immutable
9+
import scala.collection.JavaConverters._
910
import scala.collection.mutable.ListBuffer
11+
import scala.sys.process.{Process, ProcessLogger}
12+
import scala.util.Try
1013

1114
/**
1215
* A JVM class for performing NPM commands. Requires a JS engine to use.
1316
*/
14-
class Npm(engine: Engine, npmFile: File, verbose: Boolean = false) {
17+
class Npm(engine: Engine, npmFile: File, verbose: Boolean = false, preferSystemNpm: Boolean = true) {
1518

19+
// TODO deprecate all constructors, file not always necessary (when using local installed)
1620
def this(engine: Engine, npmFile: File) = this(engine, npmFile, false)
1721

22+
def install(global: Boolean = false, names: Seq[String] = Nil, outSink: String => Unit, errSink: String => Unit): JsExecutionResult = {
23+
val args = ListBuffer[String]()
24+
args += "install"
25+
if (global) args += "-g"
26+
if (verbose) args += "--verbose"
27+
//args += "--no-audit" // TODO: audit as optional
28+
args ++= names
29+
invokeNpm(args, outSink, errSink)
30+
}
31+
1832
def update(global: Boolean = false, names: Seq[String] = Nil, outSink: String => Unit, errSink: String => Unit): JsExecutionResult = {
1933
val args = ListBuffer[String]()
2034
args += "update"
2135
if (global) args += "-g"
2236
if (verbose) args += "--verbose"
37+
//args += "--no-audit" // TODO: audit as optional
2338
args ++= names
2439
invokeNpm(args, outSink, errSink)
2540
}
2641

42+
// TODO: Also provide ci subcommand, see https://github.com/typesafehub/npm/issues/28
43+
44+
private def detectNpm(command: String): Option[String] = {
45+
val npmExists = Try(Process(s"$command --version").!!).isSuccess
46+
if (!npmExists) {
47+
println(s"Warning: npm detection failed. Tried the command: $command")
48+
None
49+
} else {
50+
Some(command)
51+
}
52+
}
53+
2754
private def invokeNpm(args: ListBuffer[String], outSink: String => Unit, errSink: String => Unit): JsExecutionResult = {
2855
if (!engine.isNode) {
2956
throw new IllegalStateException("node not found: a Node.js installation is required to run npm.")
3057
}
31-
engine.executeJs(npmFile, args.to[immutable.Seq], Map.empty, outSink, errSink)
58+
59+
def executeJsNpm(): JsExecutionResult = engine.executeJs(npmFile, args.to[immutable.Seq], Map.empty, outSink, errSink)
60+
61+
engine match {
62+
case localEngine: LocalEngine if preferSystemNpm => {
63+
// The first argument always is the command of the js engine, e.g. either just "node", "phantomjs,.. or a path like "/usr/bin/node"
64+
// 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
65+
val localEngineCmd = new File(localEngine.stdArgs.head)
66+
val localNpmCmd = if (localEngineCmd.getParent() == null) {
67+
// Pretty sure the command was not a path but just something like "node"
68+
// Therefore we assume the npm command is on the path, just like the js engine command
69+
detectNpm("npm")
70+
} else {
71+
val cmdPath = new File(localEngineCmd.getParentFile, "npm").getCanonicalPath
72+
detectNpm(cmdPath).orElse(detectNpm("npm"))
73+
}
74+
localNpmCmd match {
75+
case Some(cmd) => {
76+
val allArgs = immutable.Seq(cmd) ++ args
77+
val pb = new ProcessBuilder(LocalEngine.prepareArgs(allArgs).asJava)
78+
pb.environment().putAll(localEngine.stdEnvironment.asJava)
79+
JsExecutionResult(Process(pb).!(ProcessLogger(outSink, errSink)))
80+
}
81+
case None => executeJsNpm() // TODO log that fallback is happening
82+
}
83+
}
84+
case _ => // e.g. Trireme provides node, but is not a local install and does not provide npm, therefore fallback using the webjar npm
85+
executeJsNpm()
86+
}
3287
}
3388

3489
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ class NpmSpec extends Specification {
3232
val stdOut = out.toString()
3333

3434
result.exitValue must_== 0
35-
stdErr must contain("npm http request GET https://registry.npmjs.org/amdefine")
35+
stdErr must contain("npm http request GET https://registry.npmjs.org/amdefine") // when using webjar npm
36+
.or(contain("npm http fetch GET 200 https://registry.npmjs.org/amdefine")) // when using local installed npm
3637
}
3738
}
3839
}

0 commit comments

Comments
 (0)