Support pure JavaModules (#155)

* Support pure `JavaModule`s Looks like there is no need for being a `ScalaModule` at all. There is some sanity check, but it looks like not really needed. Fix #154 * Move type check outside of task * scalafmt * Add itest for java module * Use Scala 3 as default * Use optional scalaVersion via `null` for java compatibility * Fix test case * Use `null` only when calling the "Java" API * scalafmt again * .orNull * Fix unrelated issue with `ExtraCoursierSupport` for Mill 0.11 Instead of extending `ScalaModule` it should use the `bindDependency` task provided by `CoursierSupport`. Otherwise, it can be problematic in downstream modules. --------- Co-authored-by: Lorenzo Gabriele <[email protected]>
lolgab · May 30, 2024 · 283e145 · 283e145
1 parent 880d061
commit 283e145
Show file tree

Hide file tree

Showing 8 changed files with 131 additions and 93 deletions.
diff --git a/build.sc b/build.sc
@@ -126,7 +126,12 @@ trait itestCross extends MillIntegrationTestModule with Cross.Module[String] {
           TestInvocation.Targets(Seq("prepare")),
           TestInvocation.Targets(Seq("verify")),
           TestInvocation.Targets(Seq("verifyFail"), expectedExitCode = 1)
-        )
+        ),
+        PathRef(testBase / "java") -> Seq(
+          TestInvocation.Targets(Seq("prepare")),
+          TestInvocation.Targets(Seq("verify"), expectedExitCode = 1)
+        ),
+
       )
     }
 }
diff --git a/itest/src/java/build.sc b/itest/src/java/build.sc
@@ -0,0 +1,26 @@
+import mill._
+
+import mill.scalalib._
+import mill.scalalib.publish._
+import $file.plugins
+import com.github.lolgab.mill.mima._
+
+trait Common extends JavaModule with PublishModule {
+  def publishVersion = "0.0.1"
+  def pomSettings =
+    PomSettings("", organization = "org", "", Seq(), VersionControl(), Seq())
+}
+object prev extends Common
+object curr extends Common with Mima {
+  override def mimaPreviousArtifacts = T(Agg(ivy"org:prev:0.0.1"))
+  override def mimaCheckDirection = CheckDirection.Backward
+}
+
+def prepare() = T.command {
+  prev.publishLocal(sys.props("ivy.home") + "/local")()
+}
+
+def verify() = T.command {
+  curr.mimaReportBinaryIssues()()
+  ()
+}
diff --git a/itest/src/java/curr/src/Main.java b/itest/src/java/curr/src/Main.java
@@ -0,0 +1,2 @@
+public class Main {
+}
diff --git a/itest/src/java/prev/src/Main.java b/itest/src/java/prev/src/Main.java
@@ -0,0 +1,5 @@
+public class Main {
+  public static String hello() {
+    return "Hello world!";
+  }
+}
diff --git a/mill-mima-worker-api/src/com/github/lolgab/mill/mima/worker/api/MimaWorkerApi.java b/mill-mima-worker-api/src/com/github/lolgab/mill/mima/worker/api/MimaWorkerApi.java
@@ -1,8 +1,8 @@
 package com.github.lolgab.mill.mima.worker.api;
 
 public interface MimaWorkerApi {
-  java.util.Optional<String> reportBinaryIssues(
-    String scalaBinaryVersion,
+  String reportBinaryIssues(
+    String scalaBinaryVersionOrNull,
     java.util.function.Consumer<String> logDebug,
     java.util.function.Consumer<String> logError,
     java.util.function.Consumer<String> logPrintln,

diff --git a/mill-mima-worker-impl/src/com/github/lolgab/mill/mima/worker/MimaWorkerImpl.scala b/mill-mima-worker-impl/src/com/github/lolgab/mill/mima/worker/MimaWorkerImpl.scala
@@ -12,7 +12,7 @@ import scala.jdk.CollectionConverters._
 class MimaWorkerImpl extends MimaWorkerApi {
 
   def reportBinaryIssues(
-      scalaBinaryVersion: String,
+      scalaBinaryVersionOrNull: String,
       logDebug: java.util.function.Consumer[String],
       logError: java.util.function.Consumer[String],
       logPrintln: java.util.function.Consumer[String],
@@ -25,8 +25,8 @@ class MimaWorkerImpl extends MimaWorkerApi {
       forwardFilters: java.util.Map[String, Array[ProblemFilter]],
       excludeAnnos: Array[String],
       publishVersion: String
-  ): java.util.Optional[String] = {
-    sanityCheckScalaBinaryVersion(scalaBinaryVersion)
+  ): String = {
+    sanityCheckScalaBinaryVersion(scalaBinaryVersionOrNull)
 
     val mimaLib = new MiMaLib(runClasspath.toSeq)
 
@@ -74,12 +74,10 @@ class MimaWorkerImpl extends MimaWorkerApi {
     if (problemsCount > 0) {
       val filteredNote =
         if (filteredCount > 0) s" (filtered $filteredCount)" else ""
-      java.util.Optional.of(
-        s"Failed binary compatibility check! Found $problemsCount potential problems$filteredNote"
-      )
+      s"Failed binary compatibility check! Found $problemsCount potential problems$filteredNote"
     } else {
       logPrintln.accept("Binary compatibility check passed")
-      java.util.Optional.empty()
+      null
     }
   }
 
@@ -91,12 +89,14 @@ class MimaWorkerImpl extends MimaWorkerApi {
     s" * $desc$howToFilter"
   }
 
-  private def sanityCheckScalaBinaryVersion(scalaBinaryVersion: String) = {
-    scalaBinaryVersion match {
-      case "2.11" | "2.12" | "2.13" | "3" => // ok
-      case _ =>
+  private def sanityCheckScalaBinaryVersion(
+      scalaBinaryVersionOrNull: String
+  ) = {
+    scalaBinaryVersionOrNull match {
+      case "3" | "2.13" | "2.12" | "2.11" | null => // ok
+      case other =>
         throw new IllegalArgumentException(
-          s"MiMa supports Scala 2.11, 2.12, 2.13 and 3, not $scalaBinaryVersion"
+          s"MiMa supports Scala 2.11, 2.12, 2.13 and 3, not $other"
         )
     }
   }

diff --git a/mill-mima/src-mill0.11/com/github/lolgab/mill/mima/ExtraCoursierSupport.scala b/mill-mima/src-mill0.11/com/github/lolgab/mill/mima/ExtraCoursierSupport.scala
@@ -7,11 +7,7 @@ import mill.api.Result
 import mill.define.Task
 import mill.scalalib._
 
-import scala.util.chaining._
-
-private[mima] trait ExtraCoursierSupport
-    extends CoursierModule
-    with ScalaModule {
+private[mima] trait ExtraCoursierSupport extends CoursierModule {
 
   /** Resolves each dependency independently.
     *
@@ -27,18 +23,14 @@ private[mima] trait ExtraCoursierSupport
       deps: Task[Agg[Dep]]
   ): Task[Agg[(Dep, Agg[PathRef])]] = T.task {
     val pRepositories = repositoriesTask()
+    val bind = bindDependency()
     val pDeps = deps()
-    val scalaV = scalaVersion()
     pDeps.map { dep =>
       val Result.Success(resolved) = Lib.resolveDependencies(
         repositories = pRepositories,
-        deps = Agg(dep).map(dep =>
-          Lib
-            .depToBoundDep(dep, scalaV)
-            .tap(dependency =>
-              dependency.copy(dep = dependency.dep.withTransitive(false))
-            )
-        ),
+        deps = Agg(dep)
+          .map(bind)
+          .map(dep => dep.copy(dep = dep.dep.withTransitive(false))),
         ctx = Some(implicitly[mill.api.Ctx.Log])
       )
       (dep, resolved)

diff --git a/mill-mima/src/com/github/lolgab/mill/mima/MimaBase.scala b/mill-mima/src/com/github/lolgab/mill/mima/MimaBase.scala
@@ -13,7 +13,7 @@ import scala.jdk.CollectionConverters._
 import scala.util.chaining._
 
 private[mima] trait MimaBase
-    extends ScalaModule
+    extends JavaModule
     with PublishModule
     with ExtraCoursierSupport
     with VersionSpecific {
@@ -113,72 +113,80 @@ private[mima] trait MimaBase
     */
   def mimaCurrentArtifact: T[PathRef] = T { jar() }
 
-  def mimaReportBinaryIssues(): Command[Unit] = T.command {
-    def prettyDep(dep: Dep): String = {
-      s"${dep.dep.module.orgName}:${dep.dep.version}"
+  def mimaReportBinaryIssues(): Command[Unit] = {
+    val scalaBinVersionOrNullTask: Task[Option[String]] = this match {
+      case m: ScalaModule =>
+        T.task { Some(scalaBinaryVersion(m.scalaVersion())) }
+      case _ => T.task { None }
     }
-    val log = T.ctx().log
-    val logDebug: java.util.function.Consumer[String] = log.debug(_)
-    val logError: java.util.function.Consumer[String] = log.error(_)
-    val logPrintln: java.util.function.Consumer[String] =
-      log.outputStream.println(_)
-    val runClasspathIO =
-      runClasspath().view.map(_.path).filter(os.exists).map(_.toIO).toArray
-    val current = mimaCurrentArtifact().path.pipe {
-      case p if os.exists(p) => p
-      case _                 => (T.dest / "emptyClasses").tap(os.makeDir)
-    }.toIO
-
-    val previous = resolvedMimaPreviousArtifacts().iterator.map {
-      case (dep, artifact) =>
-        new worker.api.Artifact(prettyDep(dep), artifact.path.toIO)
-    }.toArray
-
-    val checkDirection = mimaCheckDirection() match {
-      case CheckDirection.Forward  => worker.api.CheckDirection.Forward
-      case CheckDirection.Backward => worker.api.CheckDirection.Backward
-      case CheckDirection.Both     => worker.api.CheckDirection.Both
-    }
-
-    def toWorkerApi(p: ProblemFilter) =
-      new worker.api.ProblemFilter(p.name, p.problem)
-
-    val incompatibleSignatureProblemFilters =
-      if (mimaReportSignatureProblems()) Seq.empty
-      else Seq(ProblemFilter.exclude[IncompatibleSignatureProblem]("*"))
-    val binaryFilters =
-      (mimaBinaryIssueFilters() ++ incompatibleSignatureProblemFilters)
-        .map(toWorkerApi)
-        .toArray
-    val backwardFilters =
-      mimaBackwardIssueFilters().view
-        .mapValues(_.map(toWorkerApi).toArray)
-        .toMap
-        .asJava
-    val forwardFilters =
-      mimaForwardIssueFilters().view
-        .mapValues(_.map(toWorkerApi).toArray)
-        .toMap
-        .asJava
-
-    val errorOpt: java.util.Optional[String] = mimaWorker().reportBinaryIssues(
-      scalaBinaryVersion(scalaVersion()),
-      logDebug,
-      logError,
-      logPrintln,
-      checkDirection,
-      runClasspathIO,
-      previous,
-      current,
-      binaryFilters,
-      backwardFilters,
-      forwardFilters,
-      mimaExcludeAnnotations().toArray,
-      publishVersion()
-    )
+    T.command {
+      def prettyDep(dep: Dep): String = {
+        s"${dep.dep.module.orgName}:${dep.dep.version}"
+      }
+      val log = T.ctx().log
+      val logDebug: java.util.function.Consumer[String] = log.debug(_)
+      val logError: java.util.function.Consumer[String] = log.error(_)
+      val logPrintln: java.util.function.Consumer[String] =
+        log.outputStream.println(_)
+      val runClasspathIO =
+        runClasspath().view.map(_.path).filter(os.exists).map(_.toIO).toArray
+      val current = mimaCurrentArtifact().path.pipe {
+        case p if os.exists(p) => p
+        case _                 => (T.dest / "emptyClasses").tap(os.makeDir)
+      }.toIO
+
+      val previous = resolvedMimaPreviousArtifacts().iterator.map {
+        case (dep, artifact) =>
+          new worker.api.Artifact(prettyDep(dep), artifact.path.toIO)
+      }.toArray
+
+      val checkDirection = mimaCheckDirection() match {
+        case CheckDirection.Forward  => worker.api.CheckDirection.Forward
+        case CheckDirection.Backward => worker.api.CheckDirection.Backward
+        case CheckDirection.Both     => worker.api.CheckDirection.Both
+      }
+
+      def toWorkerApi(p: ProblemFilter) =
+        new worker.api.ProblemFilter(p.name, p.problem)
+
+      val incompatibleSignatureProblemFilters =
+        if (mimaReportSignatureProblems()) Seq.empty
+        else Seq(ProblemFilter.exclude[IncompatibleSignatureProblem]("*"))
+      val binaryFilters =
+        (mimaBinaryIssueFilters() ++ incompatibleSignatureProblemFilters)
+          .map(toWorkerApi)
+          .toArray
+      val backwardFilters =
+        mimaBackwardIssueFilters().view
+          .mapValues(_.map(toWorkerApi).toArray)
+          .toMap
+          .asJava
+      val forwardFilters =
+        mimaForwardIssueFilters().view
+          .mapValues(_.map(toWorkerApi).toArray)
+          .toMap
+          .asJava
+
+      val errorOrNull: String =
+        mimaWorker().reportBinaryIssues(
+          scalaBinVersionOrNullTask().orNull,
+          logDebug,
+          logError,
+          logPrintln,
+          checkDirection,
+          runClasspathIO,
+          previous,
+          current,
+          binaryFilters,
+          backwardFilters,
+          forwardFilters,
+          mimaExcludeAnnotations().toArray,
+          publishVersion()
+        )
 
-    if (errorOpt.isPresent()) Result.Failure(errorOpt.get())
-    else Result.Success(())
+      if (errorOrNull == null) Result.Success(())
+      else Result.Failure(errorOrNull)
+    }
   }
 
 }