Skip to content
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

Fix conflicting dependencies between upstream JavaModules #2735

Merged
merged 27 commits into from
Sep 14, 2023
Merged
2 changes: 1 addition & 1 deletion build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ object bsp extends MillPublishScalaModule with BuildInfo {
}

object worker extends MillPublishScalaModule {
def compileModuleDeps = Seq(bsp, scalalib, testrunner, runner)
def compileModuleDeps = Seq(bsp, scalalib, testrunner, runner) ++ scalalib.compileModuleDeps
def ivyDeps = Agg(Deps.bsp4j, Deps.sbtTestInterface)
}
}
Expand Down
6 changes: 3 additions & 3 deletions example/basic/4-builtin-commands/build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,9 @@ Inputs:

> ./mill show foo.compileClasspath
[
".../foo/compile-resources",
".../org/scala-lang/scala-library/2.13.11/scala-library-2.13.11.jar",
...
".../foo/compile-resources"
]

*/
Expand All @@ -150,9 +150,9 @@ Inputs:
".../foo/src"
],
"foo.compileClasspath": [
".../foo/compile-resources",
".../org/scala-lang/scala-library/2.13.11/scala-library-2.13.11.jar",
...
".../foo/compile-resources"
]
}

Expand All @@ -171,9 +171,9 @@ Inputs:
".../foo/src"
],
"foo.compileClasspath": [
".../foo/compile-resources",
".../org/scala-lang/scala-library/2.13.11/scala-library-2.13.11.jar",
...
".../foo/compile-resources"
]
}

Expand Down
2 changes: 1 addition & 1 deletion example/cross/10-static-blog/build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// libraries - Commonmark and Scalatags - to deal with Markdown parsing and
// HTML generation respectively:

import $ivy.`com.lihaoyi::scalatags:0.9.1`, scalatags.Text.all._
import $ivy.`com.lihaoyi::scalatags:0.12.0`, scalatags.Text.all._
import $ivy.`com.atlassian.commonmark:commonmark:0.13.1`
import org.commonmark.parser.Parser
import org.commonmark.renderer.html.HtmlRenderer
Expand Down
2 changes: 1 addition & 1 deletion example/misc/3-import-file-ivy/build.sc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import mill._, scalalib._
import $ivy.`com.lihaoyi::scalatags:0.8.2`, scalatags.Text.all._
import $ivy.`com.lihaoyi::scalatags:0.12.0`, scalatags.Text.all._
import $file.scalaversion, scalaversion.myScalaVersion

object foo extends RootModule with ScalaModule {
Expand Down
4 changes: 2 additions & 2 deletions example/misc/4-mill-build-folder/build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ compiling 1 Scala source...

> ./mill run
Foo.value: <h1>hello</h1>
scalatagsVersion: 0.8.2
scalatagsVersion: 0.12.0

> ./mill show assembly
".../out/assembly.dest/out.jar"

> ./out/assembly.dest/out.jar # mac/linux
Foo.value: <h1>hello</h1>
scalatagsVersion: 0.8.2
scalatagsVersion: 0.12.0

*/

Expand Down
2 changes: 1 addition & 1 deletion example/misc/4-mill-build-folder/mill-build/build.sc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import mill._, scalalib._

object millbuild extends MillBuildRootModule{
val scalatagsVersion = "0.8.2"
val scalatagsVersion = "0.12.0"
def ivyDeps = Agg(ivy"com.lihaoyi::scalatags:$scalatagsVersion")

def generatedSources = T {
Expand Down
25 changes: 14 additions & 11 deletions example/thirdparty/jimfs/build.sc
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import mill._, scalalib._, publish._

object jimfs extends PublishModule with MavenModule{
def sharedCompileIvyDeps = T{
Agg(
ivy"com.google.auto.service:auto-service:1.0.1",
ivy"com.google.code.findbugs:jsr305:3.0.2",
ivy"org.checkerframework:checker-compat-qual:2.5.5",
ivy"com.ibm.icu:icu4j:73.1",
)
}


object jimfs extends PublishModule with MavenModule {
def publishVersion = "1.3.3.7"

def pomSettings = PomSettings(
Expand All @@ -12,21 +22,14 @@ object jimfs extends PublishModule with MavenModule{
developers = Nil
)

def ivyDeps = Agg(
def ivyDeps = sharedCompileIvyDeps() ++ Agg(
ivy"com.google.guava:guava:31.1-android",
)

def compileIvyDeps = Agg(
ivy"com.google.auto.service:auto-service:1.0.1",
ivy"com.google.code.findbugs:jsr305:3.0.2",
ivy"org.checkerframework:checker-compat-qual:2.5.5",
ivy"com.ibm.icu:icu4j:73.1",
)

def javacOptions = Seq("-processor", "com.google.auto.service.processor.AutoServiceProcessor")

object test extends MavenModuleTests{
def ivyDeps = Agg(
object test extends MavenModuleTests {
def ivyDeps = sharedCompileIvyDeps() ++ Agg(
ivy"junit:junit:4.13.2",
ivy"com.google.guava:guava-testlib:31.1-android",
ivy"com.google.truth:truth:1.1.3",
Expand Down
114 changes: 54 additions & 60 deletions scalalib/src/mill/scalalib/JavaModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,34 @@ trait JavaModule
/** The compile-only direct dependencies of this module. */
def compileModuleDeps: Seq[JavaModule] = Seq.empty

/** The direct and indirect dependencies of this module */
def recursiveModuleDeps: Seq[JavaModule] = {
moduleDeps.flatMap(_.transitiveModuleDeps).distinct
}

/**
* Like `recursiveModuleDeps` but also include the module itself,
* basically the modules whose classpath are needed at runtime
*/
def transitiveModuleDeps: Seq[JavaModule] = Seq(this) ++ recursiveModuleDeps

/**
* All direct and indirect module dependencies of this module, including
* compile-only dependencies: basically the modules whose classpath are needed
* at compile-time.
*
* Note that `compileModuleDeps` are defined to be non-transitive, so we only
* look at the direct `compileModuleDeps` when assembling this list
*/
def transitiveModuleCompileModuleDeps: Seq[JavaModule] = {
(moduleDeps ++ compileModuleDeps).flatMap(_.transitiveModuleDeps).distinct
}

/** The compile-only transitive ivy dependencies of this module and all it's upstream compile-only modules. */
def transitiveCompileIvyDeps: T[Agg[BoundDep]] = T {
// We never include compile-only dependencies transitively, but we must include normal transitive dependencies!
compileIvyDeps().map(bindDependency()) ++ T
.traverse(compileModuleDeps)(_.transitiveIvyDeps)()
.flatten
compileIvyDeps().map(bindDependency()) ++
T.traverse(compileModuleDeps)(_.transitiveIvyDeps)().flatten
}

/**
Expand All @@ -158,16 +180,6 @@ trait JavaModule
T.log.outputStream.println(asString)
}

/** The direct and indirect dependencies of this module */
def recursiveModuleDeps: Seq[JavaModule] = {
moduleDeps.flatMap(_.transitiveModuleDeps).distinct
}

/** Like `recursiveModuleDeps` but also include the module itself */
def transitiveModuleDeps: Seq[JavaModule] = {
Seq(this) ++ recursiveModuleDeps
}

/**
* Additional jars, classfiles or resources to add to the classpath directly
* from disk rather than being downloaded from Maven Central or other package
Expand All @@ -180,28 +192,22 @@ trait JavaModule
* This is calculated from [[ivyDeps]], [[mandatoryIvyDeps]] and recursively from [[moduleDeps]].
*/
def transitiveIvyDeps: T[Agg[BoundDep]] = T {
(ivyDeps() ++ mandatoryIvyDeps()).map(bindDependency()) ++ T.traverse(moduleDeps)(
_.transitiveIvyDeps
)().flatten
(ivyDeps() ++ mandatoryIvyDeps()).map(bindDependency()) ++
T.traverse(moduleDeps)(_.transitiveIvyDeps)().flatten
}

/**
* The upstream compilation output of all this module's upstream modules
*/
def upstreamCompileOutput: T[Seq[CompilationResult]] = T {
T.traverse((recursiveModuleDeps ++ compileModuleDeps.flatMap(
_.transitiveModuleDeps
)).distinct)(_.compile)
T.traverse(transitiveModuleCompileModuleDeps)(_.compile)
}

/**
* The transitive version of `localClasspath`
*/
def transitiveLocalClasspath: T[Agg[PathRef]] = T {
T.traverse(
(moduleDeps ++ compileModuleDeps).flatMap(_.transitiveModuleDeps).distinct
)(m => m.localClasspath)()
.flatten
T.traverse(transitiveModuleCompileModuleDeps)(_.localClasspath)().flatten
}

/**
Expand All @@ -210,20 +216,16 @@ trait JavaModule
// Keep in sync with [[transitiveLocalClasspath]]
@internal
def bspTransitiveLocalClasspath: T[Agg[UnresolvedPath]] = T {
T.traverse(
(moduleDeps ++ compileModuleDeps).flatMap(_.transitiveModuleDeps).distinct
)(m => m.bspLocalClasspath)()
.flatten
T.traverse(transitiveModuleCompileModuleDeps)(_.bspLocalClasspath)().flatten
}

/**
* The transitive version of `compileClasspath`
*/
def transitiveCompileClasspath: T[Agg[PathRef]] = T {
T.traverse(
(moduleDeps ++ compileModuleDeps).flatMap(_.transitiveModuleDeps).distinct
)(m => T.task { m.compileClasspath() ++ Agg(m.compile().classes) })()
.flatten
T.traverse(transitiveModuleCompileModuleDeps)(m =>
T.task { m.localCompileClasspath() ++ Agg(m.compile().classes) }
)().flatten
}

/**
Expand All @@ -232,9 +234,7 @@ trait JavaModule
// Keep in sync with [[transitiveCompileClasspath]]
@internal
def bspTransitiveCompileClasspath: T[Agg[UnresolvedPath]] = T {
T.traverse(
(moduleDeps ++ compileModuleDeps).flatMap(_.transitiveModuleDeps).distinct
)(m =>
T.traverse(transitiveModuleCompileModuleDeps)(m =>
T.task {
m.bspCompileClasspath() ++ Agg(m.bspCompileClassesPath())
}
Expand Down Expand Up @@ -355,11 +355,11 @@ trait JavaModule
}

/**
* The output classfiles/resources from this module, excluding upstream
* modules and third-party dependencies
* The *output* classfiles/resources from this module, used for execution,
* excluding upstream modules and third-party dependencies
*/
def localClasspath: T[Seq[PathRef]] = T {
compileResources() ++ resources() ++ Agg(compile().classes)
localCompileClasspath().toSeq ++ resources() ++ Agg(compile().classes)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds unmanagedClasspath to localClasspath. This is probably the right thing to do, since it means it'll probably be aggregated from upstream modules into upstreamAssemblyClasspath, where it wouldn't before

}

/**
Expand All @@ -368,9 +368,8 @@ trait JavaModule
*/
@internal
def bspLocalClasspath: T[Agg[UnresolvedPath]] = T {
(compileResources() ++ resources()).map(p => UnresolvedPath.ResolvedPath(p.path)) ++ Agg(
bspCompileClassesPath()
)
(compileResources() ++ resources()).map(p => UnresolvedPath.ResolvedPath(p.path)) ++
Agg(bspCompileClassesPath())
}

/**
Expand All @@ -379,53 +378,51 @@ trait JavaModule
*/
// Keep in sync with [[bspCompileClasspath]]
def compileClasspath: T[Agg[PathRef]] = T {
transitiveCompileClasspath() ++
compileResources() ++
unmanagedClasspath() ++
resolvedIvyDeps()
resolvedIvyDeps() ++ transitiveCompileClasspath() ++ localCompileClasspath()
}

/**
* The *input* classfiles/resources from this module, used during compilation,
* excluding upstream modules and third-party dependencies
*/
def localCompileClasspath: T[Agg[PathRef]] = T {
compileResources() ++ unmanagedClasspath()
}

/** Same as [[compileClasspath]], but does not trigger compilation targets, if possible. */
// Keep in sync with [[compileClasspath]]
@internal
def bspCompileClasspath: T[Agg[UnresolvedPath]] = T {
bspTransitiveCompileClasspath() ++
(compileResources() ++ unmanagedClasspath() ++ resolvedIvyDeps())
(localCompileClasspath() ++ resolvedIvyDeps())
.map(p => UnresolvedPath.ResolvedPath(p.path))
}

/**
* Resolved dependencies based on [[transitiveIvyDeps]] and [[transitiveCompileIvyDeps]].
*/
def resolvedIvyDeps: T[Agg[PathRef]] = T {
resolveDeps(T.task {
transitiveCompileIvyDeps() ++ transitiveIvyDeps()
})()
resolveDeps(T.task { transitiveCompileIvyDeps() ++ transitiveIvyDeps() })()
}

/**
* All upstream classfiles and resources necessary to build and executable
* assembly, but without this module's contribution
*/
def upstreamAssemblyClasspath: T[Agg[PathRef]] = T {
transitiveLocalClasspath() ++
unmanagedClasspath() ++
resolvedRunIvyDeps()
resolvedRunIvyDeps() ++ transitiveLocalClasspath()
}

def resolvedRunIvyDeps: T[Agg[PathRef]] = T {
resolveDeps(T.task {
runIvyDeps().map(bindDependency()) ++ transitiveIvyDeps()
})()
resolveDeps(T.task { runIvyDeps().map(bindDependency()) ++ transitiveIvyDeps() })()
}

/**
* All classfiles and resources from upstream modules and dependencies
* necessary to run this module's code after compilation
*/
def runClasspath: T[Seq[PathRef]] = T {
localClasspath() ++
upstreamAssemblyClasspath()
resolvedRunIvyDeps().toSeq ++ transitiveLocalClasspath() ++ localClasspath()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We remove upstreamAssemblyClasspath so we can remove the dependency on unmanagedClasspath, since it now also comes from localClasspath, and we want to avoid duplicate classpath entries

}

/**
Expand Down Expand Up @@ -470,10 +467,7 @@ trait JavaModule
* without those from upstream modules and dependencies
*/
def jar: T[PathRef] = T {
Jvm.createJar(
localClasspath().map(_.path).filter(os.exists),
manifest()
)
Jvm.createJar(localClasspath().map(_.path).filter(os.exists), manifest())
}

/**
Expand Down
Loading