Preserve coursier system properties when shading

README.md

@@ -9,6 +9,8 @@
 
 *coursier-interface* is a zero-dependency Java library, exposing some of the features of the [API of coursier](https://get-coursier.io/docs/api). *coursier-interface* shades coursier, along with all its dependencies, so that it doesn't have any public dependency, and can be safely used along with other Scala or coursier versions.
 
+Even though coursier and its dependencies are shaded, the environment variables and Java properties that coursier reads are left untouched. That is `COURSIER_REPOSITORIES` / `coursier.repositories`, `COURSIER_CACHE` / `coursier.cache`, etc. are read by *coursier-interface* just like they are by coursier itself.
+
 *coursier-interface* aims at maintaining backward binary compatibility as much as possible. This means that if you depend on version N of coursier-interface, any version M >= N is safe to use at runtime. Backward binary compatibility has not been broken since the very first release of coursier-interface, `0.0.1` (ignoring version `0.0.11`, which exposed some dependencies that should have been shaded).
 
 *coursier-interface* doesn't support as many features as the API of coursier itself. For now, it has equivalents for:

build.mill

@@ -325,7 +325,13 @@ object proguarded extends CoursierInterfacePublishedModule {
       rule.setResult(to)
       rule
     }
-    val rules = Seq(
+    // JarJar remaps string constants that look like class names. That would rename the
+    // coursier system properties below too, which we'd rather keep as is, so that users can
+    // set them the same way, whether they use coursier directly or via coursier-interface.
+    // Identity rules, put before the more general coursier.** rule (the first matching rule
+    // wins), keep those strings untouched.
+    val identityRules = CoursierProperties.list.map(prop => rename(prop, prop))
+    val rules = identityRules ++ Seq(
       rename("scala.**", "coursierapi.shaded.scala.@1"),
       rename("coursier.**", "coursierapi.shaded.coursier.@1"),
       rename("dependency.**", "coursierapi.shaded.dependency.@1"),
@@ -392,6 +398,7 @@ object proguarded extends CoursierInterfacePublishedModule {
   def jar = Task {
     val jar0 = cleanedUpJar()
     Check.onlyNamespace("coursierapi", jar0.path.toIO)
+    Check.noShadedProperties(jar0.path.toIO)
     jar0
   }
   def docJar = interface(interfaceSv).docJar()
@@ -438,6 +445,13 @@ object `interface-test` extends Cross[InterfaceTest](Versions.scala)
 trait InterfaceTest extends CoursierInterfaceModule with CrossSbtModule with DependsOnProguarded { interfaceTest =>
   object test extends interfaceTest.SbtTests with TestModule.Junit4 {
     def junit4Version = Versions.junit
+    // SystemPropertyTests starts JVMs of its own, and needs our class path for that
+    def forkArgs = Task {
+      val classPath = runClasspath()
+        .map(_.path.toString)
+        .mkString(java.io.File.pathSeparator)
+      super.forkArgs() ++ Seq(s"-Dcoursier-interface.test-classpath=$classPath")
+    }
   }
 }
 

interface-test/src/test/java/coursierapi/test/PrintDefaultRepositories.java

@@ -0,0 +1,18 @@
+package coursierapi.test;
+
+import coursierapi.MavenRepository;
+import coursierapi.Repository;
+
+/** Prints the default repositories, one per line - used by {@link SystemPropertyTests} */
+public final class PrintDefaultRepositories {
+
+    public static void main(String[] args) {
+        for (Repository repository : Repository.defaults()) {
+            if (repository instanceof MavenRepository)
+                System.out.println(((MavenRepository) repository).getBase());
+            else
+                System.out.println(repository);
+        }
+    }
+
+}

interface-test/src/test/java/coursierapi/test/SystemPropertyTests.java

@@ -0,0 +1,71 @@
+package coursierapi.test;
+
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+
+public class SystemPropertyTests {
+
+    private static String javaCommand() {
+        boolean isWindows = System.getProperty("os.name", "")
+                .toLowerCase(Locale.ROOT)
+                .contains("windows");
+        Path javaHome = Paths.get(System.getProperty("java.home"));
+        return javaHome.resolve("bin").resolve(isWindows ? "java.exe" : "java").toString();
+    }
+
+    private static List<String> run(String property, String value, String mainClass) throws Exception {
+
+        // passed by the build, as we can't reliably get our own class path here
+        String classPath = System.getProperty("coursier-interface.test-classpath");
+        assertNotNull("coursier-interface.test-classpath not set", classPath);
+
+        List<String> command = new ArrayList<>();
+        command.add(javaCommand());
+        command.add("-cp");
+        command.add(classPath);
+        command.add("-D" + property + "=" + value);
+        command.add(mainClass);
+
+        ProcessBuilder builder = new ProcessBuilder(command);
+        // environment variables take precedence over Java properties
+        builder.environment().remove("COURSIER_REPOSITORIES");
+        builder.redirectInput(ProcessBuilder.Redirect.INHERIT);
+        builder.redirectError(ProcessBuilder.Redirect.INHERIT);
+        Path output = Files.createTempFile("coursier-interface-test-", ".txt");
+        try {
+            builder.redirectOutput(output.toFile());
+            Process p = builder.start();
+            int retCode = p.waitFor();
+            assertEquals("Non-zero exit code from " + mainClass, 0, retCode);
+            return Files.readAllLines(output);
+        } finally {
+            Files.deleteIfExists(output);
+        }
+    }
+
+    // the shading used to rename the coursier.repositories property under the hood,
+    // see https://github.com/coursier/interface/issues/477
+    @Test
+    public void repositories() throws Exception {
+
+        String repository = "https://foo.example.com/maven";
+
+        // in a JVM of its own, as coursier only reads that property once
+        List<String> defaultRepositories = run(
+                "coursier.repositories",
+                repository,
+                PrintDefaultRepositories.class.getName()
+        );
+
+        assertEquals(java.util.Collections.singletonList(repository), defaultRepositories);
+    }
+
+}

mill-build/src/interfacebuild/Check.scala

@@ -1,5 +1,7 @@
 package interfacebuild
 
+import org.objectweb.asm.ClassReader
+
 import java.io.File
 import java.util.zip.ZipFile
 
@@ -24,4 +26,50 @@ object Check {
     assert(unrecognized.isEmpty)
   }
 
+  /** String constants of a class file (its `CONSTANT_String` constant pool entries) */
+  private def stringConstants(classFile: Array[Byte]): Seq[String] = {
+    val constantStringTag = 8 // CONSTANT_String, see JVMS 4.4
+    val reader = new ClassReader(classFile)
+    val buffer = Array.ofDim[Char](reader.getMaxStringLength)
+    (1 until reader.getItemCount).flatMap { idx =>
+      val offset = reader.getItem(idx)
+      // CONSTANT_Long / CONSTANT_Double take two constant pool entries, the second one has a zero offset
+      if (offset > 0 && reader.readByte(offset - 1) == constantStringTag)
+        Seq(reader.readUTF8(offset, buffer))
+      else
+        Nil
+    }
+  }
+
+  /** Ensures no coursier system property was shaded
+    *
+    * JarJar remaps string constants that look like class names, which would rename coursier system
+    * properties like `coursier.repositories` too, see
+    * [[https://github.com/coursier/interface/issues/477]]. Those are meant to be kept as is by the
+    * identity rules built from [[CoursierProperties.list]].
+    */
+  def noShadedProperties(jar: File): Unit = {
+    // properties can't be told apart from packages for sure, we assume a string whose last
+    // element starts with a lower case letter is one (class names start with an upper case one)
+    val propertyLike = "coursierapi\\.shaded\\.coursier(\\.[A-Za-z0-9_-]+)*\\.[a-z][A-Za-z0-9_-]*".r
+    val zf = new ZipFile(jar)
+    val shaded =
+      try
+        zf.entries()
+          .asScala
+          .filter(_.getName.endsWith(".class"))
+          .flatMap(ent => stringConstants(zf.getInputStream(ent).readAllBytes()))
+          .collect { case s @ propertyLike(_*) => s }
+          .toVector
+          .distinct
+          .sorted
+      finally zf.close()
+    for (s <- shaded)
+      System.err.println(
+        s"Shaded coursier system property: $s " +
+          s"(add ${s.stripPrefix("coursierapi.shaded.")} to CoursierProperties.list if it is one)"
+      )
+    assert(shaded.isEmpty)
+  }
+
 }

mill-build/src/interfacebuild/CoursierProperties.scala

@@ -0,0 +1,43 @@
+package interfacebuild
+
+object CoursierProperties {
+
+  /** Coursier system properties, that shouldn't be shaded
+    *
+    * JarJar remaps string constants that look like class names, so that these would be
+    * renamed to `coursierapi.shaded.coursier.…` without specific rules, see
+    * [[https://github.com/coursier/interface/issues/477]].
+    */
+  def list = Seq(
+    "coursier.archive.cache",
+    "coursier.cache",
+    "coursier.cache.throw-exceptions",
+    "coursier.config-dir",
+    "coursier.core.throw-exceptions",
+    "coursier.credentials",
+    "coursier.data-dir",
+    "coursier.digest-based.cache",
+    "coursier.directories.powershell-debug",
+    "coursier.exception-retry",
+    "coursier.exception-retry-backoff-initial-delay",
+    "coursier.exception-retry-backoff-multiplier",
+    "coursier.http.maxRedirects",
+    "coursier.ivy.home",
+    "coursier.jni",
+    "coursier.jni.check.throw",
+    "coursier.jvm.cache",
+    "coursier.mirrors",
+    "coursier.mirrors.extra",
+    "coursier.mode",
+    "coursier.parallel-download-count",
+    "coursier.priviledged.archive.cache",
+    "coursier.repositories",
+    "coursier.sslexception-retry",
+    "coursier.structure-lock-retry-count",
+    "coursier.structure-lock-retry-initial-delay-ms",
+    "coursier.structure-lock-retry-multiplier",
+    "coursier.ttl",
+    "coursier.windows.disable-ffm"
+  )
+
+}

mill-build/src/interfacebuild/ZipUtil.scala

@@ -1,24 +1,9 @@
 package interfacebuild
 
-import java.io.{ByteArrayOutputStream, File, FileInputStream, FileOutputStream, InputStream}
 import java.util.zip.{ZipEntry, ZipFile, ZipInputStream, ZipOutputStream}
 
 object ZipUtil {
 
-  private 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
-  }
-
   private def zipEntries(zipStream: ZipInputStream): Iterator[(ZipEntry, Array[Byte])] =
     new Iterator[(ZipEntry, Array[Byte])] {
       private var nextEntry = Option.empty[ZipEntry]
@@ -30,7 +15,7 @@ object ZipUtil {
       def hasNext = nextEntry.nonEmpty
       def next() = {
         val ent = nextEntry.get
-        val data = readFullySync(zipStream)
+        val data = zipStream.readAllBytes()
 
         update()
 
@@ -102,7 +87,7 @@ object ZipUtil {
     val zf = new ZipFile(sourceZip.toIO)
     val entryOpt = Option(zf.getEntry(entryName))
     val content = entryOpt.map { entry =>
-      readFullySync(zf.getInputStream(entry))
+      zf.getInputStream(entry).readAllBytes()
     }
     zf.close()
     content