[2.x] fix: addressing review comments

main/src/main/scala/sbt/RepositoryUpdateStrategy.scala

@@ -16,20 +16,18 @@ import scala.concurrent.duration.FiniteDuration
  * Used with the `repositoryUpdateStrategy` setting key to control when
  * remote project dependencies are updated from their upstream repositories.
  */
-sealed trait RepositoryUpdateStrategy
-
-object RepositoryUpdateStrategy {
+enum RepositoryUpdateStrategy {
 
   /**
    * Never update automatically.
    * Users must manually delete staging directories or run `updateRemoteProjects`.
    */
-  case object Manual extends RepositoryUpdateStrategy
+  case Manual
 
   /** Update on every `update` task invocation. */
-  case object Always extends RepositoryUpdateStrategy
+  case Always
 
   /** Update at most once per the given interval. */
-  final case class Every(interval: FiniteDuration) extends RepositoryUpdateStrategy
+  case Every(interval: FiniteDuration)
 
 }

main/src/main/scala/sbt/Resolvers.scala

@@ -22,7 +22,7 @@ import java.util.Locale
 import scala.sys.process.{ BasicIO, Process }
 import scala.util.control.NonFatal
 import sbt.internal.util.Util
-import sbt.internal.{ BuildDependencies, LoadedBuild }
+import sbt.internal.{ BuildDependencies, LoadedBuild, LoadedBuildUnit }
 import sbt.util.Logger
 import sbt.internal.RetrieveUnit
 
@@ -209,17 +209,19 @@ object Resolvers {
       extracted: Extracted,
       force: Boolean
   ): Boolean = {
+    def needsUpdate(uri: URI, unit: LoadedBuildUnit): Boolean = {
+      val rootProjectId = unit.rootProjects.headOption.getOrElse("root")
+      val projectRef = ProjectRef(uri, rootProjectId)
+      val strategy = extracted
+        .getOpt(projectRef / Keys.repositoryUpdateStrategy)
+        .getOrElse(RepositoryUpdateStrategy.Manual)
+      shouldUpdate(unit.localBase, strategy)
+    }
+
     log.info("Updating remote repos")
     val repos = (for {
       (uri, unit) <- lb.units if unit.localBase.exists()
-      vcs <- RetrieveUnit(uri) if (force || {
-        val rootProjectId = unit.rootProjects.headOption.getOrElse("root")
-        val projectRef = ProjectRef(uri, rootProjectId)
-        val strategy = extracted
-          .getOpt(projectRef / Keys.repositoryUpdateStrategy)
-          .getOrElse(RepositoryUpdateStrategy.Manual)
-        shouldUpdate(unit.localBase, strategy)
-      })
+      vcs <- RetrieveUnit(uri) if force || needsUpdate(uri, unit)
     } yield (unit.localBase, vcs))
 
     val isUpdated = repos.foldLeft(false) { case (acc, (repo, vcs)) =>
@@ -270,7 +272,10 @@ object Resolvers {
   }
 
   private[sbt] def updateRepository(localCopy: File, uri: URI, log: Logger): Boolean =
-    RetrieveUnit(uri).exists(updateRepository(localCopy, _, log))
+    RetrieveUnit(uri) match {
+      case Some(vcs) => updateRepository(localCopy, vcs, log)
+      case None      => false
+    }
 
   private[sbt] def updateRepository(localCopy: File, vcs: RemoteVcs, log: Logger): Boolean =
     vcs match {
@@ -281,12 +286,21 @@ object Resolvers {
 
   private def updateGit(localCopy: File, uri: URI, log: Logger): Boolean =
     try {
-      val headBefore = captureOutput(Some(localCopy), "git", "rev-parse", "HEAD")
-      val ref = if (fromURI(uri).hasFragment) uri.getFragment else "HEAD"
-      run(Some(localCopy), Some(log), "git", "fetch", "origin", ref)
-      run(Some(localCopy), Some(log), "git", "reset", "--hard", "FETCH_HEAD")
-      markUpdated(localCopy)
-      captureOutput(Some(localCopy), "git", "rev-parse", "HEAD") != headBefore
+      val status = captureOutput(Some(localCopy), "git", "status", "--porcelain", "-uno")
+      if (status.nonEmpty) {
+        log.warn(
+          s"Skipping update of $localCopy: uncommitted changes detected. " +
+            "Commit or discard them before updating."
+        )
+        false
+      } else {
+        val headBefore = captureOutput(Some(localCopy), "git", "rev-parse", "HEAD")
+        val ref = if (fromURI(uri).hasFragment) uri.getFragment else "HEAD"
+        run(Some(localCopy), Some(log), "git", "fetch", "origin", ref)
+        run(Some(localCopy), Some(log), "git", "reset", "--hard", "FETCH_HEAD")
+        markUpdated(localCopy)
+        captureOutput(Some(localCopy), "git", "rev-parse", "HEAD") != headBefore
+      }
     } catch {
       case NonFatal(e) =>
         log.error(
@@ -303,6 +317,7 @@ object Resolvers {
 
   private def updateMercurial(localCopy: File, uri: URI, log: Logger): Boolean =
     try {
+      val idBefore = captureOutput(Some(localCopy), "hg", "id", "-i")
       if (fromURI(uri).hasFragment) {
         val branch = uri.getFragment
         run(Some(localCopy), Some(log), "hg", "pull")
@@ -311,7 +326,7 @@ object Resolvers {
         run(Some(localCopy), Some(log), "hg", "pull", "-u")
       }
       markUpdated(localCopy)
-      true
+      captureOutput(Some(localCopy), "hg", "id", "-i") != idBefore
     } catch {
       case NonFatal(e) =>
         log.error(
@@ -322,6 +337,7 @@ object Resolvers {
 
   private def updateSubversion(localCopy: File, uri: URI, log: Logger): Boolean =
     try {
+      val revBefore = captureOutput(Some(localCopy), "svn", "info", "--show-item", "revision")
       if (fromURI(uri).hasFragment) {
         val revision = uri.getFragment
         run(Some(localCopy), Some(log), "svn", "update", "-q", "-r", revision)
@@ -329,7 +345,7 @@ object Resolvers {
         run(Some(localCopy), Some(log), "svn", "update", "-q")
       }
       markUpdated(localCopy)
-      true
+      captureOutput(Some(localCopy), "svn", "info", "--show-item", "revision") != revBefore
     } catch {
       case NonFatal(e) =>
         log.error(

notes/2.0.0/remote-project-update.md

@@ -0,0 +1,25 @@
+### Updating remote VCS project dependencies
+
+sbt now supports automatic updating of remote VCS project dependencies
+(Git, Mercurial, Subversion) via a configurable update strategy.
+
+By default the strategy is `Manual` — remote projects are never updated
+automatically during `update`. To change this, set `repositoryUpdateStrategy`
+per dependency:
+
+```scala
+lazy val dep = RootProject(uri("git:file:///path/to/repo"))
+
+lazy val root = project.dependsOn(dep).settings(
+  // Pull latest on every `update`:
+  dep / repositoryUpdateStrategy := RepositoryUpdateStrategy.Always,
+  // Or pull at most once per hour:
+  dep / repositoryUpdateStrategy := RepositoryUpdateStrategy.Every(1.hour),
+)
+```
+
+When changes are detected, sbt will warn that a `reload` is needed to pick
+up the new sources.
+
+The `updateRemoteProjects` command force-updates all remote VCS dependencies
+regardless of strategy and automatically reloads the build.

sbt-app/src/sbt-test/project/remote-project-dedup/changes/MainWithNew.scala

@@ -1,13 +1,7 @@
+import upstream.{ Service, NewService }
+
 object Main {
   def main(args: Array[String]): Unit = {
-    val cls = Class.forName("upstream.Service")
-    val value = cls.getMethod("provide").invoke(null)
-    assert(value == "service", s"Unexpected Service.provide: '$value'")
-    println(s"Service loaded: $value")
-
-    val newCls = Class.forName("upstream.NewService")
-    val newValue = newCls.getMethod("provide").invoke(null)
-    assert(newValue == "new-service", s"Unexpected NewService.provide: '$newValue'")
-    println(s"NewService loaded: $newValue")
+    val _ = (Service.provide, NewService.provide)
   }
 }

sbt-app/src/sbt-test/project/remote-project-dedup/changes/build-with-deps.sbt

@@ -3,6 +3,36 @@ lazy val b = (project in file("b")).dependsOn(a)
 lazy val c = (project in file("c")).dependsOn(a)
 lazy val d = (project in file("d")).dependsOn(b, c)
 
+lazy val snapshotReflog = taskKey[Unit]("Records current HEAD reflog count in staging repo")
+lazy val checkFetchCount = inputKey[Unit]("Asserts exactly N git resets happened since snapshot")
+
 lazy val root = (project in file(".")).aggregate(b, c, d).settings(
-  a / repositoryUpdateStrategy := RepositoryUpdateStrategy.Always
+  a / repositoryUpdateStrategy := RepositoryUpdateStrategy.Always,
+  snapshotReflog := {
+    val staging = baseDirectory.value / "global" / "staging"
+    for {
+      hashDir <- staging.listFiles() if hashDir.isDirectory
+      repoDir <- hashDir.listFiles() if repoDir.isDirectory && (repoDir / ".git").exists()
+    } {
+      val headLog = repoDir / ".git" / "logs" / "HEAD"
+      val count = if (headLog.exists()) IO.readLines(headLog).size else 0
+      IO.write(repoDir / ".git" / "reflog-baseline", count.toString)
+    }
+  },
+  checkFetchCount := {
+    val expected = complete.Parsers.spaceDelimited("<expected>").parsed.head.toInt
+    val staging = baseDirectory.value / "global" / "staging"
+    val gitDirs = for {
+      hashDir <- staging.listFiles() if hashDir.isDirectory
+      repoDir <- hashDir.listFiles() if repoDir.isDirectory && (repoDir / ".git").exists()
+    } yield repoDir
+    assert(gitDirs.length == 1, s"Expected 1 staging repo, got ${gitDirs.length}")
+    val repo = gitDirs.head
+    val headLog = repo / ".git" / "logs" / "HEAD"
+    val currentCount = if (headLog.exists()) IO.readLines(headLog).size else 0
+    val baseline = repo / ".git" / "reflog-baseline"
+    val beforeCount = if (baseline.exists()) IO.read(baseline).trim.toInt else 0
+    val fetchCount = currentCount - beforeCount
+    assert(fetchCount == expected, s"Expected fetch count $expected, got $fetchCount")
+  }
 )

sbt-app/src/sbt-test/project/remote-project-dedup/d/src/main/scala/Main.scala

@@ -1,8 +1,7 @@
+import upstream.Service
+
 object Main {
   def main(args: Array[String]): Unit = {
-    val cls = Class.forName("upstream.Service")
-    val value = cls.getMethod("provide").invoke(null)
-    assert(value == "service", s"Unexpected Service.provide: '$value'")
-    println(s"Service loaded: $value")
+    val _ = Service.provide
   }
 }

sbt-app/src/sbt-test/project/remote-project-dedup/test

@@ -1,14 +1,15 @@
-# Checks that `d` project:
-# - Receives changes from a transitive dependency
-# - Calls `git pull` on a indirectly depended VCS project
-
 $ exec bash setup.sh
 $ copy-file changes/build-with-deps.sbt build.sbt
 > reload
-> d/run
+> compile
 
 $ exec bash add-newservice.sh
 $ copy-file changes/MainWithNew.scala d/src/main/scala/Main.scala
+
+# Snapshot HEAD reflog, update, verify exactly 1 fetch (dedup across diamond)
+> snapshotReflog
 > d/update
+> checkFetchCount 1
+
 > reload
-> d/run
+> compile

sbt-app/src/sbt-test/project/remote-project-git-branch/changes/MainWithBranchOnly.scala

@@ -1,13 +1,7 @@
+import upstream.{ Greeter, BranchOnly }
+
 object Main {
   def main(args: Array[String]): Unit = {
-    val cls = Class.forName("upstream.Greeter")
-    val value = cls.getMethod("greet").invoke(null)
-    assert(value == "hello-from-feature", s"Expected 'hello-from-feature' but got '$value'")
-    println(s"Greeter loaded: $value")
-
-    val branchCls = Class.forName("upstream.BranchOnly")
-    val branchValue = branchCls.getMethod("value").invoke(null)
-    assert(branchValue == "branch-only", s"Expected 'branch-only' but got '$branchValue'")
-    println(s"BranchOnly loaded: $branchValue")
+    val _ = (Greeter.greet, BranchOnly.value)
   }
 }

sbt-app/src/sbt-test/project/remote-project-git-branch/src/main/scala/Main.scala

@@ -1,8 +1,7 @@
+import upstream.Greeter
+
 object Main {
   def main(args: Array[String]): Unit = {
-    val cls = Class.forName("upstream.Greeter")
-    val value = cls.getMethod("greet").invoke(null)
-    assert(value == "hello-from-feature", s"Expected 'hello-from-feature' but got '$value'")
-    println(s"Greeter loaded: $value")
+    val _ = Greeter.greet
   }
 }

sbt-app/src/sbt-test/project/remote-project-git-branch/test

@@ -1,11 +1,11 @@
 $ exec bash setup.sh
 $ copy-file changes/build-with-branch.sbt build.sbt
 > reload
-> run
+> compile
 
 # Ensures that update fetches from the correct branch
 $ exec bash add-branch-class.sh
 $ copy-file changes/MainWithBranchOnly.scala src/main/scala/Main.scala
 > update
 > reload
-> run
+> compile

sbt-app/src/sbt-test/project/remote-project-transitive-deps/src/main/scala/Main.scala

@@ -1,13 +1,7 @@
+import upstream.{ AService, BService }
+
 object Main {
   def main(args: Array[String]): Unit = {
-    val aClass = Class.forName("upstream.AService")
-    val aValue = aClass.getMethod("provide").invoke(null)
-    assert(aValue == "from-A-via-from-B", s"Unexpected AService.provide: '$aValue'")
-    println(s"AService loaded: $aValue")
-
-    val bClass = Class.forName("upstream.BService")
-    val bValue = bClass.getMethod("provide").invoke(null)
-    assert(bValue == "from-B", s"Unexpected BService.provide: '$bValue'")
-    println(s"BService loaded: $bValue")
+    val _ = (AService.provide, BService.provide)
   }
 }

sbt-app/src/sbt-test/project/remote-project-transitive-deps/test

@@ -7,4 +7,4 @@
 $ exec bash setup.sh
 $ copy-file changes/build-with-dep.sbt build.sbt
 > reload
-> run
+> compile

sbt-app/src/sbt-test/project/remote-project-update/add-every-class.sh

@@ -0,0 +1,16 @@
+#!/usr/bin/env bash
+set -e
+
+cd upstream-repo
+git config commit.gpgsign false
+
+cat > src/main/scala/upstream/EveryClass.scala << 'EOF'
+package upstream
+
+object EveryClass {
+  def value: String = "every"
+}
+EOF
+
+git add .
+git commit -m "add EveryClass"

sbt-app/src/sbt-test/project/remote-project-update/changes/MainCheckEvery.scala

@@ -0,0 +1,7 @@
+import upstream.{ Greeter, Helper, Utils, ManualClass, EveryClass }
+
+object Main {
+  def main(args: Array[String]): Unit = {
+    val _ = (Greeter.greet, Helper.help, Utils.util, ManualClass.value, EveryClass.value)
+  }
+}

sbt-app/src/sbt-test/project/remote-project-update/changes/MainCheckManual.scala

@@ -1,28 +1,8 @@
+import upstream.{ Greeter, Helper, Utils }
+
 object Main {
   def main(args: Array[String]): Unit = {
-    val greeterClass = Class.forName("upstream.Greeter")
-    val greet = greeterClass.getMethod("greet").invoke(null)
-    assert(greet == "hello", s"Expected 'hello' but got '$greet'")
-    println(s"Greeter loaded: $greet")
-
-    val helperClass = Class.forName("upstream.Helper")
-    val help = helperClass.getMethod("help").invoke(null)
-    assert(help == "helped", s"Expected 'helped' but got '$help'")
-    println(s"Helper loaded: $help")
-
-    val utilsClass = Class.forName("upstream.Utils")
-    val util = utilsClass.getMethod("util").invoke(null)
-    assert(util == "utility", s"Expected 'utility' but got '$util'")
-    println(s"Utils loaded: $util")
-
-    try {
-      Class.forName("upstream.ManualClass")
-      sys.error("upstream.ManualClass found on classpath")
-    } catch {
-      case _: ClassNotFoundException =>
-        ()
-      case e =>
-        sys.error(s"Unknown error: ${e}")
-    }
+    val _ = (Greeter.greet, Helper.help, Utils.util)
+    val _ = ManualClass.value
   }
 }

sbt-app/src/sbt-test/project/remote-project-update/changes/MainWithHelper.scala

@@ -1,13 +1,7 @@
+import upstream.{ Greeter, Helper }
+
 object Main {
   def main(args: Array[String]): Unit = {
-    val greeterClass = Class.forName("upstream.Greeter")
-    val greet = greeterClass.getMethod("greet").invoke(null)
-    assert(greet == "hello", s"Expected 'hello' but got '$greet'")
-    println(s"Greeter loaded: $greet")
-
-    val helperClass = Class.forName("upstream.Helper")
-    val help = helperClass.getMethod("help").invoke(null)
-    assert(help == "helped", s"Expected 'helped' but got '$help'")
-    println(s"Helper loaded: $help")
+    val _ = (Greeter.greet, Helper.help)
   }
 }

sbt-app/src/sbt-test/project/remote-project-update/changes/MainWithUtils.scala

@@ -1,18 +1,7 @@
+import upstream.{ Greeter, Helper, Utils }
+
 object Main {
   def main(args: Array[String]): Unit = {
-    val greeterClass = Class.forName("upstream.Greeter")
-    val greet = greeterClass.getMethod("greet").invoke(null)
-    assert(greet == "hello", s"Expected 'hello' but got '$greet'")
-    println(s"Greeter loaded: $greet")
-
-    val helperClass = Class.forName("upstream.Helper")
-    val help = helperClass.getMethod("help").invoke(null)
-    assert(help == "helped", s"Expected 'helped' but got '$help'")
-    println(s"Helper loaded: $help")
-
-    val utilsClass = Class.forName("upstream.Utils")
-    val util = utilsClass.getMethod("util").invoke(null)
-    assert(util == "utility", s"Expected 'utility' but got '$util'")
-    println(s"Utils loaded: $util")
+    val _ = (Greeter.greet, Helper.help, Utils.util)
   }
 }

sbt-app/src/sbt-test/project/remote-project-update/changes/build-with-dep-every.sbt

@@ -0,0 +1,7 @@
+import scala.concurrent.duration.*
+
+lazy val dep = RootProject(uri(s"git:file://${System.getProperty("user.dir")}/upstream-repo"))
+
+lazy val root = (project in file(".")).dependsOn(dep).settings(
+  dep / repositoryUpdateStrategy := RepositoryUpdateStrategy.Every(3.second)
+)

sbt-app/src/sbt-test/project/remote-project-update/src/main/scala/Main.scala

@@ -1,8 +1,7 @@
+import upstream.Greeter
+
 object Main {
   def main(args: Array[String]): Unit = {
-    val greeterClass = Class.forName("upstream.Greeter")
-    val greet = greeterClass.getMethod("greet").invoke(null)
-    assert(greet == "hello", s"Expected 'hello' but got '$greet'")
-    println(s"Greeter loaded: $greet")
+    val _ = Greeter.greet
   }
 }

sbt-app/src/sbt-test/project/remote-project-update/test

@@ -2,25 +2,38 @@
 $ exec bash setup.sh
 $ copy-file changes/build-with-dep.sbt build.sbt
 > reload
-> run
+> compile
 
 # Ensures that `updateRemoteProjects` works correctly
 $ exec bash add-helper.sh
 $ copy-file changes/MainWithHelper.scala src/main/scala/Main.scala
 > updateRemoteProjects
-> run
+> compile
 
 # Ensures update with `Always` strategy
 $ exec bash add-utils.sh
 $ copy-file changes/MainWithUtils.scala src/main/scala/Main.scala
 > update
 > reload
-> run
+> compile
 
 # Ensures that `Manual` strategy does nothing on `update`
+# compile must fail
 $ copy-file changes/build-with-dep-manual.sbt build.sbt
 > reload
 $ exec bash add-manual-class.sh
 $ copy-file changes/MainCheckManual.scala src/main/scala/Main.scala
 > update
-> run
+-> compile
+
+# Ensures that `Every` strategy updates after interval elapses
+# compile must fail and succeed after sleep
+$ copy-file changes/build-with-dep-every.sbt build.sbt
+> reload
+$ exec bash add-every-class.sh
+$ copy-file changes/MainCheckEvery.scala src/main/scala/Main.scala
+-> compile
+$ sleep 3000
+> update
+> reload
+> compile