From 82e54238c2c3f6d8a2a6f66982393d6ca85b62c8 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 18 May 2023 12:47:52 +0200 Subject: [PATCH] JS engines supporting node should prefer local installed npm Instead of using webjars npm --- build.sbt | 2 +- .../com/typesafe/sbt/jse/SbtJsEngine.scala | 5 +- .../sbt/jse/engines/LocalEngine.scala | 44 ++++++------- .../scala/com/typesafe/sbt/jse/npm/Npm.scala | 61 ++++++++++++++++++- .../com/typesafe/sbt/jse/npm/NpmSpec.scala | 3 +- 5 files changed, 87 insertions(+), 28 deletions(-) diff --git a/build.sbt b/build.sbt index eb1517a..58f1ddf 100644 --- a/build.sbt +++ b/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 diff --git a/src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala b/src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala index fbdae03..91897e9 100644 --- a/src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala +++ b/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) diff --git a/src/main/scala/com/typesafe/sbt/jse/engines/LocalEngine.scala b/src/main/scala/com/typesafe/sbt/jse/engines/LocalEngine.scala index efa00bf..cd5961a 100755 --- a/src/main/scala/com/typesafe/sbt/jse/engines/LocalEngine.scala +++ b/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 } /** diff --git a/src/main/scala/com/typesafe/sbt/jse/npm/Npm.scala b/src/main/scala/com/typesafe/sbt/jse/npm/Npm.scala index 0094d65..bbc8681 100644 --- a/src/main/scala/com/typesafe/sbt/jse/npm/Npm.scala +++ b/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() + } } } diff --git a/src/test/scala/com/typesafe/sbt/jse/npm/NpmSpec.scala b/src/test/scala/com/typesafe/sbt/jse/npm/NpmSpec.scala index b3f4f62..8eccee0 100644 --- a/src/test/scala/com/typesafe/sbt/jse/npm/NpmSpec.scala +++ b/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 } } }