diff --git a/lm-core/src/main/scala/sbt/internal/librarymanagement/JsonUtil.scala b/lm-core/src/main/scala/sbt/internal/librarymanagement/JsonUtil.scala index 59f496b31..7c632dcbd 100644 --- a/lm-core/src/main/scala/sbt/internal/librarymanagement/JsonUtil.scala +++ b/lm-core/src/main/scala/sbt/internal/librarymanagement/JsonUtil.scala @@ -40,26 +40,30 @@ private[sbt] object JsonUtil { oar.organization, oar.name, oar.modules map { mr => - ModuleReport( - mr.module, - mr.artifacts, - mr.missingArtifacts, - mr.status, - mr.publicationDate, - mr.resolver, - mr.artifactResolver, - mr.evicted, - mr.evictedData, - mr.evictedReason, - mr.problem, - mr.homepage, - mr.extraAttributes, - mr.isDefault, - mr.branch, - mr.configurations, - mr.licenses, - filterOutArtificialCallers(mr.callers) - ) + val callers = filterOutArtificialCallers(mr.callers) + // Reuse the instance when filtering changed nothing, so interning is not undone here. + if (callers eq mr.callers) mr + else + ModuleReport( + mr.module, + mr.artifacts, + mr.missingArtifacts, + mr.status, + mr.publicationDate, + mr.resolver, + mr.artifactResolver, + mr.evicted, + mr.evictedData, + mr.evictedReason, + mr.problem, + mr.homepage, + mr.extraAttributes, + mr.isDefault, + mr.branch, + mr.configurations, + mr.licenses, + callers + ) } ) } diff --git a/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportInterner.scala b/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportInterner.scala new file mode 100644 index 000000000..3da755a08 --- /dev/null +++ b/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportInterner.scala @@ -0,0 +1,74 @@ +/* + * 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 sbt.librarymanagement.* + +/** + * Intern pools for the immutable `UpdateReport` components to save memory. + * + * The pools are weak, so an entry lives only as long as some report references it. + */ +object UpdateReportInterner { + + private val configRefs = new WeakInterner[ConfigRef] + private val rules = new WeakInterner[InclExclRule] + private val artifacts = new WeakInterner[Artifact] + private val moduleIds = new WeakInterner[ModuleID] + private val callers = new WeakInterner[Caller] + private val files = new WeakInterner[File] + private val moduleReports = new WeakInterner[ModuleReport] + + def intern(c: ConfigRef): ConfigRef = configRefs.intern(c) + + def intern(r: InclExclRule): InclExclRule = rules.intern(r) + + def intern(f: File): File = files.intern(f) + + def intern(a: Artifact): Artifact = + artifacts.internWith(a) { artifact => + // Canonicalize the nested vectors so value-equal artifacts share their pieces even when only + // encountered once. + if (artifact.configurations.isEmpty) artifact + else artifact.withConfigurations(artifact.configurations.map(intern)) + } + + def intern(m: ModuleID): ModuleID = + moduleIds.internWith(m) { moduleId => + if ( + moduleId.inclusions.isEmpty && moduleId.exclusions.isEmpty && + moduleId.explicitArtifacts.isEmpty + ) moduleId + else + moduleId + .withInclusions(moduleId.inclusions.map(intern)) + .withExclusions(moduleId.exclusions.map(intern)) + .withExplicitArtifacts(moduleId.explicitArtifacts.map(intern)) + } + + def intern(c: Caller): Caller = + callers.internWith(c) { caller => + caller + .withCaller(intern(caller.caller)) + .withCallerConfigurations(caller.callerConfigurations.map(intern)) + } + + def intern(mr: ModuleReport): ModuleReport = + // A publicationDate is a mutable Calendar, so canonicalize such a report but never pool it. + if (mr.publicationDate.isDefined) canonicalize(mr) + else moduleReports.internWith(mr)(canonicalize) + + private def canonicalize(mr: ModuleReport): ModuleReport = + mr.withModule(intern(mr.module)) + .withArtifacts(mr.artifacts.map { case (a, f) => (intern(a), intern(f)) }) + .withMissingArtifacts(mr.missingArtifacts.map(intern)) + .withConfigurations(mr.configurations.map(intern)) + .withCallers(mr.callers.map(intern)) +} diff --git a/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportPersistence.scala b/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportPersistence.scala index 85d8ba1b6..75d67139b 100644 --- a/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportPersistence.scala +++ b/lm-core/src/main/scala/sbt/internal/librarymanagement/UpdateReportPersistence.scala @@ -26,32 +26,13 @@ final case class UpdateReportCache( 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. + * The generated library-management codecs, with the artifact content hash disabled: nothing reads the + * hash back, and computing it re-reads the whole downloaded classpath. * - * 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. + * `fileStringLongIso` is virtual, so overriding it also reaches the `Vector[(Artifact, File)]` nested + * inside the generated `ModuleReportFormat`. */ 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), @@ -60,10 +41,27 @@ object UpdateReportPersistence: end CacheCodec - // Not the stock `LibraryManagementCodec`: see `CacheCodec` for why persisted reports must not - // content-hash the artifacts they name. import CacheCodec.given + /** Interns the modules of a decoded cache, in a pass since the generated reader has no hook. */ + private def internModules(cache: UpdateReportCache): UpdateReportCache = + cache.copy(lite = + UpdateReportLite( + cache.lite.configurations.map(cr => + ConfigurationReportLite( + cr.configuration, + cr.details.map(d => + OrganizationArtifactReport( + d.organization, + d.name, + d.modules.map(UpdateReportInterner.intern) + ) + ) + ) + ) + ) + ) + given updateReportCacheFormat: JsonFormat[UpdateReportCache] = new JsonFormat[UpdateReportCache]: override def read[J]( @@ -78,7 +76,7 @@ object UpdateReportPersistence: val stamps = unbuilder.readField[Map[String, Long]]("stamps") val cachedDescriptor = unbuilder.readField[File]("cachedDescriptor") unbuilder.endObject() - UpdateReportCache(lite, stats, stamps, cachedDescriptor) + internModules(UpdateReportCache(lite, stats, stamps, cachedDescriptor)) case None => deserializationError("Expected JsObject but found None") @@ -109,6 +107,7 @@ object UpdateReportPersistence: .orElse( Try(store.read[UpdateReport]()).toOption .map(toCache) + .map(internModules) ) def writeTo(store: CacheStore, cache: UpdateReportCache): Unit = diff --git a/lm-core/src/main/scala/sbt/internal/librarymanagement/WeakPools.scala b/lm-core/src/main/scala/sbt/internal/librarymanagement/WeakPools.scala new file mode 100644 index 000000000..91108dc53 --- /dev/null +++ b/lm-core/src/main/scala/sbt/internal/librarymanagement/WeakPools.scala @@ -0,0 +1,91 @@ +/* + * 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.lang.ref.{ ReferenceQueue, WeakReference } +import java.util.concurrent.ConcurrentHashMap + +import scala.annotation.tailrec + +/** + * Canonicalizing pool: `intern` returns one instance per distinct value, held weakly. + * + * Ported from zinc's `sbt.internal.inc.WeakInterner` to avoid a new dependency, plus `internWith`. + */ +private[librarymanagement] final class WeakInterner[A <: AnyRef] { + private val stale = new ReferenceQueue[A] + private val pool = new ConcurrentHashMap[WeakValue[A], WeakValue[A]] + + def intern(a: A): A = internWith(a)(identity) + + /** Like `intern`, but applies `canonicalize` only on a miss, since equality is structural. */ + def internWith(a: A)(canonicalize: A => A): A = { + expunge() + lookup(a) match { + case null => publish(canonicalize(a)) + case hit => hit + } + } + + /** The pooled instance value-equal to `a`, or null if there is none. */ + private def lookup(a: A): A = { + val probe = new WeakValue(a, stale) + try + pool.get(probe) match { + case null => null.asInstanceOf[A] + case existing => existing.get // null if it was collected since it matched + } + finally probe.clear() // never enqueue a reference that was not pooled + } + + private def publish(a: A): A = { + val candidate = new WeakValue(a, stale) + @tailrec def attempt(): A = pool.putIfAbsent(candidate, candidate) match { + case null => a + case existing => + existing.get match { + case null => // collected since it matched: drop the dead entry and retry + pool.remove(existing, existing) + attempt() + case canonical => + candidate.clear() + canonical + } + } + attempt() + } + + @tailrec private def expunge(): Unit = stale.poll() match { + case null => () + case dead => + pool.remove(dead, dead) + expunge() + } +} + +/** + * Weak reference that hashes and compares by its referent's value. + * + * The hash is captured eagerly: it must stay stable after the referent is cleared, or the dead entry + * could never be found and removed. + */ +private final class WeakValue[A <: AnyRef](a: A, stale: ReferenceQueue[A]) + extends WeakReference[A](a, stale) { + private val hash: Int = a.hashCode + + override def hashCode(): Int = hash + override def equals(other: Any): Boolean = other match { + case that: WeakValue[?] => + (this `eq` that) || { + val value = get + value != null && value == that.get + } + case _ => false + } +} diff --git a/lm-core/src/test/scala/sbt/internal/librarymanagement/UpdateReportInternerSpec.scala b/lm-core/src/test/scala/sbt/internal/librarymanagement/UpdateReportInternerSpec.scala new file mode 100644 index 000000000..6e6043fd3 --- /dev/null +++ b/lm-core/src/test/scala/sbt/internal/librarymanagement/UpdateReportInternerSpec.scala @@ -0,0 +1,132 @@ +/* + * 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.lang.ref.WeakReference +import java.util.Calendar +import sbt.io.IO +import sbt.librarymanagement.* + +object UpdateReportInternerSpec extends verify.BasicTestSuite: + + test("interning a value-equal ConfigRef returns the same instance"): + val a = ConfigRef("compile") + val b = ConfigRef("compile") + val ia = UpdateReportInterner.intern(a) + assert(ia eq UpdateReportInterner.intern(b), "value-equal ConfigRefs should share one instance") + assert(ia == a, "interning must preserve value equality") + + test("interning a value-equal InclExclRule shares one instance"): + val r1 = InclExclRule().withOrganization("org.bad").withName("bad-lib") + val r2 = InclExclRule().withOrganization("org.bad").withName("bad-lib") + assert(r1 ne r2) + val i1 = UpdateReportInterner.intern(r1) + assert(i1 eq UpdateReportInterner.intern(r2)) + assert(i1 == r1) + + test("interning a value-equal File shares one instance"): + val f1 = new File("/tmp/shared/module.jar") + val f2 = new File("/tmp/shared/module.jar") + assert(f1 ne f2) + assert(UpdateReportInterner.intern(f1) eq UpdateReportInterner.intern(f2)) + + test("interning a value-equal Artifact shares one instance and preserves equality"): + def artifact = + Artifact("lib", "jar", "jar", None, Vector(ConfigRef("compile")), None, Map.empty, None) + val a1 = artifact + val a2 = artifact + assert(a1 ne a2) + val i1 = UpdateReportInterner.intern(a1) + assert(i1 eq UpdateReportInterner.intern(a2)) + assert(i1 == a1) + + test("interning a value-equal ModuleID shares one instance and canonicalizes nested rules"): + def excl = InclExclRule().withOrganization("org.bad").withName("bad") + val m1 = ModuleID("org.example", "lib", "1.0.0").withExclusions(Vector(excl)) + val m2 = ModuleID("org.example", "lib", "1.0.0").withExclusions(Vector(excl)) + assert(m1 ne m2) + val i1 = UpdateReportInterner.intern(m1) + val i2 = UpdateReportInterner.intern(m2) + assert(i1 eq i2, "value-equal ModuleIDs should share one instance") + assert(i1 == m1) + assert(i1.exclusions.head eq i2.exclusions.head, "nested exclusion rules should be shared too") + + test("interning a value-equal Caller shares one instance"): + def caller = + Caller( + ModuleID("org.parent", "parent", "2.0.0"), + Vector(ConfigRef("compile")), + Map.empty, + isForceDependency = false, + isChangingDependency = false, + isTransitiveDependency = true, + isDirectlyForceDependency = false + ) + val c1 = caller + val c2 = caller + assert(c1 ne c2) + assert(UpdateReportInterner.intern(c1) eq UpdateReportInterner.intern(c2)) + + test("interning a value-equal ModuleReport shares one instance"): + IO.withTemporaryDirectory: baseDir => + val jar = new File(baseDir, "pooled.jar") + IO.touch(jar) + def mr = plainModuleReport("org.example", "pooled-lib", jar) + val a = mr + val b = mr + assert(a ne b) + val ia = UpdateReportInterner.intern(a) + assert(ia eq UpdateReportInterner.intern(b), "value-equal reports should share one instance") + assert(ia == a, "pooling must preserve value equality") + + test("a ModuleReport carrying a publicationDate is canonicalized but not pooled"): + IO.withTemporaryDirectory: baseDir => + val jar = new File(baseDir, "dated.jar") + IO.touch(jar) + val epoch = Calendar.getInstance() + epoch.setTimeInMillis(0L) + def mr = plainModuleReport("org.example", "dated-lib", jar).withPublicationDate(Some(epoch)) + val a = UpdateReportInterner.intern(mr) + val b = UpdateReportInterner.intern(mr) + assert(a ne b, "sharing would alias a mutable java.util.Calendar") + assert(a == b, "the two instances must still be value-equal") + assert(a.module eq b.module, "the immutable coordinate inside is still interned") + + test("an interned value is released once nothing else references it"): + // ModuleID has no factory cache and the name is unique, so the pool is the only thing that could + // retain it. A strong pool would pin it for the classloader's life. + val weak = new WeakReference( + UpdateReportInterner.intern(ModuleID("org.example.interner", "released-coordinate", "1.0.0")) + ) + assert(awaitCleared(weak), "the interner must release a value once no report references it") + + test("a pooled ModuleReport is released once nothing else references it"): + IO.withTemporaryDirectory: baseDir => + val jar = new File(baseDir, "transient.jar") + IO.touch(jar) + val weak = new WeakReference( + UpdateReportInterner.intern( + plainModuleReport("org.example.interner", "released-report", jar) + ) + ) + assert(awaitCleared(weak), "the pool must not pin a report no caller references") + + private def plainModuleReport(org: String, name: String, jar: File): ModuleReport = + val artifact = Artifact(name, "jar", "jar", None, Vector.empty, None, Map.empty, None) + ModuleReport(ModuleID(org, name, "1.0.0"), Vector((artifact, jar)), Vector.empty) + .withConfigurations(Vector(ConfigRef("compile"))) + + private def awaitCleared(ref: WeakReference[?]): Boolean = + var i = 0 + while i < 50 && ref.get != null do + System.gc() + Thread.sleep(20) + i += 1 + ref.get == null diff --git a/lm-core/src/test/scala/sbt/internal/librarymanagement/WeakInternerSpec.scala b/lm-core/src/test/scala/sbt/internal/librarymanagement/WeakInternerSpec.scala new file mode 100644 index 000000000..7c307c704 --- /dev/null +++ b/lm-core/src/test/scala/sbt/internal/librarymanagement/WeakInternerSpec.scala @@ -0,0 +1,119 @@ +/* + * 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.lang.ref.WeakReference +import java.util.IdentityHashMap +import java.util.concurrent.{ ConcurrentLinkedQueue, CountDownLatch } + +object WeakInternerSpec extends verify.BasicTestSuite: + + test("intern returns one instance per distinct value"): + val pool = new WeakInterner[Key] + val first = Key("shared") + assert(pool.intern(first) eq first, "the first instance interned is the canonical one") + val second = Key("shared") + assert(first ne second) + assert(pool.intern(second) eq first, "a value-equal input must return the canonical instance") + + test("interning preserves value equality"): + val pool = new WeakInterner[Key] + val k = Key("preserved") + assert(pool.intern(k) == k) + + test("distinct values get distinct instances"): + val pool = new WeakInterner[Key] + val a = pool.intern(Key("a")) + val b = pool.intern(Key("b")) + assert(a ne b) + assert(a == Key("a") && b == Key("b")) + + test("an interned value is released once nothing else references it"): + // Only a weak reference to the canonical instance is kept, so a strong pool would pin it for the + // classloader's life. The pool must let the GC reclaim it. + val pool = new WeakInterner[Key] + val weak = new WeakReference(pool.intern(Key("released"))) + assert(awaitCleared(weak), "the pool must not pin a value no caller references") + + test("a collected entry does not block re-interning the same value"): + // Exercises the retry path: putIfAbsent matches a dead entry, which has to be dropped and the + // fresh candidate published in its place. + val pool = new WeakInterner[Key] + val weak = new WeakReference(pool.intern(Key("recycled"))) + assert(awaitCleared(weak)) + val fresh = Key("recycled") + assert(pool.intern(fresh) eq fresh, "a dead entry must be replaced, not returned") + + test("internWith derives the pooled instance only when the value is not pooled yet"): + val pool = new WeakInterner[Key] + var derived = 0 + val canonicalize: Key => Key = k => + derived += 1 + k + val first = pool.internWith(Key("derived"))(canonicalize) + assert(derived == 1, "the first intern has to derive what it pools") + val second = pool.internWith(Key("derived"))(canonicalize) + assert(second eq first) + assert(derived == 1, "a hit must not derive a value it would only discard") + + test("internWith pools what canonicalize returns, not what it was given"): + // Deriving is only worth having because the pooled instance differs from the argument -- for a + // module report, by holding interned children. Probing by value still has to find it. + val pool = new WeakInterner[Key] + val canonical = Key("canonical") + val pooled = pool.internWith(Key("canonical"))(_ => canonical) + assert(pooled eq canonical) + val later = pool.intern(Key("canonical")) + assert(later eq canonical, "what canonicalize returned is the canonical instance from then on") + + test("an entry survives the collection of a value that probed for it"): + // A miss probes with one instance and pools another, value-equal one, so the two hash to the same + // bucket. Once the probe is collected, expunging it must not take the live entry with it. + val pool = new WeakInterner[Key] + val pooled = pool.internWith(Key("probed"))(_ => Key("probed")) + // The probe is already unreachable; this waits for a GC to have run, which is what would enqueue + // it. The canary is built outside the assert because the assert macro records -- and so retains -- + // every intermediate value it evaluates. + val canary = new WeakReference(Key("collectable")) + assert(awaitCleared(canary)) + val again = pool.intern(Key("probed")) + assert(again eq pooled, "expunge must not evict a live entry a dead probe hashes to") + + test("concurrent interning of one value converges on a single instance"): + // The parallel per-project update tasks hit these pools at once, so publication has to be atomic. + val pool = new WeakInterner[Key] + val results = new ConcurrentLinkedQueue[Key] + val start = new CountDownLatch(1) + val workers = (1 to 8).map: _ => + val t = new Thread(() => + start.await() + var i = 0 + while i < 200 do + results.add(pool.intern(Key("contended"))) + i += 1 + ) + t.start() + t + start.countDown() + workers.foreach(_.join()) + val distinct = new IdentityHashMap[Key, Boolean] + results.forEach(k => distinct.put(k, true)) + assert(distinct.size == 1, s"expected one canonical instance, got ${distinct.size}") + + private case class Key(name: String) + + // Weak references are cleared by the GC, which is only advisory, so retry a bounded number of times + // rather than relying on a single System.gc(). + private def awaitCleared(ref: WeakReference[?]): Boolean = + var i = 0 + while i < 50 && ref.get != null do + System.gc() + Thread.sleep(20) + i += 1 + ref.get == null diff --git a/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala b/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala index 7ee19e4c7..d1c4ee260 100644 --- a/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala +++ b/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala @@ -16,6 +16,7 @@ import coursier.core.{ } import coursier.maven.MavenAttributes import coursier.util.Artifact +import sbt.internal.librarymanagement.UpdateReportInterner import sbt.librarymanagement.{ Artifact as _, Configuration as _, * } import sbt.util.Logger import scala.annotation.nowarn @@ -147,23 +148,26 @@ private[internal] object SbtUpdateReport { sbtMissingArtifacts.toVector ) - rep - // .withStatus(None) - .withPublicationDate(publicationDate) - // .withResolver(None) - // .withArtifactResolver(None) - // .withEvicted(false) - // .withEvictedData(None) - // .withEvictedReason(None) - // .withProblem(None) - .withHomepage(Some(project.info.homePage).filter(_.nonEmpty)) - .withLicenses(project.info.licenses.toVector) - .withExtraAttributes(dependency.module.attributes ++ infoProperties(project)) - // .withIsDefault(None) - // .withBranch(None) - .withConfigurations(project.configurations.keys.toVector.map(c => ConfigRef(c.value))) - .withLicenses(project.info.licenses.toVector) - .withCallers(callers.toVector) + // Intern as each report is built, so a coordinate is one instance across every project. + UpdateReportInterner.intern( + rep + // .withStatus(None) + .withPublicationDate(publicationDate) + // .withResolver(None) + // .withArtifactResolver(None) + // .withEvicted(false) + // .withEvictedData(None) + // .withEvictedReason(None) + // .withProblem(None) + .withHomepage(Some(project.info.homePage).filter(_.nonEmpty)) + .withLicenses(project.info.licenses.toVector) + .withExtraAttributes(dependency.module.attributes ++ infoProperties(project)) + // .withIsDefault(None) + // .withBranch(None) + .withConfigurations(project.configurations.keys.toVector.map(c => ConfigRef(c.value))) + .withLicenses(project.info.licenses.toVector) + .withCallers(callers.toVector) + ) } @nowarn diff --git a/main/src/main/scala/sbt/internal/LibraryManagement.scala b/main/src/main/scala/sbt/internal/LibraryManagement.scala index 73e5f3295..935677cd9 100644 --- a/main/src/main/scala/sbt/internal/LibraryManagement.scala +++ b/main/src/main/scala/sbt/internal/LibraryManagement.scala @@ -265,7 +265,12 @@ private[sbt] object LibraryManagement { else crs1 map { cr => val mrs0 = cr.modules - val mrs1 = mrs0 map { _.withCallers(Vector()) } + // Re-intern what stripping rebuilds: dropping the callers is what makes two projects' + // reports of the same coordinate value-equal, so this is where they collapse to one. + val mrs1 = mrs0 map { mr => + if (mr.callers.isEmpty) mr + else UpdateReportInterner.intern(mr.withCallers(Vector())) + } cr.withModules(mrs1) } ur.withConfigurations(crs2) diff --git a/sbt-app/src/sbt-test/dependency-management/update-report-interning/build.sbt b/sbt-app/src/sbt-test/dependency-management/update-report-interning/build.sbt new file mode 100644 index 000000000..44e03b315 --- /dev/null +++ b/sbt-app/src/sbt-test/dependency-management/update-report-interning/build.sbt @@ -0,0 +1,43 @@ +ThisBuild / scalaVersion := "2.13.16" + +lazy val checkSharedAcrossProjects = taskKey[Unit]("Validates cross-project module report sharing") + +// Two projects resolving the same coordinate independently. Coursier memoizes moduleReport on a key +// that includes the dependees, so each project builds its own instance; they become value-equal only +// once `update` strips the callers, which is where interning collapses them. +lazy val a = project.settings( + libraryDependencies += "org.scala-lang.modules" %% "scala-xml" % "2.4.0" +) + +lazy val b = project.settings( + libraryDependencies += "org.scala-lang.modules" %% "scala-xml" % "2.4.0" +) + +lazy val root = (project in file(".")) + .aggregate(a, b) + .settings( + checkSharedAcrossProjects := { + def xml(ur: UpdateReport) = + ur.configurations + .find(_.configuration.name == "compile") + .getOrElse(sys.error("no compile configuration")) + .modules + .find(_.module.name.startsWith("scala-xml")) + .getOrElse(sys.error("scala-xml not in the report")) + val fromA = xml((a / update).value) + val fromB = xml((b / update).value) + require(fromA == fromB, s"expected value-equal reports, got\n$fromA\nand\n$fromB") + require( + fromA.callers.isEmpty, + s"update should have stripped the callers, got ${fromA.callers}" + ) + require( + fromA eq fromB, + "two projects resolving one coordinate must share a single ModuleReport instance" + ) + require( + fromA.module eq fromB.module, + "the ModuleID inside must be shared too" + ) + } + ) diff --git a/sbt-app/src/sbt-test/dependency-management/update-report-interning/test b/sbt-app/src/sbt-test/dependency-management/update-report-interning/test new file mode 100644 index 000000000..3cb072a6e --- /dev/null +++ b/sbt-app/src/sbt-test/dependency-management/update-report-interning/test @@ -0,0 +1,6 @@ +# Both projects resolve fresh here, so this exercises the coursier construction path +# rather than the cached-read path. +> checkSharedAcrossProjects + +# And again with the reports coming back from the cache. +> checkSharedAcrossProjects