From dc87950dc40d25a2fb9ab47cabb3384b03d2220c Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Tue, 27 Feb 2018 22:54:48 +0100 Subject: [PATCH 1/7] Remove some uses of scalaz.Scalaz.*Ops --- .../coursier/core/ResolutionProcess.scala | 8 +-- .../src/main/scala/coursier/maven/Pom.scala | 52 ++++++++++--------- .../main/scala/coursier/util/Traverse.scala | 20 +++++++ .../scala/coursier/test/PomParsingTests.scala | 10 ++-- 4 files changed, 56 insertions(+), 34 deletions(-) create mode 100644 core/shared/src/main/scala/coursier/util/Traverse.scala diff --git a/core/shared/src/main/scala/coursier/core/ResolutionProcess.scala b/core/shared/src/main/scala/coursier/core/ResolutionProcess.scala index e627977ca..92bc88fde 100644 --- a/core/shared/src/main/scala/coursier/core/ResolutionProcess.scala +++ b/core/shared/src/main/scala/coursier/core/ResolutionProcess.scala @@ -4,7 +4,6 @@ package core import scala.annotation.tailrec import scala.language.higherKinds import scalaz.Monad -import scalaz.Scalaz.{ToFunctorOps, ToBindOps} sealed abstract class ResolutionProcess { @@ -191,8 +190,11 @@ object ResolutionProcess { .toVector .foldLeft(F.point(Vector.empty[((Module, String), Either[Seq[String], (Artifact.Source, Project)])])) { (acc, l) => - for (v <- acc; e <- fetch(l)) - yield v ++ e + F.bind(acc) { v => + F.map(fetch(l)) { e => + v ++ e + } + } } } diff --git a/core/shared/src/main/scala/coursier/maven/Pom.scala b/core/shared/src/main/scala/coursier/maven/Pom.scala index 4a0ba003e..ccc5f3cf2 100644 --- a/core/shared/src/main/scala/coursier/maven/Pom.scala +++ b/core/shared/src/main/scala/coursier/maven/Pom.scala @@ -1,7 +1,7 @@ package coursier.maven import coursier.core._ -import scalaz.Scalaz.{eitherMonad, listInstance, ToTraverseOps} +import coursier.util.Traverse.TraverseOps object Pom { import coursier.util.Xml._ @@ -57,20 +57,23 @@ object Pom { .map(_.children.filter(_.label == "exclusion")) .getOrElse(Seq.empty) - xmlExclusions.toList.traverseU(module(_, defaultArtifactId = Some("*"))).right.map { exclusions => + xmlExclusions + .eitherTraverse(module(_, defaultArtifactId = Some("*"))) + .right + .map { exclusions => - val optional = text(node, "optional", "").right.toSeq.contains("true") + val optional = text(node, "optional", "").right.toSeq.contains("true") - scopeOpt.getOrElse("") -> Dependency( - mod, - version0, - "", - exclusions.map(mod => (mod.organization, mod.name)).toSet, - Attributes(typeOpt.getOrElse(""), classifierOpt.getOrElse("")), - optional, - transitive = true - ) - } + scopeOpt.getOrElse("") -> Dependency( + mod, + version0, + "", + exclusions.map(mod => (mod.organization, mod.name)).toSet, + Attributes(typeOpt.getOrElse(""), classifierOpt.getOrElse("")), + optional, + transitive = true + ) + } } private def profileActivation(node: Node): (Option[Boolean], Activation) = { @@ -125,7 +128,9 @@ object Pom { .getOrElse(Seq.empty) for { - deps <- xmlDeps.toList.traverseU(dependency).right + deps <- xmlDeps + .eitherTraverse(dependency) + .right depMgmts <- node .children @@ -133,8 +138,7 @@ object Pom { .flatMap(_.children.find(_.label == "dependencies")) .map(_.children.filter(_.label == "dependency")) .getOrElse(Seq.empty) - .toList - .traverseU(dependency) + .eitherTraverse(dependency) .right properties <- node @@ -142,8 +146,7 @@ object Pom { .find(_.label == "properties") .map(_.children.collect { case elem if elem.isElement => elem }) .getOrElse(Seq.empty) - .toList - .traverseU(property) + .eitherTraverse(property) .right } yield Profile(id, activeByDefault, activation, deps, depMgmts, properties.toMap) @@ -176,7 +179,7 @@ object Pom { .map(_.children.filter(_.label == "dependency")) .getOrElse(Seq.empty) ) - deps <- xmlDeps.toList.traverseU(dependency).right + deps <- xmlDeps.eitherTraverse(dependency).right xmlDepMgmts <- point( pom.children @@ -185,7 +188,7 @@ object Pom { .map(_.children.filter(_.label == "dependency")) .getOrElse(Seq.empty) ) - depMgmts <- xmlDepMgmts.toList.traverseU(dependency).right + depMgmts <- xmlDepMgmts.eitherTraverse(dependency).right groupId <- Some(projModule.organization).filter(_.nonEmpty) .orElse(parentModuleOpt.map(_.organization).filter(_.nonEmpty)) @@ -211,7 +214,7 @@ object Pom { .map(_.children.collect{case elem if elem.isElement => elem}) .getOrElse(Seq.empty) ) - properties <- xmlProperties.toList.traverseU(property).right + properties <- xmlProperties.eitherTraverse(property).right xmlProfiles <- point( pom @@ -220,7 +223,7 @@ object Pom { .map(_.children.filter(_.label == "profile")) .getOrElse(Seq.empty) ) - profiles <- xmlProfiles.toList.traverseU(profile).right + profiles <- xmlProfiles.eitherTraverse(profile).right extraAttrs <- properties .collectFirst { case ("extraDependencyAttributes", s) => extraAttributes(s) } @@ -307,7 +310,7 @@ object Pom { Project( finalProjModule, version, - (relocationDependencyOpt.toList ::: deps).map { + (relocationDependencyOpt.toSeq ++ deps).map { case (config, dep0) => val dep = extraAttrsMap.get(dep0.moduleVersion).fold(dep0)(attrs => dep0.copy(module = dep0.module.copy(attributes = attrs)) @@ -426,8 +429,7 @@ object Pom { .getOrElse(Seq.empty) xmlSnapshotVersions - .toList - .traverseU(snapshotVersion) + .eitherTraverse(snapshotVersion) .right } } yield { diff --git a/core/shared/src/main/scala/coursier/util/Traverse.scala b/core/shared/src/main/scala/coursier/util/Traverse.scala new file mode 100644 index 000000000..d20d4d069 --- /dev/null +++ b/core/shared/src/main/scala/coursier/util/Traverse.scala @@ -0,0 +1,20 @@ +package coursier.util + +import scala.collection.mutable.ListBuffer + +object Traverse { + + implicit class TraverseOps[T](val seq: Seq[T]) { + def eitherTraverse[L, R](f: T => Either[L, R]): Either[L, Seq[R]] = + // Warning: iterates on the whole sequence no matter what, even if the first element is a Left + seq.foldLeft[Either[L, ListBuffer[R]]](Right(new ListBuffer)) { + case (l @ Left(_), _) => l + case (Right(b), elem) => + f(elem) match { + case Left(l) => Left(l) + case Right(r) => Right(b += r) + } + } + } + +} diff --git a/tests/shared/src/test/scala/coursier/test/PomParsingTests.scala b/tests/shared/src/test/scala/coursier/test/PomParsingTests.scala index f7006d7b6..23aa32550 100644 --- a/tests/shared/src/test/scala/coursier/test/PomParsingTests.scala +++ b/tests/shared/src/test/scala/coursier/test/PomParsingTests.scala @@ -1,11 +1,10 @@ package coursier package test -import utest._ - -import coursier.maven.Pom - import coursier.core.compatibility._ +import coursier.util.Traverse.TraverseOps +import coursier.maven.Pom +import utest._ object PomParsingTests extends TestSuite { @@ -217,7 +216,6 @@ object PomParsingTests extends TestSuite { assert(result == expected) } 'beFineWithCommentsInProperties{ - import scalaz.Scalaz.{eitherMonad, listInstance, ToTraverseOps} val properties = """ @@ -258,7 +256,7 @@ object PomParsingTests extends TestSuite { assert(node.label == "properties") val children = node.children.collect { case elem if elem.isElement => elem } - val props0 = children.toList.traverseU(Pom.property) + val props0 = children.eitherTraverse(Pom.property) assert(props0.isRight) From a83df3e1c867a542f67e39dd8c19ae586b56c4dd Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Tue, 27 Feb 2018 22:54:48 +0100 Subject: [PATCH 2/7] Add coursier.util.ValidationNel --- .../src/main/scala/coursier/CacheParse.scala | 63 ++++++++--------- .../src/main/scala/coursier/CachePolicy.scala | 10 ++- .../main/scala-2.12/coursier/cli/Helper.scala | 18 ++--- .../src/main/scala/coursier/ivy/Pattern.scala | 67 +++++++++---------- .../main/scala/coursier/util/Traverse.scala | 47 +++++++++++-- .../scala/coursier/util/ValidationNel.scala | 27 ++++++++ 6 files changed, 145 insertions(+), 87 deletions(-) create mode 100644 core/shared/src/main/scala/coursier/util/ValidationNel.scala diff --git a/cache/src/main/scala/coursier/CacheParse.scala b/cache/src/main/scala/coursier/CacheParse.scala index 335b76c83..8125af9d9 100644 --- a/cache/src/main/scala/coursier/CacheParse.scala +++ b/cache/src/main/scala/coursier/CacheParse.scala @@ -4,19 +4,16 @@ import java.net.MalformedURLException import coursier.core.Authentication import coursier.ivy.IvyRepository -import coursier.util.Parse - -import scalaz.{Validation, ValidationNel} -import scalaz.Scalaz.vectorInstance -import scalaz.Scalaz.{ToEitherOpsFromEither, ToNelOps, ToTraverseOps, ToValidationOps} +import coursier.util.{Parse, ValidationNel} +import coursier.util.Traverse.TraverseOps object CacheParse { - def repository(s: String): Validation[String, Repository] = + def repository(s: String): Either[String, Repository] = if (s == "ivy2local" || s == "ivy2Local") - Cache.ivy2Local.success + Right(Cache.ivy2Local) else if (s == "ivy2cache" || s == "ivy2Cache") - Cache.ivy2Cache.success + Right(Cache.ivy2Cache) else { val repo = Parse.repository(s) @@ -78,34 +75,38 @@ object CacheParse { Left(s"No password found in user info of URL $url") } } - }.validation + } } def repositories(l: Seq[String]): ValidationNel[String, Seq[Repository]] = - l.toVector.traverseU { s => - repository(s).leftMap(_.wrapNel) + l.toVector.validationNelTraverse { s => + ValidationNel.fromEither(repository(s)) } def cachePolicies(s: String): ValidationNel[String, Seq[CachePolicy]] = - s.split(',').toVector.traverseM[({ type L[X] = ValidationNel[String, X] })#L, CachePolicy] { - case "offline" => - Vector(CachePolicy.LocalOnly).successNel - case "update-local-changing" => - Vector(CachePolicy.LocalUpdateChanging).successNel - case "update-local" => - Vector(CachePolicy.LocalUpdate).successNel - case "update-changing" => - Vector(CachePolicy.UpdateChanging).successNel - case "update" => - Vector(CachePolicy.Update).successNel - case "missing" => - Vector(CachePolicy.FetchMissing).successNel - case "force" => - Vector(CachePolicy.ForceDownload).successNel - case "default" => - Vector(CachePolicy.LocalOnly, CachePolicy.FetchMissing).successNel - case other => - s"Unrecognized mode: $other".failureNel - } + s + .split(',') + .toVector + .validationNelTraverse[String, Seq[CachePolicy]] { + case "offline" => + ValidationNel.success(Seq(CachePolicy.LocalOnly)) + case "update-local-changing" => + ValidationNel.success(Seq(CachePolicy.LocalUpdateChanging)) + case "update-local" => + ValidationNel.success(Seq(CachePolicy.LocalUpdate)) + case "update-changing" => + ValidationNel.success(Seq(CachePolicy.UpdateChanging)) + case "update" => + ValidationNel.success(Seq(CachePolicy.Update)) + case "missing" => + ValidationNel.success(Seq(CachePolicy.FetchMissing)) + case "force" => + ValidationNel.success(Seq(CachePolicy.ForceDownload)) + case "default" => + ValidationNel.success(Seq(CachePolicy.LocalOnly, CachePolicy.FetchMissing)) + case other => + ValidationNel.failure(s"Unrecognized mode: $other") + } + .map(_.flatten) } diff --git a/cache/src/main/scala/coursier/CachePolicy.scala b/cache/src/main/scala/coursier/CachePolicy.scala index 9c072b2d4..8757b3abc 100644 --- a/cache/src/main/scala/coursier/CachePolicy.scala +++ b/cache/src/main/scala/coursier/CachePolicy.scala @@ -1,7 +1,5 @@ package coursier -import scalaz.{Failure, Success} - sealed abstract class CachePolicy extends Product with Serializable object CachePolicy { @@ -81,15 +79,15 @@ object CachePolicy { def fromOption(value: Option[String], description: String): Option[Seq[CachePolicy]] = value.filter(_.nonEmpty).flatMap { str => - CacheParse.cachePolicies(str) match { - case Success(Seq()) => + CacheParse.cachePolicies(str).either match { + case Right(Seq()) => Console.err.println( s"Warning: no mode found in $description, ignoring it." ) None - case Success(policies) => + case Right(policies) => Some(policies) - case Failure(errors) => + case Left(_) => Console.err.println( s"Warning: unrecognized mode in $description, ignoring it." ) diff --git a/cli/src/main/scala-2.12/coursier/cli/Helper.scala b/cli/src/main/scala-2.12/coursier/cli/Helper.scala index 88c51d2a4..50309a69b 100644 --- a/cli/src/main/scala-2.12/coursier/cli/Helper.scala +++ b/cli/src/main/scala-2.12/coursier/cli/Helper.scala @@ -17,7 +17,7 @@ import coursier.util.{Parse, Print} import scala.annotation.tailrec import scala.concurrent.duration.Duration import scalaz.concurrent.{Strategy, Task} -import scalaz.{Failure, Nondeterminism, Success} +import scalaz.Nondeterminism object Helper { @@ -81,11 +81,11 @@ class Helper( if (common.mode.isEmpty) CachePolicy.default else - CacheParse.cachePolicies(common.mode) match { - case Success(cp) => cp - case Failure(errors) => + CacheParse.cachePolicies(common.mode).either match { + case Right(cp) => cp + case Left(errors) => prematureExit( - s"Error parsing modes:\n${errors.list.toList.map(" "+_).mkString("\n")}" + s"Error parsing modes:\n${errors.map(" "+_).mkString("\n")}" ) } @@ -116,12 +116,12 @@ class Helper( repos } - val standardRepositories = repositoriesValidation match { - case Success(repos) => + val standardRepositories = repositoriesValidation.either match { + case Right(repos) => repos - case Failure(errors) => + case Left(errors) => prematureExit( - s"Error with repositories:\n${errors.list.toList.map(" "+_).mkString("\n")}" + s"Error with repositories:\n${errors.map(" "+_).mkString("\n")}" ) } diff --git a/core/shared/src/main/scala/coursier/ivy/Pattern.scala b/core/shared/src/main/scala/coursier/ivy/Pattern.scala index 212c025a7..d2eadc8b2 100644 --- a/core/shared/src/main/scala/coursier/ivy/Pattern.scala +++ b/core/shared/src/main/scala/coursier/ivy/Pattern.scala @@ -1,12 +1,11 @@ package coursier.ivy -import scala.language.implicitConversions - -import scalaz.{Failure, Success, ValidationNel} -import scalaz.Scalaz.{ToEitherOpsFromEither, ToFoldableOps, ToTraverseOps, ToValidationOps, vectorInstance} - +import coursier.util.Traverse.TraverseOps +import coursier.util.ValidationNel import fastparse.all._ +import scala.language.implicitConversions + final case class PropertiesPattern(chunks: Seq[PropertiesPattern.ChunkOrProperty]) { def string: String = chunks.map(_.string).mkString @@ -15,43 +14,43 @@ final case class PropertiesPattern(chunks: Seq[PropertiesPattern.ChunkOrProperty def substituteProperties(properties: Map[String, String]): Either[String, Pattern] = { - val validation = chunks.toVector.traverseM[({ type L[X] = ValidationNel[String, X] })#L, Pattern.Chunk] { + val validation = chunks.validationNelTraverse[String, Seq[Pattern.Chunk]] { case ChunkOrProperty.Prop(name, alternativesOpt) => properties.get(name) match { case Some(value) => - Vector(Pattern.Chunk.Const(value)).successNel + ValidationNel.success(Seq(Pattern.Chunk.Const(value))) case None => alternativesOpt match { case Some(alt) => - PropertiesPattern(alt) - .substituteProperties(properties) - .right - .map(_.chunks.toVector) - .validation - .toValidationNel + ValidationNel.fromEither( + PropertiesPattern(alt) + .substituteProperties(properties) + .right + .map(_.chunks.toVector) + ) case None => - name.failureNel + ValidationNel.failure(name) } } case ChunkOrProperty.Opt(l @ _*) => - PropertiesPattern(l) - .substituteProperties(properties) - .right - .map(l => Vector(Pattern.Chunk.Opt(l.chunks: _*))) - .validation - .toValidationNel + ValidationNel.fromEither( + PropertiesPattern(l) + .substituteProperties(properties) + .right + .map(l => Seq(Pattern.Chunk.Opt(l.chunks: _*))) + ) case ChunkOrProperty.Var(name) => - Vector(Pattern.Chunk.Var(name)).successNel + ValidationNel.success(Seq(Pattern.Chunk.Var(name))) case ChunkOrProperty.Const(value) => - Vector(Pattern.Chunk.Const(value)).successNel + ValidationNel.success(Seq(Pattern.Chunk.Const(value))) - }.map(Pattern(_)) + }.map(c => Pattern(c.flatten)) - validation.toEither.left.map { notFoundProps => - s"Property(ies) not found: ${notFoundProps.toList.mkString(", ")}" + validation.either.left.map { notFoundProps => + s"Property(ies) not found: ${notFoundProps.mkString(", ")}" } } } @@ -68,30 +67,30 @@ final case class Pattern(chunks: Seq[Pattern.Chunk]) { def substituteVariables(variables: Map[String, String]): Either[String, String] = { def helper(chunks: Seq[Chunk]): ValidationNel[String, Seq[Chunk.Const]] = - chunks.toVector.traverseU[ValidationNel[String, Seq[Chunk.Const]]] { + chunks.validationNelTraverse[String, Seq[Chunk.Const]] { case Chunk.Var(name) => variables.get(name) match { case Some(value) => - Seq(Chunk.Const(value)).successNel + ValidationNel.success(Seq(Chunk.Const(value))) case None => - name.failureNel + ValidationNel.failure(name) } case Chunk.Opt(l @ _*) => val res = helper(l) if (res.isSuccess) res else - Seq().successNel + ValidationNel.success(Seq()) case c: Chunk.Const => - Seq(c).successNel + ValidationNel.success(Seq(c)) }.map(_.flatten) val validation = helper(chunks) - validation match { - case Failure(notFoundVariables) => - Left(s"Variables not found: ${notFoundVariables.toList.mkString(", ")}") - case Success(constants) => + validation.either match { + case Left(notFoundVariables) => + Left(s"Variables not found: ${notFoundVariables.mkString(", ")}") + case Right(constants) => val b = new StringBuilder constants.foreach(b ++= _.value) Right(b.result()) diff --git a/core/shared/src/main/scala/coursier/util/Traverse.scala b/core/shared/src/main/scala/coursier/util/Traverse.scala index d20d4d069..8579ce805 100644 --- a/core/shared/src/main/scala/coursier/util/Traverse.scala +++ b/core/shared/src/main/scala/coursier/util/Traverse.scala @@ -5,16 +5,49 @@ import scala.collection.mutable.ListBuffer object Traverse { implicit class TraverseOps[T](val seq: Seq[T]) { + def eitherTraverse[L, R](f: T => Either[L, R]): Either[L, Seq[R]] = // Warning: iterates on the whole sequence no matter what, even if the first element is a Left - seq.foldLeft[Either[L, ListBuffer[R]]](Right(new ListBuffer)) { - case (l @ Left(_), _) => l - case (Right(b), elem) => - f(elem) match { - case Left(l) => Left(l) - case Right(r) => Right(b += r) + seq + .foldLeft[Either[L, ListBuffer[R]]](Right(new ListBuffer)) { + case (l @ Left(_), _) => l + case (Right(b), elem) => + f(elem) match { + case Left(l) => Left(l) + case Right(r) => Right(b += r) + } + } + .right + .map(_.result()) + + def validationNelTraverse[L, R](f: T => ValidationNel[L, R]): ValidationNel[L, Seq[R]] = { + + val e = seq + .foldLeft[Either[ListBuffer[L], ListBuffer[R]]](Right(new ListBuffer)) { + case (l @ Left(b), elem) => + f(elem).either match { + case Left(l0) => Left(b ++= l0) + case Right(_) => l + } + case (Right(b), elem) => + f(elem).either match { + case Left(l) => Left(new ListBuffer[L] ++= l) + case Right(r) => Right(b += r) + } + } + .left + .map { b => + b.result() match { + case Nil => sys.error("Can't happen") + case h :: t => ::(h, t) } - } + } + .right + .map(_.result()) + + ValidationNel(e) + } + } } diff --git a/core/shared/src/main/scala/coursier/util/ValidationNel.scala b/core/shared/src/main/scala/coursier/util/ValidationNel.scala new file mode 100644 index 000000000..e19f60c0e --- /dev/null +++ b/core/shared/src/main/scala/coursier/util/ValidationNel.scala @@ -0,0 +1,27 @@ +package coursier.util + +// not covariant because scala.:: isn't (and is there a point in being covariant in R but not L?) +final case class ValidationNel[L, R](either: Either[::[L], R]) { + def isSuccess: Boolean = + either.isRight + def map[S](f: R => S): ValidationNel[L, S] = + ValidationNel(either.right.map(f)) +} + +object ValidationNel { + def fromEither[L, R](either: Either[L, R]): ValidationNel[L, R] = + ValidationNel(either.left.map(l => ::(l, Nil))) + def success[L]: SuccessBuilder[L] = + new SuccessBuilder + def failure[R]: FailureBuilder[R] = + new FailureBuilder + + final class SuccessBuilder[L] { + def apply[R](r: R): ValidationNel[L, R] = + ValidationNel(Right(r)) + } + final class FailureBuilder[R] { + def apply[L](l: L): ValidationNel[L, R] = + ValidationNel(Left(::(l, Nil))) + } +} From 5e1eeef7570d05e4988c47bf6a39d2dddf1a6523 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Tue, 27 Feb 2018 22:54:48 +0100 Subject: [PATCH 3/7] Add coursier.util.Monad --- .../src/main/scala/coursier/Fetch.scala | 4 ++-- .../main/scala/coursier/core/Repository.scala | 3 +-- .../coursier/core/ResolutionProcess.scala | 3 ++- .../scala/coursier/ivy/IvyRepository.scala | 3 +-- .../coursier/maven/MavenRepository.scala | 3 +-- .../main/scala/coursier/util/EitherT.scala | 1 - .../src/main/scala/coursier/util/Monad.scala | 21 +++++++++++++++++++ .../FallbackDependenciesRepository.scala | 3 +-- .../coursier/InterProjectRepository.scala | 3 +-- .../scala/coursier/test/TestRepository.scala | 3 +-- 10 files changed, 31 insertions(+), 16 deletions(-) create mode 100644 core/shared/src/main/scala/coursier/util/Monad.scala diff --git a/core/shared/src/main/scala/coursier/Fetch.scala b/core/shared/src/main/scala/coursier/Fetch.scala index dcf9e520e..36fe18a46 100644 --- a/core/shared/src/main/scala/coursier/Fetch.scala +++ b/core/shared/src/main/scala/coursier/Fetch.scala @@ -1,9 +1,9 @@ package coursier -import coursier.util.EitherT +import coursier.util.{EitherT, Monad} import scala.language.higherKinds -import scalaz.{Monad, Nondeterminism} +import scalaz.Nondeterminism object Fetch { diff --git a/core/shared/src/main/scala/coursier/core/Repository.scala b/core/shared/src/main/scala/coursier/core/Repository.scala index 8e88b403b..cdd9d5c69 100644 --- a/core/shared/src/main/scala/coursier/core/Repository.scala +++ b/core/shared/src/main/scala/coursier/core/Repository.scala @@ -3,9 +3,8 @@ package coursier.core import coursier.Fetch import scala.language.higherKinds -import scalaz.Monad import coursier.core.compatibility.encodeURIComponent -import coursier.util.EitherT +import coursier.util.{EitherT, Monad} trait Repository extends Product with Serializable { def find[F[_]]( diff --git a/core/shared/src/main/scala/coursier/core/ResolutionProcess.scala b/core/shared/src/main/scala/coursier/core/ResolutionProcess.scala index 92bc88fde..827d78a41 100644 --- a/core/shared/src/main/scala/coursier/core/ResolutionProcess.scala +++ b/core/shared/src/main/scala/coursier/core/ResolutionProcess.scala @@ -1,9 +1,10 @@ package coursier package core +import coursier.util.Monad + import scala.annotation.tailrec import scala.language.higherKinds -import scalaz.Monad sealed abstract class ResolutionProcess { diff --git a/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala b/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala index 0fc38d9dd..f86336c96 100644 --- a/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala +++ b/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala @@ -2,10 +2,9 @@ package coursier.ivy import coursier.Fetch import coursier.core._ -import coursier.util.{EitherT, WebPage} +import coursier.util.{EitherT, Monad, WebPage} import scala.language.higherKinds -import scalaz.Monad final case class IvyRepository( pattern: Pattern, diff --git a/core/shared/src/main/scala/coursier/maven/MavenRepository.scala b/core/shared/src/main/scala/coursier/maven/MavenRepository.scala index ee6ac4a92..3f3333165 100644 --- a/core/shared/src/main/scala/coursier/maven/MavenRepository.scala +++ b/core/shared/src/main/scala/coursier/maven/MavenRepository.scala @@ -3,10 +3,9 @@ package coursier.maven import coursier.Fetch import coursier.core._ import coursier.core.compatibility.encodeURIComponent -import coursier.util.{EitherT, WebPage} +import coursier.util.{EitherT, Monad, WebPage} import scala.language.higherKinds -import scalaz.Monad object MavenRepository { val SnapshotTimestamp = "(.*-)?[0-9]{8}\\.[0-9]{6}-[0-9]+".r diff --git a/core/shared/src/main/scala/coursier/util/EitherT.scala b/core/shared/src/main/scala/coursier/util/EitherT.scala index c4324e9f1..b69501fc0 100644 --- a/core/shared/src/main/scala/coursier/util/EitherT.scala +++ b/core/shared/src/main/scala/coursier/util/EitherT.scala @@ -1,7 +1,6 @@ package coursier.util import scala.language.higherKinds -import scalaz.Monad final case class EitherT[F[_], L, R](run: F[Either[L, R]]) { diff --git a/core/shared/src/main/scala/coursier/util/Monad.scala b/core/shared/src/main/scala/coursier/util/Monad.scala new file mode 100644 index 000000000..026ca2ea7 --- /dev/null +++ b/core/shared/src/main/scala/coursier/util/Monad.scala @@ -0,0 +1,21 @@ +package coursier.util + +import scala.language.higherKinds + +trait Monad[F[_]] { + def point[A](a: A): F[A] + def bind[A, B](elem: F[A])(f: A => F[B]): F[B] + + def map[A, B](elem: F[A])(f: A => B): F[B] = + bind(elem)(a => point(f(a))) +} + +object Monad { + + implicit def fromScalaz[F[_]](implicit M: scalaz.Monad[F]): Monad[F] = + new Monad[F] { + def point[A](a: A) = M.pure(a) + def bind[A, B](elem: F[A])(f: A => F[B]) = M.bind(elem)(f) + } + +} diff --git a/extra/src/main/scala/coursier/FallbackDependenciesRepository.scala b/extra/src/main/scala/coursier/FallbackDependenciesRepository.scala index 1f984d00d..50ea1b297 100644 --- a/extra/src/main/scala/coursier/FallbackDependenciesRepository.scala +++ b/extra/src/main/scala/coursier/FallbackDependenciesRepository.scala @@ -3,10 +3,9 @@ package coursier import java.io.{File, FileNotFoundException, IOException} import java.net.{HttpURLConnection, URL, URLConnection} -import coursier.util.EitherT +import coursier.util.{EitherT, Monad} import scala.language.higherKinds -import scalaz.Monad object FallbackDependenciesRepository { diff --git a/sbt-coursier/src/main/scala/coursier/InterProjectRepository.scala b/sbt-coursier/src/main/scala/coursier/InterProjectRepository.scala index c6c58410e..23087d480 100644 --- a/sbt-coursier/src/main/scala/coursier/InterProjectRepository.scala +++ b/sbt-coursier/src/main/scala/coursier/InterProjectRepository.scala @@ -1,9 +1,8 @@ package coursier -import coursier.util.EitherT +import coursier.util.{EitherT, Monad} import scala.language.higherKinds -import scalaz.Monad final case class InterProjectRepository(projects: Seq[Project]) extends Repository { diff --git a/tests/shared/src/test/scala/coursier/test/TestRepository.scala b/tests/shared/src/test/scala/coursier/test/TestRepository.scala index 7e29cd4e1..a0c016447 100644 --- a/tests/shared/src/test/scala/coursier/test/TestRepository.scala +++ b/tests/shared/src/test/scala/coursier/test/TestRepository.scala @@ -2,10 +2,9 @@ package coursier package test import coursier.core._ -import coursier.util.EitherT +import coursier.util.{EitherT, Monad} import scala.language.higherKinds -import scalaz.Monad final case class TestRepository(projects: Map[(Module, String), Project]) extends Repository { val source = new core.Artifact.Source { From c037dbaeebb2b00bff7f41e583b5ee5e85eff4bf Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Tue, 27 Feb 2018 22:54:48 +0100 Subject: [PATCH 4/7] Add coursier.util.Gather --- .../shared/src/main/scala/coursier/Fetch.scala | 7 +++---- .../src/main/scala/coursier/util/Gather.scala | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 core/shared/src/main/scala/coursier/util/Gather.scala diff --git a/core/shared/src/main/scala/coursier/Fetch.scala b/core/shared/src/main/scala/coursier/Fetch.scala index 36fe18a46..866e9b9a3 100644 --- a/core/shared/src/main/scala/coursier/Fetch.scala +++ b/core/shared/src/main/scala/coursier/Fetch.scala @@ -1,9 +1,8 @@ package coursier -import coursier.util.{EitherT, Monad} +import coursier.util.{EitherT, Gather, Monad} import scala.language.higherKinds -import scalaz.Nondeterminism object Fetch { @@ -59,12 +58,12 @@ object Fetch { fetch: Content[F], extra: Content[F]* )(implicit - F: Nondeterminism[F] + F: Gather[F] ): Metadata[F] = { modVers => F.map( - F.gatherUnordered { + F.gather { modVers.map { case (module, version) => def get(fetch: Content[F]) = find(repositories, module, version, fetch) diff --git a/core/shared/src/main/scala/coursier/util/Gather.scala b/core/shared/src/main/scala/coursier/util/Gather.scala new file mode 100644 index 000000000..0da299040 --- /dev/null +++ b/core/shared/src/main/scala/coursier/util/Gather.scala @@ -0,0 +1,18 @@ +package coursier.util + +import scala.language.higherKinds + +trait Gather[F[_]] extends Monad[F] { + def gather[A](elems: Seq[F[A]]): F[Seq[A]] +} + +object Gather { + + implicit def fromScalaz[F[_]](implicit N: scalaz.Nondeterminism[F]): Gather[F] = + new Gather[F] { + def point[A](a: A) = N.pure(a) + def bind[A, B](elem: F[A])(f: A => F[B]) = N.bind(elem)(f) + def gather[A](elems: Seq[F[A]]) = N.map(N.gather(elems))(l => l) + } + +} From 838a340b895e57b634820c2f132ebaeab3bb70de Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Tue, 27 Feb 2018 22:54:49 +0100 Subject: [PATCH 5/7] Remove deprecated stuff --- cache/src/main/scala/coursier/Cache.scala | 48 +++---------------- .../src/main/scala/coursier/TermDisplay.scala | 2 +- .../main/scala-2.12/coursier/cli/Helper.scala | 4 +- .../main/scala/coursier/core/Resolution.scala | 5 +- .../main/scala/coursier/CoursierPlugin.scala | 3 -- .../src/main/scala/coursier/Keys.scala | 2 - .../src/main/scala/coursier/Tasks.scala | 6 +-- .../scala/coursier/test/CacheFetchTests.scala | 2 +- .../scala/coursier/test/CentralTests.scala | 8 ++-- .../scala/coursier/test/ResolutionTests.scala | 2 +- 10 files changed, 22 insertions(+), 60 deletions(-) diff --git a/cache/src/main/scala/coursier/Cache.scala b/cache/src/main/scala/coursier/Cache.scala index 6d7e51dea..172ef0dd9 100644 --- a/cache/src/main/scala/coursier/Cache.scala +++ b/cache/src/main/scala/coursier/Cache.scala @@ -320,11 +320,9 @@ object Cache { private def contentLength( url: String, authentication: Option[Authentication], - logger0: Option[Logger] + logger: Option[Logger] ): Either[FileError, Option[Long]] = { - val logger = logger0.map(Logger.Extended(_)) - var conn: URLConnection = null try { @@ -367,14 +365,12 @@ object Cache { checksums: Set[String], cachePolicy: CachePolicy, pool: ExecutorService, - logger0: Option[Logger] = None, + logger: Option[Logger] = None, ttl: Option[Duration] = defaultTtl ): Task[Seq[((File, String), Either[FileError, Unit])]] = { implicit val pool0 = pool - val logger = logger0.map(Logger.Extended(_)) - // Reference file - if it exists, and we get not found errors on some URLs, we assume // we can keep track of these missing, and not try to get them again later. val referenceFileOpt = artifact @@ -960,7 +956,7 @@ object Cache { checksums = checksums0.collect { case Some(c) => c }.toSet, cachePolicy, pool, - logger0 = logger, + logger = logger, ttl = ttl ).map { results => val checksum = checksums0.find { @@ -1134,43 +1130,11 @@ object Cache { def downloadedArtifact(url: String, success: Boolean): Unit = {} def checkingUpdates(url: String, currentTimeOpt: Option[Long]): Unit = {} def checkingUpdatesResult(url: String, currentTimeOpt: Option[Long], remoteTimeOpt: Option[Long]): Unit = {} - } - object Logger { - // adding new methods to this one, not to break bin compat in 2.10 / 2.11 - abstract class Extended extends Logger { - def downloadLength(url: String, totalLength: Long, alreadyDownloaded: Long, watching: Boolean): Unit = { - downloadLength(url, totalLength, 0L, watching) - } + def downloadLength(url: String, totalLength: Long, alreadyDownloaded: Long, watching: Boolean): Unit = {} - def gettingLength(url: String): Unit = {} - def gettingLengthResult(url: String, length: Option[Long]): Unit = {} - } - - object Extended { - def apply(logger: Logger): Extended = - logger match { - case e: Extended => e - case _ => - new Extended { - override def foundLocally(url: String, f: File) = - logger.foundLocally(url, f) - - override def downloadingArtifact(url: String, file: File) = - logger.downloadingArtifact(url, file) - - override def downloadProgress(url: String, downloaded: Long) = - logger.downloadProgress(url, downloaded) - - override def downloadedArtifact(url: String, success: Boolean) = - logger.downloadedArtifact(url, success) - override def checkingUpdates(url: String, currentTimeOpt: Option[Long]) = - logger.checkingUpdates(url, currentTimeOpt) - override def checkingUpdatesResult(url: String, currentTimeOpt: Option[Long], remoteTimeOpt: Option[Long]) = - logger.checkingUpdatesResult(url, currentTimeOpt, remoteTimeOpt) - } - } - } + def gettingLength(url: String): Unit = {} + def gettingLengthResult(url: String, length: Option[Long]): Unit = {} } var bufferSize = 1024*1024 diff --git a/cache/src/main/scala/coursier/TermDisplay.scala b/cache/src/main/scala/coursier/TermDisplay.scala index 9ecd32bbd..e2e51c448 100644 --- a/cache/src/main/scala/coursier/TermDisplay.scala +++ b/cache/src/main/scala/coursier/TermDisplay.scala @@ -374,7 +374,7 @@ object TermDisplay { class TermDisplay( out: Writer, val fallbackMode: Boolean = TermDisplay.defaultFallbackMode -) extends Cache.Logger.Extended { +) extends Cache.Logger { import TermDisplay._ diff --git a/cli/src/main/scala-2.12/coursier/cli/Helper.scala b/cli/src/main/scala-2.12/coursier/cli/Helper.scala index 50309a69b..add604fc0 100644 --- a/cli/src/main/scala-2.12/coursier/cli/Helper.scala +++ b/cli/src/main/scala-2.12/coursier/cli/Helper.scala @@ -515,11 +515,11 @@ class Helper( errPrintln("\nMaximum number of iterations reached!") } - if (res.metadataErrors.nonEmpty) { + if (res.errors.nonEmpty) { anyError = true errPrintln( "\nError:\n" + - res.metadataErrors.map { + res.errors.map { case ((module, version), errors) => s" $module:$version\n${errors.map(" " + _.replace("\n", " \n")).mkString("\n")}" }.mkString("\n") diff --git a/core/shared/src/main/scala/coursier/core/Resolution.scala b/core/shared/src/main/scala/coursier/core/Resolution.scala index a98305233..8daaa7f4a 100644 --- a/core/shared/src/main/scala/coursier/core/Resolution.scala +++ b/core/shared/src/main/scala/coursier/core/Resolution.scala @@ -1085,7 +1085,10 @@ final case class Resolution( * Returns errors on dependencies * @return errors */ - def metadataErrors: Seq[(ModuleVersion, Seq[String])] = errorCache.toSeq + def errors: Seq[(ModuleVersion, Seq[String])] = errorCache.toSeq + + @deprecated("Use errors instead", "1.1.0") + def metadataErrors: Seq[(ModuleVersion, Seq[String])] = errors /** * Removes from this `Resolution` dependencies that are not in `dependencies` neither brought diff --git a/sbt-coursier/src/main/scala/coursier/CoursierPlugin.scala b/sbt-coursier/src/main/scala/coursier/CoursierPlugin.scala index 15fb06274..2bd8e4481 100644 --- a/sbt-coursier/src/main/scala/coursier/CoursierPlugin.scala +++ b/sbt-coursier/src/main/scala/coursier/CoursierPlugin.scala @@ -41,9 +41,6 @@ object CoursierPlugin extends AutoPlugin { val coursierParentProjectCache = Keys.coursierParentProjectCache val coursierResolutions = Keys.coursierResolutions - @deprecated("Use coursierResolutions instead", "1.0.0-RC4") - val coursierResolution = Keys.actualCoursierResolution - val coursierSbtClassifiersResolution = Keys.coursierSbtClassifiersResolution val coursierDependencyTree = Keys.coursierDependencyTree diff --git a/sbt-coursier/src/main/scala/coursier/Keys.scala b/sbt-coursier/src/main/scala/coursier/Keys.scala index 75c7441cd..015a10391 100644 --- a/sbt-coursier/src/main/scala/coursier/Keys.scala +++ b/sbt-coursier/src/main/scala/coursier/Keys.scala @@ -51,8 +51,6 @@ object Keys { private[coursier] val actualCoursierResolution = TaskKey[Resolution]("coursier-resolution") - @deprecated("Use coursierResolutions instead", "1.0.0-RC4") - val coursierResolution = actualCoursierResolution val coursierSbtClassifiersResolution = TaskKey[Resolution]("coursier-sbt-classifiers-resolution") val coursierDependencyTree = TaskKey[Unit]( diff --git a/sbt-coursier/src/main/scala/coursier/Tasks.scala b/sbt-coursier/src/main/scala/coursier/Tasks.scala index ad2649fc0..0a950ed0b 100644 --- a/sbt-coursier/src/main/scala/coursier/Tasks.scala +++ b/sbt-coursier/src/main/scala/coursier/Tasks.scala @@ -781,17 +781,17 @@ object Tasks { ).throwException() } - if (res.metadataErrors.nonEmpty) { + if (res.errors.nonEmpty) { val internalRepositoriesLen = internalRepositories.length val errors = if (repositories.length > internalRepositoriesLen) // drop internal repository errors - res.metadataErrors.map { + res.errors.map { case (dep, errs) => dep -> errs.drop(internalRepositoriesLen) } else - res.metadataErrors + res.errors ResolutionError.MetadataDownloadErrors(errors) .throwException() diff --git a/tests/jvm/src/test/scala/coursier/test/CacheFetchTests.scala b/tests/jvm/src/test/scala/coursier/test/CacheFetchTests.scala index bc06df92d..2408842a4 100644 --- a/tests/jvm/src/test/scala/coursier/test/CacheFetchTests.scala +++ b/tests/jvm/src/test/scala/coursier/test/CacheFetchTests.scala @@ -54,7 +54,7 @@ object CacheFetchTests extends TestSuite { cleanTmpDir() } - val errors = res.metadataErrors + val errors = res.errors assert(errors.isEmpty) } diff --git a/tests/shared/src/test/scala/coursier/test/CentralTests.scala b/tests/shared/src/test/scala/coursier/test/CentralTests.scala index 6f0e40025..1c86c5a10 100644 --- a/tests/shared/src/test/scala/coursier/test/CentralTests.scala +++ b/tests/shared/src/test/scala/coursier/test/CentralTests.scala @@ -45,7 +45,7 @@ abstract class CentralTests extends TestSuite { .run(fetch0) .map { res => - val metadataErrors = res.metadataErrors + val metadataErrors = res.errors val conflicts = res.conflicts val isDone = res.isDone assert(metadataErrors.isEmpty) @@ -183,7 +183,7 @@ abstract class CentralTests extends TestSuite { ): Future[T] = async { val res = await(resolve(deps, extraRepos = extraRepos)) - val metadataErrors = res.metadataErrors + val metadataErrors = res.errors val conflicts = res.conflicts val isDone = res.isDone assert(metadataErrors.isEmpty) @@ -581,7 +581,7 @@ abstract class CentralTests extends TestSuite { val res = await(resolve(deps)) - val metadataErrors = res.metadataErrors + val metadataErrors = res.errors val conflicts = res.conflicts val isDone = res.isDone assert(metadataErrors.isEmpty) @@ -619,7 +619,7 @@ abstract class CentralTests extends TestSuite { val res = await(resolve(deps)) - val metadataErrors = res.metadataErrors + val metadataErrors = res.errors val conflicts = res.conflicts val isDone = res.isDone assert(metadataErrors.isEmpty) diff --git a/tests/shared/src/test/scala/coursier/test/ResolutionTests.scala b/tests/shared/src/test/scala/coursier/test/ResolutionTests.scala index 0f5d2e028..d1ca90a2a 100644 --- a/tests/shared/src/test/scala/coursier/test/ResolutionTests.scala +++ b/tests/shared/src/test/scala/coursier/test/ResolutionTests.scala @@ -251,7 +251,7 @@ object ResolutionTests extends TestSuite { assert(directDependencyErrors.isEmpty) // metadataErrors have that - assert(res.metadataErrors == Seq((Module("acme", "missing-pom"), "1.0.0") -> List("Not found"))) + assert(res.errors == Seq((Module("acme", "missing-pom"), "1.0.0") -> List("Not found"))) } } 'single{ From ca62830d23878ceb557e797789ec5c8c7602bcd7 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Tue, 27 Feb 2018 22:54:49 +0100 Subject: [PATCH 6/7] Add coursier.util.Schedulable --- cache/src/main/scala/coursier/Cache.scala | 132 ++++++++---------- .../scala/coursier/util/Schedulable.scala | 42 ++++++ 2 files changed, 102 insertions(+), 72 deletions(-) create mode 100644 cache/src/main/scala/coursier/util/Schedulable.scala diff --git a/cache/src/main/scala/coursier/Cache.scala b/cache/src/main/scala/coursier/Cache.scala index 172ef0dd9..9949e9e12 100644 --- a/cache/src/main/scala/coursier/Cache.scala +++ b/cache/src/main/scala/coursier/Cache.scala @@ -13,12 +13,10 @@ import coursier.internal.FileUtil import coursier.util.Base64.Encoder import scala.annotation.tailrec -import scalaz.Nondeterminism -import scalaz.concurrent.{Strategy, Task} import java.io.{Serializable => _, _} import java.nio.charset.Charset -import coursier.util.EitherT +import coursier.util.{EitherT, Schedulable} import scala.concurrent.duration.{Duration, DurationInt} import scala.util.Try @@ -359,17 +357,15 @@ object Cache { } } - private def download( + private def download[F[_]]( artifact: Artifact, cache: File, checksums: Set[String], cachePolicy: CachePolicy, pool: ExecutorService, - logger: Option[Logger] = None, - ttl: Option[Duration] = defaultTtl - ): Task[Seq[((File, String), Either[FileError, Unit])]] = { - - implicit val pool0 = pool + logger: Option[Logger], + ttl: Option[Duration] + )(implicit S: Schedulable[F]): F[Seq[((File, String), Either[FileError, Unit])]] = { // Reference file - if it exists, and we get not found errors on some URLs, we assume // we can keep track of these missing, and not try to get them again later. @@ -380,9 +376,9 @@ object Cache { def referenceFileExists: Boolean = referenceFileOpt.exists(_.exists()) - def fileLastModified(file: File): EitherT[Task, FileError, Option[Long]] = + def fileLastModified(file: File): EitherT[F, FileError, Option[Long]] = EitherT { - Task { + S.schedule(pool) { Right { val lastModified = file.lastModified() if (lastModified > 0L) @@ -397,9 +393,9 @@ object Cache { url: String, currentLastModifiedOpt: Option[Long], // for the logger logger: Option[Logger] - ): EitherT[Task, FileError, Option[Long]] = + ): EitherT[F, FileError, Option[Long]] = EitherT { - Task { + S.schedule(pool) { var conn: URLConnection = null try { @@ -441,19 +437,19 @@ object Cache { } } - def fileExists(file: File): Task[Boolean] = - Task { + def fileExists(file: File): F[Boolean] = + S.schedule(pool) { file.exists() } def ttlFile(file: File): File = new File(file.getParent, s".${file.getName}.checked") - def lastCheck(file: File): Task[Option[Long]] = { + def lastCheck(file: File): F[Option[Long]] = { val ttlFile0 = ttlFile(file) - Task { + S.schedule(pool) { if (ttlFile0.exists()) Some(ttlFile0.lastModified()).filter(_ > 0L) else @@ -474,17 +470,17 @@ object Cache { } } - def shouldDownload(file: File, url: String): EitherT[Task, FileError, Boolean] = { + def shouldDownload(file: File, url: String): EitherT[F, FileError, Boolean] = { - def checkNeeded = ttl.fold(Task.now(true)) { ttl => + def checkNeeded = ttl.fold(S.point(true)) { ttl => if (ttl.isFinite()) - lastCheck(file).flatMap { - case None => Task.now(true) + S.bind(lastCheck(file)) { + case None => S.point(true) case Some(ts) => - Task(System.currentTimeMillis()).map(_ > ts + ttl.toMillis) + S.map(S.schedule(pool)(System.currentTimeMillis()))(_ > ts + ttl.toMillis) } else - Task.now(false) + S.point(false) } def check = for { @@ -500,22 +496,22 @@ object Cache { } EitherT { - fileExists(file).flatMap { + S.bind(fileExists(file)) { case false => - Task.now(Right(true)) + S.point(Right(true)) case true => - checkNeeded.flatMap { + S.bind(checkNeeded) { case false => - Task.now(Right(false)) + S.point(Right(false)) case true => - check.run.flatMap { + S.bind(check.run) { case Right(false) => - Task { + S.schedule(pool) { doTouchCheckFile(file) Right(false) } case other => - Task.now(other) + S.point(other) } } } @@ -543,9 +539,9 @@ object Cache { def remote( file: File, url: String - ): EitherT[Task, FileError, Unit] = + ): EitherT[F, FileError, Unit] = EitherT { - Task { + S.schedule(pool) { val tmp = CachePath.temporaryFile(file) @@ -677,20 +673,20 @@ object Cache { def errFile(file: File) = new File(file.getParentFile, "." + file.getName + ".error") - def remoteKeepErrors(file: File, url: String): EitherT[Task, FileError, Unit] = { + def remoteKeepErrors(file: File, url: String): EitherT[F, FileError, Unit] = { val errFile0 = errFile(file) def validErrFileExists = EitherT { - Task[Either[FileError, Boolean]] { + S.schedule[Either[FileError, Boolean]](pool) { Right(referenceFileExists && errFile0.exists()) } } def createErrFile = EitherT { - Task[Either[FileError, Unit]] { + S.schedule[Either[FileError, Unit]](pool) { if (referenceFileExists) { if (!errFile0.exists()) FileUtil.write(errFile0, "".getBytes(UTF_8)) @@ -702,7 +698,7 @@ object Cache { def deleteErrFile = EitherT { - Task[Either[FileError, Unit]] { + S.schedule[Either[FileError, Unit]](pool) { if (errFile0.exists()) errFile0.delete() @@ -712,11 +708,11 @@ object Cache { def retainError = EitherT { - remote(file, url).run.flatMap { + S.bind(remote(file, url).run) { case err @ Left(FileError.NotFound(_, Some(true))) => - createErrFile.run.map(_ => err) + S.map(createErrFile.run)(_ => err: Either[FileError, Unit]) case other => - deleteErrFile.run.map(_ => other) + S.map(deleteErrFile.run)(_ => other) } } @@ -724,7 +720,7 @@ object Cache { case CachePolicy.FetchMissing | CachePolicy.LocalOnly | CachePolicy.LocalUpdate | CachePolicy.LocalUpdateChanging => validErrFileExists.flatMap { exists => if (exists) - EitherT(Task.now[Either[FileError, Unit]](Left(FileError.NotFound(url, Some(true))))) + EitherT(S.point[Either[FileError, Unit]](Left(FileError.NotFound(url, Some(true))))) else retainError } @@ -734,7 +730,7 @@ object Cache { } } - def localInfo(file: File, url: String): EitherT[Task, FileError, Boolean] = { + def localInfo(file: File, url: String): EitherT[F, FileError, Boolean] = { val errFile0 = errFile(file) @@ -748,12 +744,12 @@ object Cache { else Right(false) - EitherT(Task(res)) + EitherT(S.schedule(pool)(res)) } - def checkFileExists(file: File, url: String, log: Boolean = true): EitherT[Task, FileError, Unit] = + def checkFileExists(file: File, url: String, log: Boolean = true): EitherT[F, FileError, Unit] = EitherT { - Task { + S.schedule(pool) { if (file.exists()) { logger.foreach(_.foundLocally(url, file)) Right(()) @@ -780,19 +776,19 @@ object Cache { val requiredArtifactCheck = artifact.extra.get("required") match { case None => - EitherT(Task.now[Either[FileError, Unit]](Right(()))) + EitherT(S.point[Either[FileError, Unit]](Right(()))) case Some(required) => cachePolicy0 match { case CachePolicy.LocalOnly | CachePolicy.LocalUpdateChanging | CachePolicy.LocalUpdate => val file = localFile(required.url, cache, artifact.authentication.map(_.user)) localInfo(file, required.url).flatMap { case true => - EitherT(Task.now[Either[FileError, Unit]](Right(()))) + EitherT(S.point[Either[FileError, Unit]](Right(()))) case false => - EitherT(Task.now[Either[FileError, Unit]](Left(FileError.NotFound(file.toString)))) + EitherT(S.point[Either[FileError, Unit]](Left(FileError.NotFound(file.toString)))) } case _ => - EitherT(Task.now[Either[FileError, Unit]](Right(()))) + EitherT(S.point[Either[FileError, Unit]](Right(()))) } } @@ -815,7 +811,7 @@ object Cache { case true => remoteKeepErrors(file, url) case false => - EitherT(Task.now[Either[FileError, Unit]](Right(()))) + EitherT(S.point[Either[FileError, Unit]](Right(()))) } cachePolicy0 match { @@ -834,13 +830,10 @@ object Cache { } } - requiredArtifactCheck - .flatMap(_ => res) - .run - .map((file, url) -> _) + S.map(requiredArtifactCheck.flatMap(_ => res).run)((file, url) -> _) } - Nondeterminism[Task].gather(tasks) + S.gather(tasks) } def parseChecksum(content: String): Option[BigInteger] = { @@ -883,14 +876,12 @@ object Cache { .mkString)) } - def validateChecksum( + def validateChecksum[F[_]]( artifact: Artifact, sumType: String, cache: File, pool: ExecutorService - ): EitherT[Task, FileError, Unit] = { - - implicit val pool0 = pool + )(implicit S: Schedulable[F]): EitherT[F, FileError, Unit] = { val localFile0 = localFile(artifact.url, cache, artifact.authentication.map(_.user)) @@ -899,7 +890,7 @@ object Cache { case Some(sumUrl) => val sumFile = localFile(sumUrl, cache, artifact.authentication.map(_.user)) - Task { + S.schedule(pool) { val sumOpt = parseRawChecksum(FileUtil.readAllBytes(sumFile)) sumOpt match { @@ -930,12 +921,12 @@ object Cache { } case None => - Task.now(Left(FileError.ChecksumNotFound(sumType, localFile0.getPath))) + S.point[Either[FileError, Unit]](Left(FileError.ChecksumNotFound(sumType, localFile0.getPath))) } } } - def file( + def file[F[_]]( artifact: Artifact, cache: File = default, cachePolicy: CachePolicy = CachePolicy.UpdateChanging, @@ -943,14 +934,12 @@ object Cache { logger: Option[Logger] = None, pool: ExecutorService = defaultPool, ttl: Option[Duration] = defaultTtl - ): EitherT[Task, FileError, File] = { - - implicit val pool0 = pool + )(implicit S: Schedulable[F]): EitherT[F, FileError, File] = { val checksums0 = if (checksums.isEmpty) Seq(None) else checksums val res = EitherT { - download( + S.map(download( artifact, cache, checksums = checksums0.collect { case Some(c) => c }.toSet, @@ -958,7 +947,7 @@ object Cache { pool, logger = logger, ttl = ttl - ).map { results => + )) { results => val checksum = checksums0.find { case None => true case Some(c) => @@ -983,20 +972,20 @@ object Cache { } res.flatMap { - case (f, None) => EitherT(Task.now[Either[FileError, File]](Right(f))) + case (f, None) => EitherT(S.point[Either[FileError, File]](Right(f))) case (f, Some(c)) => validateChecksum(artifact, c, cache, pool).map(_ => f) } } - def fetch( + def fetch[F[_]]( cache: File = default, cachePolicy: CachePolicy = CachePolicy.UpdateChanging, checksums: Seq[Option[String]] = defaultChecksums, logger: Option[Logger] = None, pool: ExecutorService = defaultPool, ttl: Option[Duration] = defaultTtl - ): Fetch.Content[Task] = { + )(implicit S: Schedulable[F]): Fetch.Content[F] = { artifact => file( artifact, @@ -1060,7 +1049,7 @@ object Cache { } else notFound(f) - EitherT(Task.now[Either[String, String]](res)) + EitherT(S.point[Either[String, String]](res)) } } @@ -1102,8 +1091,7 @@ object Cache { val defaultConcurrentDownloadCount = 6 - lazy val defaultPool = - Executors.newFixedThreadPool(defaultConcurrentDownloadCount, Strategy.DefaultDaemonThreadFactory) + lazy val defaultPool = Schedulable.fixedThreadPool(defaultConcurrentDownloadCount) lazy val defaultTtl: Option[Duration] = { def fromString(s: String) = diff --git a/cache/src/main/scala/coursier/util/Schedulable.scala b/cache/src/main/scala/coursier/util/Schedulable.scala new file mode 100644 index 000000000..b01a6f572 --- /dev/null +++ b/cache/src/main/scala/coursier/util/Schedulable.scala @@ -0,0 +1,42 @@ +package coursier.util + +import java.util.concurrent.{ExecutorService, Executors, ThreadFactory} + +import scala.language.higherKinds +import scalaz.concurrent.{Task => ScalazTask} + +trait Schedulable[F[_]] extends Gather[F] { + def schedule[A](pool: ExecutorService)(f: => A): F[A] +} + +object Schedulable { + + implicit val scalazTask: Schedulable[ScalazTask] = + new Schedulable[ScalazTask] { + def point[A](a: A) = + ScalazTask.point(a) + def schedule[A](pool: ExecutorService)(f: => A) = + ScalazTask(f)(pool) + + def gather[A](elems: Seq[ScalazTask[A]]) = + ScalazTask.taskInstance.gather(elems) + + def bind[A, B](elem: ScalazTask[A])(f: A => ScalazTask[B]) = + ScalazTask.taskInstance.bind(elem)(f) + } + + def fixedThreadPool(size: Int): ExecutorService = + Executors.newFixedThreadPool( + size, + // from scalaz.concurrent.Strategy.DefaultDaemonThreadFactory + new ThreadFactory { + val defaultThreadFactory = Executors.defaultThreadFactory() + def newThread(r: Runnable) = { + val t = defaultThreadFactory.newThread(r) + t.setDaemon(true) + t + } + } + ) + +} From f4fe44fe2c57797ae06ee699293ffa26be099680 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Tue, 27 Feb 2018 22:54:50 +0100 Subject: [PATCH 7/7] Clean-up helper method to fully read InputStream --- cache/src/main/scala/coursier/Cache.scala | 14 -------------- .../main/scala/coursier/internal/FileUtil.scala | 2 +- .../main/scala-2.12/coursier/cli/Bootstrap.scala | 4 ++-- .../coursier/cli/scaladex/Scaladex.scala | 2 +- .../main/scala-2.12/coursier/cli/util/Zip.scala | 4 +--- .../coursier/cli/CliBootstrapIntegrationTest.scala | 4 ++-- .../jvm/src/test}/scala/coursier/Platform.scala | 0 7 files changed, 7 insertions(+), 23 deletions(-) rename {cache/src/main => tests/jvm/src/test}/scala/coursier/Platform.scala (100%) diff --git a/cache/src/main/scala/coursier/Cache.scala b/cache/src/main/scala/coursier/Cache.scala index 9949e9e12..ae56f0cf6 100644 --- a/cache/src/main/scala/coursier/Cache.scala +++ b/cache/src/main/scala/coursier/Cache.scala @@ -1127,20 +1127,6 @@ object Cache { var bufferSize = 1024*1024 - def readFullySync(is: InputStream) = { - val buffer = new ByteArrayOutputStream() - val data = Array.ofDim[Byte](16384) - - var nRead = is.read(data, 0, data.length) - while (nRead != -1) { - buffer.write(data, 0, nRead) - nRead = is.read(data, 0, data.length) - } - - buffer.flush() - buffer.toByteArray - } - def withContent(is: InputStream, f: (Array[Byte], Int) => Unit): Unit = { val data = Array.ofDim[Byte](16384) diff --git a/cache/src/main/scala/coursier/internal/FileUtil.scala b/cache/src/main/scala/coursier/internal/FileUtil.scala index 5b8bcc876..5a6970101 100644 --- a/cache/src/main/scala/coursier/internal/FileUtil.scala +++ b/cache/src/main/scala/coursier/internal/FileUtil.scala @@ -66,7 +66,7 @@ object FileUtil { } } - private def readFully(is: InputStream): Array[Byte] = { + def readFully(is: InputStream): Array[Byte] = { val buffer = new ByteArrayOutputStream val data = Array.ofDim[Byte](16384) diff --git a/cli/src/main/scala-2.12/coursier/cli/Bootstrap.scala b/cli/src/main/scala-2.12/coursier/cli/Bootstrap.scala index 84ee3311c..5b1b7dac7 100644 --- a/cli/src/main/scala-2.12/coursier/cli/Bootstrap.scala +++ b/cli/src/main/scala-2.12/coursier/cli/Bootstrap.scala @@ -82,7 +82,7 @@ object Bootstrap extends CaseApp[BootstrapOptions] { val bootstrapJar = Option(Thread.currentThread().getContextClassLoader.getResourceAsStream("bootstrap.jar")) match { - case Some(is) => Cache.readFullySync(is) + case Some(is) => FileUtil.readFully(is) case None => Console.err.println(s"Error: bootstrap JAR not found") sys.exit(1) @@ -165,7 +165,7 @@ object Bootstrap extends CaseApp[BootstrapOptions] { entry.setTime(f.lastModified()) outputZip.putNextEntry(entry) - outputZip.write(Cache.readFullySync(new FileInputStream(f))) + outputZip.write(FileUtil.readFully(new FileInputStream(f))) outputZip.closeEntry() } diff --git a/cli/src/main/scala-2.12/coursier/cli/scaladex/Scaladex.scala b/cli/src/main/scala-2.12/coursier/cli/scaladex/Scaladex.scala index 1fa1030fe..a96a1dfd8 100644 --- a/cli/src/main/scala-2.12/coursier/cli/scaladex/Scaladex.scala +++ b/cli/src/main/scala-2.12/coursier/cli/scaladex/Scaladex.scala @@ -40,7 +40,7 @@ object Scaladex { val b = try { conn = new java.net.URL(url).openConnection().asInstanceOf[HttpURLConnection] - coursier.Platform.readFullySync(conn.getInputStream) + coursier.internal.FileUtil.readFully(conn.getInputStream) } finally { if (conn != null) coursier.Cache.closeConn(conn) diff --git a/cli/src/main/scala-2.12/coursier/cli/util/Zip.scala b/cli/src/main/scala-2.12/coursier/cli/util/Zip.scala index 23ed3263b..67c5d1494 100644 --- a/cli/src/main/scala-2.12/coursier/cli/util/Zip.scala +++ b/cli/src/main/scala-2.12/coursier/cli/util/Zip.scala @@ -2,8 +2,6 @@ package coursier.cli.util import java.util.zip.{ZipEntry, ZipInputStream} -import coursier.Platform - object Zip { def zipEntries(zipStream: ZipInputStream): Iterator[(ZipEntry, Array[Byte])] = @@ -17,7 +15,7 @@ object Zip { def hasNext = nextEntry.nonEmpty def next() = { val ent = nextEntry.get - val data = Platform.readFullySync(zipStream) + val data = coursier.internal.FileUtil.readFully(zipStream) update() diff --git a/cli/src/test/scala-2.12/coursier/cli/CliBootstrapIntegrationTest.scala b/cli/src/test/scala-2.12/coursier/cli/CliBootstrapIntegrationTest.scala index 6ed82df8c..8f6dce705 100644 --- a/cli/src/test/scala-2.12/coursier/cli/CliBootstrapIntegrationTest.scala +++ b/cli/src/test/scala-2.12/coursier/cli/CliBootstrapIntegrationTest.scala @@ -22,7 +22,7 @@ class CliBootstrapIntegrationTest extends FlatSpec with CliTestLib { if (e == null) throw new NoSuchElementException(s"Entry $path in zip file") else if (e.getName == path) - coursier.Platform.readFullySync(zis) + coursier.internal.FileUtil.readFully(zis) else zipEntryContent(zis, path) } @@ -53,7 +53,7 @@ class CliBootstrapIntegrationTest extends FlatSpec with CliTestLib { val content = try { fis = new FileInputStream(bootstrapFile) - coursier.Platform.readFullySync(fis) + coursier.internal.FileUtil.readFully(fis) } finally { if (fis != null) fis.close() } diff --git a/cache/src/main/scala/coursier/Platform.scala b/tests/jvm/src/test/scala/coursier/Platform.scala similarity index 100% rename from cache/src/main/scala/coursier/Platform.scala rename to tests/jvm/src/test/scala/coursier/Platform.scala