From 5f595b23a4d7e6f9dd8cc5e0af61926021141c8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Raddum=20Berg?= Date: Thu, 25 Jun 2020 23:17:47 +0200 Subject: [PATCH 1/4] Reuse thread pool from `FileCache` --- .../lmcoursier/internal/ArtifactsRun.scala | 90 +++++++++---------- .../lmcoursier/internal/ResolutionRun.scala | 85 +++++++++--------- 2 files changed, 83 insertions(+), 92 deletions(-) diff --git a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala index f208fd2ec..356aaf447 100644 --- a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala +++ b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala @@ -2,7 +2,6 @@ package lmcoursier.internal import java.io.File -import coursier.cache.internal.ThreadUtil import coursier.cache.loggers.{FallbackRefreshDisplay, ProgressBarRefreshDisplay, RefreshLogger} import coursier.core.Type import coursier.util.Artifact @@ -38,53 +37,50 @@ object ArtifactsRun { else "" - ThreadUtil.withFixedThreadPool(params.parallel) { pool => - - coursier.Artifacts() - .withResolutions(params.resolutions) - .withArtifactTypes(Set(Type.all)) - .withClassifiers(params.classifiers.getOrElse(Nil).toSet) - .withClasspathOrder(params.classpathOrder) - .addExtraArtifacts { l => - if (params.includeSignatures) - l.flatMap(_._3.extra.get("sig").toSeq) - else - Nil - } - .addTransformArtifacts { artifacts => - if (params.missingOk) - artifacts.map { - case (dependency, publication, artifact) => - (dependency, publication, artifact.withOptional(true)) + coursier.Artifacts() + .withResolutions(params.resolutions) + .withArtifactTypes(Set(Type.all)) + .withClassifiers(params.classifiers.getOrElse(Nil).toSet) + .withClasspathOrder(params.classpathOrder) + .addExtraArtifacts { l => + if (params.includeSignatures) + l.flatMap(_._3.extra.get("sig").toSeq) + else + Nil + } + .addTransformArtifacts { artifacts => + if (params.missingOk) + artifacts.map { + case (dependency, publication, artifact) => + (dependency, publication, artifact.withOptional(true)) + } + else + artifacts + } + .withCache( + params + .cache + .withLogger( + params.loggerOpt.getOrElse { + RefreshLogger.create( + if (RefreshLogger.defaultFallbackMode) + new FallbackRefreshDisplay() + else + ProgressBarRefreshDisplay.create( + if (printOptionalMessage) log.info(artifactInitialMessage), + if (printOptionalMessage || verbosityLevel >= 2) + log.info( + s"Fetched artifacts of ${params.projectName}" + + (if (params.sbtClassifiers) " (sbt classifiers)" else "") + ) + ) + ) } - else - artifacts - } - .withCache( - params - .cache - .withPool(pool) - .withLogger( - params.loggerOpt.getOrElse { - RefreshLogger.create( - if (RefreshLogger.defaultFallbackMode) - new FallbackRefreshDisplay() - else - ProgressBarRefreshDisplay.create( - if (printOptionalMessage) log.info(artifactInitialMessage), - if (printOptionalMessage || verbosityLevel >= 2) - log.info( - s"Fetched artifacts of ${params.projectName}" + - (if (params.sbtClassifiers) " (sbt classifiers)" else "") - ) - ) - ) - } - ) - ) - .eitherResult() - .map(_.fullDetailedArtifacts) // FIXME Misses extraArtifacts, that we don't use for now though - } + ) + ) + .eitherResult() + .map(_.fullDetailedArtifacts) // FIXME Misses extraArtifacts, that we don't use for now though + } } diff --git a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala index 3a976d71d..0f9c6d8ea 100644 --- a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala +++ b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala @@ -1,6 +1,5 @@ package lmcoursier.internal -import coursier.cache.internal.ThreadUtil import coursier.{Resolution, Resolve} import coursier.cache.loggers.{FallbackRefreshDisplay, ProgressBarRefreshDisplay, RefreshLogger} import coursier.core._ @@ -80,51 +79,47 @@ object ResolutionRun { if (verbosityLevel >= 2) log.info(initialMessage) - ThreadUtil.withFixedThreadPool(params.parallel) { pool => - - Resolve() - // re-using various caches from a resolution of a configuration we extend - .withInitialResolution(startingResolutionOpt) - .withDependencies( - params.dependencies.collect { - case (config, dep) if configs(config) => - dep - } - ) - .withRepositories(repositories) - .withResolutionParams( - params - .params - .addForceVersion((if (isSandboxConfig) Nil else params.interProjectDependencies.map(_.moduleVersion)): _*) - .withForceScalaVersion(params.autoScalaLibOpt.nonEmpty) - .withScalaVersionOpt(params.autoScalaLibOpt.map(_._2)) - .withTypelevel(params.params.typelevel) - .withRules(rules) - ) - .withCache( - params - .cache - .withPool(pool) - .withLogger( - params.loggerOpt.getOrElse { - RefreshLogger.create( - if (RefreshLogger.defaultFallbackMode) - new FallbackRefreshDisplay() - else - ProgressBarRefreshDisplay.create( - if (printOptionalMessage) log.info(initialMessage), - if (printOptionalMessage || verbosityLevel >= 2) - log.info(s"Resolved ${params.projectName} dependencies") - ) - ) - } - ) - ) - .either() match { - case Left(err) if params.missingOk => Right(err.resolution) - case others => others + Resolve() + // re-using various caches from a resolution of a configuration we extend + .withInitialResolution(startingResolutionOpt) + .withDependencies( + params.dependencies.collect { + case (config, dep) if configs(config) => + dep } - } + ) + .withRepositories(repositories) + .withResolutionParams( + params + .params + .addForceVersion((if (isSandboxConfig) Nil else params.interProjectDependencies.map(_.moduleVersion)): _*) + .withForceScalaVersion(params.autoScalaLibOpt.nonEmpty) + .withScalaVersionOpt(params.autoScalaLibOpt.map(_._2)) + .withTypelevel(params.params.typelevel) + .withRules(rules) + ) + .withCache( + params + .cache + .withLogger( + params.loggerOpt.getOrElse { + RefreshLogger.create( + if (RefreshLogger.defaultFallbackMode) + new FallbackRefreshDisplay() + else + ProgressBarRefreshDisplay.create( + if (printOptionalMessage) log.info(initialMessage), + if (printOptionalMessage || verbosityLevel >= 2) + log.info(s"Resolved ${params.projectName} dependencies") + ) + ) + } + ) + ) + .either() match { + case Left(err) if params.missingOk => Right(err.resolution) + case others => others + } } def resolutions( From e601f843e5379e037fd7734ae7287f9b48d41d9a Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Sat, 13 Jun 2020 16:20:51 +0200 Subject: [PATCH 2/4] Allow several resolutions to run at a time with fallback logger --- .../lmcoursier/internal/ArtifactsRun.scala | 122 ++++++++++-------- 1 file changed, 66 insertions(+), 56 deletions(-) diff --git a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala index 356aaf447..1dece31b0 100644 --- a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala +++ b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala @@ -2,6 +2,7 @@ package lmcoursier.internal import java.io.File +import coursier.cache.CacheLogger import coursier.cache.loggers.{FallbackRefreshDisplay, ProgressBarRefreshDisplay, RefreshLogger} import coursier.core.Type import coursier.util.Artifact @@ -17,70 +18,79 @@ object ArtifactsRun { verbosityLevel: Int, log: Logger ): Either[coursier.error.FetchError, Map[Artifact, File]] = - artifactsResult(params, verbosityLevel, log).map(_.collect { case (_, _, a, Some(f)) => (a, f) }.toMap) + artifactsResult(params, verbosityLevel, log) + .map(_.collect { case (_, _, a, Some(f)) => (a, f) }.toMap) def artifactsResult( params: ArtifactsParams, verbosityLevel: Int, log: Logger - ): Either[coursier.error.FetchError, Seq[(Dependency, Publication, Artifact, Option[File])]] = - // let's update only one module at once, for a better output - // Downloads are already parallel, no need to parallelize further anyway - Lock.lock.synchronized { + ): Either[coursier.error.FetchError, Seq[(Dependency, Publication, Artifact, Option[File])]] = { - val printOptionalMessage = verbosityLevel >= 0 && verbosityLevel <= 1 + val printOptionalMessage = verbosityLevel >= 0 && verbosityLevel <= 1 - val artifactInitialMessage = - if (verbosityLevel >= 0) - s"Fetching artifacts of ${params.projectName}" + - (if (params.sbtClassifiers) " (sbt classifiers)" else "") + val artifactInitialMessage = + if (verbosityLevel >= 0) + s"Fetching artifacts of ${params.projectName}" + + (if (params.sbtClassifiers) " (sbt classifiers)" else "") + else + "" + + // Ensuring only one resolution / artifact fetching runs at a time when the logger + // may rely on progress bars, as two progress bar loggers can't display stuff at the + // same time. + val needsLock = params.loggerOpt.nonEmpty || !RefreshLogger.defaultFallbackMode + + val coursierLogger = params.loggerOpt.getOrElse { + RefreshLogger.create( + if (RefreshLogger.defaultFallbackMode) + new FallbackRefreshDisplay() else - "" - - coursier.Artifacts() - .withResolutions(params.resolutions) - .withArtifactTypes(Set(Type.all)) - .withClassifiers(params.classifiers.getOrElse(Nil).toSet) - .withClasspathOrder(params.classpathOrder) - .addExtraArtifacts { l => - if (params.includeSignatures) - l.flatMap(_._3.extra.get("sig").toSeq) - else - Nil - } - .addTransformArtifacts { artifacts => - if (params.missingOk) - artifacts.map { - case (dependency, publication, artifact) => - (dependency, publication, artifact.withOptional(true)) - } - else - artifacts - } - .withCache( - params - .cache - .withLogger( - params.loggerOpt.getOrElse { - RefreshLogger.create( - if (RefreshLogger.defaultFallbackMode) - new FallbackRefreshDisplay() - else - ProgressBarRefreshDisplay.create( - if (printOptionalMessage) log.info(artifactInitialMessage), - if (printOptionalMessage || verbosityLevel >= 2) - log.info( - s"Fetched artifacts of ${params.projectName}" + - (if (params.sbtClassifiers) " (sbt classifiers)" else "") - ) - ) - ) - } - ) - ) - .eitherResult() - .map(_.fullDetailedArtifacts) // FIXME Misses extraArtifacts, that we don't use for now though - + ProgressBarRefreshDisplay.create( + if (printOptionalMessage) log.info(artifactInitialMessage), + if (printOptionalMessage || verbosityLevel >= 2) + log.info( + s"Fetched artifacts of ${params.projectName}" + + (if (params.sbtClassifiers) " (sbt classifiers)" else "") + ) + ) + ) } + if (needsLock) + Lock.lock.synchronized { + artifactsResultNoLock(params, coursierLogger) + } + else + artifactsResultNoLock(params, coursierLogger) + } + + private def artifactsResultNoLock( + params: ArtifactsParams, + coursierLogger: CacheLogger + ): Either[coursier.error.FetchError, Seq[(Dependency, Publication, Artifact, Option[File])]] = + coursier.Artifacts() + .withResolutions(params.resolutions) + .withArtifactTypes(Set(Type.all)) + .withClassifiers(params.classifiers.getOrElse(Nil).toSet) + .withClasspathOrder(params.classpathOrder) + .addExtraArtifacts { l => + if (params.includeSignatures) + l.flatMap(_._3.extra.get("sig").toSeq) + else + Nil + } + .addTransformArtifacts { artifacts => + if (params.missingOk) + artifacts.map { + case (dependency, publication, artifact) => + (dependency, publication, artifact.withOptional(true)) + } + else + artifacts + } + .withCache(params.cache.withLogger(coursierLogger)) + .eitherResult() + .map(_.fullDetailedArtifacts) // FIXME Misses extraArtifacts, that we don't use for now though + } From f5f3fd12f841a96df76c85a54c3b2ac66ad51a6d Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Sat, 13 Jun 2020 16:27:53 +0200 Subject: [PATCH 3/4] Refacto --- .../CoursierDependencyResolution.scala | 4 +-- .../lmcoursier/internal/ArtifactsRun.scala | 29 +++++-------------- .../coursier/sbtcoursier/ArtifactsTasks.scala | 4 +-- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/modules/lm-coursier/src/main/scala/lmcoursier/CoursierDependencyResolution.scala b/modules/lm-coursier/src/main/scala/lmcoursier/CoursierDependencyResolution.scala index 94ebf893e..4a9639567 100644 --- a/modules/lm-coursier/src/main/scala/lmcoursier/CoursierDependencyResolution.scala +++ b/modules/lm-coursier/src/main/scala/lmcoursier/CoursierDependencyResolution.scala @@ -222,9 +222,9 @@ class CoursierDependencyResolution(conf: CoursierConfiguration) extends Dependen val e = for { resolutions <- ResolutionRun.resolutions(resolutionParams, verbosityLevel, log) artifactsParams0 = artifactsParams(resolutions) - artifacts <- ArtifactsRun.artifactsResult(artifactsParams0, verbosityLevel, log) + artifacts <- ArtifactsRun(artifactsParams0, verbosityLevel, log) } yield { - val updateParams0 = updateParams(resolutions, artifacts) + val updateParams0 = updateParams(resolutions, artifacts.fullDetailedArtifacts) UpdateRun.update(updateParams0, verbosityLevel, log) } e.left.map(unresolvedWarningOrThrow(uwconfig, _)) diff --git a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala index 1dece31b0..839cc7868 100644 --- a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala +++ b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala @@ -1,31 +1,19 @@ package lmcoursier.internal -import java.io.File - +import coursier.Artifacts import coursier.cache.CacheLogger import coursier.cache.loggers.{FallbackRefreshDisplay, ProgressBarRefreshDisplay, RefreshLogger} import coursier.core.Type -import coursier.util.Artifact import sbt.util.Logger -import coursier.core.Dependency -import coursier.core.Publication -// private[coursier] +// private[lmcoursier] object ArtifactsRun { - def artifacts( + def apply( params: ArtifactsParams, verbosityLevel: Int, log: Logger - ): Either[coursier.error.FetchError, Map[Artifact, File]] = - artifactsResult(params, verbosityLevel, log) - .map(_.collect { case (_, _, a, Some(f)) => (a, f) }.toMap) - - def artifactsResult( - params: ArtifactsParams, - verbosityLevel: Int, - log: Logger - ): Either[coursier.error.FetchError, Seq[(Dependency, Publication, Artifact, Option[File])]] = { + ): Either[coursier.error.FetchError, Artifacts.Result] = { val printOptionalMessage = verbosityLevel >= 0 && verbosityLevel <= 1 @@ -59,16 +47,16 @@ object ArtifactsRun { if (needsLock) Lock.lock.synchronized { - artifactsResultNoLock(params, coursierLogger) + result(params, coursierLogger) } else - artifactsResultNoLock(params, coursierLogger) + result(params, coursierLogger) } - private def artifactsResultNoLock( + private def result( params: ArtifactsParams, coursierLogger: CacheLogger - ): Either[coursier.error.FetchError, Seq[(Dependency, Publication, Artifact, Option[File])]] = + ): Either[coursier.error.FetchError, Artifacts.Result] = coursier.Artifacts() .withResolutions(params.resolutions) .withArtifactTypes(Set(Type.all)) @@ -91,6 +79,5 @@ object ArtifactsRun { } .withCache(params.cache.withLogger(coursierLogger)) .eitherResult() - .map(_.fullDetailedArtifacts) // FIXME Misses extraArtifacts, that we don't use for now though } diff --git a/modules/sbt-coursier/src/main/scala/coursier/sbtcoursier/ArtifactsTasks.scala b/modules/sbt-coursier/src/main/scala/coursier/sbtcoursier/ArtifactsTasks.scala index 39e3a24db..e31a540ff 100644 --- a/modules/sbt-coursier/src/main/scala/coursier/sbtcoursier/ArtifactsTasks.scala +++ b/modules/sbt-coursier/src/main/scala/coursier/sbtcoursier/ArtifactsTasks.scala @@ -75,7 +75,7 @@ object ArtifactsTasks { missingOk = sbtClassifiers ) - val resOrError = ArtifactsRun.artifacts( + val resOrError = ArtifactsRun( params, verbosityLevel, log @@ -85,7 +85,7 @@ object ArtifactsTasks { case Left(err) => throw err case Right(res0) => - res0 + res0.artifacts.toMap } } } From 315a4bb4c0a8c33f3df96b27f7fceba1939e30df Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 26 Jun 2020 13:16:28 +0200 Subject: [PATCH 4/4] Fix Trapped errors? --- .../src/main/scala/lmcoursier/internal/ResolutionRun.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala index 0f9c6d8ea..9321cee43 100644 --- a/modules/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala +++ b/modules/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala @@ -183,7 +183,7 @@ object ResolutionRun { () } } - either.map(_ => map.toMap) + withSubResolutions.map(_ => map.toMap) } for (res <- resOrError) SbtCoursierCache.default.putResolution(params.resolutionKey, res)