Retry on ChecksumError

BUILD.tools

@@ -7,6 +7,7 @@ jar_library(
             rev = "2.12.4",
         ),
     ],
+    scope = 'runtime'
 )
 
 jar_library(

cache/src/main/scala/coursier/Cache.scala

@@ -37,7 +37,7 @@ object Cache {
   }
 
   // java.nio.charset.StandardCharsets.UTF_8 not available in Java 6
-  private val UTF_8 = Charset.forName("UTF-8")
+  val UTF_8: Charset = Charset.forName("UTF-8")
 
   // Check SHA-1 if available, else be fine with no checksum
   val defaultChecksums = Seq(Some("SHA-1"), None)
@@ -518,7 +518,7 @@ object Cache {
                       doTouchCheckFile(file)
                       Right(false)
                     }
-                  case other =>
+                  case other =>FileError
                     Task.now(other)
                 }
             }
@@ -939,21 +939,28 @@ object Cache {
     }
   }
 
+  /**
+    * This method computes the task needed to get a file.
+    *
+    * Retry only applies to [[coursier.FileError.WrongChecksum]].
+    *
+    * [[coursier.FileError.DownloadError]] is handled separately at [[downloading]]
+    */
   def file(
     artifact: Artifact,
     cache: File = default,
     cachePolicy: CachePolicy = CachePolicy.UpdateChanging,
     checksums: Seq[Option[String]] = defaultChecksums,
     logger: Option[Logger] = None,
     pool: ExecutorService = defaultPool,
-    ttl: Option[Duration] = defaultTtl
+    ttl: Option[Duration] = defaultTtl,
+    retry: Int = 1
   ): EitherT[Task, FileError, File] = {
 
     implicit val pool0 = pool
+    val checksums0: Seq[Option[String]] = if (checksums.isEmpty) Seq(None) else checksums
 
-    val checksums0 = if (checksums.isEmpty) Seq(None) else checksums
-
-    val res = EitherT {
+    val res: EitherT[Task, FileError, (File, Option[String])] = EitherT {
       download(
         artifact,
         cache,
@@ -987,10 +994,35 @@ object Cache {
     }
 
     res.flatMap {
-      case (f, None) => EitherT(Task.now[Either[FileError, File]](Right(f)))
-      case (f, Some(c)) =>
-        validateChecksum(artifact, c, cache, pool).map(_ => f)
+      case (file0, None) => EitherT(Task.now[Either[FileError, File]](Right(file0)))
+      case (file0, Some(shaType)) =>
+        validateChecksum(artifact, shaType, cache, pool).map(_ => file0)
+    }.leftFlatMap {
+      case err: FileError.WrongChecksum =>
+        if (retry == 0) {
+          logger.foreach(_.log(s"Retry exhausted for ${artifact.url}\n", None))
+          EitherT(Task.now[Either[FileError, File]](Left(err)))
+        }
+        else {
+          val badFile = localFile(artifact.url, cache, artifact.authentication.map(_.user))
+          badFile.delete()
+          logger.foreach(_.log(s"Bad file deleted: ${badFile.getAbsolutePath} due to wrong checksum. Retrying with count $retry...\n", None))
+          file(
+            artifact,
+            cache,
+            cachePolicy,
+            checksums,
+            logger,
+            pool,
+            ttl,
+            retry - 1
+          )
+        }
+      case err =>
+        EitherT(Task.now[Either[FileError, File]](Left(err)))
     }
+
+
   }
 
   def fetch(
@@ -1134,6 +1166,7 @@ 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 = {}
+    def log(content: String, currentTimeOpt: Option[Long]): Unit = {}
   }
 
   object Logger {
@@ -1168,6 +1201,8 @@ object Cache {
                 logger.checkingUpdates(url, currentTimeOpt)
               override def checkingUpdatesResult(url: String, currentTimeOpt: Option[Long], remoteTimeOpt: Option[Long]) =
                 logger.checkingUpdatesResult(url, currentTimeOpt, remoteTimeOpt)
+              override def log(content: String, currentTimeOpt: Option[Long]): Unit =
+                logger.log(content, currentTimeOpt)
             }
         }
     }

cache/src/main/scala/coursier/TermDisplay.scala

@@ -509,4 +509,9 @@ class TermDisplay(
     }
   }
 
+  override def log(content: String, currentTimeOpt: Option[Long]): Unit = {
+    updateRunnable.newEntry(content, CheckUpdateInfo(currentTimeOpt, None, isDone = true), content)
+    updateRunnable.removeEntry(content, success = true, fallbackMessage = content)(x => x)
+  }
+
 }

cli/src/main/scala-2.12/coursier/cli/Helper.scala

@@ -628,7 +628,8 @@ class Helper(
         checksums = checksums,
         logger = logger,
         pool = pool,
-        ttl = ttl0
+        ttl = ttl0,
+        retry = common.retryCount
       )
 
       (file(cachePolicies.head) /: cachePolicies.tail)(_ orElse file(_))

cli/src/main/scala-2.12/coursier/cli/options/CacheOptions.scala

@@ -5,7 +5,6 @@ import coursier.Cache
 
 final case class CacheOptions(
   @Help("Cache directory (defaults to environment variable COURSIER_CACHE or ~/.coursier/cache/v1)")
-  @Short("C")
     cache: String = Cache.default.toString
 )
 

cli/src/main/scala-2.12/coursier/cli/options/CommonOptions.scala

@@ -95,7 +95,11 @@ final case class CommonOptions(
   @Help("Swap the mainline Scala JARs by Typelevel ones")
     typelevel: Boolean = false,
   @Recurse
-    cacheOptions: CacheOptions = CacheOptions()
+    cacheOptions: CacheOptions = CacheOptions(),
+
+  @Help("Retry limit for Checksum error when fetching a file")
+    retryCount: Int = 1
+
 ) {
   val verbosityLevel = Tag.unwrap(verbose) - (if (quiet) 1 else 0)
   lazy val classifier0 = classifier.flatMap(_.split(',')).filter(_.nonEmpty).toSet

cli/src/test/scala-2.12/coursier/cli/BUILD

@@ -11,7 +11,7 @@ junit_tests(
 scala_library(
     name='lib',
     dependencies = [
-        "3rdparty/jvm:caseapp",
+        "cache/src/main/scala:cache"
     ],
     sources = ["CliTestLib.scala"],
 )

cli/src/test/scala-2.12/coursier/cli/CliFetchIntegrationTest.scala

@@ -1,12 +1,13 @@
 package coursier.cli
 
-import java.io._
-import java.util.zip.ZipInputStream
-import java.net.URLEncoder.encode
 import argonaut.Argonaut._
-import coursier.cli.util.{DepNode, ReportNode}
 import caseapp.core.RemainingArgs
+import coursier.Cache.UTF_8
 import coursier.cli.options._
+import coursier.cli.util.{DepNode, ReportNode}
+import coursier.internal.FileUtil
+import java.io._
+import java.net.URLEncoder.encode
 import org.junit.runner.RunWith
 import org.scalatest.FlatSpec
 import org.scalatest.junit.JUnitRunner
@@ -707,4 +708,58 @@ class CliFetchIntegrationTest extends FlatSpec with CliTestLib {
       assert(depNode.get.dependencies.head.contains("org.tukaani:xz:1.2"))
     }
   }
+
+  "Bad pom resolve" should "succeed with retry" in withTempDir("tmp_dir") {
+    dir => {
+      def runFetchJunit = {
+        val fetchOpt = FetchOptions(common = CommonOptions(cacheOptions = CacheOptions(cache = dir.getAbsolutePath)))
+        val fetch = Fetch(fetchOpt, RemainingArgs(Seq("junit:junit:4.12"), Seq()))
+        assert(fetch.files0.map(_.getName).toSet
+          .equals(Set("junit-4.12.jar", "hamcrest-core-1.3.jar")))
+        val junitJarPath = fetch.files0.map(_.getAbsolutePath()).filter(_.contains("junit-4.12.jar"))
+          .head
+        val junitPomFile = new File(junitJarPath.replace(".jar", ".pom"))
+        val junitPomShaFile = new File(junitJarPath.replace(".jar", ".pom.sha1"))
+        assert(junitPomFile.exists())
+        assert(junitPomShaFile.exists())
+        junitPomFile
+      }
+
+      val junitPomFile: File = runFetchJunit
+      val originalPomContent = FileUtil.readAllBytes(junitPomFile)
+
+      // Corrupt the pom content
+      FileUtil.write(junitPomFile, "bad pom".getBytes(UTF_8))
+
+      // Run fetch again and it should pass because of retrying om the bad pom.
+      val pom = runFetchJunit
+      assert(FileUtil.readAllBytes(pom).sameElements(originalPomContent))
+    }
+  }
+
+  "Bad jar resolve" should "succeed with retry" in withTempDir("tmp_dir") {
+    dir => {
+      def runFetchJunit = {
+        val fetchOpt = FetchOptions(common = CommonOptions(cacheOptions = CacheOptions(cache = dir.getAbsolutePath)))
+        val fetch = Fetch(fetchOpt, RemainingArgs(Seq("junit:junit:4.12"), Seq()))
+        assert(fetch.files0.map(_.getName).toSet
+          .equals(Set("junit-4.12.jar", "hamcrest-core-1.3.jar")))
+        val junitJarPath = fetch.files0.map(_.getAbsolutePath()).filter(_.contains("junit-4.12.jar"))
+          .head
+        val junitJarFile = new File(junitJarPath)
+        junitJarFile
+      }
+
+      val originalJunitJar: File = runFetchJunit
+      val originalJunitJarContent = FileUtil.readAllBytes(originalJunitJar)
+
+      // Corrupt the jar content
+      FileUtil.write(originalJunitJar, "bad jar".getBytes(UTF_8))
+
+      // Run fetch again and it should pass because of retrying on the bad jar.
+      val jar = runFetchJunit
+      assert(FileUtil.readAllBytes(jar).sameElements(originalJunitJarContent))
+    }
+  }
+
 }

cli/src/test/scala-2.12/coursier/cli/CliTestLib.scala

@@ -1,5 +1,6 @@
 package coursier.cli
 
+import coursier.internal.FileUtil
 import java.io.{File, FileWriter}
 
 
@@ -14,10 +15,36 @@ trait CliTestLib {
     writer.flush()
     try {
       testCode(file, writer) // "loan" the fixture to the test
-    }
-    finally {
+    } finally {
       writer.close()
       file.delete()
     }
   }
+
+  def withTempDir(
+      prefix: String
+  )(testCode: File => Any) {
+    val dir = FileUtil.createTempDirectory(prefix) // create the fixture
+    try {
+      testCode(dir) // "loan" the fixture to the test
+    } finally {
+      cleanDir(dir)
+    }
+  }
+
+  def cleanDir(tmpDir: File): Unit = {
+    def delete(f: File): Boolean =
+      if (f.isDirectory) {
+        val removedContent =
+          Option(f.listFiles()).toSeq.flatten.map(delete).forall(x => x)
+        val removedDir = f.delete()
+
+        removedContent && removedDir
+      } else
+        f.delete()
+
+    if (!delete(tmpDir))
+      Console.err.println(
+        s"Warning: unable to remove temporary directory $tmpDir")
+  }
 }

core/shared/src/main/scala/coursier/util/EitherT.scala

@@ -25,6 +25,16 @@ final case class EitherT[F[_], L, R](run: F[Either[L, R]]) {
       M.map(run)(e => e.left.map(f))
     )
 
+  def leftFlatMap[S](f: L => EitherT[F, S, R])(implicit M: Monad[F]): EitherT[F, S, R] =
+    EitherT(
+      M.bind(run) {
+        case Left(l) =>
+          f(l).run
+        case Right(r) =>
+          M.point(Right(r))
+      }
+    )
+
   def orElse(other: => EitherT[F, L, R])(implicit M: Monad[F]): EitherT[F, L, R] =
     EitherT(
       M.bind(run) {