mill icon indicating copy to clipboard operation
mill copied to clipboard

Build fails when using scala ivydep in scalamodule with java moduledep

Open nrktkt opened this issue 4 years ago • 8 comments

Given

// build.sc
import mill._, scalalib._
object core extends JavaModule {
  object scalaapi extends ScalaModule {
    def scalaVersion = "2.13.1"
    def moduleDeps = Seq(core)
    def ivyDeps = Agg(ivy"org.typelevel::cats-core:2.1.1")
  }
  object javaapi extends JavaModule {
    def moduleDeps = Seq(core, scalaapi)
  }
}

attempting to compile will yield

[42/57] core.scalaapi.compile 
[info] Compiling 6 Scala sources to /home/nfischer/Code/2Auth/out/core/scalaapi/compile/dest/classes ...
[info] Done compiling.
[55/57] core.javaapi.compileClasspath 
1 targets failed
core.javaapi.compileClasspath java.lang.AssertionError: assertion failed: Not a Java dependency: Dep(Dependency(org.typelevel:cats-core, 2.1.1, Configuration(default(compile)), Set(), Publication(, Type(), Extension(), Classifier()), false, true),Binary(false),false)
    scala.Predef$.assert(Predef.scala:223)
    mill.scalalib.Lib$.depToDependencyJava(Lib.scala:24)
    mill.scalalib.CoursierModule.$anonfun$resolveCoursierDependency$3(CoursierModule.scala:17)
    mill.scalalib.CoursierModule.$anonfun$resolveDeps$2(CoursierModule.scala:30)
    scala.collection.immutable.Stream.map(Stream.scala:418)
    mill.scalalib.Lib$.resolveDependencies(Lib.scala:65)
    mill.scalalib.CoursierModule.$anonfun$resolveDeps$1(CoursierModule.scala:27)
    mill.define.ApplyerGenerated.$anonfun$zipMap$2(ApplicativeGenerated.scala:7)
    mill.define.Task$MappedDest.evaluate(Task.scala:380)

on 0.6.2

nrktkt avatar May 06 '20 21:05 nrktkt

On a first glance, it looks like when resolving the ivy deps of javaapi we also resolve all transitive deps from other modules, which include also scala dependencies. But in the scope of the JavaModule we no longer know the scala version to use.

lefou avatar May 06 '20 21:05 lefou

2 workaround come to my mind:

  • make the javaapi project a ScalaModule and adapt the artifactSuffix to be empty
  • write the dependency as pure java dependency: ivy"org.typelevel:cats-scalaBinVersion}:2.1.1"

lefou avatar May 06 '20 21:05 lefou

as suggested by @lefou a workaround is to make javaapi a ScalaModule and def artifactSuffix = ""

nrktkt avatar May 06 '20 21:05 nrktkt

We could transform ivy-deps of other modules to be just "java" dependencies without any cross-version semantic. This can of course lead to issues too.

Thinking a bit more about this issue, I think we need some more safe-guarding when using transitive dependencies of moduleDeps, e.g. in case we depend on another module which uses a different Scala version.

lefou avatar May 06 '20 21:05 lefou

I currently hit this issue again when working on a polyglot project, which has Java, Java + AspectJ, Scala and Kotlin modules. But I think this issue will hit us more frequently now, when people start the try out the new Scala 2.13 - Scala 3 forward compatibility feature.

I think the fact, that we bind the cross value for transitive dependencies in the dependent module is wrong. Instead, each modules should resolve the cross- and platform placeholder by itself. This has the downside, that it is possible to bring more that one Scala-binary-version-line into the dependency tree. We probably should invent some other mechanism to check for consistency, or maybe we can offload that to coursier (I don't know how)?

lefou avatar Nov 20 '20 09:11 lefou

I think each module should implement some checks, e.g. a ScalaModule should check that each transitively module dependency has a compatible or event the same scala version selected. Other modules, like ScalaJsModule need to also check for the same selected platform.

lefou avatar Nov 20 '20 10:11 lefou

We could transform ivy-deps of other modules to be just "java" dependencies without any cross-version semantic. This can of course lead to issues too.

Thinking a bit more about this issue, I think we need some more safe-guarding when using transitive dependencies of moduleDeps, e.g. in case we depend on another module which uses a different Scala version.

I'm facing probably another instance of this issue in Ammonite, where Scala 3 dependencies are converted into Scala 2.13 dependencies when we try to make a Scala 2.13 module depend on a Scala 3 module. It seems like the suffix of the dependencies is evaluated according to the depending module and not according to the module that declares them.

For example, given this project:

import mill._
import mill.scalalib._

object scala213 extends ScalaModule {
  def scalaVersion = "2.13.6"
  def moduleDeps = Seq(scala3)
  def scalacOptions = Seq("-Ytasty-reader")
}
object scala3 extends ScalaModule {
  def scalaVersion = "3.0.2"
  def ivyDeps = Agg(
    ivy"com.lihaoyi::upickle:1.4.0"
  )
}

the ivyDepsTree is:

./mill --disable-ticker scala213.ivyDepsTree
└─ com.lihaoyi:upickle_2.13:1.4.0
   ├─ com.lihaoyi:ujson_2.13:1.4.0
   │  └─ com.lihaoyi:upickle-core_2.13:1.4.0
   │     └─ com.lihaoyi:geny_2.13:0.6.10
   ├─ com.lihaoyi:upack_2.13:1.4.0
   │  └─ com.lihaoyi:upickle-core_2.13:1.4.0
   │     └─ com.lihaoyi:geny_2.13:0.6.10
   └─ com.lihaoyi:upickle-implicits_2.13:1.4.0
      └─ com.lihaoyi:upickle-core_2.13:1.4.0
         └─ com.lihaoyi:geny_2.13:0.6.10

While in Sbt the dependencies are resolved using the _3 suffix (as it should be IMO):

lazy val scala213 = project
  .settings(
    scalaVersion := "2.13.6",
    scalacOptions += "-Ytasty-reader"
  )
  .dependsOn(scala3)

lazy val scala3 = project
  .settings(
    scalaVersion := "3.0.2",
    libraryDependencies += "com.lihaoyi" %% "upickle" % "1.4.0"
  )
sbt:scala3-sandwitch> scala213/dependencyTree
[info] scala213:scala213_2.13:0.1.0-SNAPSHOT [S]
[info]   +-scala3:scala3_3:0.1.0-SNAPSHOT
[info]     +-com.lihaoyi:upickle_3:1.4.0
[info]     | +-com.lihaoyi:ujson_3:1.4.0
[info]     | | +-com.lihaoyi:upickle-core_3:1.4.0
[info]     | |   +-com.lihaoyi:geny_3:0.6.10
[info]     | |   
[info]     | +-com.lihaoyi:upack_3:1.4.0
[info]     | | +-com.lihaoyi:upickle-core_3:1.4.0
[info]     | |   +-com.lihaoyi:geny_3:0.6.10
[info]     | |   
[info]     | +-com.lihaoyi:upickle-implicits_3:1.4.0
[info]     |   +-com.lihaoyi:upickle-core_3:1.4.0
[info]     |     +-com.lihaoyi:geny_3:0.6.10
[info]     |     
[info]     +-org.scala-lang:scala3-library_3:3.0.2 [S]
[info]     

lolgab avatar Nov 20 '21 14:11 lolgab

I haven't looked at the code right now, but we have a defined way to translate each mill-dependency to a coursier-dependency. I think it's resolveCoursierDependency. It's probably worth a POC to use the resolveCoursierDependency task of that module which also contributed the dependency. (Currently we always use that module which resolves the final dependency tree.) The result may contain potential conflicting dependencies, but in such a case, it probably needs intervention for the user anyways. We should try to make some minimal sanity checks, e.g. to not have multiple scala versions etc.

lefou avatar Nov 20 '21 19:11 lefou

I now have a fully working PR, which fixes this issue.

  • #1574

It comes with a slight API change. I'd welcome any review or feedback on it. If there are no objections, I'm going to merge it in the next days.

lefou avatar Dec 15 '22 20:12 lefou