Have boostraps use the main coursier cache

bootstrap/src/main/java/coursier/Bootstrap.java

@@ -4,6 +4,8 @@ import java.io.*;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.*;
+import java.nio.channels.FileLock;
+import java.nio.channels.OverlappingFileLockException;
 import java.nio.file.Files;
 import java.nio.file.StandardCopyOption;
 import java.security.CodeSource;
@@ -52,21 +54,21 @@ public class Bootstrap {
 
     /**
      *
-     * @param jarDir can be null if nothing should be downloaded!
+     * @param cacheDir can be null if nothing should be downloaded!
      * @param isolationIDs
      * @param bootstrapProtocol
      * @param loader
      * @return
      * @throws IOException
      */
-    static Map<String, URL[]> readIsolationContexts(File jarDir, String[] isolationIDs, String bootstrapProtocol, ClassLoader loader) throws IOException {
+    static Map<String, URL[]> readIsolationContexts(File cacheDir, String[] isolationIDs, String bootstrapProtocol, ClassLoader loader) throws IOException {
         final Map<String, URL[]> perContextURLs = new LinkedHashMap<String, URL[]>();
 
         for (String isolationID: isolationIDs) {
             String[] strUrls = readStringSequence("bootstrap-isolation-" + isolationID + "-jar-urls");
             String[] resources = readStringSequence("bootstrap-isolation-" + isolationID + "-jar-resources");
             List<URL> urls = getURLs(strUrls, resources, bootstrapProtocol, loader);
-            List<URL> localURLs = getLocalURLs(urls, jarDir, bootstrapProtocol);
+            List<URL> localURLs = getLocalURLs(urls, cacheDir, bootstrapProtocol);
 
             perContextURLs.put(isolationID, localURLs.toArray(new URL[localURLs.size()]));
         }
@@ -130,17 +132,6 @@ public class Bootstrap {
         return "";
     }
 
-    static File localFile(File jarDir, URL url) {
-        if (url.getProtocol().equals("file"))
-            return new File(url.getPath());
-
-        String path = url.getPath();
-        int idx = path.lastIndexOf('/');
-        // FIXME Add other components in path to prevent conflicts?
-        String fileName = path.substring(idx + 1);
-        return new File(jarDir, fileName);
-    }
-
     // from http://www.java2s.com/Code/Java/File-Input-Output/Readfiletobytearrayandsavebytearraytofile.htm
     static void writeBytesToFile(File file, byte[] bytes) throws IOException {
         BufferedOutputStream bos = null;
@@ -163,12 +154,12 @@ public class Bootstrap {
     /**
      *
      * @param urls
-     * @param jarDir: can be null if nothing should be downloaded!
+     * @param cacheDir
      * @param bootstrapProtocol
      * @return
      * @throws MalformedURLException
      */
-    static List<URL> getLocalURLs(List<URL> urls, final File jarDir, String bootstrapProtocol) throws MalformedURLException {
+    static List<URL> getLocalURLs(List<URL> urls, final File cacheDir, String bootstrapProtocol) throws MalformedURLException {
 
         ThreadFactory threadFactory = new ThreadFactory() {
             // from scalaz Strategy.DefaultDaemonThreadFactory
@@ -195,9 +186,7 @@ public class Bootstrap {
             if (protocol.equals("file") || protocol.equals(bootstrapProtocol)) {
                 localURLs.add(url);
             } else {
-                assert jarDir != null : "Should not happen, not supposed to download things";
+                File dest = CachePath.localFile(url.toString(), cacheDir, null);
-
-                File dest = localFile(jarDir, url);
 
                 if (dest.exists()) {
                     localURLs.add(dest.toURI().toURL());
@@ -207,30 +196,68 @@ public class Bootstrap {
             }
         }
 
-        final Random random = new Random();
         for (final URL url : missingURLs) {
-            assert jarDir != null : "Should not happen, not supposed to download things";
-
             completionService.submit(new Callable<URL>() {
                 @Override
                 public URL call() throws Exception {
-                    File dest = localFile(jarDir, url);
+                    final File dest = CachePath.localFile(url.toString(), cacheDir, null);
 
                     if (!dest.exists()) {
+                        FileOutputStream out = null;
+                        FileLock lock = null;
+
+                        final File tmpDest = CachePath.temporaryFile(dest);
+                        final File lockFile = CachePath.lockFile(tmpDest);
+
                         try {
-                            URLConnection conn = url.openConnection();
+
-                            long lastModified = conn.getLastModified();
+                            out = CachePath.withStructureLock(cacheDir, new Callable<FileOutputStream>() {
-                            InputStream s = conn.getInputStream();
+                                @Override
-                            byte[] b = readFullySync(s);
+                                public FileOutputStream call() throws FileNotFoundException {
-                            File tmpDest = new File(dest.getParentFile(), dest.getName() + "-" + random.nextInt());
+                                    tmpDest.getParentFile().mkdirs();
-                            tmpDest.deleteOnExit();
+                                    lockFile.getParentFile().mkdirs();
-                            writeBytesToFile(tmpDest, b);
+                                    dest.getParentFile().mkdirs();
-                            Files.move(tmpDest.toPath(), dest.toPath(), StandardCopyOption.ATOMIC_MOVE);
+
-                            dest.setLastModified(lastModified);
+                                    return new FileOutputStream(lockFile);
+                                }
+                            });
+
+                            try {
+                                lock = out.getChannel().tryLock();
+                                if (lock == null)
+                                    throw new RuntimeException("Ongoing concurrent download for " + url);
+                                else
+                                    try {
+                                        URLConnection conn = url.openConnection();
+                                        long lastModified = conn.getLastModified();
+                                        InputStream s = conn.getInputStream();
+                                        byte[] b = readFullySync(s);
+                                        tmpDest.deleteOnExit();
+                                        writeBytesToFile(tmpDest, b);
+                                        tmpDest.setLastModified(lastModified);
+                                        Files.move(tmpDest.toPath(), dest.toPath(), StandardCopyOption.ATOMIC_MOVE);
+                                    }
+                                    finally {
+                                        lock.release();
+                                        lock = null;
+                                        out.close();
+                                        out = null;
+                                        lockFile.delete();
+                                    }
+                            }
+                            catch (OverlappingFileLockException e) {
+                                throw new RuntimeException("Ongoing concurrent download for " + url);
+                            }
+                            finally {
+                                if (lock != null) lock.release();
+                            }
                         } catch (Exception e) {
                             System.err.println("Error while downloading " + url + ": " + e.getMessage() + ", ignoring it");
                             throw e;
                         }
+                        finally {
+                            if (out != null) out.close();
+                        }
                     }
 
                     return dest.toURI().toURL();
@@ -345,19 +372,8 @@ public class Bootstrap {
         setExtraProperties("bootstrap.properties");
 
         String mainClass0 = System.getProperty("bootstrap.mainClass");
-        String jarDir0 = System.getProperty("bootstrap.jarDir");
 
-        File jarDir = null;
+        File cacheDir = CachePath.defaultCacheDirectory();
-
-        if (jarDir0 != null) {
-            jarDir = new File(jarDir0);
-
-            if (jarDir.exists()) {
-                if (!jarDir.isDirectory())
-                    exit("Error: " + jarDir0 + " is not a directory");
-            } else if (!jarDir.mkdirs())
-                System.err.println("Warning: cannot create " + jarDir0 + ", continuing anyway.");
-        }
 
         Random rng = new Random();
         String protocol = "bootstrap" + rng.nextLong();
@@ -369,10 +385,10 @@ public class Bootstrap {
         String[] strUrls = readStringSequence(defaultURLResource);
         String[] resources = readStringSequence(defaultJarResource);
         List<URL> urls = getURLs(strUrls, resources, protocol, contextLoader);
-        List<URL> localURLs = getLocalURLs(urls, jarDir, protocol);
+        List<URL> localURLs = getLocalURLs(urls, cacheDir, protocol);
 
         String[] isolationIDs = readStringSequence(isolationIDsResource);
-        Map<String, URL[]> perIsolationContextURLs = readIsolationContexts(jarDir, isolationIDs, protocol, contextLoader);
+        Map<String, URL[]> perIsolationContextURLs = readIsolationContexts(cacheDir, isolationIDs, protocol, contextLoader);
 
         Thread thread = Thread.currentThread();
         ClassLoader parentClassLoader = thread.getContextClassLoader();

build.sbt

@@ -82,6 +82,12 @@ lazy val `proxy-tests` = project
     sharedTestResources
   )
 
+lazy val paths = project
+  .settings(
+    pureJava,
+    dontPublish
+  )
+
 lazy val cache = project
   .dependsOn(coreJvm)
   .settings(
@@ -89,13 +95,15 @@ lazy val cache = project
     Mima.previousArtifacts,
     coursierPrefix,
     libs += Deps.scalazConcurrent,
-    Mima.cacheFilters
+    Mima.cacheFilters,
+    addPathsSources
   )
 
 lazy val bootstrap = project
   .settings(
     pureJava,
     dontPublish,
+    addPathsSources,
     // seems not to be automatically found with sbt 0.13.16-M1 :-/
     mainClass := Some("coursier.Bootstrap"),
     renameMainJar("bootstrap.jar")
@@ -266,6 +274,7 @@ lazy val jvm = project
     coreJvm,
     testsJvm,
     `proxy-tests`,
+    paths,
     cache,
     bootstrap,
     extra,
@@ -324,6 +333,7 @@ lazy val coursier = project
     testsJvm,
     testsJs,
     `proxy-tests`,
+    paths,
     cache,
     bootstrap,
     extra,
@@ -414,3 +424,7 @@ lazy val proguardedCli = Seq(
 lazy val sharedTestResources = {
   unmanagedResourceDirectories.in(Test) += baseDirectory.in(LocalRootProject).value / "tests" / "shared" / "src" / "test" / "resources"
 }
+
+lazy val addPathsSources = {
+  unmanagedSourceDirectories.in(Compile) ++= unmanagedSourceDirectories.in(Compile).in(paths).value
+}

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

@@ -4,7 +4,7 @@ import java.math.BigInteger
 import java.net.{ HttpURLConnection, URL, URLConnection, URLStreamHandler, URLStreamHandlerFactory }
 import java.nio.channels.{ OverlappingFileLockException, FileLock }
 import java.security.MessageDigest
-import java.util.concurrent.{ ConcurrentHashMap, Executors, ExecutorService }
+import java.util.concurrent.{ Callable, ConcurrentHashMap, Executors, ExecutorService }
 import java.util.regex.Pattern
 
 import coursier.core.Authentication
@@ -47,63 +47,8 @@ object Cache {
   // Check SHA-1 if available, else be fine with no checksum
   val defaultChecksums = Seq(Some("SHA-1"), None)
 
-  private val unsafeChars: Set[Char] = " %$&+,:;=?@<>#".toSet
+  def localFile(url: String, cache: File, user: Option[String]): File =
-
+    CachePath.localFile(url, cache, user.orNull)
-  // Scala version of http://stackoverflow.com/questions/4571346/how-to-encode-url-to-avoid-special-characters-in-java/4605848#4605848
-  // '/' was removed from the unsafe character list
-  private def escape(input: String): String = {
-
-    def toHex(ch: Int) =
-      (if (ch < 10) '0' + ch else 'A' + ch - 10).toChar
-
-    def isUnsafe(ch: Char) =
-      ch > 128 || ch < 0 || unsafeChars(ch)
-
-    input.flatMap {
-      case ch if isUnsafe(ch) =>
-        "%" + toHex(ch / 16) + toHex(ch % 16)
-      case other =>
-        other.toString
-    }
-  }
-
-  def localFile(url: String, cache: File, user: Option[String]): File = {
-    val path =
-      if (url.startsWith("file:///"))
-        url.stripPrefix("file://")
-      else if (url.startsWith("file:/"))
-        url.stripPrefix("file:")
-      else
-        // FIXME Should we fully parse the URL here?
-        // FIXME Should some safeguards be added against '..' components in paths?
-        url.split(":", 2) match {
-          case Array(protocol, remaining) =>
-            val remaining0 =
-              if (remaining.startsWith("///"))
-                remaining.stripPrefix("///")
-              else if (remaining.startsWith("/"))
-                remaining.stripPrefix("/")
-              else
-                throw new Exception(s"URL $url doesn't contain an absolute path")
-
-            val remaining1 =
-              if (remaining0.endsWith("/"))
-                // keeping directory content in .directory files
-                remaining0 + ".directory"
-              else
-                remaining0
-
-            new File(
-              cache,
-              escape(protocol + "/" + user.fold("")(_ + "@") + remaining1.dropWhile(_ == '/'))
-            ).toString
-
-          case _ =>
-            throw new Exception(s"No protocol found in URL $url")
-        }
-
-    new File(path)
-  }
 
   private def readFullyTo(
     in: InputStream,
@@ -129,45 +74,14 @@ object Cache {
     helper(alreadyDownloaded)
   }
 
-  private val processStructureLocks = new ConcurrentHashMap[File, AnyRef]
-
   /**
     * Should be acquired when doing operations changing the file structure of the cache (creating
     * new directories, creating / acquiring locks, ...), so that these don't hinder each other.
     *
     * Should hopefully address some transient errors seen on the CI of ensime-server.
     */
-  private def withStructureLock[T](cache: File)(f: => T): T = {
+  private def withStructureLock[T](cache: File)(f: => T): T =
-
+    CachePath.withStructureLock(cache, new Callable[T] { def call() = f })
-    val intraProcessLock = Option(processStructureLocks.get(cache)).getOrElse {
-      val lock = new AnyRef
-      val prev = Option(processStructureLocks.putIfAbsent(cache, lock))
-      prev.getOrElse(lock)
-    }
-
-    intraProcessLock.synchronized {
-      val lockFile = new File(cache, ".structure.lock")
-      lockFile.getParentFile.mkdirs()
-      var out = new FileOutputStream(lockFile)
-
-      try {
-        var lock: FileLock = null
-        try {
-          lock = out.getChannel.lock()
-
-          try f
-          finally {
-            lock.release()
-            lock = null
-            out.close()
-            out = null
-            lockFile.delete()
-          }
-        }
-        finally if (lock != null) lock.release()
-      } finally if (out != null) out.close()
-    }
-  }
 
   private def withLockOr[T](
     cache: File,
@@ -177,7 +91,7 @@ object Cache {
     ifLocked: => Option[FileError \/ T]
   ): FileError \/ T = {
 
-    val lockFile = new File(file.getParentFile, s"${file.getName}.lock")
+    val lockFile = CachePath.lockFile(file)
 
     var out: FileOutputStream = null
 
@@ -289,12 +203,6 @@ object Cache {
     helper(retry)
   }
 
-  private def temporaryFile(file: File): File = {
-    val dir = file.getParentFile
-    val name = file.getName
-    new File(dir, s"$name.part")
-  }
-
   private val partialContentResponseCode = 206
 
   private val handlerClsCache = new ConcurrentHashMap[String, Option[URLStreamHandler]]
@@ -647,7 +555,7 @@ object Cache {
       EitherT {
         Task {
 
-          val tmp = temporaryFile(file)
+          val tmp = CachePath.temporaryFile(file)
 
           var lenOpt = Option.empty[Option[Long]]
 
@@ -1187,12 +1095,7 @@ object Cache {
     throw new Exception("Cannot happen")
   )
 
-  lazy val default = new File(
+  lazy val default: File = CachePath.defaultCacheDirectory()
-    sys.env.getOrElse(
-      "COURSIER_CACHE",
-      sys.props("user.home") + "/.coursier/cache/v1"
-    )
-  ).getAbsoluteFile
 
   val defaultConcurrentDownloadCount = 6
 

paths/src/main/java/coursier/CachePath.java

@@ -0,0 +1,149 @@
+package coursier;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.net.MalformedURLException;
+import java.nio.channels.FileLock;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Cache paths logic, shared by the cache and bootstrap modules
+ */
+public class CachePath {
+
+    // based on https://stackoverflow.com/questions/4571346/how-to-encode-url-to-avoid-special-characters-in-java/4605848#4605848
+    // '/' was removed from the unsafe list
+    private static String escape(String input) {
+        StringBuilder resultStr = new StringBuilder();
+        for (char ch : input.toCharArray()) {
+            if (isUnsafe(ch)) {
+                resultStr.append('%');
+                resultStr.append(toHex(ch / 16));
+                resultStr.append(toHex(ch % 16));
+            } else {
+                resultStr.append(ch);
+            }
+        }
+        return resultStr.toString();
+    }
+
+    private static char toHex(int ch) {
+        return (char) (ch < 10 ? '0' + ch : 'A' + ch - 10);
+    }
+
+    private static boolean isUnsafe(char ch) {
+        return ch > 128 || " %$&+,:;=?@<>#%".indexOf(ch) >= 0;
+    }
+
+    public static File localFile(String url, File cache, String user) throws MalformedURLException {
+
+        // use the File constructor accepting a URI in case of problem with the two cases below?
+
+        if (url.startsWith("file:///"))
+            return new File(url.substring("file://".length()));
+
+        if (url.startsWith("file:/"))
+            return new File(url.substring("file:".length()));
+
+        String[] split = url.split(":", 2);
+        if (split.length != 2)
+            throw new MalformedURLException("No protocol found in URL " + url);
+
+        String protocol = split[0];
+        String remaining = split[1];
+
+        if (remaining.startsWith("///"))
+            remaining = remaining.substring("///".length());
+        else if (remaining.startsWith("/"))
+            remaining = remaining.substring("/".length());
+        else
+            throw new MalformedURLException("URL " + url + " doesn't contain an absolute path");
+
+        if (remaining.endsWith("/"))
+            // keeping directory content in .directory files
+            remaining = remaining + ".directory";
+
+        while (remaining.startsWith("/"))
+            remaining = remaining.substring(1);
+
+        String userPart = "";
+        if (user != null)
+            userPart = user + "@";
+
+        return new File(cache, escape(protocol + "/" + userPart + remaining));
+    }
+
+    public static File temporaryFile(File file) {
+        File dir = file.getParentFile();
+        String name = file.getName();
+        return new File(dir, name + ".part");
+    }
+
+    public static File lockFile(File file) {
+        return new File(file.getParentFile(), file.getName() + ".lock");
+    }
+
+    public static File defaultCacheDirectory() {
+
+        // cache this method result so that it's only ever computed once?
+
+        String path = System.getenv("COURSIER_CACHE");
+
+        if (path == null)
+          path = System.getProperty("coursier.cache");
+
+        if (path == null)
+            path = System.getProperty("user.home") + "/.coursier/cache/v1"; // shouldn't have put a "v1" component here...
+
+        return new File(path).getAbsoluteFile();
+    }
+
+    private static ConcurrentHashMap<File, Object> processStructureLocks = new ConcurrentHashMap<File, Object>();
+
+    public static <V> V withStructureLock(File cache, Callable<V> callable) throws Exception {
+
+        Object intraProcessLock = processStructureLocks.get(cache);
+
+        if (intraProcessLock == null) {
+            Object lock = new Object();
+            Object prev = processStructureLocks.putIfAbsent(cache, lock);
+            if (prev == null)
+                intraProcessLock = lock;
+            else
+                intraProcessLock = prev;
+        }
+
+        synchronized (intraProcessLock) {
+            File lockFile = new File(cache, ".structure.lock");
+            lockFile.getParentFile().mkdirs();
+            FileOutputStream out = null;
+
+            try {
+                out = new FileOutputStream(lockFile);
+
+                FileLock lock = null;
+                try {
+                    lock = out.getChannel().lock();
+
+                    try {
+                        return callable.call();
+                    }
+                    finally {
+                        lock.release();
+                        lock = null;
+                        out.close();
+                        out = null;
+                        lockFile.delete();
+                    }
+                }
+                finally {
+                    if (lock != null) lock.release();
+                }
+            } finally {
+                if (out != null) out.close();
+            }
+        }
+    }
+
+}