-
-
Notifications
You must be signed in to change notification settings - Fork 413
Simplified and improved build importer implementations #5428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
ajaychandran
wants to merge
29
commits into
com-lihaoyi:main
Choose a base branch
from
ajaychandran:importer
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
d57983b
Added standalone tests for SBT import
ajaychandran 55eb124
Rebase
ajaychandran 5410f43
Merged code from new modules in to existing ones
ajaychandran 2e82d15
Restored log statement required by examples
ajaychandran a2b17d2
Updated failing example
ajaychandran 0c135ff
Cleanup
ajaychandran 18b0e78
Refined core design and extended cross development support.
ajaychandran ff61532
Cleanup
ajaychandran 6e10b2d
Removed build transformation options.
ajaychandran 2113a73
Cleanup
ajaychandran 50ec630
Updated incomplete example
ajaychandran d90a39a
Cleanup
ajaychandran 997ee71
Added base trait and merge features, fixed issue with missing modules…
ajaychandran f3802b8
Cleanup
ajaychandran 3948dbb
Render output using concatenation
ajaychandran 335bca9
Updated Gradle implementation
ajaychandran aa3c626
Cleanup
ajaychandran fcf163b
Set .mill-jvm-version for Mockito tests
ajaychandran 63731fb
Set .mill-jvm-version for Fluency example
ajaychandran 59055b7
Updated Maven conversion
ajaychandran cce84a5
Cleanup
ajaychandran 5a889fc
Fix mima
ajaychandran 199b26e
Revert name change
ajaychandran d8c8089
Updated Scaladocs
ajaychandran 9f49707
Eliminated MiMa filters
ajaychandran 6bfd04c
Cleanup
ajaychandran 66f7bf9
Reinstated mill-jvm-version in build header and added lower bound for…
ajaychandran aeb3fbb
Merge remote-tracking branch 'upstream/main' into importer
ajaychandran fa2a015
Added fixes for binary compatibility
ajaychandran File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
34 changes: 34 additions & 0 deletions
34
integration/manual/migrating/src/MillInitGradleAsmTests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitGradleAsmTests extends GitRepoIntegrationTestSuite { | ||
|
||
// gradle 8.3 | ||
def gitRepoUrl = "https://gitlab.ow2.org/asm/asm.git" | ||
def gitRepoBranch = "ASM_9_8" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
os.write(workspacePath / ".mill-jvm-version", "11") | ||
|
||
eval("init", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("asm.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("asm.publishLocal", stdout = os.Inherit, stderr = os.Inherit) | ||
|
||
// modules are tested with asm-test module? | ||
eval("asm.test", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
|
||
// custom sourceSets not supported | ||
eval( | ||
"tools.retrofitter.compile", | ||
stdout = os.Inherit, | ||
stderr = os.Inherit | ||
).isSuccess ==> false | ||
} | ||
} | ||
} |
35 changes: 35 additions & 0 deletions
35
integration/manual/migrating/src/MillInitGradleEhcache3Tests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitGradleEhcache3Tests extends GitRepoIntegrationTestSuite { | ||
|
||
// gradle 7.2 | ||
// custom dependency configurations | ||
// dependencies with version constraints | ||
// custom layout | ||
// custom repository | ||
// bom dependencies | ||
// modules with pom packaging | ||
// Junit4 | ||
def gitRepoUrl = "https://github.com/ehcache/ehcache3.git" | ||
def gitRepoBranch = "v3.10.8" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
// https://docs.gradle.org/current/userguide/compatibility.html#java_runtime | ||
os.write(workspacePath / ".mill-jvm-version", "11") | ||
|
||
eval("init", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("ehcache-api.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("ehcache-api.test", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
|
||
// custom dependency configurations not supported | ||
eval("ehcache-xml.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
} | ||
} | ||
} |
33 changes: 33 additions & 0 deletions
33
integration/manual/migrating/src/MillInitGradleFastCsvTests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitGradleFastCsvTests extends GitRepoIntegrationTestSuite { | ||
|
||
// gradle 9.0.0-rc-1 | ||
// Junit5 | ||
// uses ErrorProne | ||
def gitRepoUrl = "https://github.com/osiegmar/FastCSV.git" | ||
def gitRepoBranch = "v4.0.0" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
os.write(workspacePath / ".mill-jvm-version", "17") | ||
|
||
eval("init", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("lib.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("lib.publishLocal", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
|
||
// requires support for Java modules | ||
eval("example.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
|
||
// junit-platform-launcher version is set implicitly (tasks.test.useJunitPlatform()) | ||
// https://docs.gradle.org/8.14.3/userguide/upgrading_version_8.html#test_framework_implementation_dependencies | ||
eval("lib.test.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
} | ||
} | ||
} |
29 changes: 29 additions & 0 deletions
29
integration/manual/migrating/src/MillInitGradleJCommanderTests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitGradleJCommanderTests extends GitRepoIntegrationTestSuite { | ||
|
||
// gradle 8.9 | ||
// single module | ||
// testng 7.0.0 | ||
def gitRepoUrl = "https://github.com/cbeust/jcommander.git" | ||
def gitRepoBranch = "2.0" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
os.write(workspacePath / ".mill-jvm-version", "11") | ||
|
||
eval("init", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("publishLocal", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
|
||
// annotations defined in main-module cannot be used in test-module? | ||
eval("test.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
} | ||
} | ||
} |
25 changes: 25 additions & 0 deletions
25
integration/manual/migrating/src/MillInitGradleMicroconfigTests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitGradleMicroconfigTests extends GitRepoIntegrationTestSuite { | ||
|
||
// gradle 8.10.1 | ||
// uses spring-boot-dependencies BOM | ||
// Junit5 | ||
def gitRepoUrl = "https://github.com/microconfig/microconfig.git" | ||
def gitRepoBranch = "v4.9.5" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
eval(("init", "-u"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("__.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("__.publishLocal", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval("__.test", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
} | ||
} | ||
} |
27 changes: 27 additions & 0 deletions
27
integration/manual/migrating/src/MillInitGradleMockitoTests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitGradleMockitoTests extends GitRepoIntegrationTestSuite { | ||
|
||
// gradle 8.14.2 | ||
// contains BOM module | ||
def gitRepoUrl = "https://github.com/mockito/mockito.git" | ||
def gitRepoBranch = "v5.19.0" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
// project requires Java 11 but Gradle version in use requires Java 17 | ||
os.write(workspacePath / ".mill-jvm-version", "17") | ||
|
||
eval("init", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
|
||
// requires javacOptions for Java modules | ||
eval("mockito-core.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
} | ||
} | ||
} |
25 changes: 25 additions & 0 deletions
25
integration/manual/migrating/src/MillInitGradlePCollectionsTests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitGradlePCollectionsTests extends GitRepoIntegrationTestSuite { | ||
|
||
// gradle 8.14.3 | ||
// single module | ||
// Junit5 | ||
def gitRepoUrl = "https://github.com/hrldcpr/pcollections.git" | ||
def gitRepoBranch = "v5.0.0" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
eval("init", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
|
||
// https://github.com/com-lihaoyi/mill/issues/5782 | ||
eval("compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
} | ||
} | ||
} |
37 changes: 37 additions & 0 deletions
37
integration/manual/migrating/src/MillInitGradleSpotbugsTests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitGradleSpotbugsTests extends GitRepoIntegrationTestSuite { | ||
|
||
// gradle 9.0.0 | ||
// custom dependency configurations | ||
// dependencies with version constraints | ||
// custom layout | ||
// Junit5 | ||
def gitRepoUrl = "https://github.com/spotbugs/spotbugs.git" | ||
def gitRepoBranch = "4.9.4" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
eval("init", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval( | ||
"spotbugs-annotations.compile", | ||
stdout = os.Inherit, | ||
stderr = os.Inherit | ||
).isSuccess ==> true | ||
eval( | ||
"spotbugs-annotations.publishLocal", | ||
stdout = os.Inherit, | ||
stderr = os.Inherit | ||
).isSuccess ==> true | ||
|
||
// missing sources from custom layout | ||
eval("spotbugs-tests.test", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
} | ||
} | ||
} |
29 changes: 29 additions & 0 deletions
29
integration/manual/migrating/src/MillInitGradleSpringFrameworkTests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitGradleSpringFrameworkTests extends GitRepoIntegrationTestSuite { | ||
|
||
// gradle 8.14.3 | ||
// custom repository | ||
// dependencies with version constraints | ||
// BOM modules | ||
// uses errorprone | ||
def gitRepoUrl = "https://github.com/spring-projects/spring-framework.git" | ||
def gitRepoBranch = "v6.2.10" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
os.write(workspacePath / ".mill-jvm-version", "17") | ||
|
||
eval("init", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
|
||
// requires support for dependency version constraints | ||
eval("spring-jcl.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
} | ||
} | ||
} |
22 changes: 22 additions & 0 deletions
22
integration/manual/migrating/src/MillInitMavenAntlr4Tests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package mill.integration | ||
|
||
import mill.testkit.GitRepoIntegrationTestSuite | ||
import utest.* | ||
|
||
object MillInitMavenAntlr4Tests extends GitRepoIntegrationTestSuite { | ||
|
||
def gitRepoUrl = "https://github.com/antlr/antlr4.git" | ||
def gitRepoBranch = "v4.11.1" | ||
|
||
def tests = Tests { | ||
test - integrationTest { tester => | ||
import tester.* | ||
|
||
eval("init", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
eval(("resolve", "_"), stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> true | ||
|
||
// custom layout not supported | ||
eval("__.compile", stdout = os.Inherit, stderr = os.Inherit).isSuccess ==> false | ||
} | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we no longer support
--jvm-id
?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conversion now auto-configures
JavaHomeModule.jvmId
for Gradle and Maven projects.Based on the projects tested, JDK version enforcement is quite rare. It is much more common to ensure compatibility with
javacOptions
like-target
.The Netty project is an example where the version,
1.8
, is enforced. But we cannot support this value due to #5782.The option was also being used to configure the JVM version in the build header. We could rename it to
--mill-jvm-version
and use it for this purpose.I am not convinced that the conversion should configure this property since, in the typical case, the preferred JDK is the one already installed.
Consider the Mockito example in
5-gradle-incomplete
.Since
init
runs the Gradle daemon, we need at least version 17. Post conversion, JDK 11 would suffice (this won't be configured at the module level since it is not enforced in the build).But why not use the latest version? The Mill project itself uses
zulu:21
.The reason the version needs to be specified here is that we do not know the JDK version in the CI environment. We could move the value to
.mill-jvm-version
file to "hide" this from the user.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we look at versions below 11 like
1.8
and generatemill-jvm-version: 11
? That won't work for all projects, but it should hopefully work for many of them which aren't too picky about the version they are using. Then the user won't have to create the.mill-jvm-version
file manually.I think trying to specify a version as close as possible to that configured within their build via
javacOptions
sounds best. Not all projects work with newer versions, e.g. Scala, Kotlin, Spring, etc. all can potentially have problems if we pick a too-new version of the JVM to run them onThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why should the conversion explicitly specify the
.mill-jvm-version
at all? Isn't the default Mill behavior of using the installed JDK sufficient for real world usage?The conversion should be concerned with configuring the JVM only at the module level and only if it is strictly enforced in the Gradle / Maven build.
I don't think
javacOptions
should be used to determine a version since a low value is targeted in many cases. Downgrading the JDK will likely adversely affect build times and require a fix in the build file (assuming the cause for the slowdown is identified).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the Mill
main
branch, we need to opt-into using the installed JDK viamill-jvm-version: system
. But in general that is discouraged (hence making it no longer the default) and so we shouldn't generate it on-import if possible.Pulling a low Java version is fine as long as it works. People can tweak and adjust it later, but our goal for the import "a version that is most likely to work while avoiding
mill-jvm-version: system
". So if there is a-target
or-release
flag we should use as close to a version as possible, and if there is no flag we maybe just leave it unspecified and Mill will default to Java 21There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reinstated
mill-jvm-version
in build header and added lower bound for Mill/module JVM version.