Revised build importer implementations #5428

ajaychandran · 2025-07-02T17:46:28Z

This PR rewrites importer implementations using a new architecture that is easy to maintain and extend.

Motivation

While working on adding support for cross projects, the build models used in the existing implementations turned out to be insufficient for the use-case.
This prompted work on an alternate architecture that culminated in this pull request.

Solution

New data models are introduced to define a build specification. This includes an ADT whose members have a one-to-one correspondence with module types defined within Mill. Importer implementations were updated to target the new models.

New and dropped features

Added support for converting cross-platform and cross version SBT projects.
Added support for configuring ErrorProneModule for Gradle and Maven projects.
Added support for configuring jvmId.
Dropped support for mapping resources.
Redundant command line options were removed.

Other improvements

Fixed mapping for PomSettings in Maven conversion.
Fixed mapping for artifactName in SBT conversion.
Publish is skipped for Maven modules where it is explicitly disabled. Example, testsuite-osgi module in netty project.
Prevent runtime failure by handling Maven ModelBuildingException.

Limitations

As before, build customizations are not supported. This includes custom sources, custom Gradle configurations, and Maven profiles.
SBT cross-platform support is limited to projects using sbt-crossproject plugin.

Summary of code changes

The previous implementation has been completely removed. Source files for application entry points and some tests were retained, but the contents have changed.

The new shared models are housed in the buildgen.api module. Together with new code in buildgen these account for ~1300 LOC.
The new Gradle plugin is isolated in the gradle.exportplugin module. The Gradle importer implementation and unit tests account for ~430 and ~700 LOC, respectively.
The Maven importer implementation and unit tests account for ~500 and ~1350 LOC, respectively.
The SBT importer implementation and unit tests account for ~600 and ~2200 LOC, respectively.
The integration tests and golden files account for ~150 and ~11500 LOC, respectively.
The manual integration tests account for ~400 LOC.

Testing

Updated integration tests to use golden testing for checking settings.
Added integration.manual[migrating] module to develop and test the conversions against real world projects.

Summary of fixes/enhancements made to improve support for test projects.

Fixed init error in path handling for spring-ai.
Fixed init/configuration error by removing custom resources for byte-buddy/joda-beans.
Added spring-boot-dependencies BOM dependency for microconfig.
Generated ErrorPluginModule configurations for mockito.
Generated cross-platform/version configurations for Airstream, cats, enumeratum, fs2, nscala-time, refined, scala-logging, scopt, scrypto.

Future work

Add support for converting Kotlin/Android projects.
Add support for converting SBT projects that use the sbt-projectmatrix plugin.

ajaychandran · 2025-07-03T05:31:35Z

@lihaoyi As per the Maven POM schema, the description, organization and url fields are not mandatory. Is there a reason why these are required in Mill PomSettings?

lihaoyi · 2025-07-03T05:48:25Z

No reason I'm aware of, we can make them optional

lefou · 2025-07-03T07:59:40Z

I think the Maven Central validator rejects projects without these fields. Although it's specific to Maven Central, having these fields mandatory avoids some late pitfalls, when publishing artifacts.

We only use PomSettings for publishing/sharing, so having some minimal standards on meta-data is probably not bad.

ajaychandran · 2025-07-03T16:02:11Z

We could allow null values and handle in the render method. This would satisfy the empty use-case without requiring changes in user code.

ajaychandran · 2025-07-29T18:59:26Z

A new SBT importer implementation has been added that can import projects that use sbt-crossproject plugin for cross platform development. The implementation also generates cross modules for cross Scala versions.

This was tested against 12 projects and all but 2 were successfully imported.

scala/scala3: Multiple SBT projects use the same base directory.
scalapb/ScalaPB: Uses sbt-projectmatrix for cross development.

One of the tests can be run using the following command:

./mill standalone.migrate[sbt].testOnly mill.standalone.MillInitAirstreamTests

ajaychandran · 2025-07-29T19:18:20Z

@lihaoyi

I am going to be working on adding support for some more settings such as jvmId and generating base trait feature.

What are your thoughts on supporting more plugins? We already have tests that require the following:

sbt-projectmatrix
sbt-play
ScalablyTypedConverterGenSourcePlugin

lihaoyi · 2025-07-30T00:54:03Z

@ajaychandran update the PR title and description to summarize the motivation, major changes, how it integrates with the existing codebase, and testing strategy. It's a large PR and that would make it easier to review

lihaoyi · 2025-07-30T00:57:12Z

libs/init/migrate/src/mill/init/migrate/PackageWriter.scala

+            ps.println()
+            ps.print("def repositories = super.repositories() ++ ")
+            ps.print("Seq(")
+            ps.print(literalize(repositories.head))


Please use string interpolation for this and all similar code snippets so it's not so verbose

lihaoyi · 2025-07-30T00:58:55Z

standalone/testkit/src/mill/standalone/StandaloneTester.scala

+import mill.constants.OutFiles.{millDaemon, millNoDaemon, out}
+import os.{Path, ProcessInput, ProcessOutput, Shellable}
+
+class StandaloneTester(


Why does this need to be different from normal IntegrationTester? We should make these normal integration tests using the normal IntegrationTester utilities

It doesn't have the same requirements as IntegrationTester. The workspace creation and initialization happens outside the tester.
Given that this PR proposal will take some time, I chose to isolate this to avoid any conflicts when re-basing.

What workspace creation and initialization do we expect with IntegrationTester? If run on an empty folder, the workspace starts empty

IntegrationTester clears the workspace and copies resources. I suppose this can be circumvented with an empty resource folder.

I am not opposed to the idea of sharing existing code. I just kept all new code isolated initially. I will merge the testkit classes and move the tests to integration/manual.

lihaoyi · 2025-07-30T00:59:11Z

standalone/package.mill

@@ -0,0 +1,33 @@
+package build.standalone


Let's put this in the integration/manual/, indicating that these are tests meant to be run manually but won't be run in CI, but are otherwise normal integration tests

lihaoyi · 2025-07-30T01:02:02Z

libs/init/migrate/api/src/mill/init/migrate/ModuleConfig.scala

@@ -0,0 +1,108 @@
+package mill.init.migrate


How does this file related to the code we already have in libs/init/buildgen/src/mill/main/buildgen/ir.scala? Does it replace it entirely, or does it re-use parts of the existing model?

It is meant to replace it.
In essence, it is a refactoring of the existing model that organizes settings into groups that correspond to actual module types.
Also, the IR types with the rendered code are not required and do not exist in the new model. This was primarily introduced, by me, for file merging. The new implementation achieves the same in a sane way.

lihaoyi · 2025-07-30T01:04:49Z

libs/init/migrate/api/src/mill/init/migrate/ModuleTypedef.scala

+    "org.testng" -> "TestModule.TestNg",
+    "org.scalatest" -> "TestModule.ScalaTest",
+    "org.specs2" -> "TestModule.Specs2",
+    "com.lihaoyi" -> "TestModule.Utest",


This seems a bit imprecise. For example a lot of people use com.lihaoyi::os-lib but don't use utest. Could we match on a whitelist of artifact names (or prefixes, ignoring the _{scala-version}) as well?

Also, this seems to duplicate logic we already have in BuildgenUtil. We should consolidate it and avoid copy-pasting code

Yes, it is the same as the list in BuildGenUtil.
Given that this is a rewrite of the core design, I have kept the new implementation separate. If the proposal is approved, this can be consolidated.

lihaoyi · 2025-07-30T01:06:06Z

libs/init/migrate/api/src/mill/init/migrate/Tree.scala

+
+import upickle.default.{ReadWriter, macroRW}
+
+case class Tree[+A](root: A, children: Seq[Tree[A]] = Nil) {


How is this different from libs/init/buildgen/src/mill/main/buildgen/Tree.scala?

It's in Scala 2 and the unused features are removed.
But basically it is the same and only exists because the new implementation is isolated.

lihaoyi · 2025-07-30T01:08:21Z

libs/init/migrate/sbt/src/mill/init/migrate/sbt/SbtPackageWriter.scala

+  }
+
+  def renderSbtPlatformModule =
+    s"""trait SbtPlatformModule extends SbtModule { outer =>


How is this different from SbtModule with PlatformScalaModule? If it's necessary, we should put it (and CrossSbtPlatformModule) in libs/scalalib/src/mill/scalalib/ rather than codegen-ing it every import

SbtModule resets sources

override def sources = Task.Sources("src/main/scala", "src/main/java")

and PlatformModuleBase operates on sourceFolders.

override def sourcesFolders: Seq[os.SubPath] = super.sourcesFolders.flatMap { source => Seq(source, source / os.up / s"${source.last}-${platformCrossSuffix}") }

Are these compatible in any scenario?

This is being used to replicate the layout used by sbt-crossproject plugin. But now we also have sbt-projectmatrix for cross development which seems to use a completely different structure. Is it okay to add a trait to scalalib per plugin?

We should make SbtModule use sourceFolders as well, just for consistency with the rest.

I think having traits in scalalib for each of them is fine as long as they're meaningful and well documented .

lihaoyi · 2025-07-30T01:10:01Z

libs/init/migrate/sbt/exportscript/src/mill/init/migrate/sbt/ExportSbtBuildScript.scala

@@ -0,0 +1,225 @@
+package mill.init.migrate.sbt


How does this relate to the existing libs/init/sbt/exportplugin/src/mill/main/sbt/ExportBuildPlugin.scala? Can we consolidate them?

Functionally they are the same.
But the script offers certain advantages such as not requiring any extra build files to add a plugin. The idea was borrowed from IDEA :)

lihaoyi · 2025-07-30T01:10:54Z

libs/init/migrate/src/mill/init/migrate/PackageTree.scala

+import scala.collection.mutable
+import scala.util.Using
+
+opaque type PackageTree = Tree[PackageRepr]


Let's just use a normal case class here with normal methods, rather than this opaque type with extensions

A PackageTree is supposed to have the following invariants:

The root package is located at the workspace root.

The tree structure represents the filesystem structure.
An opaque type seemed the "right" choice.
The extension methods can be defined as normal methods.

Could you provide the reason behind the comment? This is just for my learning.

In general, we aim to use simpler language features over more powerful ones, unless there is a strong reason otherwise. In this case, there is no strong reason to use an opaque type, so we use case class which people are more familiar with

lihaoyi · 2025-07-30T01:13:28Z

@ajaychandran Overall this looks decent as a first pass, but what is missing is how this interacts with the existing libs/init/sbt implementation

Does the new implementation pass all the same tests? I saw some tests failing on your fork's CI
Does the new implementation replace the old one? If so can we delete the old one entirely?
Can we share code between the old and new implementations? I see a bunch of duplicate logic that can be consolidated

lihaoyi · 2025-07-30T01:14:37Z

@lihaoyi

I am going to be working on adding support for some more settings such as jvmId and generating base trait feature.

What are your thoughts on supporting more plugins? We already have tests that require the following:

sbt-projectmatrix
sbt-play
ScalablyTypedConverterGenSourcePlugin

@ajaychandran Let's focus on getting this PR merged first before we start looking at follow ups, I think there's enough work here to keep you occupied for at least a few more days, and I don't want us to get distracted and end up leaving work half finished

lihaoyi · 2025-07-30T01:19:24Z

libs/init/migrate/src/mill/init/migrate/PackageWriter.scala

+import java.io.PrintStream
+import scala.collection.immutable.SortedSet
+
+trait PackageWriter {


We should re-use as much of BuildGenUtil as possible here, which already has logic for rendering def mvnDeps, def moduleDeps, etc. We should not need to re-implement it from scratch and end up maintaining two different implementations. We should only need to implement that parts that BuildGenutil does not already implement

I was hoping to port the other importers to the new design. This might eliminate BuildGenUtil entirely.
Can we discuss this?

lihaoyi · 2025-07-30T01:20:34Z

libs/init/migrate/sbt/exportscript/src/mill/init/migrate/sbt/ExportSbtBuildScript.scala

+          val platform = crossVersion match {
+            case v: librarymanagement.Full if v.prefix.nonEmpty => "::"
+            case v: librarymanagement.Binary if v.prefix.nonEmpty => "::"
+            case v: librarymanagement.For2_13Use3 if v.prefix.nonEmpty => "_3" + "::"


Why not just "_3::"? Same for the "_2.13" + "::" below

ajaychandran · 2025-07-30T08:26:39Z

The PR title and description have been updated.

lihaoyi · 2025-07-30T08:30:18Z

@ajaychandran Thanks for updating the PR description. Given this is a cleanroom implementation that aims to completely replace the old one, the PR description will need to be a lot more detailed to explain what the differences between the old implementation and new one are, to justify why it needs to be replaced and why the new implementation is better.

As mentioned over email, this will need to be integrated into the existing utils and Maven/Gradle implementation before merging. While it's fine for you to experiment with a new design in a separate module, it needs to be fully integrated into the existing code before merging, so we can see the results of your new "better" architecture in a realistic scenarios with the same requirements. The old SBT implementation should be fully removed, and whatever code that can be shared between the new SBT implementation and the Maven/Gradle implementations should be consolidated

lihaoyi · 2025-10-16T03:05:03Z

testkit/src/mill/testkit/GitRepoIntegrationTestSuite.scala

@@ -0,0 +1,36 @@
+package mill.testkit


This should not be in mill.testkit, it should live inside the integration/manual/migrating/src/ since that's the only place that is using it

lihaoyi · 2025-10-16T03:10:13Z

@ajaychandran given that you are adding 12,000 lines of code to the repo, can you summarize in the PR description what the breakdown of these 12,000 lines is? Is it all tests? test cases? Implementation code? 12,000 lines is a large burden to maintain going forward, and we need to ensure we minimize that burden where possible

ajaychandran · 2025-10-16T20:31:53Z

@ajaychandran please clean up the boilerplate in the manual tests as I mentioned earlier. They are part of the testing plan in the PR description, and so they need to be of decent quality and not just copy-paste boilerplate

Fixed, both manual and CI integrations.

ajaychandran · 2025-10-16T20:33:17Z

@ajaychandran given that you are adding 12,000 lines of code to the repo, can you summarize in the PR description what the breakdown of these 12,000 lines is? Is it all tests? test cases? Implementation code? 12,000 lines is a large burden to maintain going forward, and we need to ensure we minimize that burden where possible

Updated PR description. The "Layout changes" section was replaced with "Summary of code changes".

libs/init/maven/src/mill/main/maven/MavenBuildGenMain.scala

libs/init/sbt/src/mill/main/sbt/SbtBuildGenMain.scala

lefou

Lots of public defs do not have a return type annotation, which is unnecessary hard to read. Please add them.

libs/init/src/mill/init/Util.scala

ajaychandran · 2025-10-17T07:04:34Z

Lots of public defs do not have a return type annotation, which is unnecessary hard to read. Please add them.

Added return type annotation for public methods.

ajaychandran · 2025-10-17T07:43:28Z

Diagnosis of a problem reported in a test case revealed an issue in Mill.

For testing, the jcommander project uses TestNg that itself has a dependency on the project.
Post import, this resulted in a situation where the classes in compile.dest were permanently overshadowed due their trailing position in compileClasspath.
Perhaps Mill, like Gradle, should prioritize the localCompileClasspath.

lefou · 2025-10-17T08:18:29Z

Diagnosis of a problem reported in a test case revealed an issue in Mill.

For testing, the jcommander project uses TestNg that itself has a dependency on the project. Post import, this resulted in a situation where the classes in compile.dest were permanently overshadowed due their trailing position in compileClasspath. Perhaps Mill, like Gradle, should prioritize the localCompileClasspath.

This is an issue specific to jcommander / TestNG. I saw it multiple times. IMHO, there is no appropriate generic fix for that in tooling. You can either workaround it (by reordering the classpath, if you're in the lucky case that jcommander versions are binary compatible) or properly fix it by changing the design of TestNG. Both approaches are out of scope for a generic build converter.

ajaychandran · 2025-10-17T08:32:18Z

This is an issue specific to jcommander / TestNG. I saw it multiple times. IMHO, there is no appropriate generic fix for that in tooling. You can either workaround it (by reordering the classpath, if you're in the lucky case that jcommander versions are binary compatible) or properly fix it by changing the design of TestNG. Both approaches are out of scope for a generic build converter.

I didn't mean this is an issue with respect to conversion; this is an issue in Mill.
Shouldn't localCompileClasspath be prioritized over other dependencies? Should I open a separate ticket for this discussion?

lefou · 2025-10-17T08:34:42Z

@ajaychandran

This is an issue specific to jcommander / TestNG. I saw it multiple times. IMHO, there is no appropriate generic fix for that in tooling. You can either workaround it (by reordering the classpath, if you're in the lucky case that jcommander versions are binary compatible) or properly fix it by changing the design of TestNG. Both approaches are out of scope for a generic build converter.

I didn't mean this is an issue with respect to conversion; this is an issue in Mill. Shouldn't localCompileClasspath be prioritized over other dependencies? Should I open a separate ticket for this discussion?

If unrelated, please open a new issue.

ajaychandran · 2025-10-22T07:08:13Z

@lihaoyi Is there anything pending here?

lihaoyi · 2025-10-22T07:21:13Z

libs/init/gradle/test/resources/gradle-6-0/build.gradle

@@ -0,0 +1,39 @@
+/*


For all these files was generated by the Gradle 'init' task, can we generate them as part of our libs/init/package.mill in a generated resource folder? That would let us avoid committing all this generated code into main and having it potentiall drift or fall out of sync over time

lihaoyi · 2025-10-22T07:24:58Z

integration/migrating/init/src/MillInitImportTestSuite.scala

+      val initRes = eval("init" +: initArgs)
+      assert(initRes.isSuccess)
+
+      if (configsGoldenFile != null) {


Let's replace this and the one below with a single val taskNames: Seq[String] that we loop over and then .mkString into a single big string. That would remove all the duplication that comes from assigning things to local vals then interpolating them

Also, how is this different from the version in IntegrationTesterUtil, and the version below in renderAllImportedConfigurations? Can they be combined somehow?

Also, rather than generating a flat file, we can do a single ./mill show __.{repositories,jvmId,mvnDeps,...} and which should return a JSON object we can parse and re-render with indent = 4, sortKeys = true as one big JSON object to use as our golden file contents. That will make it much easier for anyone reading these golden files later to understand what all these values mean, and avoid the messiness of the line-separated JSON objects you are using now

Refactored test method to remove duplication.
Removed unused IntegrationTesterUtil and renderAllImportedConfigurations.

lihaoyi · 2025-10-22T07:30:14Z

@ajaychandran i left some last comments, let's give a day or so to see if @lefou has anything else, after that we can merge it

lihaoyi force-pushed the main branch 2 times, most recently from 1d3b959 to 3ba698a Compare July 10, 2025 04:00

ajaychandran mentioned this pull request Jul 15, 2025

./mill init doesn't seem to set proper java version for iluwatar/java-design-patterns repo #5531

Closed

ajaychandran force-pushed the importer branch 3 times, most recently from 3088228 to e398858 Compare July 21, 2025 01:03

ajaychandran force-pushed the importer branch from e398858 to a767592 Compare July 29, 2025 18:48

lihaoyi reviewed Jul 30, 2025

View reviewed changes

ajaychandran changed the title ~~WIP: Improve support for importing projects to Mill from SBT, Gradle, Maven~~ WIP: Proposal for new SBT importer implementation supporting cross projects using a revised core design Jul 30, 2025

lihaoyi reviewed Oct 16, 2025

View reviewed changes

ajaychandran added 2 commits October 17, 2025 01:14

Refactored integration tests

593caa0

Merge remote-tracking branch 'upstream/main' into importer

388f4d0

ajaychandran changed the title ~~Simplified and improved build importer implementations~~ Revised build importer implementations Oct 16, 2025

lefou reviewed Oct 16, 2025

View reviewed changes

libs/init/maven/src/mill/main/maven/MavenBuildGenMain.scala Show resolved Hide resolved

lefou reviewed Oct 16, 2025

View reviewed changes

libs/init/sbt/src/mill/main/sbt/SbtBuildGenMain.scala Outdated Show resolved Hide resolved

lefou reviewed Oct 16, 2025

View reviewed changes

libs/init/src/mill/init/Util.scala Outdated Show resolved Hide resolved

ajaychandran added 3 commits October 17, 2025 10:24

Defined alternate entry point for conversion.

39fa1f9

Added annotations for return types.

35e5bc9

Merge remote-tracking branch 'upstream/main' into importer

deb22ad

ajaychandran added 3 commits October 17, 2025 18:56

Added sbt.api module for exported types.

083c925

Switch to Scala 3 dialect for code generation/formatting.

cc29eef

Fix formatting in golden files

3e60aca

lihaoyi reviewed Oct 22, 2025

View reviewed changes

ajaychandran added 4 commits October 22, 2025 13:02

Remove unused file

d71160d

Reduced duplication in test method

72afe31

Minified test projects by removing comments and unused files

50eaf98

Merge remote-tracking branch 'upstream/main' into importer

5e2c921


		import upickle.default.{ReadWriter, macroRW}

		case class Tree[+A](root: A, children: Seq[Tree[A]] = Nil) {

Uh oh!

Revised build importer implementations #5428

Are you sure you want to change the base?

Revised build importer implementations #5428

Conversation

ajaychandran commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

New and dropped features

Other improvements

Limitations

Summary of code changes

Testing

Future work

Uh oh!

ajaychandran commented Jul 3, 2025

Uh oh!

lihaoyi commented Jul 3, 2025

Uh oh!

lefou commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ajaychandran commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ajaychandran commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ajaychandran commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lihaoyi commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lihaoyi Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lihaoyi Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lihaoyi Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lihaoyi Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lihaoyi Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

ajaychandran commented Jul 2, 2025 •

edited

Loading

lefou commented Jul 3, 2025 •

edited

Loading

ajaychandran commented Jul 3, 2025 •

edited

Loading

ajaychandran commented Jul 29, 2025 •

edited

Loading

ajaychandran commented Jul 29, 2025 •

edited

Loading

lihaoyi commented Jul 30, 2025 •

edited

Loading

lihaoyi Jul 30, 2025 •

edited

Loading

lihaoyi Jul 30, 2025 •

edited

Loading

lihaoyi Jul 30, 2025 •

edited

Loading

lihaoyi Jul 30, 2025 •

edited

Loading

lihaoyi Jul 30, 2025 •

edited

Loading

lihaoyi Jul 30, 2025 •

edited

Loading

lihaoyi commented Jul 30, 2025 •

edited

Loading