From 7cd4f773b22d973ebaa22af5b531093f11b90a95 Mon Sep 17 00:00:00 2001 From: Alex Archambault Date: Sat, 29 Aug 2026 12:13:46 +0200 Subject: [PATCH] Preserve coursier system properties when shading Add identity rules and validation to keep coursier properties unchanged in the shaded interface. Co-authored-by: Claude Opus 5 --- README.md | 2 + build.mill | 16 ++++- .../test/PrintDefaultRepositories.java | 18 +++++ .../coursierapi/test/SystemPropertyTests.java | 71 +++++++++++++++++++ mill-build/src/interfacebuild/Check.scala | 48 +++++++++++++ .../interfacebuild/CoursierProperties.scala | 43 +++++++++++ mill-build/src/interfacebuild/ZipUtil.scala | 19 +---- 7 files changed, 199 insertions(+), 18 deletions(-) create mode 100644 interface-test/src/test/java/coursierapi/test/PrintDefaultRepositories.java create mode 100644 interface-test/src/test/java/coursierapi/test/SystemPropertyTests.java create mode 100644 mill-build/src/interfacebuild/CoursierProperties.scala diff --git a/README.md b/README.md index 192a2b3..bb4b5f7 100644 --- a/README.md +++ b/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: diff --git a/build.mill b/build.mill index 90a47e3..ec83f14 100644 --- a/build.mill +++ b/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") + } } } diff --git a/interface-test/src/test/java/coursierapi/test/PrintDefaultRepositories.java b/interface-test/src/test/java/coursierapi/test/PrintDefaultRepositories.java new file mode 100644 index 0000000..9d44ff8 --- /dev/null +++ b/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); + } + } + +} diff --git a/interface-test/src/test/java/coursierapi/test/SystemPropertyTests.java b/interface-test/src/test/java/coursierapi/test/SystemPropertyTests.java new file mode 100644 index 0000000..dabe515 --- /dev/null +++ b/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 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 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 defaultRepositories = run( + "coursier.repositories", + repository, + PrintDefaultRepositories.class.getName() + ); + + assertEquals(java.util.Collections.singletonList(repository), defaultRepositories); + } + +} diff --git a/mill-build/src/interfacebuild/Check.scala b/mill-build/src/interfacebuild/Check.scala index 74714eb..225ff06 100644 --- a/mill-build/src/interfacebuild/Check.scala +++ b/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) + } + } diff --git a/mill-build/src/interfacebuild/CoursierProperties.scala b/mill-build/src/interfacebuild/CoursierProperties.scala new file mode 100644 index 0000000..72cdfda --- /dev/null +++ b/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" + ) + +} diff --git a/mill-build/src/interfacebuild/ZipUtil.scala b/mill-build/src/interfacebuild/ZipUtil.scala index 31c36ce..aa0d874 100644 --- a/mill-build/src/interfacebuild/ZipUtil.scala +++ b/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