Skip to content

Commit

Permalink
Introduce mill.internal package to hold Mill utility classes not in…
Browse files Browse the repository at this point in the history
…tended for public use (com-lihaoyi#4507)

This PR pulls out a bunch of classes from `mill.api` and `mill.util`
that aren't meant for public use and moves them into `mill.internal`. We
also try to assign proper meanings to `mill.api` and `mill.util` that
were previously just grab-bags of random helpers

The meaning of the folders is now:

- `mill.api`: things that form the shared API between mill "core"
`define`/`eval`/`resolve`, and user-land Mill code
- `mill.internal`: things that are only used by Mill core code
- `mill.util`: things that are only used by user-land Mill code

This lets us clearly demarcate what is meant to be exposed to user code
and what isn't, which was very hard to see when we use `private`
modifiers on individual classes in `mill.api` and `mill.util`. Most of
the stuff in `mill.internal` is not related to any user-land APIs at all
- not even the `*lib` language toolchains - and truly is internal to
Mill's internal functionality.

`internal` is marked as a `MillPublishScalaModule` rather than
`MillStableScalaModule`: this lets us mark them as unstable and clearly
not meant to be relied upon by user code, while also leaving them
exposed so if anyone was relying on them before they can continue doing
so (for the moment) to aid in the migration.

We can continue to use `private` on individual classes or members where
appropriate for fine-grained visibility control; this just gives us a
big coarse-grained way of marking things as `internal`

The `main/` folder in Mill also probably should be re-organized along
these lines, with a split between `core` and non-`core` code. That can
come in a separate PR

This split can also serve the purpose of
com-lihaoyi#3260, where `mill.api` forms
the boundary between user-land code resolved by Coursier and Mill-Core
code bundled in the assembly. But that can also come later
  • Loading branch information
lihaoyi authored Feb 8, 2025
1 parent fcc7238 commit 9f64b4b
Show file tree
Hide file tree
Showing 80 changed files with 195 additions and 227 deletions.
11 changes: 4 additions & 7 deletions bsp/src/mill/bsp/BspWorker.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package mill.bsp

import mill.api.{Ctx, Logger, SystemStreams}
import os.Path
import mill.api.{Logger, SystemStreams}

import java.io.PrintStream
import java.net.URL
Expand Down Expand Up @@ -31,7 +30,7 @@ private object BspWorker {
case None =>
val urls = workerLibs.map { urls =>
log.debug("Using direct submitted worker libs")
urls
urls.map(url => os.Path(url.getPath))
}.getOrElse {
// load extra classpath entries from file
val resources = s"${Constants.serverName}-${mill.main.BuildInfo.millVersion}.resources"
Expand All @@ -44,13 +43,11 @@ private object BspWorker {

// read the classpath from resource file
log.debug(s"Reading worker classpath from file: ${cpFile}")
os.read(cpFile).linesIterator.map(u => new URL(u)).toSeq
os.read(cpFile).linesIterator.map(u => os.Path(new URL(u).getPath)).toSeq
}

// create classloader with bsp.worker and deps
val cl = mill.api.ClassLoader.create(urls, getClass().getClassLoader())(
new Ctx.Home { override def home: Path = home0 }
)
val cl = mill.util.Jvm.createClassLoader(urls, getClass().getClassLoader())

val workerCls = cl.loadClass(Constants.bspWorkerImplClass)
val ctr = workerCls.getConstructor()
Expand Down
4 changes: 2 additions & 2 deletions bsp/worker/src/mill/bsp/worker/MillBspLogger.scala
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package mill.bsp.worker

import ch.epfl.scala.bsp4j._
import mill.api.Logger
import mill.util.{ColorLogger, ProxyLogger}
import mill.api.{ColorLogger, Logger}
import mill.internal.ProxyLogger

/**
* BSP-specialized logger class which sends `task-progress`
Expand Down
10 changes: 8 additions & 2 deletions bsp/worker/src/mill/bsp/worker/MillBuildServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@ import ch.epfl.scala.bsp4j
import ch.epfl.scala.bsp4j._
import com.google.gson.JsonObject
import mill.api.Loose.Agg
import mill.api.{CompileProblemReporter, DummyTestReporter, Result, Strict, TestReporter}
import mill.api.{
ColorLogger,
CompileProblemReporter,
DummyTestReporter,
Result,
Strict,
TestReporter
}
import mill.bsp.{BspServerResult, Constants}
import mill.bsp.worker.Utils.{makeBuildTarget, outputPaths, sanitizeUri}
import mill.define.Segment.Label
Expand All @@ -15,7 +22,6 @@ import mill.main.MainModule
import mill.runner.MillBuildRootModule
import mill.scalalib.bsp.{BspModule, JvmBuildTarget, ScalaBuildTarget}
import mill.scalalib.{JavaModule, SemanticDbJavaModule, TestModule}
import mill.util.ColorLogger
import mill.given

import java.io.PrintStream
Expand Down
2 changes: 2 additions & 0 deletions build.mill
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,8 @@ trait MillScalaModule extends ScalaModule with MillJavaModule with ScalafixModul
if (ZincWorkerUtil.isScala3(scalaVersion())) Some(T.workspace / ".scalafix-3.conf") else None
}
def semanticDbVersion = Deps.semanticDBscala.version

def scaladocOptions = Seq("-Xsource:3")
def scalacOptions =
super.scalacOptions() ++ Seq(
"-deprecation",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package mill.contrib.gitlab

import mill.contrib.gitlab.GitlabTokenLookup._
import mill.internal.DummyLogger
import mill.scalalib.publish._
import mill.util.DummyLogger
import utest.{TestSuite, Tests, assert, assertMatch, test}

import scala.collection.mutable.ListBuffer
Expand Down
7 changes: 3 additions & 4 deletions contrib/playlib/src/mill/playlib/RouteCompilerWorker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ private[playlib] class RouteCompilerWorker extends AutoCloseable {
routeCompilerInstanceCache match {
case Some((sig, bridge)) if sig == classloaderSig => bridge
case _ =>
val toolsClassPath = toolsClasspath.map(_.path.toIO.toURI.toURL).toVector
val toolsClassPath = toolsClasspath.map(_.path).toVector
ctx.log.debug("Loading classes from\n" + toolsClassPath.mkString("\n"))
val cl = mill.api.ClassLoader.create(
val cl = mill.util.Jvm.createClassLoader(
toolsClassPath,
null,
sharedLoader = getClass().getClassLoader(),
sharedPrefixes = Seq("mill.playlib.api."),
logger = Some(ctx.log)
sharedPrefixes = Seq("mill.playlib.api.")
)
val bridge = cl
.loadClass("mill.playlib.worker.RouteCompilerWorker")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ class ScalaPBWorker extends AutoCloseable {
scalaPBInstanceCache match {
case Some((sig, instance)) if sig == classloaderSig => instance
case _ =>
val pbcClasspath = scalaPBClasspath.map(_.path.toIO.toURI.toURL).toVector
val cl = mill.api.ClassLoader.create(pbcClasspath, null)
val pbcClasspath = scalaPBClasspath.map(_.path).toVector
val cl = mill.util.Jvm.createClassLoader(pbcClasspath, null)
val scalaPBCompilerClass = cl.loadClass("scalapb.ScalaPBC")
val mainMethod = scalaPBCompilerClass.getMethod("main", classOf[Array[java.lang.String]])

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package mill.contrib.scoverage

import mill.{Agg, Task}
import mill.api.{ClassLoader, Ctx, PathRef}
import mill.api.{Ctx, PathRef}
import mill.contrib.scoverage.api.ScoverageReportWorkerApi2
import mill.define.{Discover, ExternalModule, Worker}

Expand All @@ -19,9 +19,9 @@ class ScoverageReportWorker extends AutoCloseable {
val cl = scoverageClCache match {
case Some((sig, cl)) if sig == classloaderSig => cl
case _ =>
val toolsClassPath = classpath.map(_.path.toIO.toURI.toURL).toVector
val toolsClassPath = classpath.map(_.path).toVector
ctx.log.debug("Loading worker classes from\n" + toolsClassPath.mkString("\n"))
val cl = ClassLoader.create(
val cl = mill.util.Jvm.createClassLoader(
toolsClassPath,
getClass.getClassLoader
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package build
import mill._, scalalib._
import mill.util.Jvm
import mill.api.CachedFactory
import mill.util.CachedFactory
import java.net.{URL, URLClassLoader}

trait FooModule extends JavaModule {
Expand Down
2 changes: 1 addition & 1 deletion example/fundamentals/tasks/6-workers/build.mill
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def myWorker = Task.Worker { new MyWorker() }
//
import mill._
import java.lang.AutoCloseable
import mill.api.CachedFactory
import mill.util.CachedFactory
import java.io.ByteArrayOutputStream

def cachedCompressWorker = Task.Worker { new CachedCompressWorker(Task.dest) }
Expand Down
59 changes: 0 additions & 59 deletions main/api/src/mill/api/ClassLoader.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package mill.api

import java.net.{URL, URLClassLoader}

/**
* Utilities for creating classloaders for running compiled Java/Scala code in
* isolated classpaths.
Expand All @@ -17,61 +15,4 @@ object ClassLoader {
} finally thread.setContextClassLoader(oldCl)
}
def java9OrAbove: Boolean = !System.getProperty("java.specification.version").startsWith("1.")

@deprecated("Use callClassLoader", "Mill 0.12.7")
def create(
urls: Seq[URL],
parent: java.lang.ClassLoader,
sharedLoader: java.lang.ClassLoader = getClass.getClassLoader,
sharedPrefixes: Seq[String] = Seq(),
logger: Option[mill.api.Logger] = None
)(implicit ctx: Ctx.Home): URLClassLoader = {
new URLClassLoader(urls.toArray, refinePlatformParent(parent)) {
override def findClass(name: String): Class[?] = {
if (sharedPrefixes.exists(name.startsWith)) {
logger.foreach(
_.debug(s"About to load class [${name}] from shared classloader [${sharedLoader}]")
)
sharedLoader.loadClass(name)
} else super.findClass(name)
}
}
}

/**
* Return `ClassLoader.getPlatformClassLoader` for java 9 and above, if parent class loader is null,
* otherwise return same parent class loader.
* More details: https://docs.oracle.com/javase/9/migrate/toc.htm#JSMIG-GUID-A868D0B9-026F-4D46-B979-901834343F9E
*
* `ClassLoader.getPlatformClassLoader` call is implemented via runtime reflection, cause otherwise
* mill could be compiled only with jdk 9 or above. We don't want to introduce this restriction now.
*/
private def refinePlatformParent(parent: java.lang.ClassLoader): ClassLoader = {
if (parent != null) parent
else if (java9OrAbove) {
// Make sure when `parent == null`, we only delegate java.* classes
// to the parent getPlatformClassLoader. This is necessary because
// in Java 9+, somehow the getPlatformClassLoader ends up with all
// sorts of other non-java stuff on it's classpath, which is not what
// we want for an "isolated" classloader!
classOf[ClassLoader]
.getMethod("getPlatformClassLoader")
.invoke(null)
.asInstanceOf[ClassLoader]
} else {
// With Java 8 we want a clean classloader that still contains classes
// coming from com.sun.* etc.
// We get the application classloader parent which happens to be of
// type sun.misc.Launcher$ExtClassLoader
// We can't call the method directly since it would not compile on Java 9+
// So we load it via reflection to allow compilation in Java 9+ but only
// on Java 8
val launcherClass = getClass.getClassLoader().loadClass("sun.misc.Launcher")
val getLauncherMethod = launcherClass.getMethod("getLauncher")
val launcher = getLauncherMethod.invoke(null)
val getClassLoaderMethod = launcher.getClass().getMethod("getClassLoader")
val appClassLoader = getClassLoaderMethod.invoke(launcher).asInstanceOf[ClassLoader]
appClassLoader.getParent()
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
package mill.util

import mill.api.Logger
package mill.api

import java.io.PrintStream

Expand Down
2 changes: 1 addition & 1 deletion main/codesig/package.mill
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import mill._, scalalib._

object `package` extends RootModule with build.MillPublishScalaModule {
override def ivyDeps = Agg(build.Deps.asmTree, build.Deps.osLib, build.Deps.pprint)
def moduleDeps = Seq(build.main.util)
def moduleDeps = Seq(build.main.internal)

override lazy val test: CodeSigTests = new CodeSigTests {}
trait CodeSigTests extends MillScalaTests {
Expand Down
2 changes: 1 addition & 1 deletion main/codesig/src/ReachabilityAnalysis.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package mill.codesig
import mill.util.{SpanningForest, Tarjans}
import mill.internal.{SpanningForest, Tarjans}
import upickle.default.{Writer, writer}
import JvmModel.*

Expand Down
6 changes: 3 additions & 3 deletions main/codesig/src/ResolvedCalls.scala
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package mill.codesig
import JvmModel._
import JType.{Cls => JCls}
import mill.util.SpanningForest
import mill.util.SpanningForest.breadthFirst
import mill.internal.SpanningForest
import mill.internal.SpanningForest.breadthFirst
import upickle.default.{ReadWriter, macroRW}

case class ResolvedCalls(
Expand Down Expand Up @@ -122,7 +122,7 @@ object ResolvedCalls {
sig -> breadthFirst(Seq(cls))(cls => directDescendents.getOrElse(cls, Nil))
}

val allSamImplementors = mill.util.SpanningForest.reverseEdges(allSamImplementors0)
val allSamImplementors = SpanningForest.reverseEdges(allSamImplementors0)

allSamImplementors.mapValues(_.toSet).toMap
}
Expand Down
2 changes: 1 addition & 1 deletion main/define/src/mill/define/BaseModule.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package mill.define

import mill.api.PathRef
import mill.util.Watchable
import mill.internal.Watchable

import scala.collection.mutable

Expand Down
3 changes: 2 additions & 1 deletion main/define/src/mill/define/Cross.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package mill.define

import mill.api.{BuildScriptException, Lazy}
import mill.api.BuildScriptException
import mill.internal.Lazy

import scala.collection.mutable
import scala.reflect.ClassTag
Expand Down
4 changes: 2 additions & 2 deletions main/eval/src/mill/eval/Evaluator.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package mill.eval

import mill.api.{CompileProblemReporter, DummyTestReporter, Result, TestReporter, Val}
import mill.api.{ColorLogger, CompileProblemReporter, DummyTestReporter, Result, TestReporter, Val}
import mill.api.Strict.Agg
import mill.define.{BaseModule, Segments, Task}
import mill.eval.Evaluator.{Results, formatFailing}
import mill.util.{ColorLogger, MultiBiMap}
import mill.internal.MultiBiMap

import scala.jdk.CollectionConverters._
import scala.reflect.ClassTag
Expand Down
4 changes: 2 additions & 2 deletions main/eval/src/mill/eval/EvaluatorCore.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import mill.api.Strict.Agg
import mill.api._
import mill.define._
import mill.eval.Evaluator.TaskResult
import mill.util._
import mill.internal.{PrefixLogger, MultiBiMap}

import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.atomic.{AtomicBoolean, AtomicInteger}
Expand Down Expand Up @@ -124,7 +124,7 @@ private[mill] trait EvaluatorCore extends GroupEvaluator {
} else {
futures(terminal) = Future.sequence(deps.map(futures)).map { upstreamValues =>
try {
val countMsg = mill.util.Util.leftPad(
val countMsg = mill.internal.Util.leftPad(
count.getAndIncrement().toString,
terminals.length.toString.length,
'0'
Expand Down
7 changes: 3 additions & 4 deletions main/eval/src/mill/eval/EvaluatorImpl.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package mill.eval

import mill.api.{Strict, SystemStreams, Val}
import mill.api.{ColorLogger, Strict, SystemStreams, Val}
import mill.api.Strict.Agg
import mill.define._
import mill.util._
import mill.main.client.OutFiles._
import mill.define.*
import mill.main.client.OutFiles.*

import scala.collection.mutable
import scala.reflect.ClassTag
Expand Down
2 changes: 1 addition & 1 deletion main/eval/src/mill/eval/EvaluatorLogs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package mill.eval

import mill.define.{InputImpl, Task}
import mill.main.client.OutFiles
import mill.util.SpanningForest
import mill.internal.SpanningForest

import java.util.concurrent.ConcurrentHashMap
import scala.jdk.CollectionConverters.EnumerationHasAsScala
Expand Down
7 changes: 4 additions & 3 deletions main/eval/src/mill/eval/GroupEvaluator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package mill.eval

import mill.api.Result.{OuterStack, Success}
import mill.api.Strict.Agg
import mill.api._
import mill.define._
import mill.api.*
import mill.define.*
import mill.internal.MultiLogger
import mill.eval.Evaluator.TaskResult
import mill.util._
import mill.internal.FileLogger

import java.lang.reflect.Method
import scala.collection.mutable
Expand Down
4 changes: 2 additions & 2 deletions main/eval/src/mill/eval/Plan.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package mill.eval
import mill.api.Loose.Agg
import mill.define.{NamedTask, Task}
import mill.util.MultiBiMap
import mill.internal.MultiBiMap

private[mill] class Plan(
val transitive: Agg[Task[?]],
Expand Down Expand Up @@ -97,7 +97,7 @@ private[mill] object Plan {
for (t <- transitiveTargets.items)
yield t.inputs.collect(targetIndices).toArray

val sortedClusters = mill.util.Tarjans(numberedEdges.toArray)
val sortedClusters = mill.internal.Tarjans(numberedEdges.toArray)
val nonTrivialClusters = sortedClusters.filter(_.length > 1)
assert(nonTrivialClusters.isEmpty, nonTrivialClusters)
new TopoSorted(Agg.from(sortedClusters.flatten.map(indexed)))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package mill.util
package mill.internal

import java.io.Writer

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package mill.util
package mill.internal

case class Colors(info: fansi.Attrs, error: fansi.Attrs)
object Colors {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package mill.util
package mill.internal

import mill.api.{Logger, SystemStreams}

Expand Down
Loading

0 comments on commit 9f64b4b

Please sign in to comment.