Merge pull request #9543 from eed3si9n/bport2/backports

[2.0.x] Backports

build.sbt

@@ -15,7 +15,7 @@ ThisBuild / version := {
   nightlyVersion.getOrElse(v)
 }
 // update sbt.sh at root
-ThisBuild / Utils.sbtnVersion := "2.0.0-a0c4773a"
+ThisBuild / Utils.sbtnVersion := "2.0.0-731e6666"
 ThisBuild / versionScheme := Some("early-semver")
 ThisBuild / Utils.version2_13 := "2.0.0-SNAPSHOT"
 ThisBuild / scalafmtOnCompile := !(Global / insideCI).value

launcher-package/src/universal/bin/sbt.bat

@@ -51,7 +51,7 @@ set sbt_args_allow_empty=
 set sbt_args_sbt_dir=
 set sbt_args_sbt_version=
 set sbt_args_mem=
-set sbt_args_client=
+set sbt_args_client=-1
 set sbt_args_jvm_client=
 set sbt_args_no_server=
 set sbt_args_experimental_execution_log=
@@ -201,6 +201,14 @@ if defined _client_arg (
   goto args_loop
 )
 
+if "%~0" == "--server" set _server_arg=true
+
+if defined _server_arg (
+  set _server_arg=
+  set sbt_args_client=0
+  goto args_loop
+)
+
 if "%~0" == "--jvm-client" set _jvm_client_arg=true
 
 if defined _jvm_client_arg (
@@ -800,7 +808,7 @@ if defined sbt_args_verbose (
   set "SBT_ARGS=-v !SBT_ARGS!"
 )
 
-set "SBT_SCRIPT=!SBT_BIN_DIR: =%%20!sbt.bat"
+for %%I in ("!SBT_BIN_DIR!sbt.bat") do set "SBT_SCRIPT=%%~sI"
 set "SBT_ARGS=--sbt-script=!SBT_SCRIPT! %SBT_ARGS%"
 
 rem Microsoft Visual C++ 2010 SP1 Redistributable Package (x64) is required

lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportPersistence.scala

@@ -0,0 +1,53 @@
+/*
+ * sbt
+ * Copyright 2023, Scala center
+ * Copyright 2011 - 2022, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt.internal.librarymanagement
+
+import java.io.File
+import java.net.URI
+import sjsonnew.IsoStringLong
+import sbt.io.IO
+import sbt.librarymanagement.*
+
+object UpdateReportPersistence:
+
+  /**
+   * The generated library-management codecs, with the artifact content hash disabled. Persisted update
+   * reports are the only thing that uses them; everything else keeps the stock `LibraryManagementCodec`
+   * object, including the `inputs` store, so `Tracked.inputChanged` still hashes contents for
+   * invalidation.
+   *
+   * sjsonnew serializes a `File` as a `(uri, Long)` pair whose Long is
+   * `HashUtil.sha256ToLong(file.toPath())` -- a full content hash of the file. Nothing reads it back:
+   * `IsoStringLong[File].from` parses the URI and drops the Long, and `update` decides staleness in
+   * `LibraryManagement.fileUptodate`, which checks `File.exists` and the modification time against
+   * `UpdateReport.stamps`. Meanwhile a report names an artifact once per configuration it resolved in,
+   * and the projects of a build largely share their dependencies, so writing the caches re-reads the
+   * whole downloaded classpath many times over -- easily the dominant cost of writing them -- to produce
+   * bytes no reader looks at.
+   *
+   * `fileStringLongIso` is an `implicit lazy val` in `sjsonnew.FileIsoStringLongs`, so it is a virtual
+   * member and every generated format resolves `JsonFormat[File]` as
+   * `isoStringLongFormat[File](fileStringLongIso)` through its self-type. Overriding it here therefore
+   * also reaches the `Vector[(Artifact, File)]` nested inside the generated `ModuleReportFormat`, which
+   * a locally-scoped `JsonFormat[File]` could not.
+   *
+   * The JSON shape is unchanged -- only the Long's value is -- so caches stay readable by sbt versions
+   * that still write the hash, and the ones written here stay readable by them.
+   */
+  private[sbt] object CacheCodec extends LibraryManagementCodec:
+
+    /** `IO.toURI` emits the same text the stock iso puts in `first`, and `IO.toFile` inverts it. */
+    override implicit lazy val fileStringLongIso: IsoStringLong[File] =
+      IsoStringLong.iso[File](
+        (f: File) => (IO.toURI(f).toASCIIString, 0L),
+        (p: (String, Long)) => IO.toFile(new URI(p._1))
+      )
+
+  end CacheCodec
+end UpdateReportPersistence

lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala

@@ -237,8 +237,15 @@ private[internal] object SbtUpdateReport {
         .withOptional(false)
         .clearOverrides
 
+    // `Resolution.projectCache` is not a field. It builds a version-string-keyed view of
+    // `projectCache0` from scratch on every call, so reading it per dependency -- as the lookups
+    // below do, once per module and again per parent while assembling inherited info -- rebuilds a
+    // map of every resolved project once per module. Read it once and the lookups become what they
+    // read like.
+    val projectCache = res.projectCache
+
     def lookupProject(mv: coursier.core.Resolution.ModuleVersion): Option[Project] =
-      res.projectCache.get(mv) match {
+      projectCache.get(mv) match {
         case Some((_, p)) => Some(p)
         case _            =>
           interProjectDependencies.find(p => mv == (p.module, p.version))
@@ -360,11 +367,15 @@ private[internal] object SbtUpdateReport {
         classLoaders = classLoaders,
       )
 
+      // Rebuilt on every read; see the note in `moduleReports`. The eviction loop below reads it
+      // three times per conflict.
+      val subProjectCache = subRes.projectCache
+
       val reports0 = subRes.rootDependencies match {
-        case Seq(dep) if subRes.projectCache.contains(dep.moduleVersion) =>
+        case Seq(dep) if subProjectCache.contains(dep.moduleVersion) =>
           // quick hack ensuring the module for the only root dependency
           // appears first in the update report, see https://github.com/coursier/coursier/issues/650
-          val (_, proj) = subRes.projectCache(dep.moduleVersion)
+          val (_, proj) = subProjectCache(dep.moduleVersion)
           val mod = moduleId((dep, proj.version, infoProperties(proj).toMap))
           val (main, other) = reports.partition { r =>
             r.module.organization == mod.organization &&
@@ -389,14 +400,14 @@ private[internal] object SbtUpdateReport {
         // rather than handing them for each dependency (where each dependency could have its own forced
         // versions, and apply and pass them to its transitive dependencies, just like for exclusions today).
         if !forceVersions.contains(c.module)
-        projOpt = subRes.projectCache
+        projOpt = subProjectCache
           .get((c.module, c.wantedVersion))
-          .orElse(subRes.projectCache.get((c.module, c.version)))
+          .orElse(subProjectCache.get((c.module, c.version)))
         (_, proj) <- projOpt.toSeq
       } yield {
         val dep = Dependency(c.module, c.wantedVersion)
         val dependee = Dependency(c.dependeeModule, c.dependeeVersion)
-        val dependeeProj = subRes.projectCache.get((c.dependeeModule, c.dependeeVersion)) match {
+        val dependeeProj = subProjectCache.get((c.dependeeModule, c.dependeeVersion)) match {
           case Some((_, p)) =>
             ProjectInfo(
               p.version,

main/src/main/scala/sbt/Defaults.scala

@@ -1366,7 +1366,11 @@ object Defaults extends BuildCommon {
 
   /** Fork options for run-like tasks: the forked process inherits sbt's working directory. */
   private[sbt] def runForkOptionsTask: Initialize[Task[ForkOptions]] =
-    Def.task(forkOptionsTask.value.withWorkingDirectory(None))
+    Def.task {
+      // this uses Compile / run / baseDirectory, which defaults to ThisBuild / baseDirectory
+      forkOptionsTask.value
+        .withWorkingDirectory(Some(baseDirectory.value))
+    }
 
   def testExecutionTask(task: Scoped): Initialize[Task[Tests.Execution]] =
     Def.task {
@@ -2596,7 +2600,8 @@ object Defaults extends BuildCommon {
   private lazy val newRunnerSettings: Seq[Setting[?]] =
     Seq(
       runner := Def.uncached(ClassLoaders.runner.value),
-      forkOptions := Def.uncached(runForkOptionsTask.value)
+      forkOptions := Def.uncached(runForkOptionsTask.value),
+      baseDirectory := (ThisBuild / baseDirectory).value,
     )
 
   lazy val baseTasks: Seq[Setting[?]] = projectTasks ++ packageBase

main/src/main/scala/sbt/internal/ClassLoaders.scala

@@ -224,11 +224,11 @@ private[sbt] object ClassLoaders {
               scalaReflectLayer,
               () =>
                 new ReverseLookupClassLoaderHolder(
-                  allDependencies,
-                  scalaReflectLayer,
-                  close,
-                  allowZombies,
-                  logger
+                  classpath = allDependencies,
+                  parent = scalaReflectLayer,
+                  closeThis = close,
+                  allowZombies = allowZombies,
+                  logger = logger,
                 )
             )
           } else scalaReflectLayer

main/src/main/scala/sbt/internal/LibraryManagement.scala

@@ -126,7 +126,7 @@ private[sbt] object LibraryManagement {
 
     /* Skip resolve if last output exists, otherwise error. */
     def skipResolve(cache: CacheStore)(inputs: UpdateInputs): UpdateReport = {
-      import sbt.librarymanagement.LibraryManagementCodec.given
+      import UpdateReportPersistence.CacheCodec.given
       val cachedReport = Tracked
         .lastOutput[UpdateInputs, UpdateReport](cache) {
           case (_, Some(out)) => out
@@ -143,8 +143,8 @@ private[sbt] object LibraryManagement {
       ur.withStats(ur.stats.withCached(true))
 
     def doResolve(cache: CacheStore): UpdateInputs => UpdateReport = {
+      import UpdateReportPersistence.CacheCodec.given
       val doCachedResolve = { (inChanged: Boolean, updateInputs: UpdateInputs) =>
-        import sbt.librarymanagement.LibraryManagementCodec.given
         try
           var isCached = false
           val report = Tracked
@@ -173,7 +173,6 @@ private[sbt] object LibraryManagement {
             log.trace(t)
             resolvedAgain
       }
-      import LibraryManagementCodec.given
       Tracked.inputChanged(cacheStoreFactory.make("inputs"))(doCachedResolve)
     }
 
@@ -279,7 +278,7 @@ private[sbt] object LibraryManagement {
 
   val moduleIdJsonKeyFormat: sjsonnew.JsonKeyFormat[ModuleID] =
     new sjsonnew.JsonKeyFormat[ModuleID] {
-      import LibraryManagementCodec.given
+      import UpdateReportPersistence.CacheCodec.given
       import sjsonnew.support.scalajson.unsafe.*
       val moduleIdFormat: JsonFormat[ModuleID] = implicitly[JsonFormat[ModuleID]]
       def write(key: ModuleID): String =
@@ -423,7 +422,7 @@ private[sbt] object LibraryManagement {
   def withExcludes(out: File, classifiers: Seq[String], lock: xsbti.GlobalLock)(
       f: Map[ModuleID, Vector[ConfigRef]] => UpdateReport
   ): UpdateReport = {
-    import sbt.librarymanagement.LibraryManagementCodec.given
+    import UpdateReportPersistence.CacheCodec.given
     import sbt.util.FileBasedStore
     val exclName = "exclude_classifiers"
     val file = out / exclName

main/src/main/scala/sbt/internal/SysProp.scala

@@ -132,7 +132,7 @@ object SysProp:
    */
   lazy val color: Boolean = ITerminal.isColorEnabled
 
-  def closeClassLoaders: Boolean = getOrFalse("sbt.classloader.close")
+  def closeClassLoaders: Boolean = getOrTrue("sbt.classloader.close")
 
   def fileCacheSize: Long =
     SizeParser(System.getProperty("sbt.file.cache.size", "128M")).getOrElse(128L * 1024 * 1024)

sbt

@@ -25,7 +25,7 @@ declare use_sbtn=
 declare use_jvm_client=
 declare no_server=
 declare sbtn_command="$SBTN_CMD"
-declare sbtn_version="2.0.0-a0c4773a"
+declare sbtn_version="2.0.0-731e6666"
 declare use_colors=1
 declare is_this_dir_sbt=""
 declare hide_jdk_warnings=1

sbt-app/src/sbt-test/classloader-cache/close-run/src/main/scala/Main.scala

@@ -1,13 +0,0 @@
-object Main {
-  class Foo
-
-  def main(args: Array[String]): Unit = {
-    new Thread {
-      override def run(): Unit = {
-        Thread.sleep(500)
-        try new Foo
-        catch { case t: Throwable => sys.exit(1) }
-      }
-    }.start()
-  }
-}

sbt-app/src/sbt-test/classloader-cache/close-run/test

@@ -1 +0,0 @@
-> run

sbt-app/src/sbt-test/run/fork-working-directory/app/Hello.scala

@@ -0,0 +1,10 @@
+package example
+
+import java.io.File
+import java.nio.file.{ Files, Path }
+
+@main
+def hello(arg: String*): Unit =
+  val x = new File(".").getAbsolutePath
+  println(s"hi $x")
+  Files.createFile(Path.of("flag"))

sbt-app/src/sbt-test/run/fork-working-directory/changes/a.sbt

@@ -0,0 +1,17 @@
+scalaVersion := "3.8.4"
+
+@transient
+lazy val check = taskKey[Unit]("")
+
+lazy val root = rootProject
+  .autoAggregate
+
+lazy val app = project
+  .settings(
+    check := {
+      val b = (ThisBuild / baseDirectory).value
+      val fo = (Compile / run / forkOptions).value
+      assert(fo.workingDirectory == Some(b), s"${fo.workingDirectory}")
+    },
+    Compile / run / fork := true,
+  )

sbt-app/src/sbt-test/run/fork-working-directory/changes/b.sbt

@@ -0,0 +1,20 @@
+scalaVersion := "3.8.4"
+
+@transient
+lazy val check = taskKey[Unit]("")
+
+lazy val root = rootProject
+  .autoAggregate
+
+lazy val app = project
+  .settings(
+    check := {
+      val b = baseDirectory.value
+      val fo = (Compile / run / forkOptions).value
+      assert(fo.workingDirectory == Some(b), s"${fo.workingDirectory}")
+    },
+    Compile / run / fork := true,
+    // app's own baseDirectory is explicitly requested as run's working
+    // directory, so `app/run` is expected to execute from app/.
+    Compile / run / baseDirectory := baseDirectory.value,
+  )

sbt-app/src/sbt-test/run/fork-working-directory/test

@@ -0,0 +1,20 @@
+# app sets Compile / run / baseDirectory to its own baseDirectory, so
+# `app/run` is expected to execute with app/ as its working directory.
+
+$ copy-file changes/a.sbt build.sbt
+> reload
+> app/check
+
+> app/run
+$ exists flag
+$ absent app/flag
+$ delete flag
+
+$ copy-file changes/b.sbt build.sbt
+> reload
+> app/check
+
+> app/run
+$ exists app/flag
+$ absent flag
+$ delete app/flag

sbt-remote-cache/src/main/scala/sbt/internal/GrpcActionCacheStore.scala

@@ -142,9 +142,12 @@ object GrpcActionCacheStore:
 
   class AuthCallCredentials(remoteHeaders: List[String]) extends CallCredentials:
     val pairs = remoteHeaders.map: h =>
-      h.split("=").toList match
+      // Split on the first '=' only. Splitting on every '=' would drop trailing
+      // padding from values such as Basic auth credentials ("Basic dXNlcjpwdw==")
+      // and reject values that legitimately contain '='.
+      h.split("=", 2).toList match
         case List(k, v) => Metadata.Key.of(k, Metadata.ASCII_STRING_MARSHALLER) -> v
-        case _          => sys.error("remote header must contain one '='")
+        case _          => sys.error("remote header must contain '='")
     override def applyRequestMetadata(
         requestInfo: CallCredentials.RequestInfo,
         executor: java.util.concurrent.Executor,

sbt-remote-cache/src/test/scala/sbt/internal/GrpcActionCacheStoreTest.scala

@@ -36,6 +36,27 @@ object GrpcActionCacheStoreTest extends verify.BasicTestSuite:
     // Distinct Deadline instances derived at call time, not a single shared frozen one.
     assert(!deadline1.eq(deadline2))
 
+  // Regression test: header values may legitimately contain '=' -- Basic auth credentials
+  // end in base64 padding. Splitting on every '=' silently truncated the value, so the
+  // server rejected the credential with UNAUTHENTICATED while the build still succeeded,
+  // leaving the cache permanently empty with no error reported.
+  test("header values retain '=' such as base64 padding"):
+    val twoPad = GrpcActionCacheStore.AuthCallCredentials(List("authorization=Basic dXNlcjpwdw=="))
+    val (key, value) = twoPad.pairs.head
+    assert(key.name == "authorization")
+    assert(value == "Basic dXNlcjpwdw==")
+
+    val onePad = GrpcActionCacheStore.AuthCallCredentials(List("authorization=Basic dXNlcjpwdzE="))
+    assert(onePad.pairs.head._2 == "Basic dXNlcjpwdzE=")
+
+    // An interior '=' is part of the value, not a second separator.
+    val interior = GrpcActionCacheStore.AuthCallCredentials(List("x-api-key=ab=cd"))
+    assert(interior.pairs.head._2 == "ab=cd")
+
+    // No '=' at all remains an error.
+    intercept[RuntimeException]:
+      GrpcActionCacheStore.AuthCallCredentials(List("bogus")).pairs
+
   private def newStore(): GrpcActionCacheStore =
     val base = Files.createTempDirectory("grpc-action-cache-test")
     val disk = DiskActionCacheStore(base, PlainVirtualFileConverter.converter)