[2.0.x] fix: Fixes --addPluginSbtFile getting lost after reboot (#9669)

reboot restarts sbt with a fresh state, so the extra plugin sbt files registered in BasicKeys.extraMetaSbtFiles were dropped and their plugins disappeared. Prepend an early(addPluginSbtFile=<path>) command for each registered file to the arguments handed to the restarted sbt, so they are re-registered.

---------

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>

main-command/src/main/scala/sbt/State.scala

@@ -248,12 +248,40 @@ object State {
     val app = state.configuration.provider
     new Reboot(
       app.scalaProvider.version,
-      state.remainingCommands map { case e: Exec => e.commandLine },
+      addPluginSbtFileArguments(state) ::: state.remainingCommands.map(_.commandLine),
       app.id,
       state.configuration.baseDirectory
     )
   }
 
+  /**
+   * Builds the `early(...)` commands that add the extra plugin sbt files back after a reboot.
+   * A reboot clears the state, so sbt forgets these files. The commands are passed to the new
+   * sbt as arguments, and they run before it loads the build.
+   *
+   * The `early(addPluginSbtFile=...)` form is used, not `--addPluginSbtFile=...`, because the
+   * `--` form loses the quotes around the path, and then a path with a space in it fails.
+   * A path with a space can be added using `addPluginSbtFile "<path>"`.
+   */
+  private[sbt] def addPluginSbtFileArguments(state: State): List[String] =
+    state.get(BasicKeys.extraMetaSbtFiles).toList.flatten.distinct.map { vf =>
+      val path = vf match {
+        case f: xsbti.PathBasedFile => f.toPath.toString
+        case f                      => f.id
+      }
+      val command = s"${BasicCommandStrings.AddPluginSbtFileCommand}=${quote(path)}"
+      s"${BasicCommandStrings.EarlyCommand}($command)"
+    }
+
+  /**
+   * Puts `path` in quotes so the new sbt session reads it back as one whole string. There the path goes
+   * through the `Parsers.StringBasic` in `BasicCommands.addPluginSbtFileParser`, which stops at
+   * the first space when there are no quotes. A path with a space can be added, so it also has
+   * to come back after a reboot.
+   */
+  private def quote(path: String): String =
+    s"\"${path.replace("\\", "\\\\").replace("\"", "\\\"")}\""
+
   @deprecated("Import State._ or State.StateOpsImpl to access state extension methods", "1.3.0")
   def stateOps(s: State): StateOps = new StateOpsImpl(s)
 
@@ -331,8 +359,9 @@ object State {
           StartServer :: remaining.dropWhile(!_.startsWith(ReportResult)).tail ::: "shell" :: Nil
         case _ => remaining
       }
-      if (currentOnly) throw new RebootCurrent(fullRemaining)
-      else throw new xsbti.FullReload(fullRemaining.toArray, full)
+      val arguments = State.addPluginSbtFileArguments(s) ::: fullRemaining
+      if (currentOnly) throw new RebootCurrent(arguments)
+      else throw new xsbti.FullReload(arguments.toArray, full)
     }
 
     def reload = runExitHooks().setNext(new Return(defaultReload(s)))

sbt-app/src/sbt-test/global-plugin/global-plugin/test

@@ -13,3 +13,9 @@ $ copy-file changes/global-plugins.sbt global/plugins/plugins.sbt
 $ copy-file changes/plugins.sbt project/plugins.sbt
 > reload
 > check
+
+# The tests of one scripted run share a directory, and `global` is the part of it that is kept between
+# them, so delete what this test installed there.
+$ delete global/plugins global/useGlobalAutoPlugin.sbt
+$ absent global/plugins global/useGlobalAutoPlugin.sbt
+> reload

sbt-app/src/sbt-test/project/addPluginSbtFile-reboot/build.sbt

@@ -0,0 +1,46 @@
+import sbt.internal.LoadedBuild
+
+lazy val root = project.in(file("."))
+
+def detectedPlugins(lb: LoadedBuild): Seq[String] =
+  lb.units(lb.root).unit.plugins.detected.autoPlugins.map(_.name)
+
+InputKey[Unit]("checkPlugins") := {
+  val args = Def.spaceDelimited("<names>").parsed
+  val detected = detectedPlugins(loadedBuild.value)
+  args.foreach { name =>
+    assert(
+      detected.exists(_.contains(name)),
+      s"expected plugin $name to be detected, got: ${detected.mkString(", ")}"
+    )
+  }
+}
+
+InputKey[Unit]("checkPluginsAbsent") := {
+  val args = Def.spaceDelimited("<names>").parsed
+  val detected = detectedPlugins(loadedBuild.value)
+  args.foreach { name =>
+    assert(
+      !detected.exists(_.contains(name)),
+      s"expected plugin $name not to be detected, got: ${detected.mkString(", ")}"
+    )
+  }
+}
+
+// Compares the whole list of registered paths, in order. This also catches a path that comes
+// back from a reboot changed, doubled or in a different place, not only one that is lost.
+// With no arguments it asserts that no file is registered.
+InputKey[Unit]("checkFiles") := {
+  val expected = Def.spaceDelimited("<paths>").parsed.toList
+  val actual = state.value.get(BasicKeys.extraMetaSbtFiles).toList.flatten.map(_.id)
+  assert(
+    actual == expected,
+    s"expected registered files to be [${expected.mkString(", ")}], got: [${actual.mkString(", ")}]"
+  )
+}
+
+// Tests in a scripted group share one sbt session, and the extra files now survive reboot on
+// purpose, so they have to be dropped before the next test.
+commands += Command.command("clearExtraPluginSbtFiles") { s =>
+  s.remove(BasicKeys.extraMetaSbtFiles)
+}

sbt-app/src/sbt-test/project/addPluginSbtFile-reboot/temp with spaces/extraC.sbt

@@ -0,0 +1 @@
+addSbtPlugin("org.scala-native" % "sbt-scala-native" % "0.5.11")

sbt-app/src/sbt-test/project/addPluginSbtFile-reboot/temp/extraA.sbt

@@ -0,0 +1 @@
+addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.13.1")

sbt-app/src/sbt-test/project/addPluginSbtFile-reboot/temp/extraB.sbt

@@ -0,0 +1 @@
+addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.22.0")

sbt-app/src/sbt-test/project/addPluginSbtFile-reboot/test

@@ -0,0 +1,43 @@
+# Regression test for sbt/sbt#4303: files added with --addPluginSbtFile must survive reboot.
+
+# Neither plugin is on the meta-build classpath to begin with, and nothing is registered.
+> checkPluginsAbsent BuildInfoPlugin ScalaJSPlugin ScalaNativePlugin
+> checkFiles
+
+# Add the first extra plugin sbt file and check that its plugin is picked up.
+> early(addPluginSbtFile=temp/extraA.sbt); reload
+> checkPlugins BuildInfoPlugin
+> checkFiles temp/extraA.sbt
+
+# The first reboot must not lose it.
+> reboot
+> checkPlugins BuildInfoPlugin
+> checkFiles temp/extraA.sbt
+
+# Add a second extra plugin sbt file on top of the first one.
+> early(addPluginSbtFile=temp/extraB.sbt); reload
+> checkPlugins BuildInfoPlugin ScalaJSPlugin
+> checkFiles temp/extraA.sbt temp/extraB.sbt
+
+# The second reboot must keep both of them, and must not add either of them twice.
+> reboot
+> checkPlugins BuildInfoPlugin ScalaJSPlugin
+> checkFiles temp/extraA.sbt temp/extraB.sbt
+
+# A third file, this one in a directory whose name contains spaces. The path has to be quoted
+# for sbt to parse it as one argument, and the reboot has to keep it quoted to replay it.
+> addPluginSbtFile "temp with spaces/extraC.sbt"
+> reload
+> checkPlugins BuildInfoPlugin ScalaJSPlugin ScalaNativePlugin
+> checkFiles temp/extraA.sbt temp/extraB.sbt "temp with spaces/extraC.sbt"
+
+# The third reboot must keep all three, in order, with the spaced path intact.
+> reboot
+> checkPlugins BuildInfoPlugin ScalaJSPlugin ScalaNativePlugin
+> checkFiles temp/extraA.sbt temp/extraB.sbt "temp with spaces/extraC.sbt"
+
+# Drop the extra files, because the sbt session can be shared with the other scripted tests.
+> clearExtraPluginSbtFiles
+> reload
+> checkPluginsAbsent BuildInfoPlugin ScalaJSPlugin ScalaNativePlugin
+> checkFiles