From dbff439982b59f1131141e9353b85bc577d6f8c5 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 18 May 2023 12:47:52 +0200 Subject: [PATCH 1/2] JS engines supporting node should prefer local installed npm Instead of using webjars npm --- build.sbt | 2 +- .../com/typesafe/sbt/jse/SbtJsEngine.scala | 10 ++- .../sbt/jse/engines/LocalEngine.scala | 44 +++++----- .../scala/com/typesafe/sbt/jse/npm/Npm.scala | 83 +++++++++++++++++-- .../com/typesafe/sbt/jse/npm/NpmSpec.scala | 5 +- 5 files changed, 112 insertions(+), 32 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..4255fa1 100644 --- a/src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala +++ b/src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala @@ -34,6 +34,8 @@ 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 npmPreferSystemInstalledNpm = SettingKey[Boolean]("jse-npm-prefer-system-installed-npm","Prefer detecting and using locally installed NPM when using a local engine that provides Node support") + // TODO: Run install, update or ci ? } } @@ -80,7 +82,9 @@ object SbtJsEngine extends AutoPlugin { private lazy val autoDetectNode: Boolean = { val nodeExists = Try(Process("node --version").!!).isSuccess if (!nodeExists) { + println("!!!") println("Warning: node.js detection failed, sbt will use the Rhino based Trireme JavaScript engine instead to run JavaScript assets compilation, which in some cases may be orders of magnitude slower than using node.js.") + println("!!!") } nodeExists } @@ -96,10 +100,11 @@ object SbtJsEngine extends AutoPlugin { val nodeModulesDirectory = (Plugin / webJarsNodeModulesDirectory).value val logger = streams.value.log val baseDir = baseDirectory.value + val preferSystemInstalledNpm = npmPreferSystemInstalledNpm.value val runUpdate = FileFunction.cached(cacheDirectory, FilesInfo.hash) { _ => if (npmPackageJson.exists) { - val npm = new Npm(engine, nodeModulesDirectory / "npm" / "lib" / "npm.js") + val npm = new Npm(engine, Some(nodeModulesDirectory / "npm" / "lib" / "npm.js"), preferSystemNpm = preferSystemInstalledNpm) val result = npm.update(global = false, Seq("--prefix", baseDir.getPath), logger.info(_), logger.error(_)) if (result.exitValue != 0) { sys.error("Problems with NPM resolution. Aborting build.") @@ -125,7 +130,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), + npmPreferSystemInstalledNpm := true, ) ++ 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..b9ae918 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,105 @@ 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: Option[File] = None, preferSystemNpm: Boolean = true, verbose: Boolean = false) { - def this(engine: Engine, npmFile: File) = this(engine, npmFile, false) + @deprecated("Use one of the other non-deprecated constructors instead", "1.3.0") + def this(engine: Engine, npmFile: File) = this(engine, Some(npmFile)) + @deprecated("Use one of the other non-deprecated constructors instead", "1.3.0") + def this(engine: Engine, npmFile: File, verbose: Boolean) = this(engine, Some(npmFile), verbose) + + /** + * https://docs.npmjs.com/cli/commands/npm-install + */ + def install(global: Boolean = false, names: Seq[String] = Nil, outSink: String => Unit, errSink: String => Unit): JsExecutionResult = { + cmd("install", global, names, outSink, errSink) + } + + /** + * https://docs.npmjs.com/cli/commands/npm-update + */ def update(global: Boolean = false, names: Seq[String] = Nil, outSink: String => Unit, errSink: String => Unit): JsExecutionResult = { + cmd("update", global, names, outSink, errSink) + } + + /** + * https://docs.npmjs.com/cli/commands/npm-ci + */ + def ci(names: Seq[String] = Nil, outSink: String => Unit, errSink: String => Unit): JsExecutionResult = { + cmd("ci", false, names, outSink, errSink) // ci subcommand does not support -g (global) flag + } + + private def cmd(cmd: String, global: Boolean = false, names: Seq[String] = Nil, outSink: String => Unit, errSink: String => Unit): JsExecutionResult = { val args = ListBuffer[String]() - args += "update" + args += cmd if (global) args += "-g" if (verbose) args += "--verbose" args ++= names invokeNpm(args, outSink, errSink) } + private def detectNpm(command: String): Option[String] = { + val npmExists = Try(Process(s"$command --version").!!).isSuccess + if (!npmExists) { + println("!!!") + println(s"Warning: npm detection failed. Tried the command: $command") + println("!!!") + 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.getOrElse(throw new RuntimeException("No NPM JavaScript file passed to the Npm instance via the npmFile param")), 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, if the command is a path, we first try to detect if there is a npm command available in the same folder + 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 operating system path, just like the js engine command + detectNpm("npm") + } else { + // Looks like the command was a valid path, so let's see if we can detect a npm command within the same folder + // If we can't, try to fallback to a npm command that is on the operating system path + 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 => + println("!!!") + println(s"Warning: npm detection failed. Falling back to npm provided by WebJars, which is outdated and will not work with Node versions 14 and newer.") + println("!!!") + executeJsNpm() + } + 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..e0eb1c1 100644 --- a/src/test/scala/com/typesafe/sbt/jse/npm/NpmSpec.scala +++ b/src/test/scala/com/typesafe/sbt/jse/npm/NpmSpec.scala @@ -19,7 +19,7 @@ class NpmSpec extends Specification { val err = new StringBuilder val to = new File(new File("target"), "webjars") - val npm = new Npm(Node(), NpmLoader.load(to, this.getClass.getClassLoader), verbose = true) + val npm = new Npm(Node(), Some(NpmLoader.load(to, this.getClass.getClassLoader)), verbose = true) val result = npm.update(false, Nil, s => { println("stdout: " + s) out.append(s + "\n") @@ -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 } } } From 9f38033e71e86fab0e6e1e5dd0ffe48fd2c19b55 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 18 May 2023 16:56:46 +0200 Subject: [PATCH 2/2] npmSubcommand to be able to use install, update or ci --- .../scala/com/typesafe/sbt/jse/SbtJsEngine.scala | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala b/src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala index 4255fa1..fc9fe1d 100644 --- a/src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala +++ b/src/main/scala/com/typesafe/sbt/jse/SbtJsEngine.scala @@ -27,6 +27,10 @@ object JsEngineImport { AutoDetect = Value } + object NpmSubcommand extends Enumeration { + val Install, Update, Ci = Value + } + val command = SettingKey[Option[File]]("jse-command", "An optional path to the command used to invoke the engine.") val engineType = SettingKey[EngineType.Value]("jse-engine-type", "The type of engine to use.") @deprecated("No longer used", "1.3.0") @@ -35,7 +39,7 @@ object JsEngineImport { 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 npmPreferSystemInstalledNpm = SettingKey[Boolean]("jse-npm-prefer-system-installed-npm","Prefer detecting and using locally installed NPM when using a local engine that provides Node support") - // TODO: Run install, update or ci ? + val npmSubcommand = SettingKey[NpmSubcommand.Value]("jse-npm-subcommand", "The subcommand to use in NPM: install, update or ci") } } @@ -101,11 +105,16 @@ object SbtJsEngine extends AutoPlugin { val logger = streams.value.log val baseDir = baseDirectory.value val preferSystemInstalledNpm = npmPreferSystemInstalledNpm.value + val subcommand = npmSubcommand.value val runUpdate = FileFunction.cached(cacheDirectory, FilesInfo.hash) { _ => if (npmPackageJson.exists) { val npm = new Npm(engine, Some(nodeModulesDirectory / "npm" / "lib" / "npm.js"), preferSystemNpm = preferSystemInstalledNpm) - val result = npm.update(global = false, Seq("--prefix", baseDir.getPath), logger.info(_), logger.error(_)) + val result = subcommand match { + case NpmSubcommand.Install => npm.install(global = false, Seq("--prefix", baseDir.getPath), logger.info(_), logger.error(_)) + case NpmSubcommand.Update => npm.update(global = false, Seq("--prefix", baseDir.getPath), logger.info(_), logger.error(_)) + case NpmSubcommand.Ci => npm.ci(Seq("--prefix", baseDir.getPath), logger.info(_), logger.error(_)) + } if (result.exitValue != 0) { sys.error("Problems with NPM resolution. Aborting build.") } @@ -132,6 +141,7 @@ object SbtJsEngine extends AutoPlugin { }), command := sys.props.get("sbt.jse.command").map(file), npmPreferSystemInstalledNpm := true, + npmSubcommand := NpmSubcommand.Update, ) ++ inConfig(Assets)(jsEngineUnscopedSettings) ++ inConfig(TestAssets)(jsEngineUnscopedSettings)